From 30ff7bb1cdf22f53419b0fd938da59d32a8582da Mon Sep 17 00:00:00 2001
From: Marten Deinum <mdeinum@gmail.com>
Date: Mon, 19 Jan 2015 20:54:19 +0100
Subject: [PATCH] Cleanup of close methods.

Removed the doCloseWorkbook method in favor of the normal doClose method.

The PoiItemReader now closes the InputStream and Workbook (for Apache POI >= 3.11)
---
 spring-batch-excel/pom.xml                    |  2 +-
 .../item/excel/AbstractExcelItemReader.java   | 21 -----------
 .../batch/item/excel/jxl/JxlItemReader.java   |  2 +-
 .../batch/item/excel/poi/PoiItemReader.java   | 35 ++++++++++++++++++-
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/spring-batch-excel/pom.xml b/spring-batch-excel/pom.xml
index bbf8d65..4840fd6 100644
--- a/spring-batch-excel/pom.xml
+++ b/spring-batch-excel/pom.xml
@@ -28,7 +28,7 @@
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <spring.batch.version>3.0.3.RELEASE</spring.batch.version>
         <jxl.version>2.6.12</jxl.version>
-        <poi.version>3.10.1</poi.version>
+        <poi.version>3.11</poi.version>
     </properties>
 
     <dependencies>
diff --git a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/AbstractExcelItemReader.java b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/AbstractExcelItemReader.java
index b666215..651ea55 100644
--- a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/AbstractExcelItemReader.java
+++ b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/AbstractExcelItemReader.java
@@ -136,27 +136,6 @@ public abstract class AbstractExcelItemReader<T> extends AbstractItemCountingIte
 
     }
 
-    @Override
-    protected final void doClose() throws Exception {
-        doCloseWorkbook();
-        if (this.resource != null) {
-            try {
-                InputStream is = this.resource.getInputStream();
-                is.close();
-            } catch (IOException ioe) {
-                logger.warn("Exception whilst obtaining or closing the inputstream.", ioe);
-            }
-        }
-    }
-
-    /**
-     * Method which can be overriden by subclasses to do cleanup additional resources.
-     *
-     * @throws Exception
-     */
-    protected void doCloseWorkbook() throws Exception {
-    }
-
     public void setResource(final Resource resource) {
         this.resource = resource;
     }
diff --git a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/jxl/JxlItemReader.java b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/jxl/JxlItemReader.java
index add08f0..24dfc1a 100644
--- a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/jxl/JxlItemReader.java
+++ b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/jxl/JxlItemReader.java
@@ -51,7 +51,7 @@ public class JxlItemReader<T> extends AbstractExcelItemReader<T> {
     }
 
     @Override
-    protected void doCloseWorkbook() throws Exception {
+    protected void doClose() throws Exception {
         if (this.workbook != null) {
             this.workbook.close();
         }
diff --git a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/poi/PoiItemReader.java b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/poi/PoiItemReader.java
index f94f749..718c705 100644
--- a/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/poi/PoiItemReader.java
+++ b/spring-batch-excel/src/main/java/org/springframework/batch/item/excel/poi/PoiItemReader.java
@@ -23,6 +23,10 @@ import org.springframework.batch.item.excel.AbstractExcelItemReader;
 import org.springframework.batch.item.excel.Sheet;
 import org.springframework.core.io.Resource;
 
+import java.io.Closeable;
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+
 /**
  * {@link org.springframework.batch.item.ItemReader} implementation which uses apache POI to read an Excel
  * file. It will read the file sheet for sheet and row for row. It is based on
@@ -36,6 +40,8 @@ public class PoiItemReader<T> extends AbstractExcelItemReader<T> {
 
     private Workbook workbook;
 
+    private InputStream workbookStream;
+
     @Override
     protected Sheet getSheet(final int sheet) {
         return new PoiSheet(this.workbook.getSheetAt(sheet));
@@ -46,9 +52,36 @@ public class PoiItemReader<T> extends AbstractExcelItemReader<T> {
         return this.workbook.getNumberOfSheets();
     }
 
+    @Override
+    protected void doClose() throws Exception {
+        // As of Apache POI 3.11 there is a close method on the Workbook, prior version
+        // lack this method.
+        if (workbook instanceof Closeable) {
+            this.workbook.close();
+        }
+
+        if (workbookStream != null) {
+            workbookStream.close();
+        }
+        this.workbook=null;
+        this.workbookStream=null;
+    }
+
+    /**
+     * Open the underlying file using the {@code WorkbookFactory}. We keep track of the used {@code InputStream} so that
+     * it can be closed cleanly on the end of reading the file. This to be able to release the resources used by
+     * Apache POI.
+     *
+     * @param resource
+     * @throws Exception is thrown for any errors.
+     */
     @Override
     protected void openExcelFile(final Resource resource) throws Exception {
-        this.workbook = WorkbookFactory.create(resource.getInputStream());
+        workbookStream = resource.getInputStream();
+        if (!workbookStream.markSupported() && !(workbookStream instanceof PushbackInputStream)) {
+            throw new IllegalStateException("InputStream MUST either support mark/reset, or be wrapped as a PushbackInputStream");
+        }
+        this.workbook = WorkbookFactory.create(workbookStream);
         this.workbook.setMissingCellPolicy(Row.CREATE_NULL_AS_BLANK);
     }