You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "Mikkey-f (via GitHub)" <gi...@apache.org> on 2026/08/23 07:06:00 UTC
[PR] fix: resolve Windows fill template copy failures (#1021) [fesod]
Mikkey-f opened a new pull request, #1034:
URL: https://github.com/apache/fesod/pull/1034
### What and why
Fixes the `Copy template failure` errors that the fill examples
(`FillBasicExample`, `FillComplexExample`) hit on Windows. Two root causes:
**1. `WriteWorkbookHolder` leaks the output stream it opens when initialization fails**
The constructor opens a `FileOutputStream` for the target file before copying
the template. When the copy fails (or any later initialization step throws),
the stream was never closed — the file stays locked on Windows. Unix lets you
delete an open file, which is why CI stayed green. This is a cross-platform
resource leak; Windows just surfaces it first.
Fix: wrap `initHandler` + `copyTemplate` so the stream opened by the holder is
closed before the exception propagates. Caller-provided streams are untouched —
ownership stays with the caller.
**2. `ExampleFileUtil` returns the percent-encoded URL path**
`URL.getPath()` does not decode. On Windows, template file names built with
`File.separator` (`\`) are encoded as `%5C` in the resource URL, so the returned
path pointed at a file literally named `templates%5Clist.xlsx` — which does not
exist → `Copy template failure`. On Unix the separator `/` needs no encoding,
which is why CI passed.
Fix: decode via `URL.toURI()` in a shared `toFilePath` helper. This also fixes
paths containing spaces or other reserved characters (`%20` etc.).
### Tests
- New `WriteWorkbookHolderOutputStreamTest` (2 cases): the stream opened by the
holder is closed on init failure (on Windows an open stream locks the file, so
deletion fails without the fix); caller-provided streams are not closed.
- The fill ITCases (`FillBasicExampleITCase`, `FillComplexExampleITCase`) now
pass on Windows; they previously failed with `Copy template failure`.
- Full module suite green: 846 tests, 0 failures (fesod-common / fesod-shaded /
fesod-sheet / fesod-sheet-examples), `spotless:check` and `javadoc:javadoc` pass.
---
Closes #1021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: resolve Windows fill template copy failures (#1021) [fesod]
Posted by "Mikkey-f (via GitHub)" <gi...@apache.org>.
Mikkey-f commented on code in PR #1034:
URL: https://github.com/apache/fesod/pull/1034#discussion_r3838785096
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteWorkbookHolder.java:
##########
@@ -240,12 +240,15 @@ public WriteWorkbookHolder(WriteWorkbook writeWorkbook) {
}
// init handler
- initHandler(writeWorkbook, null);
-
try {
+ initHandler(writeWorkbook, null);
copyTemplate();
} catch (IOException e) {
+ closeOutputStreamIfOwned();
Review Comment:
Thanks for the suggestion — updated. The failure path in `WriteWorkbookHolder` now gates the close on `autoCloseStream` instead of `file != null`, which mirrors the success-path behavior. Callers using `autoCloseStream(false)` keep their streams unclosed on failure as well, and the test now sets `autoCloseStream(false)` explicitly to assert that contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: resolve Windows fill template copy failures (#1021) [fesod]
Posted by "delei (via GitHub)" <gi...@apache.org>.
delei merged PR #1034:
URL: https://github.com/apache/fesod/pull/1034
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: resolve Windows fill template copy failures (#1021) [fesod]
Posted by "delei (via GitHub)" <gi...@apache.org>.
delei commented on code in PR #1034:
URL: https://github.com/apache/fesod/pull/1034#discussion_r3838697132
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteWorkbookHolder.java:
##########
@@ -240,12 +240,15 @@ public WriteWorkbookHolder(WriteWorkbook writeWorkbook) {
}
// init handler
- initHandler(writeWorkbook, null);
-
try {
+ initHandler(writeWorkbook, null);
copyTemplate();
} catch (IOException e) {
+ closeOutputStreamIfOwned();
Review Comment:
could we gate `closeOutputStreamIfOwned()` with `autoCloseStream` instead of `file != null?`
`autoCloseStream` is more consistent with existing close behavior (while still not closing caller-provided streams).
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteWorkbookHolder.java:
##########
@@ -240,12 +240,15 @@ public WriteWorkbookHolder(WriteWorkbook writeWorkbook) {
}
// init handler
- initHandler(writeWorkbook, null);
-
try {
+ initHandler(writeWorkbook, null);
copyTemplate();
} catch (IOException e) {
+ closeOutputStreamIfOwned();
Review Comment:
Could we gate `closeOutputStreamIfOwned()` with `autoCloseStream` instead of `file != null?`
`autoCloseStream` is more consistent with existing close behavior (while still not closing caller-provided streams).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org