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
This commit is contained in:
Dominik Stadler 2025-01-11 09:23:46 +00:00
parent 147c034cfd
commit 38e7fe63a8
6 changed files with 192 additions and 13 deletions

View File

@ -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;

View File

@ -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);
}
}

View File

@ -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.<p>
*
@ -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.<p>
*

View File

@ -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);

View File

@ -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<org.apache.poi.hssf.record.Record> 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);
}
}
}

View File

@ -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);
}
}
}