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
This commit is contained in:
Dominik Stadler 2024-11-20 20:29:38 +00:00
parent 83384ccf90
commit c1f018f79c
10 changed files with 147 additions and 174 deletions

View File

@ -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 {
* <li>the JDK xmlsec provider</li>
* </ol>
*
* @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

View File

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

View File

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

View File

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

View File

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

View File

@ -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
*/

View File

@ -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<Entry>
{
implements DirectoryEntry, POIFSViewable, Iterable<Entry> {
// Map of Entry instances, keyed by their literal names as stored
private final Map<String,Entry> _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<Property> iter = property.getChildren();
while (iter.hasNext())
{
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,8 +157,7 @@ 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);
@ -199,17 +178,14 @@ public class DirectoryNode
*
* @return true if the operation succeeded, else false
*/
boolean changeName(final String oldName, final String newName)
{
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<Entry> getEntries()
{
public Iterator<Entry> getEntries() {
return _entries.iterator();
}
@ -278,8 +251,7 @@ public class DirectoryNode
* DirectoryEntry is empty)
*/
@Override
public Set<String> getEntryNames()
{
public Set<String> 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<Entry> spliterator() {
return _entries.spliterator();
}
/* ********** END begin implementation of POIFSViewable ********** */
} // end public class DirectoryNode
}

View File

@ -270,7 +270,7 @@ public class DataValidationEvaluator {
* 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;
@ -458,7 +458,7 @@ public class DataValidationEvaluator {
* 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;

View File

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

View File

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