Fix an issue with the HSMF tests working on some machines but not others - Make poifs...

to correctly match up chunks

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@911878 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 10ae36634936d833d345388f0f526de0def6888f..4498876f5d4cc64887a519e5129a1a9923f0bfe7 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.7-SNAPSHOT" date="2010-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">Make poifs.filesystem.DirectoryNode preserve the original ordering of its files, which HSMF needs to be able to correctly match up chunks</action>
            <action dev="POI-DEVELOPERS" type="add">Support evaluation of indirect defined names in INDIRECT</action>
            <action dev="POI-DEVELOPERS" type="fix">43670 - Improve HDGF ChunkV11 separator detection, and short string detection, to solve the "Negative length of ChunkHeader" problem</action>
            <action dev="POI-DEVELOPERS" type="add">48617 - Optionally allow the overriding of the Locale used by DataFormatter to control how the default number and date formats should look</action>

diff --git a/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java b/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java
index 2b2d9e38e09bb9b3244c98fb0919918090bb5e4d..cb61e9e0423819dfc8aeec339f21cef561669a61 100644 (file)
--- a/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java
+++ b/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java
@@ -41,7 +41,9 @@ public class DirectoryNode
 {
 
     // Map of Entry instances, keyed by their names
-    private Map<String,Entry> _entries;
+    private Map<String,Entry> _byname;
+    // Our list of entries, kept sorted to preserve order
+    private ArrayList<Entry> _entries;
 
     // the POIFSFileSystem we belong to
     private POIFSFileSystem   _filesystem;
@@ -75,7 +77,8 @@ public class DirectoryNode
             });
         }
         _filesystem = filesystem;
-        _entries    = new HashMap<String, Entry>();
+        _byname     = new HashMap<String, Entry>();
+        _entries    = new ArrayList<Entry>();
         Iterator<Property> iter = property.getChildren();
 
         while (iter.hasNext())
@@ -93,7 +96,8 @@ public class DirectoryNode
                 childNode = new DocumentNode(( DocumentProperty ) child,
                                              this);
             }
-            _entries.put(childNode.getName(), childNode);
+            _entries.add(childNode);
+            _byname.put(childNode.getName(), childNode);
         }
     }
 
@@ -149,7 +153,8 @@ public class DirectoryNode
 
         (( DirectoryProperty ) getProperty()).addChild(property);
         _filesystem.addDocument(document);
-        _entries.put(property.getName(), rval);
+        _entries.add(rval);
+        _byname.put(property.getName(), rval);
         return rval;
     }
 
@@ -165,7 +170,7 @@ public class DirectoryNode
     boolean changeName(final String oldName, final String newName)
     {
         boolean   rval  = false;
-        EntryNode child = ( EntryNode ) _entries.get(oldName);
+        EntryNode child = ( EntryNode ) _byname.get(oldName);
 
         if (child != null)
         {
@@ -173,8 +178,8 @@ public class DirectoryNode
                 .changeName(child.getProperty(), newName);
             if (rval)
             {
-                _entries.remove(oldName);
-                _entries.put(child.getProperty().getName(), child);
+                _byname.remove(oldName);
+                _byname.put(child.getProperty().getName(), child);
             }
         }
         return rval;
@@ -196,7 +201,8 @@ public class DirectoryNode
 
         if (rval)
         {
-            _entries.remove(entry.getName());
+            _entries.remove(entry);
+               _byname.remove(entry.getName());
             _filesystem.remove(entry);
         }
         return rval;
@@ -217,7 +223,7 @@ public class DirectoryNode
 
     public Iterator<Entry> getEntries()
     {
-        return _entries.values().iterator();
+        return _entries.iterator();
     }
 
     /**
@@ -263,7 +269,7 @@ public class DirectoryNode
 
         if (name != null)
         {
-            rval = _entries.get(name);
+            rval = _byname.get(name);
         }
         if (rval == null)
         {
@@ -332,7 +338,8 @@ public class DirectoryNode
 
         (( DirectoryProperty ) getProperty()).addChild(property);
         _filesystem.addDirectory(property);
-        _entries.put(name, rval);
+        _entries.add(rval);
+        _byname.put(name, rval);
         return rval;
     }
 
@@ -416,10 +423,7 @@ public class DirectoryNode
         List components = new ArrayList();
 
         components.add(getProperty());
-        SortedMap<String,Entry> sortedEntries = 
-                  new TreeMap<String,Entry>(_entries);
-        Iterator<Entry> iter = sortedEntries.values().iterator();
-
+        Iterator<Entry> iter = _entries.iterator();
         while (iter.hasNext())
         {
             components.add(iter.next());

diff --git a/src/scratchpad/src/org/apache/poi/hsmf/datatypes/AttachmentChunks.java b/src/scratchpad/src/org/apache/poi/hsmf/datatypes/AttachmentChunks.java
index 9e5f7f8be0d3a8860f7c42c3bd7b9dd8d897170e..5bc00c4b0f72d23b6b96dd5a22321b84c919c5c5 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hsmf/datatypes/AttachmentChunks.java
+++ b/src/scratchpad/src/org/apache/poi/hsmf/datatypes/AttachmentChunks.java
@@ -105,7 +105,6 @@ public class AttachmentChunks implements ChunkGroup {
     * Orders by the attachment number.
     */
    public static class AttachmentChunksSorter implements Comparator<AttachmentChunks> {
-      @Override
       public int compare(AttachmentChunks a, AttachmentChunks b) {
          return a.poifsName.compareTo(b.poifsName);
       }

diff --git a/src/scratchpad/src/org/apache/poi/hsmf/datatypes/RecipientChunks.java b/src/scratchpad/src/org/apache/poi/hsmf/datatypes/RecipientChunks.java
index b20aba8a39e688ee510e4f418029d61a0c34e416..d04ec18b54f239a8c4d2aab58d16a895310d114d 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hsmf/datatypes/RecipientChunks.java
+++ b/src/scratchpad/src/org/apache/poi/hsmf/datatypes/RecipientChunks.java
@@ -197,7 +197,6 @@ public final class RecipientChunks implements ChunkGroup {
     * Orders by the recipient number.
     */
    public static class RecipientChunksSorter implements Comparator<RecipientChunks> {
-      @Override
       public int compare(RecipientChunks a, RecipientChunks b) {
          if(a.recipientNumber < b.recipientNumber)
             return -1;

diff --git a/src/scratchpad/testcases/org/apache/poi/hsmf/parsers/TestPOIFSChunkParser.java b/src/scratchpad/testcases/org/apache/poi/hsmf/parsers/TestPOIFSChunkParser.java
index 68094e2ee883985d591421c682883ba8c09b4eff..8dcc6bf5fde20e10e649d76f165f5b4d3a430943 100644 (file)
--- a/src/scratchpad/testcases/org/apache/poi/hsmf/parsers/TestPOIFSChunkParser.java
+++ b/src/scratchpad/testcases/org/apache/poi/hsmf/parsers/TestPOIFSChunkParser.java
@@ -151,10 +151,10 @@ public final class TestPOIFSChunkParser extends TestCase {
       assertEquals(9, groups.length);
       assertTrue(groups[0] instanceof Chunks);
       assertTrue(groups[1] instanceof RecipientChunks);
-      assertTrue(groups[2] instanceof AttachmentChunks);
+      assertTrue(groups[2] instanceof RecipientChunks);
       assertTrue(groups[3] instanceof RecipientChunks);
       assertTrue(groups[4] instanceof RecipientChunks);
-      assertTrue(groups[5] instanceof RecipientChunks);
+      assertTrue(groups[5] instanceof AttachmentChunks);
       assertTrue(groups[6] instanceof RecipientChunks);
       assertTrue(groups[7] instanceof RecipientChunks);
       assertTrue(groups[8] instanceof NameIdChunks);
@@ -162,33 +162,33 @@ public final class TestPOIFSChunkParser extends TestCase {
       // In FS order initially
       RecipientChunks[] chunks = new RecipientChunks[] {
             (RecipientChunks)groups[1],
+            (RecipientChunks)groups[2],
             (RecipientChunks)groups[3],
             (RecipientChunks)groups[4],
-            (RecipientChunks)groups[5],
             (RecipientChunks)groups[6],
             (RecipientChunks)groups[7],
       };
       assertEquals(6, chunks.length);
       assertEquals(0, chunks[0].recipientNumber);
-      assertEquals(4, chunks[1].recipientNumber);
-      assertEquals(3, chunks[2].recipientNumber);
-      assertEquals(2, chunks[3].recipientNumber);
-      assertEquals(1, chunks[4].recipientNumber);
-      assertEquals(5, chunks[5].recipientNumber);
+      assertEquals(2, chunks[1].recipientNumber);
+      assertEquals(4, chunks[2].recipientNumber);
+      assertEquals(5, chunks[3].recipientNumber);
+      assertEquals(3, chunks[4].recipientNumber);
+      assertEquals(1, chunks[5].recipientNumber);
       
       // Check
       assertEquals("'Ashutosh Dandavate'", chunks[0].getRecipientName());
       assertEquals("ashutosh.dandavate@alfresco.com", chunks[0].getRecipientEmailAddress());
-      assertEquals("nick.burch@alfresco.com", chunks[1].getRecipientName());
-      assertEquals("nick.burch@alfresco.com", chunks[1].getRecipientEmailAddress());
-      assertEquals("nickb@alfresco.com", chunks[2].getRecipientName());
-      assertEquals("nickb@alfresco.com", chunks[2].getRecipientEmailAddress());
-      assertEquals("'Mike Farman'", chunks[3].getRecipientName());
-      assertEquals("mikef@alfresco.com", chunks[3].getRecipientEmailAddress());
-      assertEquals("'Paul Holmes-Higgin'", chunks[4].getRecipientName());
-      assertEquals("paul.hh@alfresco.com", chunks[4].getRecipientEmailAddress());
-      assertEquals("'Roy Wetherall'", chunks[5].getRecipientName());
-      assertEquals("roy.wetherall@alfresco.com", chunks[5].getRecipientEmailAddress());
+      assertEquals("'Mike Farman'", chunks[1].getRecipientName());
+      assertEquals("mikef@alfresco.com", chunks[1].getRecipientEmailAddress());
+      assertEquals("nick.burch@alfresco.com", chunks[2].getRecipientName());
+      assertEquals("nick.burch@alfresco.com", chunks[2].getRecipientEmailAddress());
+      assertEquals("'Roy Wetherall'", chunks[3].getRecipientName());
+      assertEquals("roy.wetherall@alfresco.com", chunks[3].getRecipientEmailAddress());
+      assertEquals("nickb@alfresco.com", chunks[4].getRecipientName());
+      assertEquals("nickb@alfresco.com", chunks[4].getRecipientEmailAddress());
+      assertEquals("'Paul Holmes-Higgin'", chunks[5].getRecipientName());
+      assertEquals("paul.hh@alfresco.com", chunks[5].getRecipientEmailAddress());
       
       // Now sort, and re-check
       Arrays.sort(chunks, new RecipientChunksSorter());

diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestDirectoryNode.java b/src/testcases/org/apache/poi/poifs/filesystem/TestDirectoryNode.java
index fe9eb8ad02aeb83801cb66d3b75703e24272b039..78e6bee9c14b537f8ade67445c6c534e91448e42 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/filesystem/TestDirectoryNode.java
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestDirectoryNode.java
@@ -158,26 +158,36 @@ public final class TestDirectoryNode extends TestCase {
         DirectoryEntry  root = fs.getRoot();
 
         // verify cannot delete the root directory
-        assertTrue(!root.delete());
+        assertFalse(root.delete());
+        assertTrue(root.isEmpty());
+        
         DirectoryEntry dir = fs.createDirectory("myDir");
 
-        assertTrue(!root.isEmpty());
+        assertFalse(root.isEmpty());
+        assertTrue(dir.isEmpty());
 
         // verify can delete empty directory
+        assertFalse(root.delete());
         assertTrue(dir.delete());
+        
+        // Now look at a non-empty one
         dir = fs.createDirectory("NextDir");
         DocumentEntry doc =
             dir.createDocument("foo",
                                new ByteArrayInputStream(new byte[ 1 ]));
 
-        assertTrue(!dir.isEmpty());
+        assertFalse(root.isEmpty());
+        assertFalse(dir.isEmpty());
 
-        // verify cannot delete empty directory
-        assertTrue(!dir.delete());
+        // verify cannot delete non-empty directory
+        assertFalse(dir.delete());
+        
+        // but we can delete it if we remove the document
         assertTrue(doc.delete());
-
-        // verify now we can delete it
+        assertTrue(dir.isEmpty());
         assertTrue(dir.delete());
+        
+        // It's really gone!
         assertTrue(root.isEmpty());
     }