METAMODEL-187: Fixed
authorKasper Sørensen <i.am.kasper.sorensen@gmail.com>
Tue, 6 Oct 2015 12:47:10 +0000 (14:47 +0200)
committerKasper Sørensen <i.am.kasper.sorensen@gmail.com>
Tue, 6 Oct 2015 12:47:10 +0000 (14:47 +0200)
Fixes #56
Closer #49

15 files changed:
core/src/main/java/org/apache/metamodel/data/RowPublisherDataSet.java
core/src/main/java/org/apache/metamodel/util/FileHelper.java
core/src/main/java/org/apache/metamodel/util/ObjectComparator.java
excel/pom.xml
excel/src/main/java/org/apache/metamodel/excel/DefaultSpreadsheetReaderDelegate.java
excel/src/main/java/org/apache/metamodel/excel/ExcelDataContext.java
excel/src/main/java/org/apache/metamodel/excel/ExcelUpdateCallback.java
excel/src/main/java/org/apache/metamodel/excel/ExcelUtils.java
excel/src/main/java/org/apache/metamodel/excel/SpreadsheetReaderDelegate.java
excel/src/main/java/org/apache/metamodel/excel/XlsDataSet.java
excel/src/main/java/org/apache/metamodel/excel/XlsxSpreadsheetReaderDelegate.java
excel/src/test/java/org/apache/metamodel/excel/DefaultSpreadsheetReaderDelegateTest.java
excel/src/test/java/org/apache/metamodel/excel/ExcelDataContextTest.java
excel/src/test/java/org/apache/metamodel/excel/ExcelUpdateCallbackTest.java
pom.xml

index 47f0325..2640246 100644 (file)
  */
 package org.apache.metamodel.data;
 
+import java.io.Closeable;
+
 import org.apache.metamodel.query.SelectItem;
 import org.apache.metamodel.util.Action;
+import org.apache.metamodel.util.FileHelper;
 import org.apache.metamodel.util.SharedExecutorService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -33,80 +36,85 @@ import org.slf4j.LoggerFactory;
  */
 public final class RowPublisherDataSet extends AbstractDataSet {
 
-       private static final Logger logger = LoggerFactory
-                       .getLogger(RowPublisherDataSet.class);
+    private static final Logger logger = LoggerFactory.getLogger(RowPublisherDataSet.class);
+
+    private final int _maxRows;
+    private final Action<RowPublisher> _publishAction;
+    private final Closeable[] _closeables;
+    private RowPublisherImpl _rowPublisher;
+    private boolean _closed;
 
-       private final int _maxRows;
-       private final Action<RowPublisher> _publishAction;
-       private RowPublisherImpl _rowPublisher;
-       private boolean _closed;
+    public RowPublisherDataSet(SelectItem[] selectItems, int maxRows, Action<RowPublisher> publishAction) {
+        this(selectItems, maxRows, publishAction, new Closeable[0]);
+    }
 
-       public RowPublisherDataSet(SelectItem[] selectItems, int maxRows,
-                       Action<RowPublisher> publishAction) {
-           super(selectItems);
-               _maxRows = maxRows;
-               _publishAction = publishAction;
-               _closed = false;
-       }
+    public RowPublisherDataSet(SelectItem[] selectItems, int maxRows, Action<RowPublisher> publishAction,
+            Closeable... closeables) {
+        super(selectItems);
+        _maxRows = maxRows;
+        _publishAction = publishAction;
+        _closed = false;
+        _closeables = closeables;
+    }
 
-       public int getMaxRows() {
-               return _maxRows;
-       }
+    public int getMaxRows() {
+        return _maxRows;
+    }
 
-       @Override
-       public void close() {
-               super.close();
-               _closed = true;
-               if (_rowPublisher != null) {
-                   _rowPublisher.finished();
-                   _rowPublisher = null;
-               }
-       }
+    @Override
+    public void close() {
+        super.close();
+        _closed = true;
+        if (_rowPublisher != null) {
+            _rowPublisher.finished();
+            _rowPublisher = null;
+        }
+        if (_closeables != null) {
+            FileHelper.safeClose((Object[]) _closeables);
+        }
+    }
 
-       @Override
-       protected void finalize() throws Throwable {
-               super.finalize();
-               if (!_closed) {
-                       logger.warn(
-                                       "finalize() invoked, but DataSet is not closed. Invoking close() on {}",
-                                       this);
-                       close();
-               }
-       }
+    @Override
+    protected void finalize() throws Throwable {
+        super.finalize();
+        if (!_closed) {
+            logger.warn("finalize() invoked, but DataSet is not closed. Invoking close() on {}", this);
+            close();
+        }
+    }
 
-       @Override
-       public boolean next() {
-               if (_rowPublisher == null) {
-                       // first time, create the publisher
-                       _rowPublisher = new RowPublisherImpl(this);
-                       logger.info("Starting separate thread for publishing action: {}",
-                                       _publishAction);
-                       Runnable runnable = new Runnable() {
-                               public void run() {
-                                       boolean successful = false;
-                                       try {
-                                               _publishAction.run(_rowPublisher);
-                                               logger.debug("Publshing action finished!");
-                                               successful = true;
-                                       } catch (Exception e) {
-                                               _rowPublisher.failed(e);
-                                       }
-                                       if (successful) {
-                                               _rowPublisher.finished();
-                                       }
-                               };
-                       };
-                       SharedExecutorService.get().submit(runnable);
-               }
-               return _rowPublisher.next();
-       }
+    @Override
+    public boolean next() {
+        if (_rowPublisher == null) {
+            // first time, create the publisher
+            _rowPublisher = new RowPublisherImpl(this);
+            logger.info("Starting separate thread for publishing action: {}", _publishAction);
+            Runnable runnable = new Runnable() {
+                public void run() {
+                    boolean successful = false;
+                    try {
+                        _publishAction.run(_rowPublisher);
+                        logger.debug("Publshing action finished!");
+                        successful = true;
+                    } catch (Exception e) {
+                        _rowPublisher.failed(e);
+                    }
+                    if (successful) {
+                        _rowPublisher.finished();
+                    }
+                };
+            };
+            SharedExecutorService.get().submit(runnable);
+        }
+        return _rowPublisher.next();
+    }
 
-       @Override
-       public Row getRow() {
-               if (_rowPublisher == null) {
-                       return null;
-               }
-               return _rowPublisher.getRow();
-       }
+    @Override
+    public Row getRow() {
+        if (_rowPublisher == null) {
+            return null;
+        }
+        return _rowPublisher.getRow();
+    }
 
 }
index d5676cc..0ff06a8 100644 (file)
@@ -39,6 +39,8 @@ import java.io.Reader;
 import java.io.Writer;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.Statement;
@@ -90,7 +92,8 @@ public final class FileHelper {
                 logger.error("Could not create tempFile in order to find temporary dir", e);
                 result = new File("metamodel.tmp.dir");
                 if (!result.mkdir()) {
-                    throw new IllegalStateException("Could not create directory for temporary files: " + result.getName());
+                    throw new IllegalStateException("Could not create directory for temporary files: "
+                            + result.getName());
                 }
                 result.deleteOnExit();
             }
@@ -110,7 +113,8 @@ public final class FileHelper {
         return getWriter(outputStream, encoding, false);
     }
 
-    public static Writer getWriter(OutputStream outputStream, String encoding, boolean insertBom) throws IllegalStateException {
+    public static Writer getWriter(OutputStream outputStream, String encoding, boolean insertBom)
+            throws IllegalStateException {
         if (!(outputStream instanceof BufferedOutputStream)) {
             outputStream = new BufferedOutputStream(outputStream);
         }
@@ -128,7 +132,8 @@ public final class FileHelper {
         }
     }
 
-    public static Writer getWriter(File file, String encoding, boolean append, boolean insertBom) throws IllegalStateException {
+    public static Writer getWriter(File file, String encoding, boolean append, boolean insertBom)
+            throws IllegalStateException {
         if (append && insertBom) {
             throw new IllegalArgumentException("Can not insert BOM into appending writer");
         }
@@ -144,13 +149,13 @@ public final class FileHelper {
     public static Reader getReader(InputStream inputStream, String encoding) throws IllegalStateException {
         try {
             if (encoding == null || encoding.toLowerCase().indexOf("utf") != -1) {
-                byte bom[] = new byte[4];
+                final byte bom[] = new byte[4];
                 int unread;
 
                 // auto-detect byte-order-mark
                 @SuppressWarnings("resource")
-                PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream, bom.length);
-                int n = pushbackInputStream.read(bom, 0, bom.length);
+                final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream, bom.length);
+                final int n = pushbackInputStream.read(bom, 0, bom.length);
 
                 // Read ahead four bytes and check for BOM marks.
                 if ((bom[0] == (byte) 0xEF) && (bom[1] == (byte) 0xBB) && (bom[2] == (byte) 0xBF)) {
@@ -324,7 +329,8 @@ public final class FileHelper {
         return new BufferedReader(reader);
     }
 
-    public static BufferedReader getBufferedReader(InputStream inputStream, String encoding) throws IllegalStateException {
+    public static BufferedReader getBufferedReader(InputStream inputStream, String encoding)
+            throws IllegalStateException {
         Reader reader = getReader(inputStream, encoding);
         return new BufferedReader(reader);
     }
@@ -349,7 +355,8 @@ public final class FileHelper {
         writeString(outputStream, string, DEFAULT_ENCODING);
     }
 
-    public static void writeString(OutputStream outputStream, String string, String encoding) throws IllegalStateException {
+    public static void writeString(OutputStream outputStream, String string, String encoding)
+            throws IllegalStateException {
         final Writer writer = getWriter(outputStream, encoding);
         writeString(writer, string, encoding);
     }
@@ -416,16 +423,45 @@ public final class FileHelper {
         }
     }
 
-    public static void copy(File from, File to) throws IllegalStateException {
-        assert from.exists();
+    public static void copy(Resource from, Resource to) throws IllegalStateException {
+        assert from.isExists();
 
-        final InputStream fromStream = getInputStream(from);
-        final OutputStream toStream = getOutputStream(to);
+        if (from instanceof FileResource && to instanceof FileResource) {
+            final File fromFile = ((FileResource) from).getFile();
+            final File toFile = ((FileResource) to).getFile();
+            copy(fromFile, toFile);
+            return;
+        }
 
+        final InputStream fromStream = from.read();
         try {
-            copy(fromStream, toStream);
+            if (to instanceof FileResource) {
+                final File toFile = ((FileResource) to).getFile();
+                try {
+                    Files.copy(fromStream, toFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+                } catch (IOException e) {
+                    throw new IllegalStateException(e);
+                }
+            } else {
+                final OutputStream toStream = to.write();
+                try {
+                    copy(fromStream, toStream);
+                } finally {
+                    safeClose(toStream);
+                }
+            }
         } finally {
-            safeClose(fromStream, toStream);
+            safeClose(fromStream);
+        }
+    }
+
+    public static void copy(File from, File to) throws IllegalStateException {
+        assert from.exists();
+
+        try {
+            Files.copy(from.toPath(), to.toPath(), StandardCopyOption.REPLACE_EXISTING);
+        } catch (IOException e) {
+            throw new IllegalStateException(e);
         }
     }
 
index 697cf46..6f442d5 100644 (file)
@@ -96,7 +96,7 @@ public final class ObjectComparator implements Comparator<Object> {
                                return -1 * c2.compareTo(o1);
                        }
                }
-               logger.info("Using ToStringComparator because no apparent better comparison method could be found");
+               logger.debug("Using ToStringComparator because no apparent better comparison method could be found");
                return ToStringComparator.getComparator().compare(o1, o2);
        }
 }
\ No newline at end of file
index c26e654..f8f6d67 100644 (file)
@@ -35,13 +35,13 @@ under the License.
                </dependency>
                <dependency>
                        <groupId>org.slf4j</groupId>
-                       <artifactId>slf4j-nop</artifactId>
+                       <artifactId>slf4j-simple</artifactId>
                        <scope>test</scope>
                </dependency>
                <dependency>
                        <groupId>org.apache.poi</groupId>
                        <artifactId>poi-ooxml</artifactId>
-                       <version>3.12</version>
+                       <version>3.13</version>
                        <exclusions>
                                <exclusion>
                                        <groupId>commons-logging</groupId>
index 80d91d6..1b8b534 100644 (file)
  */
 package org.apache.metamodel.excel;
 
-import java.io.InputStream;
 import java.util.Iterator;
 
-import org.apache.poi.ss.usermodel.Cell;
-import org.apache.poi.ss.usermodel.Row;
-import org.apache.poi.ss.usermodel.Sheet;
-import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.metamodel.data.DataSet;
 import org.apache.metamodel.data.EmptyDataSet;
 import org.apache.metamodel.data.MaxRowsDataSet;
@@ -36,7 +31,12 @@ import org.apache.metamodel.schema.MutableTable;
 import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
 import org.apache.metamodel.util.AlphabeticSequence;
-import org.apache.metamodel.util.Ref;
+import org.apache.metamodel.util.FileHelper;
+import org.apache.metamodel.util.Resource;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -48,30 +48,35 @@ final class DefaultSpreadsheetReaderDelegate implements SpreadsheetReaderDelegat
 
     private static final Logger logger = LoggerFactory.getLogger(DefaultSpreadsheetReaderDelegate.class);
 
+    private final Resource _resource;
     private final ExcelConfiguration _configuration;
 
-    public DefaultSpreadsheetReaderDelegate(ExcelConfiguration configuration) {
+    public DefaultSpreadsheetReaderDelegate(Resource resource, ExcelConfiguration configuration) {
+        _resource = resource;
         _configuration = configuration;
     }
 
     @Override
-    public Schema createSchema(InputStream inputStream, String schemaName) {
+    public Schema createSchema(String schemaName) {
         final MutableSchema schema = new MutableSchema(schemaName);
-        final Workbook wb = ExcelUtils.readWorkbook(inputStream);
+        final Workbook wb = ExcelUtils.readWorkbook(_resource);
+        try {
+            for (int i = 0; i < wb.getNumberOfSheets(); i++) {
+                final Sheet currentSheet = wb.getSheetAt(i);
+                final MutableTable table = createTable(wb, currentSheet);
+                table.setSchema(schema);
+                schema.addTable(table);
+            }
 
-        for (int i = 0; i < wb.getNumberOfSheets(); i++) {
-            final Sheet currentSheet = wb.getSheetAt(i);
-            final MutableTable table = createTable(wb, currentSheet);
-            table.setSchema(schema);
-            schema.addTable(table);
+            return schema;
+        } finally {
+            FileHelper.safeClose(wb);
         }
-
-        return schema;
     }
 
     @Override
-    public DataSet executeQuery(InputStream inputStream, Table table, Column[] columns, int maxRows) {
-        final Workbook wb = ExcelUtils.readWorkbook(inputStream);
+    public DataSet executeQuery(Table table, Column[] columns, int maxRows) {
+        final Workbook wb = ExcelUtils.readWorkbook(_resource);
         final Sheet sheet = wb.getSheet(table.getName());
 
         if (sheet == null || sheet.getPhysicalNumberOfRows() == 0) {
@@ -87,7 +92,7 @@ final class DefaultSpreadsheetReaderDelegate implements SpreadsheetReaderDelegat
     }
 
     @Override
-    public void notifyTablesModified(Ref<InputStream> inputStreamRef) {
+    public void notifyTablesModified() {
         // do nothing
     }
 
@@ -106,7 +111,6 @@ final class DefaultSpreadsheetReaderDelegate implements SpreadsheetReaderDelegat
             return table;
         }
 
-
         Row row = null;
 
         if (_configuration.isSkipEmptyLines()) {
index 5963e69..28c1f8e 100644 (file)
@@ -21,9 +21,7 @@ package org.apache.metamodel.excel;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.PushbackInputStream;
 
-import org.apache.poi.POIXMLDocument;
 import org.apache.metamodel.DataContext;
 import org.apache.metamodel.MetaModelException;
 import org.apache.metamodel.QueryPostprocessDataContext;
@@ -34,11 +32,10 @@ import org.apache.metamodel.schema.Column;
 import org.apache.metamodel.schema.MutableSchema;
 import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
-import org.apache.metamodel.util.FileHelper;
 import org.apache.metamodel.util.FileResource;
-import org.apache.metamodel.util.LazyRef;
-import org.apache.metamodel.util.Ref;
+import org.apache.metamodel.util.Func;
 import org.apache.metamodel.util.Resource;
+import org.apache.poi.POIXMLDocument;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -145,25 +142,19 @@ public final class ExcelDataContext extends QueryPostprocessDataContext implemen
 
     @Override
     public DataSet materializeMainSchemaTable(Table table, Column[] columns, int maxRows) {
-
-        Ref<InputStream> inputStreamRef = getInputStreamRef();
-        InputStream inputStream = null;
         try {
-            SpreadsheetReaderDelegate delegate = getSpreadsheetReaderDelegate(inputStreamRef);
-            inputStream = inputStreamRef.get();
+            SpreadsheetReaderDelegate delegate = getSpreadsheetReaderDelegate();
 
             // METAMODEL-47: Ensure that we have loaded the schema at this point
             getDefaultSchema();
 
-            DataSet dataSet = delegate.executeQuery(inputStream, table, columns, maxRows);
+            DataSet dataSet = delegate.executeQuery(table, columns, maxRows);
             return dataSet;
         } catch (Exception e) {
             if (e instanceof RuntimeException) {
                 throw (RuntimeException) e;
             }
             throw new MetaModelException("Unexpected exception while materializing main schema table", e);
-        } finally {
-            FileHelper.safeClose(inputStream);
         }
     }
 
@@ -173,12 +164,9 @@ public final class ExcelDataContext extends QueryPostprocessDataContext implemen
             logger.info("Resource does not exist, returning empty schema");
             return new MutableSchema(getMainSchemaName());
         }
-        Ref<InputStream> inputStreamRef = getInputStreamRef();
-        InputStream inputStream = null;
         try {
-            SpreadsheetReaderDelegate delegate = getSpreadsheetReaderDelegate(inputStreamRef);
-            inputStream = inputStreamRef.get();
-            Schema schema = delegate.createSchema(inputStream, getMainSchemaName());
+            SpreadsheetReaderDelegate delegate = getSpreadsheetReaderDelegate();
+            Schema schema = delegate.createSchema(getMainSchemaName());
             assert getMainSchemaName().equals(schema.getName());
             return schema;
         } catch (Exception e) {
@@ -186,8 +174,6 @@ public final class ExcelDataContext extends QueryPostprocessDataContext implemen
                 throw (RuntimeException) e;
             }
             throw new MetaModelException("Unexpected exception while building main schema", e);
-        } finally {
-            FileHelper.safeClose(inputStream);
         }
     }
 
@@ -209,55 +195,33 @@ public final class ExcelDataContext extends QueryPostprocessDataContext implemen
         return null;
     }
 
-    private SpreadsheetReaderDelegate getSpreadsheetReaderDelegate(Ref<InputStream> inputStream)
-            throws MetaModelException {
+    private SpreadsheetReaderDelegate getSpreadsheetReaderDelegate() throws MetaModelException {
         if (_spreadsheetReaderDelegate == null) {
             synchronized (this) {
                 if (_spreadsheetReaderDelegate == null) {
-                    try {
-                        if (POIXMLDocument.hasOOXMLHeader(inputStream.get())) {
-                            _spreadsheetReaderDelegate = new XlsxSpreadsheetReaderDelegate(_configuration);
-                        } else {
-                            _spreadsheetReaderDelegate = new DefaultSpreadsheetReaderDelegate(_configuration);
+                    _spreadsheetReaderDelegate = _resource.read(new Func<InputStream, SpreadsheetReaderDelegate>() {
+                        @Override
+                        public SpreadsheetReaderDelegate eval(InputStream in) {
+                            try {
+                                if (POIXMLDocument.hasOOXMLHeader(in)) {
+                                    return new XlsxSpreadsheetReaderDelegate(_resource, _configuration);
+                                } else {
+                                    return new DefaultSpreadsheetReaderDelegate(_resource, _configuration);
+                                }
+                            } catch (IOException e) {
+                                logger.warn("Could not identify spreadsheet type, using default", e);
+                                return new DefaultSpreadsheetReaderDelegate(_resource, _configuration);
+                            }
                         }
-                    } catch (IOException e) {
-                        logger.error("Could not identify spreadsheet type, using default", e);
-                        _spreadsheetReaderDelegate = new DefaultSpreadsheetReaderDelegate(_configuration);
-                    }
+                    });
                 }
             }
         }
         return _spreadsheetReaderDelegate;
     }
 
-    private InputStream getInputStream() throws MetaModelException {
-        InputStream inputStream = _resource.read();
-        if (!inputStream.markSupported()) {
-            inputStream = new PushbackInputStream(inputStream, 8);
-        }
-        return inputStream;
-    }
-
-    private LazyRef<InputStream> getInputStreamRef() throws MetaModelException {
-        final LazyRef<InputStream> inputStreamRef = new LazyRef<InputStream>() {
-            @Override
-            public InputStream fetch() {
-                InputStream inputStream = getInputStream();
-                return inputStream;
-            }
-        };
-        return inputStreamRef;
-    }
-
     protected void notifyTablesModified() {
-        LazyRef<InputStream> inputStreamRef = getInputStreamRef();
-        try {
-            getSpreadsheetReaderDelegate(inputStreamRef).notifyTablesModified(inputStreamRef);
-        } finally {
-            if (inputStreamRef.isFetched()) {
-                FileHelper.safeClose(inputStreamRef.get());
-            }
-        }
+        getSpreadsheetReaderDelegate().notifyTablesModified();
     }
 
     @Override
index 125c063..f99ec89 100644 (file)
  */
 package org.apache.metamodel.excel;
 
+import org.apache.metamodel.AbstractUpdateCallback;
+import org.apache.metamodel.UpdateCallback;
+import org.apache.metamodel.create.TableCreationBuilder;
+import org.apache.metamodel.data.DataSet;
+import org.apache.metamodel.data.Style.Color;
+import org.apache.metamodel.delete.RowDeletionBuilder;
+import org.apache.metamodel.drop.TableDropBuilder;
+import org.apache.metamodel.insert.RowInsertionBuilder;
+import org.apache.metamodel.schema.Schema;
+import org.apache.metamodel.schema.Table;
 import org.apache.poi.hssf.usermodel.HSSFPalette;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.util.HSSFColor;
@@ -29,16 +39,6 @@ import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.xssf.streaming.SXSSFSheet;
 import org.apache.poi.xssf.streaming.SXSSFWorkbook;
 import org.apache.poi.xssf.usermodel.XSSFWorkbook;
-import org.apache.metamodel.AbstractUpdateCallback;
-import org.apache.metamodel.UpdateCallback;
-import org.apache.metamodel.create.TableCreationBuilder;
-import org.apache.metamodel.data.DataSet;
-import org.apache.metamodel.data.Style.Color;
-import org.apache.metamodel.delete.RowDeletionBuilder;
-import org.apache.metamodel.drop.TableDropBuilder;
-import org.apache.metamodel.insert.RowInsertionBuilder;
-import org.apache.metamodel.schema.Schema;
-import org.apache.metamodel.schema.Table;
 
 final class ExcelUpdateCallback extends AbstractUpdateCallback implements UpdateCallback {
 
@@ -57,8 +57,8 @@ final class ExcelUpdateCallback extends AbstractUpdateCallback implements Update
     }
 
     @Override
-    public TableCreationBuilder createTable(Schema schema, String name) throws IllegalArgumentException,
-            IllegalStateException {
+    public TableCreationBuilder createTable(Schema schema, String name)
+            throws IllegalArgumentException, IllegalStateException {
         return new ExcelTableCreationBuilder(this, schema, name);
     }
 
@@ -78,7 +78,7 @@ final class ExcelUpdateCallback extends AbstractUpdateCallback implements Update
 
     protected void close() {
         if (_workbook != null) {
-            ExcelUtils.writeWorkbook(_dataContext, _workbook);
+            ExcelUtils.writeAndCloseWorkbook(_dataContext, _workbook);
 
             _workbook = null;
             _dateCellFormat = null;
@@ -93,7 +93,7 @@ final class ExcelUpdateCallback extends AbstractUpdateCallback implements Update
     protected Workbook getWorkbook(boolean streamingAllowed) {
         if (_workbook == null || (!streamingAllowed && _workbook instanceof SXSSFWorkbook)) {
             if (_workbook != null) {
-                ExcelUtils.writeWorkbook(_dataContext, _workbook);
+                ExcelUtils.writeAndCloseWorkbook(_dataContext, _workbook);
             }
             _workbook = ExcelUtils.readWorkbook(_dataContext);
             if (streamingAllowed && _workbook instanceof XSSFWorkbook) {
index d73721f..7504d64 100644 (file)
@@ -18,6 +18,7 @@
  */
 package org.apache.metamodel.excel;
 
+import java.io.File;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.text.NumberFormat;
@@ -60,8 +61,11 @@ import org.apache.metamodel.query.SelectItem;
 import org.apache.metamodel.schema.Table;
 import org.apache.metamodel.util.Action;
 import org.apache.metamodel.util.DateUtils;
+import org.apache.metamodel.util.FileHelper;
+import org.apache.metamodel.util.FileResource;
 import org.apache.metamodel.util.FormatHelper;
 import org.apache.metamodel.util.Func;
+import org.apache.metamodel.util.InMemoryResource;
 import org.apache.metamodel.util.Resource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -91,25 +95,35 @@ final class ExcelUtils {
         }
     }
 
-    /**
-     * Initializes a workbook instance based on a inputstream.
-     * 
-     * @return a workbook instance based on the inputstream.
-     */
-    public static Workbook readWorkbook(InputStream inputStream) {
-        try {
-            return WorkbookFactory.create(inputStream);
-        } catch (Exception e) {
-            logger.error("Could not open workbook", e);
-            throw new IllegalStateException("Could not open workbook", e);
+    public static Workbook readWorkbook(Resource resource) {
+        if (!resource.isExists()) {
+            // resource does not exist- create a blank workbook
+            if (isXlsxFile(resource)) {
+                return new SXSSFWorkbook(1000);
+            } else {
+                return new HSSFWorkbook();
+            }
+        }
+
+        if (resource instanceof FileResource) {
+            final File file = ((FileResource) resource).getFile();
+            try {
+                return WorkbookFactory.create(file);
+            } catch (Exception e) {
+                logger.error("Could not open workbook", e);
+                throw new IllegalStateException("Could not open workbook", e);
+            }
         }
-    }
 
-    public static Workbook readWorkbook(Resource resource) {
         return resource.read(new Func<InputStream, Workbook>() {
             @Override
             public Workbook eval(InputStream inputStream) {
-                return readWorkbook(inputStream);
+                try {
+                    return WorkbookFactory.create(inputStream);
+                } catch (Exception e) {
+                    logger.error("Could not open workbook", e);
+                    throw new IllegalStateException("Could not open workbook", e);
+                }
             }
         });
     }
@@ -128,24 +142,45 @@ final class ExcelUtils {
      */
     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) {
+        // first write to a temp file to avoid that workbook source is the same
+        // as the target (will cause read+write cyclic overflow)
+
+        final Resource realResource = dataContext.getResource();
+        final Resource tempResource = new InMemoryResource(realResource.getQualifiedPath());
+
+        tempResource.write(new Action<OutputStream>() {
             @Override
             public void run(OutputStream outputStream) throws Exception {
                 wb.write(outputStream);
             }
         });
+
+        if (wb instanceof HSSFWorkbook && realResource instanceof FileResource && realResource.isExists()) {
+            // TODO POI has a problem with closing a file-reference/channel
+            // after wb.write() is invoked. See POI issue to be fixed:
+            // https://bz.apache.org/bugzilla/show_bug.cgi?id=58480
+            System.gc();
+            System.runFinalization();
+            try {
+                Thread.sleep(800);
+            } catch (InterruptedException e) {
+            }
+        }
+
+        FileHelper.safeClose(wb);
+
+        FileHelper.copy(tempResource, realResource);
     }
 
     public static String getCellValue(Workbook wb, Cell cell) {
@@ -229,8 +264,8 @@ final class ExcelUtils {
         // evaluate cell first, if possible
         try {
             if (logger.isInfoEnabled()) {
-                logger.info("cell({},{}) is a formula. Attempting to evaluate: {}",
-                        new Object[] { cell.getRowIndex(), cell.getColumnIndex(), cell.getCellFormula() });
+                logger.info("cell({},{}) is a formula. Attempting to evaluate: {}", new Object[] { cell.getRowIndex(),
+                        cell.getColumnIndex(), cell.getCellFormula() });
             }
 
             final FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
@@ -240,8 +275,8 @@ final class ExcelUtils {
 
             return getCellValue(wb, evaluatedCell);
         } catch (RuntimeException e) {
-            logger.warn("Exception occurred while evaluating formula at position ({},{}): {}", new Object[] { cell.getRowIndex(),
-                    cell.getColumnIndex(), e.getMessage() });
+            logger.warn("Exception occurred while evaluating formula at position ({},{}): {}",
+                    new Object[] { cell.getRowIndex(), cell.getColumnIndex(), e.getMessage() });
             // Some exceptions we simply log - result will be then be the
             // actual formula
             if (e instanceof FormulaParseException) {
@@ -324,7 +359,8 @@ final class ExcelUtils {
                     styleBuilder.background(argb.substring(2));
                 }
             } else {
-                throw new IllegalStateException("Unexpected color type: " + (color == null ? "null" : color.getClass()) + ")");
+                throw new IllegalStateException("Unexpected color type: " + (color == null ? "null" : color.getClass())
+                        + ")");
             }
         }
 
@@ -408,6 +444,7 @@ final class ExcelUtils {
         final Iterator<Row> rowIterator = getRowIterator(sheet, configuration, true);
         if (!rowIterator.hasNext()) {
             // no more rows!
+            FileHelper.safeClose(workbook);
             return new EmptyDataSet(selectItems);
         }
 
index e18553e..1181b07 100644 (file)
  */
 package org.apache.metamodel.excel;
 
-import java.io.InputStream;
-
 import org.apache.metamodel.data.DataSet;
 import org.apache.metamodel.schema.Column;
 import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
-import org.apache.metamodel.util.Ref;
 
 /**
  * Delegate for spreadsheet-implementation specific operations in an
@@ -32,12 +29,10 @@ import org.apache.metamodel.util.Ref;
  */
 interface SpreadsheetReaderDelegate {
 
-       public void notifyTablesModified(Ref<InputStream> inputStreamRef);
+    public void notifyTablesModified();
 
-       public Schema createSchema(InputStream inputStream, String schemaName)
-                       throws Exception;
+    public Schema createSchema(String schemaName) throws Exception;
 
-       public DataSet executeQuery(InputStream inputStream, Table table,
-                       Column[] columns, int maxRows) throws Exception;
+    public DataSet executeQuery(Table table, Column[] columns, int maxRows) throws Exception;
 
 }
index 35f2b38..4e097e4 100644 (file)
@@ -25,52 +25,62 @@ import org.apache.metamodel.data.AbstractDataSet;
 import org.apache.metamodel.data.DataSet;
 import org.apache.metamodel.data.Row;
 import org.apache.metamodel.query.SelectItem;
+import org.apache.metamodel.util.FileHelper;
 
 /**
  * Stream {@link DataSet} implementation for Excel support.
  */
 final class XlsDataSet extends AbstractDataSet {
 
-       private final Iterator<org.apache.poi.ss.usermodel.Row> _rowIterator;
-       private final Workbook _workbook;
+    private final Iterator<org.apache.poi.ss.usermodel.Row> _rowIterator;
+    private final Workbook _workbook;
 
-       private volatile org.apache.poi.ss.usermodel.Row _row;
-       private volatile boolean _closed;
+    private volatile org.apache.poi.ss.usermodel.Row _row;
+    private volatile boolean _closed;
 
-       /**
-        * Creates an XLS dataset
-        
-        * @param selectItems
-        *            the selectitems representing the columns of the table
-        * @param workbook
-        * @param rowIterator
-        */
-       public XlsDataSet(SelectItem[] selectItems, Workbook workbook,
-                       Iterator<org.apache.poi.ss.usermodel.Row> rowIterator) {
-           super(selectItems);
-               _workbook = workbook;
-               _rowIterator = rowIterator;
-               _closed = false;
-       }
+    /**
+     * Creates an XLS dataset
+     * 
+     * @param selectItems
+     *            the selectitems representing the columns of the table
+     * @param workbook
+     * @param rowIterator
+     */
+    public XlsDataSet(SelectItem[] selectItems, Workbook workbook,
+            Iterator<org.apache.poi.ss.usermodel.Row> rowIterator) {
+        super(selectItems);
+        _workbook = workbook;
+        _rowIterator = rowIterator;
+        _closed = false;
+    }
 
-       @Override
-       public boolean next() {
-               if (_rowIterator.hasNext()) {
-                       _row = _rowIterator.next();
-                       return true;
-               } else {
-                       _row = null;
-                       _closed = true;
-                       return false;
-               }
-       }
+    @Override
+    public boolean next() {
+        if (_rowIterator.hasNext()) {
+            _row = _rowIterator.next();
+            return true;
+        } else {
+            _row = null;
+            close();
+            return false;
+        }
+    }
 
-       @Override
-       public Row getRow() {
-               if (_closed) {
-                       return null;
-               }
+    @Override
+    public Row getRow() {
+        if (_closed) {
+            return null;
+        }
 
-               return ExcelUtils.createRow(_workbook, _row, getHeader());
-       }
+        return ExcelUtils.createRow(_workbook, _row, getHeader());
+    }
+
+    @Override
+    public void close() {
+        super.close();
+        if (!_closed) {
+            FileHelper.safeClose(_workbook);
+            _closed = true;
+        }
+    }
 }
index 85dbdb1..ab34ef6 100644 (file)
@@ -18,6 +18,9 @@
  */
 package org.apache.metamodel.excel;
 
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
@@ -38,7 +41,8 @@ import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
 import org.apache.metamodel.util.AlphabeticSequence;
 import org.apache.metamodel.util.FileHelper;
-import org.apache.metamodel.util.Ref;
+import org.apache.metamodel.util.FileResource;
+import org.apache.metamodel.util.Resource;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.xssf.eventusermodel.XSSFReader;
 import org.slf4j.Logger;
@@ -55,67 +59,83 @@ final class XlsxSpreadsheetReaderDelegate implements SpreadsheetReaderDelegate {
 
     private static final Logger logger = LoggerFactory.getLogger(XlsxSpreadsheetReaderDelegate.class);
 
+    private final Resource _resource;
     private final ExcelConfiguration _configuration;
     private final Map<String, String> _tableNamesToInternalIds;
 
-    public XlsxSpreadsheetReaderDelegate(ExcelConfiguration configuration) {
+    public XlsxSpreadsheetReaderDelegate(Resource resource, ExcelConfiguration configuration) {
+        _resource = resource;
         _configuration = configuration;
         _tableNamesToInternalIds = new ConcurrentHashMap<String, String>();
     }
 
     @Override
-    public DataSet executeQuery(InputStream inputStream, Table table, Column[] columns, int maxRows) throws Exception {
-        final OPCPackage pkg = OPCPackage.open(inputStream);
+    public DataSet executeQuery(Table table, Column[] columns, int maxRows) throws Exception {
+        final OPCPackage pkg = openOPCPackage();
         final XSSFReader xssfReader = new XSSFReader(pkg);
         final String relationshipId = _tableNamesToInternalIds.get(table.getName());
-        
+
         if (relationshipId == null) {
             throw new IllegalStateException("No internal relationshipId found for table: " + table);
         }
 
-        return buildDataSet(columns, maxRows, relationshipId, xssfReader);
+        return buildDataSet(columns, maxRows, relationshipId, xssfReader, pkg);
+    }
+
+    private OPCPackage openOPCPackage() throws Exception {
+        if (_resource instanceof FileResource) {
+            final File file = ((FileResource) _resource).getFile();
+            return OPCPackage.open(file);
+        }
+
+        return OPCPackage.open(_resource.read());
     }
 
     @Override
-    public Schema createSchema(InputStream inputStream, String schemaName) throws Exception {
+    public Schema createSchema(String schemaName) throws Exception {
         final MutableSchema schema = new MutableSchema(schemaName);
-        final OPCPackage pkg = OPCPackage.open(inputStream);
-        final XSSFReader xssfReader = new XSSFReader(pkg);
+        final OPCPackage pkg = openOPCPackage();
+        try {
+            final XSSFReader xssfReader = new XSSFReader(pkg);
 
-        final XlsxWorkbookToTablesHandler workbookToTables = new XlsxWorkbookToTablesHandler(schema,
-                _tableNamesToInternalIds);
-        buildTables(xssfReader, workbookToTables);
+            final XlsxWorkbookToTablesHandler workbookToTables = new XlsxWorkbookToTablesHandler(schema,
+                    _tableNamesToInternalIds);
+            buildTables(xssfReader, workbookToTables);
 
-        for (Entry<String, String> entry : _tableNamesToInternalIds.entrySet()) {
+            for (Entry<String, String> entry : _tableNamesToInternalIds.entrySet()) {
 
-            final String tableName = entry.getKey();
-            final String relationshipId = entry.getValue();
+                final String tableName = entry.getKey();
+                final String relationshipId = entry.getValue();
 
-            final MutableTable table = (MutableTable) schema.getTableByName(tableName);
+                final MutableTable table = (MutableTable) schema.getTableByName(tableName);
 
-            buildColumns(table, relationshipId, xssfReader);
+                buildColumns(table, relationshipId, xssfReader);
+            }
+        } finally {
+            pkg.revert();
         }
         return schema;
     }
 
     @Override
-    public void notifyTablesModified(Ref<InputStream> inputStreamRef) {
-        InputStream inputStream = inputStreamRef.get();
+    public void notifyTablesModified() {
         final XlsxWorkbookToTablesHandler workbookToTables = new XlsxWorkbookToTablesHandler(null,
                 _tableNamesToInternalIds);
         try {
-            final OPCPackage pkg = OPCPackage.open(inputStream);
-            final XSSFReader xssfReader = new XSSFReader(pkg);
-            buildTables(xssfReader, workbookToTables);
+            final OPCPackage pkg = openOPCPackage();
+            try {
+                final XSSFReader xssfReader = new XSSFReader(pkg);
+                buildTables(xssfReader, workbookToTables);
+            } finally {
+                pkg.revert();
+            }
         } catch (Exception e) {
             throw new IllegalStateException(e);
-        } finally {
-            FileHelper.safeClose(inputStream);
         }
     }
 
     private DataSet buildDataSet(final Column[] columns, int maxRows, final String relationshipId,
-            final XSSFReader xssfReader) throws Exception {
+            final XSSFReader xssfReader, final OPCPackage pkg) throws Exception {
 
         List<SelectItem> selectItems = new ArrayList<SelectItem>(columns.length);
         for (Column column : columns) {
@@ -124,7 +144,13 @@ final class XlsxSpreadsheetReaderDelegate implements SpreadsheetReaderDelegate {
         final XlsxRowPublisherAction publishAction = new XlsxRowPublisherAction(_configuration, columns,
                 relationshipId, xssfReader);
 
-        return new RowPublisherDataSet(selectItems.toArray(new SelectItem[selectItems.size()]), maxRows, publishAction);
+        return new RowPublisherDataSet(selectItems.toArray(new SelectItem[selectItems.size()]), maxRows, publishAction,
+                new Closeable() {
+                    @Override
+                    public void close() throws IOException {
+                        pkg.revert();
+                    }
+                });
     }
 
     private void buildColumns(final MutableTable table, final String relationshipId, final XSSFReader xssfReader)
index fdf1e67..a2e4395 100644 (file)
@@ -21,8 +21,6 @@ package org.apache.metamodel.excel;
 import java.io.File;
 import java.lang.reflect.Field;
 
-import junit.framework.TestCase;
-
 import org.apache.metamodel.data.DataSet;
 import org.apache.metamodel.data.Row;
 import org.apache.metamodel.data.Style;
@@ -31,214 +29,199 @@ import org.apache.metamodel.schema.Column;
 import org.apache.metamodel.schema.Schema;
 import org.apache.metamodel.schema.Table;
 
+import junit.framework.TestCase;
+
 public class DefaultSpreadsheetReaderDelegateTest extends TestCase {
 
-       public void testReadAllTestResourceFiles() {
-               File[] listFiles = new File("src/test/resources").listFiles();
-               for (File file : listFiles) {
-                       if (file.isFile() && file.getName().indexOf(".xls") != -1) {
-                               try {
-                                       runTest(file);
-                               } catch (Throwable e) {
-                                       throw new IllegalStateException("Exception in file: "
-                                                       + file, e);
-                               }
-                       }
-               }
-       }
-
-       private void runTest(File file) throws Exception {
-               ExcelDataContext mainDataContext = new ExcelDataContext(file);
-               applyReaderDelegate(mainDataContext);
-
-               ExcelDataContext comparedDataContext = null;
-               if (file.getName().endsWith(".xlsx")) {
-                       comparedDataContext = new ExcelDataContext(file);
-               }
-
-               Schema schema = mainDataContext.getDefaultSchema();
-               assertNotNull(schema);
-               assertEquals(file.getName(), schema.getName());
-
-               if (comparedDataContext != null) {
-                       assertEquals(comparedDataContext.getDefaultSchema().getName(),
-                                       schema.getName());
-               }
-
-               assertEquals(DefaultSpreadsheetReaderDelegate.class,
-                               mainDataContext.getSpreadsheetReaderDelegateClass());
-
-               Table[] tables = schema.getTables();
-               assertTrue(tables.length > 0);
-
-               Table[] comparedTables = null;
-               if (comparedDataContext != null) {
-                       assertEquals(XlsxSpreadsheetReaderDelegate.class,
-                                       comparedDataContext.getSpreadsheetReaderDelegateClass());
-                       comparedTables = comparedDataContext.getDefaultSchema().getTables();
-                       assertEquals(comparedTables.length, tables.length);
-               }
-
-               for (int i = 0; i < tables.length; i++) {
-                       Table table = tables[i];
-                       Column[] columns = table.getColumns();
-                       Query query = mainDataContext.query().from(table).select(columns)
-                                       .toQuery();
-                       DataSet dataSet = mainDataContext.executeQuery(query);
-
-                       DataSet comparedDataSet = null;
-                       if (comparedDataContext != null) {
-                               Table comparedTable = comparedTables[i];
-                               assertEquals(comparedTable.getName(), table.getName());
-                               assertEquals(comparedTable.getColumnCount(),
-                                               table.getColumnCount());
-
-                               Column[] comparedColumns = comparedTable.getColumns();
-                               for (int j = 0; j < comparedColumns.length; j++) {
-                                       assertEquals(columns[j].getColumnNumber(),
-                                                       comparedColumns[j].getColumnNumber());
-                               }
-
-                               Query comparedQuery = comparedDataContext.query()
-                                               .from(comparedTable).select(comparedColumns).toQuery();
-                               comparedDataSet = comparedDataContext
-                                               .executeQuery(comparedQuery);
-                       }
-
-                       while (dataSet.next()) {
-                               Row row = dataSet.getRow();
-                               assertNotNull(row);
-                               Object[] values = row.getValues();
-
-                               assertEquals(values.length, table.getColumnCount());
-
-                               if (comparedDataSet != null) {
-                                       boolean next = comparedDataSet.next();
-                                       assertTrue("No comparable row exists for: " + row, next);
-                                       Row comparedRow = comparedDataSet.getRow();
-                                       assertNotNull(comparedRow);
-                                       Object[] comparedValues = comparedRow.getValues();
-                                       assertEquals(comparedValues.length, table.getColumnCount());
-
-                                       for (int j = 0; j < comparedValues.length; j++) {
-                                               assertEquals(comparedValues[j], values[j]);
-                                       }
-
-                                       // compare styles
-                                       for (int j = 0; j < comparedValues.length; j++) {
-                                               Style style1 = comparedRow.getStyle(j);
-                                               Style style2 = row.getStyle(j);
-                                               assertEquals("Diff in style on row: " + row
-                                                               + " (value index = " + j + ")\nStyle 1: "
-                                                               + style1 + "\nStyle 2: " + style2 + ". ",
-                                                               style1, style2);
-                                       }
-                               }
-                       }
-                       dataSet.close();
-
-                       if (comparedDataSet != null) {
-                               assertFalse(comparedDataSet.next());
-                               comparedDataSet.close();
-                       }
-               }
-       }
-
-       /**
-        * Applies the {@link DefaultSpreadsheetReaderDelegate} through reflection.
-        * 
-        * @param dataContext
-        * @throws NoSuchFieldException
-        * @throws IllegalAccessException
-        */
-       private void applyReaderDelegate(ExcelDataContext dataContext)
-                       throws NoSuchFieldException, IllegalAccessException {
-               Field field = ExcelDataContext.class
-                               .getDeclaredField("_spreadsheetReaderDelegate");
-               assertNotNull(field);
-               field.setAccessible(true);
-               field.set(
-                               dataContext,
-                               new DefaultSpreadsheetReaderDelegate(dataContext
-                                               .getConfiguration()));
-       }
-
-       public void testStylingOfDateCell() throws Exception {
-               ExcelDataContext dc = new ExcelDataContext(new File(
-                               "src/test/resources/Spreadsheet2007.xlsx"));
-               applyReaderDelegate(dc);
-
-               Table table = dc.getDefaultSchema().getTables()[0];
-
-               final String expectedStyling = "";
-
-               DataSet dataSet = dc.query().from(table).select("date").execute();
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertFalse(dataSet.next());
-               dataSet.close();
-       }
-
-       public void testStylingOfNullCell() throws Exception {
-               ExcelDataContext dc = new ExcelDataContext(new File(
-                               "src/test/resources/formulas.xlsx"));
-               applyReaderDelegate(dc);
-
-               Table table = dc.getDefaultSchema().getTables()[0];
-
-               DataSet dataSet = dc.query().from(table).select("Foo").and("Bar")
-                               .where("Foo").isEquals("7").execute();
-               assertTrue(dataSet.next());
-               Row row = dataSet.getRow();
-               assertNotNull(row.getStyle(0));
-
-               final String expectedStyling = "";
-
-               assertEquals(expectedStyling, row.getStyle(0).toCSS());
-               assertNotNull(row.getStyle(1));
-               assertEquals(expectedStyling, row.getStyle(1).toCSS());
-               assertFalse(dataSet.next());
-               dataSet.close();
-
-               dataSet = dc.query().from(table).select("Foo").and("Bar").execute();
-               assertTrue(dataSet.next());
-               row = dataSet.getRow();
-               assertNotNull(row.getStyle(0));
-               assertEquals(expectedStyling, row.getStyle(0).toCSS());
-               assertNotNull(row.getStyle(1));
-               assertEquals(expectedStyling, row.getStyle(1).toCSS());
-
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertTrue(dataSet.next());
-               assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
-               assertFalse(dataSet.next());
-               dataSet.close();
-       }
+    public void testReadAllTestResourceFiles() {
+        File[] listFiles = new File("src/test/resources").listFiles();
+        for (File file : listFiles) {
+            if (file.isFile() && file.getName().indexOf(".xls") != -1) {
+                try {
+                    runTest(file);
+                } catch (Throwable e) {
+                    throw new IllegalStateException("Exception in file: " + file, e);
+                }
+            }
+        }
+    }
+
+    private void runTest(File file) throws Exception {
+        ExcelDataContext mainDataContext = new ExcelDataContext(file);
+        applyReaderDelegate(mainDataContext);
+
+        ExcelDataContext comparedDataContext = null;
+        if (file.getName().endsWith(".xlsx")) {
+            comparedDataContext = new ExcelDataContext(file);
+        }
+
+        Schema schema = mainDataContext.getDefaultSchema();
+        assertNotNull(schema);
+        assertEquals(file.getName(), schema.getName());
+
+        if (comparedDataContext != null) {
+            assertEquals(comparedDataContext.getDefaultSchema().getName(), schema.getName());
+        }
+
+        assertEquals(DefaultSpreadsheetReaderDelegate.class, mainDataContext.getSpreadsheetReaderDelegateClass());
+
+        Table[] tables = schema.getTables();
+        assertTrue(tables.length > 0);
+
+        Table[] comparedTables = null;
+        if (comparedDataContext != null) {
+            assertEquals(XlsxSpreadsheetReaderDelegate.class, comparedDataContext.getSpreadsheetReaderDelegateClass());
+            comparedTables = comparedDataContext.getDefaultSchema().getTables();
+            assertEquals(comparedTables.length, tables.length);
+        }
+
+        for (int i = 0; i < tables.length; i++) {
+            Table table = tables[i];
+            Column[] columns = table.getColumns();
+            Query query = mainDataContext.query().from(table).select(columns).toQuery();
+            DataSet dataSet = mainDataContext.executeQuery(query);
+
+            DataSet comparedDataSet = null;
+            if (comparedDataContext != null) {
+                Table comparedTable = comparedTables[i];
+                assertEquals(comparedTable.getName(), table.getName());
+                assertEquals(comparedTable.getColumnCount(), table.getColumnCount());
+
+                Column[] comparedColumns = comparedTable.getColumns();
+                for (int j = 0; j < comparedColumns.length; j++) {
+                    assertEquals(columns[j].getColumnNumber(), comparedColumns[j].getColumnNumber());
+                }
+
+                Query comparedQuery = comparedDataContext.query().from(comparedTable).select(comparedColumns).toQuery();
+                comparedDataSet = comparedDataContext.executeQuery(comparedQuery);
+            }
+
+            while (dataSet.next()) {
+                Row row = dataSet.getRow();
+                assertNotNull(row);
+                Object[] values = row.getValues();
+
+                assertEquals(values.length, table.getColumnCount());
+
+                if (comparedDataSet != null) {
+                    boolean next = comparedDataSet.next();
+                    assertTrue("No comparable row exists for: " + row, next);
+                    Row comparedRow = comparedDataSet.getRow();
+                    assertNotNull(comparedRow);
+                    Object[] comparedValues = comparedRow.getValues();
+                    assertEquals(comparedValues.length, table.getColumnCount());
+
+                    for (int j = 0; j < comparedValues.length; j++) {
+                        assertEquals(comparedValues[j], values[j]);
+                    }
+
+                    // compare styles
+                    for (int j = 0; j < comparedValues.length; j++) {
+                        Style style1 = comparedRow.getStyle(j);
+                        Style style2 = row.getStyle(j);
+                        assertEquals("Diff in style on row: " + row + " (value index = " + j + ")\nStyle 1: " + style1
+                                + "\nStyle 2: " + style2 + ". ", style1, style2);
+                    }
+                }
+            }
+            dataSet.close();
+
+            if (comparedDataSet != null) {
+                assertFalse(comparedDataSet.next());
+                comparedDataSet.close();
+            }
+        }
+    }
+
+    /**
+     * Applies the {@link DefaultSpreadsheetReaderDelegate} through reflection.
+     * 
+     * @param dataContext
+     * @throws NoSuchFieldException
+     * @throws IllegalAccessException
+     */
+    private void applyReaderDelegate(ExcelDataContext dataContext) throws NoSuchFieldException, IllegalAccessException {
+        final SpreadsheetReaderDelegate delegate = new DefaultSpreadsheetReaderDelegate(dataContext.getResource(),
+                dataContext.getConfiguration());
+        final Field field = ExcelDataContext.class.getDeclaredField("_spreadsheetReaderDelegate");
+        assertNotNull(field);
+        field.setAccessible(true);
+        field.set(dataContext, delegate);
+    }
+
+    public void testStylingOfDateCell() throws Exception {
+        ExcelDataContext dc = new ExcelDataContext(new File("src/test/resources/Spreadsheet2007.xlsx"));
+        applyReaderDelegate(dc);
+
+        Table table = dc.getDefaultSchema().getTables()[0];
+
+        final String expectedStyling = "";
+
+        DataSet dataSet = dc.query().from(table).select("date").execute();
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertFalse(dataSet.next());
+        dataSet.close();
+    }
+
+    public void testStylingOfNullCell() throws Exception {
+        ExcelDataContext dc = new ExcelDataContext(new File("src/test/resources/formulas.xlsx"));
+        applyReaderDelegate(dc);
+
+        Table table = dc.getDefaultSchema().getTables()[0];
+
+        DataSet dataSet = dc.query().from(table).select("Foo").and("Bar").where("Foo").isEquals("7").execute();
+        assertTrue(dataSet.next());
+        Row row = dataSet.getRow();
+        assertNotNull(row.getStyle(0));
+
+        final String expectedStyling = "";
+
+        assertEquals(expectedStyling, row.getStyle(0).toCSS());
+        assertNotNull(row.getStyle(1));
+        assertEquals(expectedStyling, row.getStyle(1).toCSS());
+        assertFalse(dataSet.next());
+        dataSet.close();
+
+        dataSet = dc.query().from(table).select("Foo").and("Bar").execute();
+        assertTrue(dataSet.next());
+        row = dataSet.getRow();
+        assertNotNull(row.getStyle(0));
+        assertEquals(expectedStyling, row.getStyle(0).toCSS());
+        assertNotNull(row.getStyle(1));
+        assertEquals(expectedStyling, row.getStyle(1).toCSS());
+
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertTrue(dataSet.next());
+        assertEquals(expectedStyling, dataSet.getRow().getStyle(0).toCSS());
+        assertFalse(dataSet.next());
+        dataSet.close();
+    }
 }
index c7c3696..dd27c31 100644 (file)
@@ -526,7 +526,7 @@ public class ExcelDataContextTest extends TestCase {
     }
 
     public void testInsertInto() throws Exception {
-        File file = new File("target/xls_people_modified.xls");
+        final File file = new File("target/xls_people_modified.xls");
 
         if (file.exists()) {
             assertTrue(file.delete());
@@ -536,9 +536,10 @@ public class ExcelDataContextTest extends TestCase {
 
         assertTrue(file.exists());
 
-        ExcelDataContext dc = new ExcelDataContext(file);
+        final ExcelDataContext dc = new ExcelDataContext(file);
         final Table table = dc.getDefaultSchema().getTables()[0];
         final Column nameColumn = table.getColumnByName("name");
+
         dc.executeUpdate(new UpdateScript() {
             @Override
             public void run(UpdateCallback cb) {
index 3ac116b..97101bd 100644 (file)
@@ -74,7 +74,7 @@ public class ExcelUpdateCallbackTest extends TestCase {
 
                        assertEquals(1000, rows.size());
 
-                       ExcelUtils.writeWorkbook(dc, sheet.getWorkbook());
+                       ExcelUtils.writeAndCloseWorkbook(dc, sheet.getWorkbook());
                }
 
                assertTrue("Usually the file size will be circa 42000, but it was: "
diff --git a/pom.xml b/pom.xml
index 6d9d862..92d7fa6 100644 (file)
--- a/pom.xml
+++ b/pom.xml
@@ -693,6 +693,12 @@ under the License.
                        </dependency>
                        <dependency>
                                <groupId>org.slf4j</groupId>
+                               <artifactId>slf4j-simple</artifactId>
+                               <version>${slf4j.version}</version>
+                               <scope>test</scope>
+                       </dependency>
+                       <dependency>
+                               <groupId>org.slf4j</groupId>
                                <artifactId>slf4j-log4j12</artifactId>
                                <version>${slf4j.version}</version>
                                <scope>test</scope>