From: Dominik Stadler Date: Wed, 20 Nov 2024 20:29:38 +0000 (+0000) Subject: Apply some IDE suggestions, JavaDoc and GitHub PR X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c1f018f79c6193e4f989e51dfcb41f7653b8478f;p=poi.git Apply some IDE suggestions, JavaDoc and GitHub PR Update assertion-message Adjust JavaDoc Add tests Reformat class DirectoryNode, adjust/move some comments Closes #730 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1921980 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureConfig.java b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureConfig.java index 5888732c47..2764644812 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureConfig.java +++ b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureConfig.java @@ -26,7 +26,6 @@ import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.PrivateKey; -import java.security.Provider; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -52,14 +51,11 @@ import javax.xml.crypto.URIDereferencer; import javax.xml.crypto.dsig.CanonicalizationMethod; import javax.xml.crypto.dsig.DigestMethod; import javax.xml.crypto.dsig.Transform; -import javax.xml.crypto.dsig.XMLSignatureFactory; -import javax.xml.crypto.dsig.keyinfo.KeyInfoFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.poi.EncryptedDocumentException; import org.apache.poi.hpsf.ClassID; -import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.poifs.crypt.HashAlgorithm; import org.apache.poi.poifs.crypt.dsig.facets.KeyInfoSignatureFacet; import org.apache.poi.poifs.crypt.dsig.facets.OOXMLSignatureFacet; @@ -73,7 +69,6 @@ import org.apache.poi.poifs.crypt.dsig.services.TimeStampHttpClient; import org.apache.poi.poifs.crypt.dsig.services.TimeStampService; import org.apache.poi.poifs.crypt.dsig.services.TimeStampServiceValidator; import org.apache.poi.poifs.crypt.dsig.services.TimeStampSimpleHttpClient; -import org.apache.poi.util.Internal; import org.apache.poi.util.LocaleUtil; import org.apache.poi.util.Removal; import org.apache.xml.security.signature.XMLSignature; @@ -382,7 +377,7 @@ public class SignatureConfig { * @since POI 4.0.0 */ public void setExecutionTime(String executionTime) { - if (executionTime != null && !"".equals(executionTime)){ + if (executionTime != null && !executionTime.isEmpty()){ final DateFormat fmt = new SimpleDateFormat(SIGNATURE_TIME_FORMAT, Locale.ROOT); fmt.setTimeZone(LocaleUtil.TIMEZONE_UTC); try { @@ -992,12 +987,12 @@ public class SignatureConfig { *
  • the JDK xmlsec provider
    • * * - * @return a list of possible XMLSEC provider class names + * @return an array of possible XMLSEC provider class names */ public static String[] getProviderNames() { // need to check every time, as the system property might have been changed in the meantime String sysProp = System.getProperty("jsr105Provider"); - return (sysProp == null || "".equals(sysProp)) + return (sysProp == null || sysProp.isEmpty()) ? new String[]{XMLSEC_SANTUARIO, XMLSEC_JDK} : new String[]{sysProp, XMLSEC_SANTUARIO, XMLSEC_JDK}; } @@ -1031,7 +1026,7 @@ public class SignatureConfig { /** * The signature config can be updated if a document is succesful validated. - * This flag is used for activating this modifications. + * This flag is used for activating these modifications. * Defaults to {@code false} * * @param updateConfigOnValidate if true, update config on validate diff --git a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFTextParagraph.java b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFTextParagraph.java index f6c5570003..7580745404 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFTextParagraph.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFTextParagraph.java @@ -19,6 +19,7 @@ package org.apache.poi.xslf.usermodel; import static org.apache.poi.sl.usermodel.BaseTestSlideShow.getColor; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -467,21 +468,21 @@ class TestXSLFTextParagraph { attributes = iterator.getAttributes(); } - if ("This is a".equals(sb.toString())) { + if ("This is a".contentEquals(sb)) { // Should be no background. assertNotNull(attributes); Object background = attributes.get(TextAttribute.BACKGROUND); assertNull(background); } - if ("highlight".equals(sb.toString())) { + if ("highlight".contentEquals(sb)) { // Should be yellow background. assertNotNull(attributes); Object background = attributes.get(TextAttribute.BACKGROUND); assertNotNull(background); - assertTrue(background instanceof Color); + assertInstanceOf(Color.class, background); assertEquals(Color.yellow, background); } - if (" test".equals(sb.toString())) { + if (" test".contentEquals(sb)) { // Should be no background. assertNotNull(attributes); Object background = attributes.get(TextAttribute.BACKGROUND); @@ -494,5 +495,4 @@ class TestXSLFTextParagraph { ppt.getSlides().get(0).draw(dgfx); } } - } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index 7901b438d7..ae2f3c37f2 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -3331,7 +3331,7 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { LOG.atInfo().log(between(start, now())); assertTrue(between(start, now()).getSeconds() < 25, - "Had start: " + start + ", now: " + now() + + "Expected to have less than 25s duration for test, but had start: " + start + ", now: " + now() + ", diff: " + Duration.between(start, now()).getSeconds()); } } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hsmf/datatypes/PropertiesChunk.java b/poi-scratchpad/src/main/java/org/apache/poi/hsmf/datatypes/PropertiesChunk.java index 64a3a31c21..b409748504 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hsmf/datatypes/PropertiesChunk.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hsmf/datatypes/PropertiesChunk.java @@ -447,8 +447,9 @@ public abstract class PropertiesChunk extends Chunk { /** * Writes out pre-calculated raw values which assume any variable length property `data` * field to already have size, reserved and manually written header - * @param out - * @throws IOException + * + * @param out The OutputStream to write the data to + * @throws IOException If an I/O error occurs while writing data */ private void writeVariableLengthPreCalculatedValue(OutputStream out, PropertyValue value) throws IOException { // variable length header diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/HWPFDocument.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/HWPFDocument.java index 12577ea186..3c4275b2a9 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/HWPFDocument.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/HWPFDocument.java @@ -291,7 +291,7 @@ public final class HWPFDocument extends HWPFDocumentCore { _text = _tpt.getText(); /* - * in this mode we preserving PAPX/CHPX structure from file, so text may + * in this mode we are preserving PAPX/CHPX structure from file, so text may * miss from output, and text order may be corrupted */ boolean preserveBinTables = false; 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 d665ba0eb9..16f1a04d04 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 @@ -173,7 +173,7 @@ public final class RecordFactory { * * @param in the InputStream from which the records will be obtained * - * @return an array of Records created from the InputStream + * @return a list of Records created from the InputStream * * @throws org.apache.poi.util.RecordFormatException on error processing the InputStream */ diff --git a/poi/src/main/java/org/apache/poi/poifs/filesystem/DirectoryNode.java b/poi/src/main/java/org/apache/poi/poifs/filesystem/DirectoryNode.java index 6e7f7508d0..07945f97cd 100644 --- a/poi/src/main/java/org/apache/poi/poifs/filesystem/DirectoryNode.java +++ b/poi/src/main/java/org/apache/poi/poifs/filesystem/DirectoryNode.java @@ -16,16 +16,13 @@ limitations under the License. ==================================================================== */ - package org.apache.poi.poifs.filesystem; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -44,8 +41,7 @@ import org.apache.poi.poifs.property.Property; */ public class DirectoryNode extends EntryNode - implements DirectoryEntry, POIFSViewable, Iterable -{ + implements DirectoryEntry, POIFSViewable, Iterable { // Map of Entry instances, keyed by their literal names as stored private final Map _byname = new HashMap<>(); @@ -72,36 +68,25 @@ public class DirectoryNode */ DirectoryNode(final DirectoryProperty property, final POIFSFileSystem filesystem, - final DirectoryNode parent) - { + final DirectoryNode parent) { super(property, parent); this._filesystem = filesystem; - if (parent == null) - { + if (parent == null) { _path = new POIFSDocumentPath(); - } - else - { - _path = new POIFSDocumentPath(parent._path, new String[] - { - property.getName() - }); + } else { + _path = new POIFSDocumentPath(parent._path, new String[] { property.getName() }); } Iterator iter = property.getChildren(); - while (iter.hasNext()) - { - Property child = iter.next(); - Entry childNode; + while (iter.hasNext()) { + Property child = iter.next(); + Entry childNode; - if (child.isDirectory()) - { + if (child.isDirectory()) { DirectoryProperty childDir = (DirectoryProperty) child; childNode = new DirectoryNode(childDir, _filesystem, this); - } - else - { + } else { childNode = new DocumentNode((DocumentProperty) child, this); } _entries.add(childNode); @@ -113,17 +98,14 @@ public class DirectoryNode /** * @return this directory's path representation */ - - public POIFSDocumentPath getPath() - { + public POIFSDocumentPath getPath() { return _path; } /** * @return the filesystem that this belongs to */ - public POIFSFileSystem getFileSystem() - { + public POIFSFileSystem getFileSystem() { return _filesystem; } @@ -139,8 +121,7 @@ public class DirectoryNode */ public DocumentInputStream createDocumentInputStream( final String documentName) - throws IOException - { + throws IOException { return createDocumentInputStream(getEntryCaseInsensitive(documentName)); } @@ -156,8 +137,7 @@ public class DirectoryNode */ public DocumentInputStream createDocumentInputStream( final Entry document) - throws IOException - { + throws IOException { if (!document.isDocumentEntry()) { throw new IOException("Entry '" + document.getName() + "' is not a DocumentEntry"); @@ -177,10 +157,9 @@ public class DirectoryNode * @throws IOException if the document can't be created */ DocumentEntry createDocument(final POIFSDocument document) - throws IOException - { + throws IOException { DocumentProperty property = document.getDocumentProperty(); - DocumentNode rval = new DocumentNode(property, this); + DocumentNode rval = new DocumentNode(property, this); (( DirectoryProperty ) getProperty()).addChild(property); _filesystem.addDocument(document); @@ -199,17 +178,14 @@ public class DirectoryNode * * @return true if the operation succeeded, else false */ - boolean changeName(final String oldName, final String newName) - { - boolean rval = false; + boolean changeName(final String oldName, final String newName) { + boolean rval = false; EntryNode child = ( EntryNode ) _byUCName.get(oldName.toUpperCase(Locale.ROOT)); - if (child != null) - { + if (child != null) { rval = (( DirectoryProperty ) getProperty()) .changeName(child.getProperty(), newName); - if (rval) - { + if (rval) { _byname.remove(oldName); _byname.put(child.getProperty().getName(), child); _byUCName.remove(oldName.toUpperCase(Locale.ROOT)); @@ -227,14 +203,12 @@ public class DirectoryNode * @return true if the entry was deleted, else false */ - boolean deleteEntry(final EntryNode entry) - { + boolean deleteEntry(final EntryNode entry) { boolean rval = (( DirectoryProperty ) getProperty()) .deleteChild(entry.getProperty()); - if (rval) - { + if (rval) { _entries.remove(entry); _byname.remove(entry.getName()); _byUCName.remove(entry.getName().toUpperCase(Locale.ROOT)); @@ -263,8 +237,7 @@ public class DirectoryNode */ @Override - public Iterator getEntries() - { + public Iterator getEntries() { return _entries.iterator(); } @@ -278,8 +251,7 @@ public class DirectoryNode * DirectoryEntry is empty) */ @Override - public Set getEntryNames() - { + public Set getEntryNames() { return _byname.keySet(); } @@ -290,8 +262,7 @@ public class DirectoryNode */ @Override - public boolean isEmpty() - { + public boolean isEmpty() { return _entries.isEmpty(); } @@ -304,32 +275,29 @@ public class DirectoryNode */ @Override - public int getEntryCount() - { + public int getEntryCount() { return _entries.size(); } /** * Checks for a specific entry in a case-sensitive way. * - * @param name + * @param name the name of the Entry to check * @return whether or not an entry exists for that name (case-sensitive) */ @Override - public boolean hasEntry(String name ) - { + public boolean hasEntry(String name ) { return name != null && _byname.containsKey(name); } /** * Checks for a specific entry in a case-insensitive way. * - * @param name + * @param name the name of the Entry to check * @return whether or not an entry exists for that name (case-insensitive) */ @Override - public boolean hasEntryCaseInsensitive(String name ) - { + public boolean hasEntryCaseInsensitive(String name ) { return name != null && _byUCName.containsKey(name.toUpperCase(Locale.ROOT)); } @@ -351,6 +319,7 @@ public class DirectoryNode if (name != null) { rval = _byname.get(name); } + if (rval == null) { // throw more useful exceptions for known wrong file-extensions if(_byname.containsKey("Workbook")) { @@ -386,6 +355,7 @@ public class DirectoryNode if (name != null) { rval = _byUCName.get(name.toUpperCase(Locale.ROOT)); } + if (rval == null) { // throw more useful exceptions for known wrong file-extensions if(_byname.containsKey("Workbook")) { @@ -414,12 +384,9 @@ public class DirectoryNode * * @throws IOException if the document can't be created */ - @Override public DocumentEntry createDocument(final String name, - final InputStream stream) - throws IOException - { + final InputStream stream) throws IOException { return createDocument(new POIFSDocument(name, _filesystem, stream)); } @@ -434,12 +401,9 @@ public class DirectoryNode * * @throws IOException if the document can't be created */ - @Override public DocumentEntry createDocument(final String name, final int size, - final POIFSWriterListener writer) - throws IOException - { + final POIFSWriterListener writer) throws IOException { return createDocument(new POIFSDocument(name, size, _filesystem, writer)); } @@ -452,11 +416,8 @@ public class DirectoryNode * * @throws IOException if the directory can't be created */ - @Override - public DirectoryEntry createDirectory(final String name) - throws IOException - { + public DirectoryEntry createDirectory(final String name) throws IOException { DirectoryProperty property = new DirectoryProperty(name); DirectoryNode rval = new DirectoryNode(property, _filesystem, this); @@ -482,9 +443,7 @@ public class DirectoryNode */ @SuppressWarnings("WeakerAccess") public DocumentEntry createOrUpdateDocument(final String name, - final InputStream stream) - throws IOException - { + final InputStream stream) throws IOException { if (! hasEntryCaseInsensitive(name)) { return createDocument(name, stream); } else { @@ -501,8 +460,7 @@ public class DirectoryNode * @return storage Class ID */ @Override - public ClassID getStorageClsid() - { + public ClassID getStorageClsid() { return getProperty().getStorageClsid(); } @@ -512,8 +470,7 @@ public class DirectoryNode * @param clsidStorage storage Class ID */ @Override - public void setStorageClsid(ClassID clsidStorage) - { + public void setStorageClsid(ClassID clsidStorage) { getProperty().setStorageClsid(clsidStorage); } @@ -527,8 +484,7 @@ public class DirectoryNode */ @Override - public boolean isDirectoryEntry() - { + public boolean isDirectoryEntry() { return true; } @@ -544,9 +500,7 @@ public class DirectoryNode */ @Override - protected boolean isDeleteOK() - { - + protected boolean isDeleteOK() { // if this directory is empty, we can delete it return isEmpty(); } @@ -562,8 +516,7 @@ public class DirectoryNode */ @Override - public Object [] getViewableArray() - { + public Object [] getViewableArray() { return new Object[ 0 ]; } @@ -592,8 +545,7 @@ public class DirectoryNode */ @Override - public boolean preferArray() - { + public boolean preferArray() { return false; } @@ -605,11 +557,12 @@ public class DirectoryNode */ @Override - public String getShortDescription() - { + public String getShortDescription() { return getName(); } + /* ********** END begin implementation of POIFSViewable ********** */ + /** * Returns an Iterator over all the entries */ @@ -627,7 +580,4 @@ public class DirectoryNode public Spliterator spliterator() { return _entries.spliterator(); } - - /* ********** END begin implementation of POIFSViewable ********** */ -} // end public class DirectoryNode - +} diff --git a/poi/src/main/java/org/apache/poi/ss/formula/DataValidationEvaluator.java b/poi/src/main/java/org/apache/poi/ss/formula/DataValidationEvaluator.java index dcd98bb1dc..b2e4bc1e06 100644 --- a/poi/src/main/java/org/apache/poi/ss/formula/DataValidationEvaluator.java +++ b/poi/src/main/java/org/apache/poi/ss/formula/DataValidationEvaluator.java @@ -47,20 +47,20 @@ import org.apache.poi.util.StringUtil; /** * Evaluates Data Validation constraints.

    * - * For performance reasons, this class keeps a cache of all previously retrieved {@link DataValidation} instances. - * Be sure to call {@link #clearAllCachedValues()} if any workbook validation definitions are + * For performance reasons, this class keeps a cache of all previously retrieved {@link DataValidation} instances. + * Be sure to call {@link #clearAllCachedValues()} if any workbook validation definitions are * added, modified, or deleted. *

    * Changing cell values should be fine, as long as the corresponding {@link WorkbookEvaluator#clearAllCachedResultValues()} * is called as well. - * + * */ public class DataValidationEvaluator { /** * Expensive to compute, so cache them as they are retrieved. *

    - * Sheets don't implement equals, and since its an interface, + * Sheets don't implement equals, and since its an interface, * there's no guarantee instances won't be recreated on the fly by some implementation. * So we use sheet name. */ @@ -79,14 +79,14 @@ public class DataValidationEvaluator { this.workbook = wb; this.workbookEvaluator = provider._getWorkbookEvaluator(); } - + /** * @return evaluator */ protected WorkbookEvaluator getWorkbookEvaluator() { return workbookEvaluator; } - + /** * Call this whenever validation structures change, * so future results stay in sync with the Workbook state. @@ -94,7 +94,7 @@ public class DataValidationEvaluator { public void clearAllCachedValues() { validations.clear(); } - + /** * Lazy load validations by sheet, since reading the CT* types is expensive * @@ -109,14 +109,14 @@ public class DataValidationEvaluator { } return dvs; } - + /** * Finds and returns the {@link DataValidation} for the cell, if there is * one. Lookup is based on the first match from * {@link DataValidation#getRegions()} for the cell's sheet. DataValidation * regions must be in the same sheet as the DataValidation. Allowed values * expressions may reference other sheets, however. - * + * * @param cell reference to check - use this in case the cell does not actually exist yet * @return the DataValidation applicable to the given cell, or null if no * validation applies @@ -132,7 +132,7 @@ public class DataValidationEvaluator { * {@link DataValidation#getRegions()} for the cell's sheet. DataValidation * regions must be in the same sheet as the DataValidation. Allowed values * expressions may reference other sheets, however. - * + * * @param cell reference to check * @return the DataValidationContext applicable to the given cell, or null if no * validation applies @@ -166,7 +166,7 @@ public class DataValidationEvaluator { * This method could throw an exception if the validation type is not LIST, * but since this method is mostly useful in UI contexts, null seems the * easier path. - * + * * @param cell reference to check - use this in case the cell does not actually exist yet * @return returns an unmodifiable {@link List} of {@link ValueEval}s if applicable, or * null @@ -175,7 +175,7 @@ public class DataValidationEvaluator { DataValidationContext context = getValidationContextForCell(cell); if (context == null) return null; - + return getValidationValuesForConstraint(context); } @@ -186,11 +186,11 @@ public class DataValidationEvaluator { protected static List getValidationValuesForConstraint(DataValidationContext context) { final DataValidationConstraint val = context.getValidation().getValidationConstraint(); if (val.getValidationType() != ValidationType.LIST) return null; - + String formula = val.getFormula1(); - + final List values = new ArrayList<>(); - + if (val.getExplicitListValues() != null && val.getExplicitListValues().length > 0) { // assumes parsing interprets the overloaded property right for XSSF for (String s : val.getExplicitListValues()) { @@ -221,7 +221,7 @@ public class DataValidationEvaluator { * Note that to properly apply some validations, care must be taken to * offset the base validation formula by the relative position of the * current cell, or the wrong value is checked. - * + * * @param cellRef The reference of the cell to evaluate * @return true if the cell has no validation or the cell value passes the * defined validation, false if it fails @@ -230,23 +230,23 @@ public class DataValidationEvaluator { final DataValidationContext context = getValidationContextForCell(cellRef); if (context == null) return true; - + final Cell cell = SheetUtil.getCell(workbook.getSheet(cellRef.getSheetName()), cellRef.getRow(), cellRef.getCol()); - + // now we can validate the cell - + // if empty, return not allowed flag if ( cell == null - || isType(cell, CellType.BLANK) - || (isType(cell,CellType.STRING) + || isType(cell, CellType.BLANK) + || (isType(cell,CellType.STRING) && (cell.getStringCellValue() == null || cell.getStringCellValue().isEmpty()) ) ) { return context.getValidation().getEmptyCellAllowed(); } - + // cell has a value - + return ValidationEnum.isValid(cell, context); } @@ -259,18 +259,18 @@ public class DataValidationEvaluator { */ public static boolean isType(Cell cell, CellType type) { final CellType cellType = cell.getCellType(); - return cellType == type - || (cellType == CellType.FORMULA + return cellType == type + || (cellType == CellType.FORMULA && cell.getCachedFormulaResultType() == type ); } - + /** * Not calling it ValidationType to avoid confusion for now with DataValidationConstraint.ValidationType. * Definition order matches OOXML type ID indexes */ - public static enum ValidationEnum { + public enum ValidationEnum { ANY { public boolean isValidValue(Cell cell, DataValidationContext context) { return true; @@ -291,11 +291,11 @@ public class DataValidationEvaluator { public boolean isValidValue(Cell cell, DataValidationContext context) { final List valueList = getValidationValuesForConstraint(context); if (valueList == null) return true; // special case - + // compare cell value to each item for (ValueEval listVal : valueList) { ValueEval comp = listVal instanceof RefEval ? ((RefEval) listVal).getInnerValueEval(context.getSheetIndex()) : listVal; - + // any value is valid if the list contains a blank value per Excel help if (comp instanceof BlankEval) return true; if (comp instanceof ErrorEval) continue; // nothing to check @@ -310,7 +310,7 @@ public class DataValidationEvaluator { // could this have trouble with double precision/rounding errors and date/time values? // do we need to allow a "close enough" double fractional range? // I see 17 digits after the decimal separator in XSSF files, and for time values, - // there are sometimes discrepancies in the final decimal place. + // there are sometimes discrepancies in the final decimal place. // I don't have a validation test case yet though. - GW if (isType(cell, CellType.NUMERIC) && ((NumberEval) comp).getNumberValue() == cell.getNumericCellValue()) { return true; @@ -319,7 +319,7 @@ public class DataValidationEvaluator { } } if (comp instanceof StringEval) { - // interestingly, in testing, a validation value of the string "TRUE" or "true" + // interestingly, in testing, a validation value of the string "TRUE" or "true" // did not match a boolean cell value of TRUE - so apparently cell type matters // also, Excel validation is case insensitive - "true" is valid for the list value "TRUE" if (isType(cell, CellType.STRING) && ((StringEval) comp).getStringValue().equalsIgnoreCase(cell.getStringCellValue())) { @@ -368,16 +368,16 @@ public class DataValidationEvaluator { } }, ; - + public boolean isValidValue(Cell cell, DataValidationContext context) { return isValidNumericCell(cell, context); } - + /** * Uses the cell value, which may be the cached formula result value. * We won't re-evaluate cells here. This validation would be after the cell value was updated externally. - * Excel allows invalid values through methods like copy/paste, and only validates them when the user - * interactively edits the cell. + * Excel allows invalid values through methods like copy/paste, and only validates them when the user + * interactively edits the cell. * @return if the cell is a valid numeric cell for the validation or not */ protected boolean isValidNumericCell(Cell cell, DataValidationContext context) { @@ -394,12 +394,12 @@ public class DataValidationEvaluator { try { Double t1 = evalOrConstant(context.getFormula1(), context); // per Excel, a blank value for a numeric validation constraint formula validates true - if (t1 == null) return true; + if (t1 == null) return true; Double t2 = null; if (context.getOperator() == OperatorType.BETWEEN || context.getOperator() == OperatorType.NOT_BETWEEN) { t2 = evalOrConstant(context.getFormula2(), context); // per Excel, a blank value for a numeric validation constraint formula validates true - if (t2 == null) return true; + if (t2 == null) return true; } return OperatorEnum.values()[context.getOperator()].isValid(value, t1, t2); } catch (NumberFormatException e) { @@ -407,7 +407,7 @@ public class DataValidationEvaluator { return false; } } - + /** * Evaluate a numeric formula value as either a constant or numeric expression. * Note that Excel treats validations with constraint formulas that evaluate to null as valid, @@ -435,12 +435,12 @@ public class DataValidationEvaluator { if (eval instanceof StringEval) { final String value = ((StringEval) eval).getStringValue(); if (StringUtil.isBlank(value)) return null; - // try to parse the cell value as a double and return it + // try to parse the cell value as a double and return it return Double.valueOf(value); } throw new NumberFormatException("Formula '" + formula + "' evaluates to something other than a number"); } - + /** * Validates against the type defined in context, as an index of the enum values array. * @param cell Cell to check validity of @@ -451,14 +451,14 @@ public class DataValidationEvaluator { public static boolean isValid(Cell cell, DataValidationContext context) { return values()[context.getValidation().getValidationConstraint().getValidationType()].isValidValue(cell, context); } - + } - + /** * Not calling it OperatorType to avoid confusion for now with DataValidationConstraint.OperatorType. * Definition order matches OOXML type ID indexes */ - public static enum OperatorEnum { + public enum OperatorEnum { BETWEEN { public boolean isValid(Double cellValue, Double v1, Double v2) { return cellValue.compareTo(v1) >= 0 && cellValue.compareTo(v2) <= 0; @@ -500,9 +500,9 @@ public class DataValidationEvaluator { } }, ; - + public static final OperatorEnum IGNORED = BETWEEN; - + /** * Evaluates comparison using operator instance rules * @param cellValue won't be null, assumption is previous checks handled that @@ -512,7 +512,7 @@ public class DataValidationEvaluator { */ public abstract boolean isValid(Double cellValue, Double v1, Double v2); } - + /** * This class organizes and encapsulates all the pieces of information related to a single * data validation configuration for a single cell. It cleanly separates the validation region, @@ -524,7 +524,7 @@ public class DataValidationEvaluator { private final DataValidationEvaluator dve; private final CellRangeAddressBase region; private final CellReference target; - + /** * Populate the context with the necessary values. */ @@ -558,30 +558,30 @@ public class DataValidationEvaluator { public CellReference getTarget() { return target; } - + public int getOffsetColumns() { return target.getCol() - region.getFirstColumn(); } - + public int getOffsetRows() { return target.getRow() - region.getFirstRow(); } - + public int getSheetIndex() { return dve.getWorkbookEvaluator().getSheetIndex(target.getSheetName()); } - + public String getFormula1() { return dv.getValidationConstraint().getFormula1(); } - + public String getFormula2() { return dv.getValidationConstraint().getFormula2(); } - + public int getOperator() { return dv.getValidationConstraint().getOperator(); } - + } } diff --git a/poi/src/test/java/org/apache/poi/hssf/record/TestRecordInputStream.java b/poi/src/test/java/org/apache/poi/hssf/record/TestRecordInputStream.java index 8622230338..7485a9738a 100644 --- a/poi/src/test/java/org/apache/poi/hssf/record/TestRecordInputStream.java +++ b/poi/src/test/java/org/apache/poi/hssf/record/TestRecordInputStream.java @@ -20,6 +20,8 @@ package org.apache.poi.hssf.record; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.io.IOException; + import org.apache.poi.util.HexRead; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -53,9 +55,8 @@ final class TestRecordInputStream { + "1A 59 00 8A 9E 8A " // 3 uncompressed unicode chars ; - @Test - void testChangeOfCompressionFlag_bug25866() { + void testChangeOfCompressionFlag_bug25866() throws IOException { byte[] changingFlagSimpleData = HexRead.readFromString("" + "AA AA " // fake SID + "06 00 " // first rec len 6 @@ -66,10 +67,13 @@ final class TestRecordInputStream { // bug 45866 - compressByte in continue records must be 1 while reading unicode LE string String actual = in.readUnicodeLEString(18); assertEquals("\u591A\u8A00\u8A9E - Multilingual", actual); + + in.mark(10); + in.reset(); } @Test - void testChangeFromUnCompressedToCompressed() { + void testChangeFromUnCompressedToCompressed() throws IOException { byte[] changingFlagSimpleData = HexRead.readFromString("" + "AA AA " // fake SID + "0F 00 " // first rec len 15 @@ -78,10 +82,13 @@ final class TestRecordInputStream { RecordInputStream in = TestcaseRecordInputStream.create(changingFlagSimpleData); String actual = in.readCompressedUnicode(18); assertEquals("Multilingual - \u591A\u8A00\u8A9E", actual); + + in.mark(10); + in.reset(); } @Test - void testReadString() { + void testReadString() throws IOException { byte[] changingFlagFullData = HexRead.readFromString("" + "AA AA " // fake SID + "12 00 " // first rec len 18 (15 + next 3 bytes) @@ -92,6 +99,9 @@ final class TestRecordInputStream { RecordInputStream in = TestcaseRecordInputStream.create(changingFlagFullData); String actual = in.readString(); assertEquals("Multilingual - \u591A\u8A00\u8A9E", actual); + + in.mark(10); + in.reset(); } @ParameterizedTest diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java index 7e16d69df6..ceea6b1b88 100644 --- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java +++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java @@ -537,6 +537,7 @@ public abstract class BaseTestWorkbook { /** * Tests that all the unicode capable string fields can be set, written and then read back */ + @SuppressWarnings("UnnecessaryUnicodeEscape") @Test protected void unicodeInAll() throws IOException { try (Workbook wb1 = _testDataProvider.createWorkbook()) { @@ -850,7 +851,7 @@ public abstract class BaseTestWorkbook { wb.removeSheetAt(0); wb.removeSheetAt(2); - // ensure that sheets are moved up and removed sheets are not found any more + // ensure that sheets are moved up and removed sheets are not found anymore assertEquals(-1, wb.getSheetIndex(sheet1)); assertEquals(0, wb.getSheetIndex(sheet2)); assertEquals(1, wb.getSheetIndex(sheet3)); @@ -936,4 +937,20 @@ public abstract class BaseTestWorkbook { ); } } + + @Test + void testSheetNameDifferOnlyLowercaseUppercase() throws IOException { + try (Workbook wb = _testDataProvider.createWorkbook()) { + wb.createSheet("abc"); + assertEquals(1, wb.getNumberOfSheets()); + + assertThrows(IllegalArgumentException.class, + () -> wb.createSheet("ABC")); + assertEquals(1, wb.getNumberOfSheets()); + + Sheet sheet = wb.getSheet("abc"); + assertNotNull(sheet); + assertEquals("abc", sheet.getSheetName()); + } + } }