From 3b8055baa0a26e48c8e5cfecbe6120a147da7777 Mon Sep 17 00:00:00 2001 From: Vladislav Galas Date: Thu, 3 Jan 2019 00:08:52 +0000 Subject: [PATCH] Bug 62993: XSSFEvaluationSheet now retrieves valid last row index from underlying XSSFSheet. Thanks to Axel Howind. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1850212 13f79535-47bb-0310-9956-ffa450edef68 --- .../hssf/usermodel/HSSFEvaluationSheet.java | 5 +- .../xssf/streaming/SXSSFEvaluationSheet.java | 5 +- .../xssf/usermodel/XSSFEvaluationSheet.java | 11 ++--- .../apache/poi/xssf/usermodel/XSSFSheet.java | 6 ++- .../streaming/TestSXSSFEvaluationSheet.java | 32 ++++++++++++ .../usermodel/TestXSSFEvaluationSheet.java | 18 ++++++- .../usermodel/TestHSSFEvaluationSheet.java | 33 +++++++++++++ .../usermodel/BaseTestXEvaluationSheet.java | 49 +++++++++++++++++++ 8 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFEvaluationSheet.java create mode 100644 src/testcases/org/apache/poi/hssf/usermodel/TestHSSFEvaluationSheet.java create mode 100644 src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java index 6d5729446d..d2d6927c82 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java @@ -28,11 +28,9 @@ import org.apache.poi.util.Internal; final class HSSFEvaluationSheet implements EvaluationSheet { private final HSSFSheet _hs; - private int _lastDefinedRow = -1; public HSSFEvaluationSheet(HSSFSheet hs) { _hs = hs; - _lastDefinedRow = _hs.getLastRowNum(); } public HSSFSheet getHSSFSheet() { @@ -45,7 +43,7 @@ final class HSSFEvaluationSheet implements EvaluationSheet { */ @Override public int getLastRowNum() { - return _lastDefinedRow; + return _hs.getLastRowNum(); } @Override @@ -66,6 +64,5 @@ final class HSSFEvaluationSheet implements EvaluationSheet { */ @Override public void clearAllCachedResultValues() { - _lastDefinedRow = _hs.getLastRowNum(); } } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java index 882150c903..bb1d1f1698 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java @@ -27,11 +27,9 @@ import org.apache.poi.util.Internal; @Internal final class SXSSFEvaluationSheet implements EvaluationSheet { private final SXSSFSheet _xs; - private int _lastDefinedRow = -1; public SXSSFEvaluationSheet(SXSSFSheet sheet) { _xs = sheet; - _lastDefinedRow = _xs.getLastRowNum(); } public SXSSFSheet getSXSSFSheet() { @@ -44,7 +42,7 @@ final class SXSSFEvaluationSheet implements EvaluationSheet { */ @Override public int getLastRowNum() { - return _lastDefinedRow; + return _xs.getLastRowNum(); } @Override @@ -68,6 +66,5 @@ final class SXSSFEvaluationSheet implements EvaluationSheet { */ @Override public void clearAllCachedResultValues() { - _lastDefinedRow = _xs.getLastRowNum(); } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java index c8c3b9d173..74ac23215a 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java @@ -34,11 +34,9 @@ final class XSSFEvaluationSheet implements EvaluationSheet { private final XSSFSheet _xs; private Map _cellCache; - private int _lastDefinedRow = -1; public XSSFEvaluationSheet(XSSFSheet sheet) { _xs = sheet; - _lastDefinedRow = _xs.getLastRowNum(); } public XSSFSheet getXSSFSheet() { @@ -51,7 +49,7 @@ final class XSSFEvaluationSheet implements EvaluationSheet { */ @Override public int getLastRowNum() { - return _lastDefinedRow; + return _xs.getLastRowNum(); } /* (non-JavaDoc), inherit JavaDoc from EvaluationWorkbook @@ -60,15 +58,16 @@ final class XSSFEvaluationSheet implements EvaluationSheet { @Override public void clearAllCachedResultValues() { _cellCache = null; - _lastDefinedRow = _xs.getLastRowNum(); } @Override public EvaluationCell getCell(int rowIndex, int columnIndex) { // shortcut evaluation if reference is outside the bounds of existing data // see issue #61841 for impact on VLOOKUP in particular - if (rowIndex > _lastDefinedRow) return null; - + if (rowIndex > getLastRowNum()) { + return null; + } + // cache for performance: ~30% speedup due to caching if (_cellCache == null) { _cellCache = new HashMap<>(_xs.getLastRowNum() * 3); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index 555968c42a..d95decccef 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -27,7 +27,6 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -131,7 +130,6 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTSheetViews; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTTablePart; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTTableParts; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheetSource; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCalcMode; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellFormulaType; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPane; @@ -1211,6 +1209,10 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { @Override public int getLastRowNum() { + // _rows.getLastKey() (O(logN)) or caching last row (O(1))? + // A test with 1_000_000 rows shows that querying getLastRowNum with lastKey() implementation takes ~40 ms, + // and ~1.2 ms with cached implementation. 40 ms is negligible compared to the time of evaluation a million + // cells, and the lastKey implementation is much more elegant and less error prone than caching. return _rows.isEmpty() ? 0 : _rows.lastKey(); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFEvaluationSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFEvaluationSheet.java new file mode 100644 index 0000000000..1c20bb815e --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFEvaluationSheet.java @@ -0,0 +1,32 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.xssf.streaming; + +import org.apache.poi.ss.formula.EvaluationSheet; +import org.apache.poi.ss.usermodel.BaseTestXEvaluationSheet; +import org.apache.poi.ss.usermodel.Sheet; + +import java.util.AbstractMap; +import java.util.Map; + +public class TestSXSSFEvaluationSheet extends BaseTestXEvaluationSheet { + @Override + protected Map.Entry getInstance() { + SXSSFSheet sheet = new SXSSFWorkbook().createSheet(); + return new AbstractMap.SimpleEntry<>(sheet, new SXSSFEvaluationSheet(sheet)); + } +} diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFEvaluationSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFEvaluationSheet.java index 1b1417fbb3..fdb8576361 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFEvaluationSheet.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFEvaluationSheet.java @@ -16,11 +16,19 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel; +import org.apache.poi.ss.formula.EvaluationSheet; +import org.apache.poi.ss.usermodel.BaseTestXEvaluationSheet; +import org.apache.poi.ss.usermodel.Sheet; import org.junit.Test; +import java.util.AbstractMap; +import java.util.Map; + import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; -public class TestXSSFEvaluationSheet { +public class TestXSSFEvaluationSheet extends BaseTestXEvaluationSheet { @Test public void test() throws Exception { @@ -52,4 +60,10 @@ public class TestXSSFEvaluationSheet { // other things assertEquals(sheet, evalsheet.getXSSFSheet()); } -} \ No newline at end of file + + @Override + protected Map.Entry getInstance() { + XSSFSheet sheet = new XSSFWorkbook().createSheet(); + return new AbstractMap.SimpleEntry<>(sheet, new XSSFEvaluationSheet(sheet)); + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFEvaluationSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFEvaluationSheet.java new file mode 100644 index 0000000000..81700ae56a --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFEvaluationSheet.java @@ -0,0 +1,33 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.usermodel; + +import org.apache.poi.ss.formula.EvaluationSheet; +import org.apache.poi.ss.usermodel.BaseTestXEvaluationSheet; +import org.apache.poi.ss.usermodel.Sheet; + +import java.util.AbstractMap; +import java.util.Map; + +public class TestHSSFEvaluationSheet extends BaseTestXEvaluationSheet { + @Override + protected Map.Entry getInstance() { + HSSFSheet sheet = new HSSFWorkbook().createSheet(); + return new AbstractMap.SimpleEntry<>(sheet, new HSSFEvaluationSheet(sheet)); + } +} diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java new file mode 100644 index 0000000000..cd60b3f6ff --- /dev/null +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java @@ -0,0 +1,49 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.ss.usermodel; + +import org.apache.poi.ss.formula.EvaluationSheet; +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public abstract class BaseTestXEvaluationSheet { + /** + * Get a pair of underlying sheet and evaluation sheet. + */ + protected abstract Map.Entry getInstance(); + + @Test + public void lastRowNumIsUpdatedFromUnderlyingSheet_bug62993() { + Map.Entry sheetPair = getInstance(); + Sheet underlyingSheet = sheetPair.getKey(); + EvaluationSheet instance = sheetPair.getValue(); + + assertEquals(0, instance.getLastRowNum()); + + underlyingSheet.createRow(0); + underlyingSheet.createRow(1); + underlyingSheet.createRow(2); + assertEquals(2, instance.getLastRowNum()); + + underlyingSheet.removeRow(underlyingSheet.getRow(2)); + assertEquals(1, instance.getLastRowNum()); + } +} -- 2.39.5