From a95e0cd7d77cb0a8d07c08aaf23e852a6b197566 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Fri, 19 Feb 2010 17:55:32 +0000 Subject: [PATCH] Fix an issue with the HSMF tests working on some machines but not others - Make poifs.filesystem.DirectoryNode preserve the original ordering of its files, which HSMF needs to be able to correctly match up chunks git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@911878 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../poi/poifs/filesystem/DirectoryNode.java | 34 ++++++++++-------- .../poi/hsmf/datatypes/AttachmentChunks.java | 1 - .../poi/hsmf/datatypes/RecipientChunks.java | 1 - .../hsmf/parsers/TestPOIFSChunkParser.java | 36 +++++++++---------- .../poifs/filesystem/TestDirectoryNode.java | 24 +++++++++---- 6 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 10ae366349..4498876f5d 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + Make poifs.filesystem.DirectoryNode preserve the original ordering of its files, which HSMF needs to be able to correctly match up chunks Support evaluation of indirect defined names in INDIRECT 43670 - Improve HDGF ChunkV11 separator detection, and short string detection, to solve the "Negative length of ChunkHeader" problem 48617 - Optionally allow the overriding of the Locale used by DataFormatter to control how the default number and date formats should look diff --git a/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java b/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java index 2b2d9e38e0..cb61e9e042 100644 --- 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 _entries; + private Map _byname; + // Our list of entries, kept sorted to preserve order + private ArrayList _entries; // the POIFSFileSystem we belong to private POIFSFileSystem _filesystem; @@ -75,7 +77,8 @@ public class DirectoryNode }); } _filesystem = filesystem; - _entries = new HashMap(); + _byname = new HashMap(); + _entries = new ArrayList(); Iterator 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 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 sortedEntries = - new TreeMap(_entries); - Iterator iter = sortedEntries.values().iterator(); - + Iterator 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 9e5f7f8be0..5bc00c4b0f 100644 --- 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 { - @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 b20aba8a39..d04ec18b54 100644 --- 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 { - @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 68094e2ee8..8dcc6bf5fd 100644 --- 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 fe9eb8ad02..78e6bee9c1 100644 --- 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()); } -- 2.39.5