Github user kaspersorensen commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/56#discussion_r41114786
  
    --- Diff: excel/src/main/java/org/apache/metamodel/excel/ExcelUtils.java ---
    @@ -128,24 +142,31 @@ public static boolean isXlsxFile(Resource resource) {
          */
         public static Workbook readWorkbook(ExcelDataContext dataContext) {
             Resource resource = dataContext.getResource();
    -        if (!resource.isExists()) {
    -            if (isXlsxFile(resource)) {
    -                return new SXSSFWorkbook(1000);
    -            } else {
    -                return new HSSFWorkbook();
    -            }
    -        }
             return readWorkbook(resource);
         }
     
    -    public static void writeWorkbook(ExcelDataContext dataContext, final 
Workbook wb) {
    -        final Resource resource = dataContext.getResource();
    -        resource.write(new Action<OutputStream>() {
    +    /**
    +     * Writes the {@link Workbook} to a {@link Resource}. The {@link 
Workbook}
    +     * will be closed as a result of this operation!
    +     * 
    +     * @param dataContext
    +     * @param wb
    +     */
    +    public static void writeAndCloseWorkbook(ExcelDataContext dataContext, 
final Workbook wb) {
    --- End diff --
    
    Please wait before merging. I would like to try one potential optimization:
    
    ```
    if (dataContext.getResource() instanceof FileResource) {
      wb.close();
      return;
    }
    ```
    
    I believe this should work because it seems that workbooks are aware if 
they have a file handle or not. And upon close() they will persist to the file, 
if available. That's at least my working assumption since what I saw previously 
when I was doing the copy BEFORE close, was that I ended up with an empty sheet 
in the end.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to