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

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<PackagePart> getPartsByName(final Pattern namePattern) {
 	    if (namePattern == null) {
 	        throw new IllegalArgumentException("name pattern must not be null");
 	    }
+	    Matcher matcher = namePattern.matcher("");
 	    ArrayList<PackagePart> result = new ArrayList<PackagePart>();
 	    for (PackagePart part : partList.values()) {
 	        PackagePartName partName = part.getPartName();
-	        String name = partName.getName();
+	        if (matcher.reset(partName.getName()).matches()) {
-	        Matcher matcher = namePattern.matcher(name);
-	        if (matcher.matches()) {
 	            result.add(part);
 	        }
 	    }
+	    Collections.sort(result);
 	    return result;
 	}
 
@@ -727,7 +738,9 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 				}
 			}
 		}
-		return new ArrayList<PackagePart>(partList.values());
+		ArrayList<PackagePart> result = new ArrayList<PackagePart>(partList.values());
+		java.util.Collections.sort(result);
+		return result;
 	}
 
 	/**

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<PackagePart> {
 
 	/**
 	 * 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 ------------- */
 
 	/**

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<PackagePartName> {
 	}
 
 	/**
-	 * 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) {
+        @Override
-		if (otherPartName == null)
+        public int compareTo(PackagePartName other)
-			return -1;
+        {
-		return this.partNameURI.toASCIIString().toLowerCase().compareTo(
+            // compare with natural sort order
-				otherPartName.partNameURI.toASCIIString().toLowerCase());
+            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<PackagePartName> {
 	 * packages with equivalent part names. [M1.12]
 	 */
 	@Override
-	public boolean equals(Object otherPartName) {
+	public boolean equals(Object other) {
-		if (otherPartName == null
+            if (other instanceof PackagePartName) {
-				|| !(otherPartName instanceof PackagePartName))
+                // String.equals() is compatible with our compareTo(), but cheaper
-			return false;
+                return this.partNameURI.toASCIIString().toLowerCase().equals
-		return this.partNameURI.toASCIIString().toLowerCase().equals(
+                (
-				((PackagePartName) otherPartName).partNameURI.toASCIIString()
+                    ((PackagePartName) other).partNameURI.toASCIIString().toLowerCase()
-						.toLowerCase());
+                );
-	}
+            } else {
+                return false;
+            }
+        }
 
 	@Override
 	public int hashCode() {
@@ -503,4 +508,106 @@ public final class PackagePartName implements Comparable<PackagePartName> {
 	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.
+     * <p>
+     * For example, this sorts "file10.png" after "file2.png" (comparing the
+     * numerical portion), but sorts "File10.png" before "file2.png"
+     * (lexigraphical sort)
+     *
+     * <p>
+     * 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.
+     * <p>
+     * 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);
+    }
+
 }
+
+/* ************************************************************************** */

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"}) {