https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
Bug ID: 59252 Summary: Close workbook does not save file Product: POI Version: 3.14-FINAL Hardware: PC Status: NEW Severity: major Priority: P2 Component: XSSF Assignee: [hidden email] Reporter: [hidden email] As speciffied in OPCPackage API: https://poi.apache.org/apidocs/org/apache/poi/openxml4j/opc/OPCPackage.html#close%28%29 close should save an Excel file if writable, but it doesn't. Instead, you need to save to a second file, and then it saves also the original one. It happens the same (at least) if created with WorkbookFactory.create or OPCPackage.open with Write permissions. I wrote a basic test: //Create excel file File file= new File("C:\\file2.xlsx"); Workbook wbx= new XSSFWorkbook(); wbx.createSheet(); Sheet s= wbx.getSheetAt(0); Row r= s.createRow(0); Cell c= r.createCell(0); c.setCellType(Cell.CELL_TYPE_STRING); c.setCellValue("Wrong"); OutputStream os= new FileOutputStream(file); wbx.write(os); wbx.close(); //Update Excel file wbx= WorkbookFactory.create(file, null, false); /*OR OPCPackage pkg = OPCPackage.open(file,PackageAccess.READ_WRITE); wbx= new XSSFWorkbook(pkg); */ s= wbx.getSheetAt(0); r= s.getRow(0); c= r.getCell(0); c.setCellValue("Right"); //Without this, now it doesn't save /* File trash= File.createTempFile("basura", ".tmp"); FileOutputStream trahsOs= new FileOutputStream(trash); wbx.write(trahsOs); trahsOs.close(); trash.delete(); */ //This should save the file wbx.close(); //Reopen and check wbx= WorkbookFactory.create(file, null, false); s= wbx.getSheetAt(0); r= s.getRow(0); c= r.getCell(0); String value= c.getStringCellValue(); wbx.close(); assert("Right".equals(value)); -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
Javen O'Neal <[hidden email]> changed: What |Removed |Added ---------------------------------------------------------------------------- OS| |All --- Comment #1 from Javen O'Neal <[hidden email]> --- Since you're using File and not FileInputStream, it should write back to disk. See bug 58779. If you open the file through a FileInputStream vs File with WorkbookFactory.create(File|FileInputStream) or XSSFWorkbook(File|FileInputStream), or OPCPackage(File|FileInputStream) you get different behavior. Saving to an input stream does nothing to the file system. I'm thinking about changing the behavior of close to not save changes to disk, but have not decided yet. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #2 from Nick Burch <[hidden email]> --- OPCPackage has always supported in-place write for Files. OPOIFS never did, while NPOIFS now does. Because OPOIFS never did, and that's all we had for agest, the usermodel APIs never did either Now that the default POIFS is NPOIFS which does, there has been talk of adding in-place write at the usermodel level (eg XSSFWorkbook.write() / HSSFWorkbook.write() in addition to the write(OutputStream) method), but no-one has so far volunteered to do the work to implement -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #3 from Alex <[hidden email]> --- Ok,I think I understand your point, close() shouldn't save to file (and I actually agree with that) but what I'm saing its that it actually doesn't save on close() with 3.14 release unless you call write(FileOutputStream). You said it should be saving to file as from now but it's not. May be I didn't understood your answers. I'll try to take a look to source conde (I hope I can follow it), but I would like to clarify if that test I wrote should be working at 3.14 release or not. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
This is a candidate for GSOC or Help Wanted.
Need WorkbookFactory.create and XSSFWorkbook constructor to open read only Should not write to file on close if a workbook is merely opened and not modified. Sounds like changing close's behavior is contentious, so we'd need to decide what direction we want to go (make close write changes to disk, deprecate/remove FileInputStream constructors or add note that close won't write changes back to disk; alternatively, make close non-saving or add a close(boolean save) method) However this is handled, we should harmonize all of POI so that close behavior is clear and logical. On Mar 30, 2016 10:43 AM, <[hidden email]> wrote: > https://bz.apache.org/bugzilla/show_bug.cgi?id=59252 > > --- Comment #2 from Nick Burch <[hidden email]> --- > OPCPackage has always supported in-place write for Files. OPOIFS never did, > while NPOIFS now does. Because OPOIFS never did, and that's all we had for > agest, the usermodel APIs never did either > > Now that the default POIFS is NPOIFS which does, there has been talk of > adding > in-place write at the usermodel level (eg XSSFWorkbook.write() / > HSSFWorkbook.write() in addition to the write(OutputStream) method), but > no-one > has so far volunteered to do the work to implement > > -- > You are receiving this mail because: > You are the assignee for the bug. > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > |
I never understood why the default is to write changes to disc.
I'd prefer the default to leave the document unchanged and only write the changes when commit() is called. close() should be callable in any case, possibly throwing away any temporary objects - and it should be ok, to call it again. As mentioned, harmonizing the various modules should be part of the change. To stick with our deprecation policy, I would deprecate revert() and after two releases change the handling completely :P So what to do with the in-place write functionality? The default is to open documents read-only - for read/write we would work on a copy and overwrite the original file when commit() is called. Andi --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #4 from Alex <[hidden email]> --- Ok, I took a look over source code and found the problem. This is method write(OutputStream) in POIXMLDocument: public final void write(OutputStream stream) throws IOException { //force all children to commit their changes into the underlying OOXML Package Set<PackagePart> context = new HashSet<PackagePart>(); onSave(context); context.clear(); //save extended and custom properties getProperties().commit(); getPackage().save(stream); } Key is the properties commit, that it's not done on close() method, so package doesn't notice any change. To test this, I updated my code so instead doing this: File trash= File.createTempFile("basura", ".tmp"); FileOutputStream trahsOs= new FileOutputStream(trash); wbx.write(trahsOs); trahsOs.close(); trash.delete(); It's enough to call with null and catch the Exception: try { wbx.write(null); } catch (Exception e) {} Do you think it could be right to change close() from: public void close() throws IOException { if (pkg != null) { if (pkg.getPackageAccess() == PackageAccess.READ) { pkg.revert(); } else { pkg.close(); } pkg= null; } } to: public void close() throws IOException { if (pkg != null) { if (pkg.getPackageAccess() == PackageAccess.READ) { pkg.revert(); } else { // force all children to commit their changes into the underlying OOXML Package Set<PackagePart> context= new HashSet<PackagePart>(); onSave(context); context.clear(); // save extended and custom properties getProperties().commit(); pkg.close(); } pkg= null; } } That leaving the close() discussion for other moment. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #5 from Javen O'Neal <[hidden email]> --- Testing the change to POIXMLDocument.close from comment 4, I got an error on TestXSSFBugs.bug45431 (which relates to a macro-enabled workbook): "Rule M2.4 exception : this error should NEVER happen!" Full stack trace: https://paste.apache.org/MU6J ContentTypeManager.getContentType throws the exception Code: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ContentTypeManager.java?revision=1722433&view=markup#l323 According to the javadocs for getContentType: @exception OpenXML4JRuntimeException Throws if the content type manager is not able to find the content from an existing part. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
Dominik Stadler <[hidden email]> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |59287 -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
Javen O'Neal <[hidden email]> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |57919 -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #6 from Marcel M. <[hidden email]> --- I just hit this bug in 5.0.0. A (wasteful) workaround is to write to a NullOutputStream to trigger the in place update of the File: try (Workbook wb = WorkbookFactory.create(new File("foo")); OutputStream out = OutputStream.nullOutputStream()) { ... wb.write(out); } This is especially interesting because the doc for WorkbooFactory.create(InputStream) encourages the use of the File version: "Note also that loading from an InputStream requires more memory than loading from a File, so prefer {@link #create(File)} where possible." -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
--- Comment #7 from Francisco Castaneda <[hidden email]> --- I had the same problem, unless you call write on an outputstream, changes won't be saved. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59252
Sz.Noemi <[hidden email]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[hidden email] -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Free forum by Nabble | Edit this page |