You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2025/06/14 20:28:07 UTC
[Bug 69714] New: Allow custom TempFileCreationStrategy per document
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
Bug ID: 69714
Summary: Allow custom TempFileCreationStrategy per document
Product: POI
Version: unspecified
Hardware: PC
Status: NEW
Severity: enhancement
Priority: P2
Component: POI Overall
Assignee: dev@poi.apache.org
Reporter: attila.kelemen85@gmail.com
Target Milestone: ---
As is now, we can only set a global TempFileCreationStrategy which assumes that
all documents within the same process has the same security profile (and quota)
which may or may not be the case. Currently it is not possible to workaround
this limitation in a multithreaded application.
I have an open PR for this on https://github.com/apache/poi/pull/823
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #4 from Attila Kelemen <at...@gmail.com> ---
I don't really see how my "middle ground" proposal is a drastic change. It
would just declare a thread local, and if the thread local value is set, then
would use that strategy. That would basically only involve a small adjustments
made to the `TempFile` class and nothing else.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #3 from PJ Fanning <fa...@yahoo.com> ---
I am against big new temp file API changes.
You can plug in your own strategy using
https://poi.apache.org/apidocs/dev/org/apache/poi/util/TempFile.html#setTempFileCreationStrategy-org.apache.poi.util.TempFileCreationStrategy-
POI is 20 years old. I don't think we need to make drastic changes to suit 1
user.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #10 from PJ Fanning <fa...@yahoo.com> ---
I have merged the PR and have tidied up the ThreadLocal to call remove when
nulls are in play.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
PJ Fanning <fa...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
OS| |All
--- Comment #1 from PJ Fanning <fa...@yahoo.com> ---
I am against this change. We made the temp file management POI pluggable so
that we wouldn't need to worry about having special API methods passing around
strategy instances.
I think it is possible to use POI as is to achieve this requirement.
1. Write a custom global strategy that checks uses a ThreadLocal to get the
local strategy.
2. Register the custom global strategy with POI TempFile class.
3. In a scenario like wanting to create an SXSSFWorkbook and write it out
(which creates a number of temp files), you could update your ThreadLocal to
register to the temp file strategy for this thread. You can unset the
ThreadLocal setting after you finish.
If you are using more recent versions of Java and want to avoid ThreadLocal,
you could look at using Scoped Values instead.
https://www.baeldung.com/java-20-scoped-values
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #6 from PJ Fanning <fa...@yahoo.com> ---
Ok. If the change can be kept to your approx 30 lines, I will review it.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #2 from Attila Kelemen <at...@gmail.com> ---
I think POI's mistaken assumption here is that what uses tmp files and what
doesn't is an implementation detail, but tmp file usage has very important
implications (mainly security and quota) to be just treated as such.
Thread locals do not solve the issue, because when I have a module who knows
where to put temporary files generated for particular documents, it cannot
encapsulate POI (as it doesn't know who else might be using POI independently).
Though I have other problems using thread locals as well, the rest (what I'm
aware of) can be solved with some performance loss (not too bad probably).
If you are really against passing around the strategy, then as a middle ground,
if I could do this:
```
TempFile.withTempStrategy(myStrategy, () -> {
// Use POI.
});
```
and then POI would promise me that every POI calls within the scope of the
lambda will use `myStrategy`, then that I would consider as a solution to my
main issue.
If you think this would be an acceptable on your part, then I can implement
this instead of my current PR.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #7 from PJ Fanning <fa...@yahoo.com> ---
I added some changes to TempFile to allow a thread specific strategy.
https://github.com/apache/poi/commit/9c08d29fcbd0f3f4fec5f27349246bf6a84fe78a
and
https://github.com/apache/poi/commit/66055698623ce2b7ca5e8bfeb5f2145c8b54cbb7
I will look at adding more tests later. If there are any suggestions about
changes or additional tests, I will look at them.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per thread
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
PJ Fanning <fa...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Allow custom |Allow custom
|TempFileCreationStrategy |TempFileCreationStrategy
|per document |per thread
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #8 from Attila Kelemen <at...@gmail.com> ---
My issue with your implementation is that I cannot restore the previous
implementation to avoid changing the strategy to the caller (and also cannot
cleanup garbage manually).
In the mean time I also wrote my own implementation:
https://github.com/apache/poi/pull/825/commits/0d636901adf11cc0cfee8ed9dcdee004cfd11600
The benefit of my implementation are:
- Cannot be misused (i.e., cannot forget to restore the strategy).
- Doesn't require a cleaner.
- Doesn't leave unset garbage after a `ThreadLocal.get` returning null.
I only noticed your implementation after I implemented mine. So, I didn't yet
rebased mine on top of trunk. If you want to use mine, then you can copy-paste
my solution, or just ask me to rebase my changes (whichever is more convenient
for you).
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #5 from Attila Kelemen <at...@gmail.com> ---
In fact, I just checked, it is literally just 30 lines of extra code (pretty
much trivial) containing one extra public method, with 2 modified lines
(replacing `strategy` with `getStrategy()` within `TempFile`). I feel that
calling it a "drastic change" is a major exaggeration.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per thread
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
PJ Fanning <fa...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Hardware|PC |All
Resolution|--- |FIXED
Status|NEW |RESOLVED
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 69714] Allow custom TempFileCreationStrategy per document
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69714
--- Comment #9 from Attila Kelemen <at...@gmail.com> ---
Some notes on your current implementation:
- I cannot restore the strategy of my caller. The simplest change you can do to
allow this is to return the previous strategy in
`setThreadLocalTempFileCreationStrategy`.
- `getStrategy()` should call `threadLocalStrategy.remove()` if
`threadLocalStrategy.get()` returns null to avoid penalizing people who don't
use the thread local strategy with a garbage thread local value (with a value
of null).
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org