[SHRINKWRAP-389] Only use IllegalOverwriteException when attempting t…

…o overwrite directories with files, else overwrite the content in-place.
shrinkwrap · Oct 30, 2012 · 25f645d · 25f645d
1 parent d2eacc7
commit 25f645d
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 43 deletions.
diff --git a/api/src/main/java/org/jboss/shrinkwrap/api/IllegalOverwriteException.java b/api/src/main/java/org/jboss/shrinkwrap/api/IllegalOverwriteException.java
@@ -18,7 +18,7 @@
 
 /**
  * Exception thrown when trying to add an {@code Asset} into an archive under an {@code ArchivePath} which is already
- * taken.
+ * taken by a directory.
  *
  * @author <a href="mailto:alr@jboss.org">Andrew Lee Rubinger</a>
  */

diff --git a/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/MemoryMapArchiveBase.java b/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/MemoryMapArchiveBase.java
@@ -181,32 +181,29 @@ public T addAsDirectory(final ArchivePath path) throws IllegalArgumentException
     }
 
     private T addAsset(ArchivePath path, Asset asset) {
-       Asset handledAsset = invokeHandlers(path, asset);
-
-       // Check if it exists. If it doesn't, create it and add it.
-       if (!contains(path)) {
-          // Add the node to the content of the archive
-          NodeImpl node = new NodeImpl(path, handledAsset);
-          content.put(path, node);
-
-          // Add the new node to the parent as a child
-          NodeImpl parentNode = obtainParent(path.getParent());
-          if (parentNode != null) {
-             parentNode.addChild(node);
-          }
-       } else {
-           // Get the Node
-           final Node node = this.get(path);
-
-            // Disallow if we're dealing with an existing Asset or a non-empty dir
-            final Asset currentAsset = node.getAsset();
-            if (currentAsset != null || node.getChildren().size() == 0) {
-                // Path exists, throw an exception
-                throw new IllegalOverwriteException("Cannot add requested path " + path.get() + " to archive "
-                    + this.getName() + "; path already exists");
+        Asset handledAsset = invokeHandlers(path, asset);
+
+        // Disallow if we're dealing with a non-empty dir
+        if (contains(path) && asset != null) {
+            final Node node = this.get(path);
+            if (node.getAsset() == null) {
+                // Path exists as a dir, throw an exception
+                throw new IllegalOverwriteException("Cannot add requested asset " + asset + " to path " + path.get()
+                    + " to archive " + this.getName() + "; path already exists as directory");
             }
-       }
-       return covariantReturn();
+        }
+
+        // Add the node to the content of the archive
+        NodeImpl node = new NodeImpl(path, handledAsset);
+        content.put(path, node);
+
+        // Add the new node to the parent as a child
+        NodeImpl parentNode = obtainParent(path.getParent());
+        if (parentNode != null) {
+            parentNode.addChild(node);
+        }
+
+        return covariantReturn();
     }
 
     /**

diff --git a/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/test/DynamicContainerTestBase.java b/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/test/DynamicContainerTestBase.java
@@ -625,11 +625,17 @@ public void testAddResourceStringTargetResourceOverride() throws Exception {
         ArchivePath targetPath2 = new BasicPath("META-INF");
 
         getResourceContainer().addAsResource(NAME_TEST_PROPERTIES, targetPath);
-        getResourceContainer().addAsResource(NAME_TEST_PROPERTIES, targetPath2);
 
-        ArchivePath testPath = new BasicPath(getResourcePath(), "META-INF/Test.txt");
+        boolean gotExpectedException = false;
+        try {
+            getResourceContainer().addAsResource(NAME_TEST_PROPERTIES, targetPath2);
+        } catch (final IllegalOverwriteException ioe) {
+            gotExpectedException = true;
+        }
 
+        ArchivePath testPath = new BasicPath(getResourcePath(), "META-INF/Test.txt");
         Assert.assertTrue("Archive should contain " + testPath, getArchive().contains(testPath));
+        Assert.assertTrue(gotExpectedException);
     }
 
     @Test
@@ -1906,27 +1912,24 @@ public void testAddingEmptyResourceDirectory() throws Exception {
     }
 
     /**
-     * SHRINKWRAP-329
+     * SHRINKWRAP-329 SHRINKWRAP-389
      */
     @Test
-    public void addDuplicateResourceThrowsIllegalOverwriteException() {
+    public void addDuplicateResourceMakesOverwrite() throws IOException {
         // Create the new archive
         final Archive<?> archive = createNewArchive();
 
         // Put in an asset
         final ArchivePath path = ArchivePaths.create("testPath");
         archive.add(EmptyAsset.INSTANCE, path);
 
-        // Now try again with a new asset, and this should fail
-        try {
-            archive.add(new StringAsset("failContent"), path);
-        } catch (final IllegalOverwriteException ioe) {
-            // Good
-            return;
-        }
+        // Now try again with a new asset, and this should replace the old content
+        final String content = "newContent";
+        archive.add(new StringAsset(content), path);
 
-        // Fail us
-        TestCase.fail("Expected " + IllegalOverwriteException.class.getName() + " not received");
+        final String contentFound = new BufferedReader(new InputStreamReader(archive.get(path).getAsset().openStream()))
+            .readLine();
+        Assert.assertEquals(content, contentFound);
     }
 
     /**

diff --git a/impl-nio2/src/main/java/org/jboss/shrinkwrap/impl/nio/file/ShrinkWrapFileSystemProvider.java b/impl-nio2/src/main/java/org/jboss/shrinkwrap/impl/nio/file/ShrinkWrapFileSystemProvider.java
@@ -52,7 +52,6 @@
 import org.jboss.shrinkwrap.api.Archive;
 import org.jboss.shrinkwrap.api.ArchivePath;
 import org.jboss.shrinkwrap.api.ArchivePaths;
-import org.jboss.shrinkwrap.api.IllegalOverwriteException;
 import org.jboss.shrinkwrap.api.Node;
 import org.jboss.shrinkwrap.api.asset.Asset;
 import org.jboss.shrinkwrap.api.nio.file.MemoryNamedAsset;
@@ -263,10 +262,8 @@ public SeekableByteChannel newByteChannel(final Path path, final Set<? extends O
             || options.contains(StandardOpenOption.WRITE)) {
 
             // Plug channel as Asset into the archive
-            try {
-                archive.add(channel);
-            } catch (final IllegalOverwriteException ioe) {
 
+            if (archive.contains(channel.getName())) {
                 // Appending?
                 if (options.contains(StandardOpenOption.APPEND)) {
                     // Read in the existing content
@@ -281,6 +278,8 @@ public SeekableByteChannel newByteChannel(final Path path, final Set<? extends O
                     // Exception translate
                     throw new FileAlreadyExistsException(archivePath.get());
                 }
+            } else {
+                archive.add(channel);
             }
 
             // Return the channel