From 38e7fe63a89207e02da67743a0894b1873b21a1d Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 11 Jan 2025 09:23:46 +0000 Subject: [PATCH] Apply IDE suggestions, code-formating, tests, ... Add test for DefaultTempFileCreationStrategy Adjust comments, add test, improve error message git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1923054 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/record/RecordFactory.java | 4 +- .../poi/hssf/record/RecordInputStream.java | 3 +- .../ss/formula/functions/TextFunction.java | 13 +- .../poi/ss/usermodel/DataFormatter.java | 4 +- .../poi/hssf/record/TestRecordFactory.java | 32 +++- .../DefaultTempFileCreationStrategyTest.java | 149 ++++++++++++++++++ 6 files changed, 192 insertions(+), 13 deletions(-) create mode 100644 poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java index 16f1a04d04..08b49f5be4 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java @@ -34,8 +34,8 @@ import org.apache.poi.util.RecordFormatException; public final class RecordFactory { private static final int NUM_RECORDS = 512; - // how many records we read at max by default (can be adjusted via IOUtils) - //increased to 5 million due to https://bz.apache.org/bugzilla/show_bug.cgi?id=65887 + // how many records we read at max by default (can be adjusted via the static setters) + // increased to 5 million due to https://bz.apache.org/bugzilla/show_bug.cgi?id=65887 private static final int DEFAULT_MAX_NUMBER_OF_RECORDS = 5_000_000; private static int MAX_NUMBER_OF_RECORDS = DEFAULT_MAX_NUMBER_OF_RECORDS; diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java index 06420f394b..561768f32b 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java @@ -216,7 +216,8 @@ public final class RecordInputStream implements LittleEndianInput { _currentDataLength = _bhi.readDataSize(); if (_currentDataLength > MAX_RECORD_DATA_SIZE) { throw new RecordFormatException("The content of an excel record cannot exceed " - + MAX_RECORD_DATA_SIZE + " bytes"); + + MAX_RECORD_DATA_SIZE + " bytes, but had: " + _currentDataLength + + " for record with sid: " + _currentSid); } } diff --git a/poi/src/main/java/org/apache/poi/ss/formula/functions/TextFunction.java b/poi/src/main/java/org/apache/poi/ss/formula/functions/TextFunction.java index 518fe08e0c..5a2b613c93 100644 --- a/poi/src/main/java/org/apache/poi/ss/formula/functions/TextFunction.java +++ b/poi/src/main/java/org/apache/poi/ss/formula/functions/TextFunction.java @@ -108,12 +108,14 @@ public abstract class TextFunction implements Function { return new NumberEval(arg.length()); } }; + public static final Function LOWER = new SingleArgTextFunc() { @Override protected ValueEval evaluate(String arg) { return new StringEval(arg.toLowerCase(Locale.ROOT)); } }; + public static final Function UPPER = new SingleArgTextFunc() { @Override protected ValueEval evaluate(String arg) { @@ -246,13 +248,16 @@ public abstract class TextFunction implements Function { private static final class LeftRight extends Var1or2ArgFunction { private static final ValueEval DEFAULT_ARG1 = new NumberEval(1.0); private final boolean _isLeft; + protected LeftRight(boolean isLeft) { _isLeft = isLeft; } + @Override public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0) { return evaluate(srcRowIndex, srcColumnIndex, arg0, DEFAULT_ARG1); } + @Override public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0, ValueEval arg1) { @@ -369,7 +374,6 @@ public abstract class TextFunction implements Function { try { valueDouble = DateUtil.parseDateTime(evaluated); } catch (Exception ignored) { - valueDouble = null; } } } @@ -393,7 +397,7 @@ public abstract class TextFunction implements Function { * Using it instead of {@link OperandResolver#coerceValueToString(ValueEval)} in order to handle booleans differently. */ private String formatPatternValueEval2String(ValueEval ve) { - String format = null; + final String format; if (!(ve instanceof BoolEval) && (ve instanceof StringValueEval)) { StringValueEval sve = (StringValueEval) ve; format = sve.getStringValue(); @@ -414,6 +418,7 @@ public abstract class TextFunction implements Function { public SearchFind(boolean isCaseSensitive) { _isCaseSensitive = isCaseSensitive; } + @Override public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0, ValueEval arg1) { try { @@ -424,6 +429,7 @@ public abstract class TextFunction implements Function { return e.getErrorEval(); } } + @Override public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0, ValueEval arg1, ValueEval arg2) { @@ -440,6 +446,7 @@ public abstract class TextFunction implements Function { return e.getErrorEval(); } } + private ValueEval eval(String haystack, String needle, int startIndex) { int result; if (_isCaseSensitive) { @@ -454,6 +461,7 @@ public abstract class TextFunction implements Function { return new NumberEval(result + 1.); } } + /** * Implementation of the FIND() function.

* @@ -468,6 +476,7 @@ public abstract class TextFunction implements Function { * Author: Torstein Tauno Svendsen (torstei@officenet.no) */ public static final Function FIND = new SearchFind(true); + /** * Implementation of the FIND() function.

* diff --git a/poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java b/poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java index b3de0462b0..95b0243bce 100644 --- a/poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java +++ b/poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java @@ -380,7 +380,7 @@ public class DataFormatter { // String formatStr = (i < formatBits.length) ? formatBits[i] : formatBits[0]; // this replace is done to fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63211 - String formatStr = formatStrIn.replace("\\%", "\'%\'"); + String formatStr = formatStrIn.replace("\\%", "'%'"); // Excel supports 2+ part conditional data formats, eg positive/negative/zero, // or (>1000),(>0),(0),(negative). As Java doesn't handle these kinds @@ -700,7 +700,7 @@ public class DataFormatter { private String cleanFormatForNumber(String formatStrIn) { // this replace is done to fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63211 - String formatStr = formatStrIn.replace("\\%", "\'%\'"); + String formatStr = formatStrIn.replace("\\%", "'%'"); StringBuilder sb = new StringBuilder(formatStr); diff --git a/poi/src/test/java/org/apache/poi/hssf/record/TestRecordFactory.java b/poi/src/test/java/org/apache/poi/hssf/record/TestRecordFactory.java index 26793e1930..56ad402776 100644 --- a/poi/src/test/java/org/apache/poi/hssf/record/TestRecordFactory.java +++ b/poi/src/test/java/org/apache/poi/hssf/record/TestRecordFactory.java @@ -19,7 +19,8 @@ package org.apache.poi.hssf.record; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -27,8 +28,10 @@ import java.io.InputStream; import java.util.List; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.HexRead; +import org.apache.poi.util.RecordFormatException; import org.junit.jupiter.api.Test; /** @@ -178,11 +181,11 @@ final class TestRecordFactory { List records = RecordFactory.createRecords(new ByteArrayInputStream(data)); assertEquals(5, records.size()); - assertTrue(records.get(0) instanceof ObjRecord); - assertTrue(records.get(1) instanceof DrawingRecord); - assertTrue(records.get(2) instanceof TextObjectRecord); - assertTrue(records.get(3) instanceof ContinueRecord); - assertTrue(records.get(4) instanceof ObjRecord); + assertInstanceOf(ObjRecord.class, records.get(0)); + assertInstanceOf(DrawingRecord.class, records.get(1)); + assertInstanceOf(TextObjectRecord.class, records.get(2)); + assertInstanceOf(ContinueRecord.class, records.get(3)); + assertInstanceOf(ObjRecord.class, records.get(4)); //serialize and verify that the serialized data is the same as the original UnsynchronizedByteArrayOutputStream out = UnsynchronizedByteArrayOutputStream.builder().get(); @@ -231,4 +234,21 @@ final class TestRecordFactory { assertEquals(5, outRecs.size()); } } + + @Test + void testMaxNumberOfRecords() { + int prev = RecordFactory.getMaxNumberOfRecords(); + + try { + // check setter/getter + RecordFactory.setMaxNumberOfRecords(0); + assertEquals(0, RecordFactory.getMaxNumberOfRecords()); + + // check exception when exceeding the limit + assertThrows(RecordFormatException.class, + () -> HSSFTestDataSamples.openSampleWorkbook("SampleSS.xls")); + } finally { + RecordFactory.setMaxNumberOfRecords(prev); + } + } } diff --git a/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java b/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java new file mode 100644 index 0000000000..4e4f6b779e --- /dev/null +++ b/poi/src/test/java/org/apache/poi/util/DefaultTempFileCreationStrategyTest.java @@ -0,0 +1,149 @@ +/* ==================================================================== + 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.util; + +import static org.apache.poi.util.DefaultTempFileCreationStrategy.DELETE_FILES_ON_EXIT; +import static org.apache.poi.util.DefaultTempFileCreationStrategy.POIFILES; +import static org.apache.poi.util.TempFile.JAVA_IO_TMPDIR; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.io.IOException; + +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class DefaultTempFileCreationStrategyTest { + + private String propBefore; + private String tmpBefore; + + @BeforeEach + void before() { + propBefore = System.getProperty(DELETE_FILES_ON_EXIT); + tmpBefore = System.getProperty(JAVA_IO_TMPDIR); + } + + @AfterEach + void after() { + if (propBefore == null) { + System.clearProperty(DELETE_FILES_ON_EXIT); + } else { + System.setProperty(DELETE_FILES_ON_EXIT, propBefore); + } + + System.setProperty(JAVA_IO_TMPDIR, tmpBefore); + } + + @Test + void testDefaultFile() throws IOException { + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(); + checkGetFile(strategy); + } + + private static void checkGetFile(DefaultTempFileCreationStrategy strategy) throws IOException { + File file = strategy.createTempFile("POITest", ".tmp"); + try { + assertTrue(file.getParentFile().exists(), + "Failed for " + file.getParentFile()); + + assertTrue(file.exists(), + "Failed for " + file); + } finally { + assertTrue(file.delete()); + } + } + + @Test + void testDefaultDir() throws IOException { + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(); + File dir = strategy.createTempDirectory("POITest"); + try { + assertTrue(dir.getParentFile().exists(), + "Failed for " + dir.getParentFile()); + + assertTrue(dir.exists(), + "Failed for " + dir); + } finally { + assertTrue(dir.delete()); + } + } + + @Test + void testWithProperty() throws IOException { + System.setProperty(DELETE_FILES_ON_EXIT, "true"); + + // we can set the property, but not easily check if it works + // so let's just call the main method + testDefaultFile(); + } + + @Test + void testEmptyTempDir() { + System.clearProperty(JAVA_IO_TMPDIR); + + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(); + assertThrows(IOException.class, + () -> strategy.createTempDirectory("POITest")); + } + + @Test + void testCustomDir() throws IOException { + File dirTest = File.createTempFile("POITest", ".dir"); + try { + assertTrue(dirTest.delete()); + + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(dirTest); + checkGetFile(strategy); + } finally { + FileUtils.deleteDirectory(dirTest); + } + } + + @Test + void testCustomDirExists() throws IOException { + File dirTest = File.createTempFile("POITest", ".dir"); + try { + assertTrue(dirTest.delete()); + assertTrue(dirTest.mkdir()); + + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(dirTest); + checkGetFile(strategy); + } finally { + FileUtils.deleteDirectory(dirTest); + } + } + + @Test + void testCustomDirAndPoiFilesExists() throws IOException { + File dirTest = File.createTempFile("POITest", ".dir"); + try { + assertTrue(dirTest.delete()); + assertTrue(dirTest.mkdir()); + assertTrue(new File(dirTest, POIFILES).mkdir()); + + DefaultTempFileCreationStrategy strategy = new DefaultTempFileCreationStrategy(dirTest); + checkGetFile(strategy); + } finally { + FileUtils.deleteDirectory(dirTest); + } + } +} \ No newline at end of file