From c8f8cde62d032d2a75305a92edd7670377e1ec1e Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Sun, 3 May 2015 08:47:50 +0000 Subject: [PATCH] Patch from Mark Olesen from bug #57552: Sort PackagePart returns from OPCPackage by name considering numbers in filenames, so Image10.png comes after Image9.png, fixing problems with XSLF adding 10+ images to a slide git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1677373 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/openxml4j/opc/OPCPackage.java | 21 ++- .../apache/poi/openxml4j/opc/PackagePart.java | 18 ++- .../poi/openxml4j/opc/PackagePartName.java | 137 ++++++++++++++++-- .../org/apache/poi/xslf/TestXSLFBugs.java | 1 - 4 files changed, 153 insertions(+), 24 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java index 9dbddddd7..8b65418e0 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java @@ -29,6 +29,7 @@ import java.io.OutputStream; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.Hashtable; import java.util.List; @@ -581,6 +582,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { if (part.getContentType().equals(contentType)) retArr.add(part); } + Collections.sort(retArr); return retArr; } @@ -604,22 +606,31 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { retArr.add(part); } } + Collections.sort(retArr); return retArr; } + /** + * Retrieve parts by name + * + * @param namePattern + * The pattern for matching the names + * @return All parts associated to the specified content type, sorted + * in alphanumerically by the part-name + */ public List getPartsByName(final Pattern namePattern) { if (namePattern == null) { throw new IllegalArgumentException("name pattern must not be null"); } + Matcher matcher = namePattern.matcher(""); ArrayList result = new ArrayList(); for (PackagePart part : partList.values()) { PackagePartName partName = part.getPartName(); - String name = partName.getName(); - Matcher matcher = namePattern.matcher(name); - if (matcher.matches()) { + if (matcher.reset(partName.getName()).matches()) { result.add(part); } } + Collections.sort(result); return result; } @@ -727,7 +738,9 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { } } } - return new ArrayList(partList.values()); + ArrayList result = new ArrayList(partList.values()); + java.util.Collections.sort(result); + return result; } /** diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java index 8cfdb0fb3..1943cf6a5 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java @@ -30,11 +30,8 @@ import org.apache.poi.openxml4j.opc.internal.ContentType; /** * Provides a base class for parts stored in a Package. - * - * @author Julien Chable - * @version 0.9 */ -public abstract class PackagePart implements RelationshipSource { +public abstract class PackagePart implements RelationshipSource, Comparable { /** * This part's container. @@ -650,6 +647,19 @@ public abstract class PackagePart implements RelationshipSource { + this._contentType.toString(); } + /** + * Compare based on the package part name, using a natural sort order + */ + @Override + public int compareTo(PackagePart other) + { + // NOTE could also throw a NullPointerException() if desired + if (other == null) + return -1; + + return PackagePartName.compare(this._partName, other._partName); + } + /*-------------- Abstract methods ------------- */ /** diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePartName.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePartName.java index 6f946a278..9ac6941bc 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePartName.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePartName.java @@ -17,6 +17,7 @@ package org.apache.poi.openxml4j.opc; +import java.math.BigInteger; import java.net.URI; import java.net.URISyntaxException; @@ -428,19 +429,20 @@ public final class PackagePartName implements Comparable { } /** - * Compare two part name following the rule M1.12 : + * Compare two part names following the rule M1.12 : * * Part name equivalence is determined by comparing part names as * case-insensitive ASCII strings. Packages shall not contain equivalent * part names and package implementers shall neither create nor recognize * packages with equivalent part names. [M1.12] */ - public int compareTo(PackagePartName otherPartName) { - if (otherPartName == null) - return -1; - return this.partNameURI.toASCIIString().toLowerCase().compareTo( - otherPartName.partNameURI.toASCIIString().toLowerCase()); - } + @Override + public int compareTo(PackagePartName other) + { + // compare with natural sort order + return compare(this, other); + } + /** * Retrieves the extension of the part name if any. If there is no extension @@ -474,14 +476,17 @@ public final class PackagePartName implements Comparable { * packages with equivalent part names. [M1.12] */ @Override - public boolean equals(Object otherPartName) { - if (otherPartName == null - || !(otherPartName instanceof PackagePartName)) - return false; - return this.partNameURI.toASCIIString().toLowerCase().equals( - ((PackagePartName) otherPartName).partNameURI.toASCIIString() - .toLowerCase()); - } + public boolean equals(Object other) { + if (other instanceof PackagePartName) { + // String.equals() is compatible with our compareTo(), but cheaper + return this.partNameURI.toASCIIString().toLowerCase().equals + ( + ((PackagePartName) other).partNameURI.toASCIIString().toLowerCase() + ); + } else { + return false; + } + } @Override public int hashCode() { @@ -503,4 +508,106 @@ public final class PackagePartName implements Comparable { public URI getURI() { return this.partNameURI; } + + + /** + * A natural sort order for package part names, consistent with the + * requirements of {@code java.util.Comparator}, but simply implemented + * as a static method. + *

+ * For example, this sorts "file10.png" after "file2.png" (comparing the + * numerical portion), but sorts "File10.png" before "file2.png" + * (lexigraphical sort) + * + *

+ * When comparing part names, the rule M1.12 is followed: + * + * Part name equivalence is determined by comparing part names as + * case-insensitive ASCII strings. Packages shall not contain equivalent + * part names and package implementers shall neither create nor recognize + * packages with equivalent part names. [M1.12] + */ + public static int compare(PackagePartName obj1, PackagePartName obj2) + { + // NOTE could also throw a NullPointerException() if desired + if (obj1 == null) + { + // (null) == (null), (null) < (non-null) + return (obj2 == null ? 0 : -1); + } + else if (obj2 == null) + { + // (non-null) > (null) + return 1; + } + + return compare + ( + obj1.getURI().toASCIIString().toLowerCase(), + obj2.getURI().toASCIIString().toLowerCase() + ); + } + + + /** + * A natural sort order for strings, consistent with the + * requirements of {@code java.util.Comparator}, but simply implemented + * as a static method. + *

+ * For example, this sorts "file10.png" after "file2.png" (comparing the + * numerical portion), but sorts "File10.png" before "file2.png" + * (lexigraphical sort) + */ + public static int compare(String str1, String str2) + { + if (str1 == null) + { + // (null) == (null), (null) < (non-null) + return (str2 == null ? 0 : -1); + } + else if (str2 == null) + { + // (non-null) > (null) + return 1; + } + + int len1 = str1.length(); + int len2 = str2.length(); + for (int idx1 = 0, idx2 = 0; idx1 < len1 && idx2 < len2; /*nil*/) + { + char c1 = str1.charAt(idx1++); + char c2 = str2.charAt(idx2++); + + if (Character.isDigit(c1) && Character.isDigit(c2)) + { + int beg1 = idx1 - 1; // undo previous increment + while (idx1 < len1 && Character.isDigit(str1.charAt(idx1))) + { + ++idx1; + } + + int beg2 = idx2 - 1; // undo previous increment + while (idx2 < len2 && Character.isDigit(str2.charAt(idx2))) + { + ++idx2; + } + + // note: BigInteger for extra safety + int cmp = new BigInteger(str1.substring(beg1, idx1)).compareTo + ( + new BigInteger(str2.substring(beg2, idx2)) + ); + if (cmp != 0) return cmp; + } + else if (c1 != c2) + { + return (c1 - c2); + } + } + + return (len1 - len2); + } + } + +/* ************************************************************************** */ diff --git a/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java b/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java index 9f28049fa..8cf3aca1b 100644 --- a/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java @@ -265,7 +265,6 @@ public class TestXSLFBugs { * that image10.foo isn't between image1.foo and image2.foo */ @Test - @Ignore public void test57552() throws Exception { XMLSlideShow ss = new XMLSlideShow(); for (String s : new String[]{"Slide1","Slide2"}) {