[SHRINKWRAP-233] Fixes:

* Added validation for non existent packages * Removed addDefaultPackage recursive since adds everything * Added test case for addDefaultPackage
shrinkwrap · Jul 11, 2011 · bae88c5 · bae88c5
1 parent 6289e75
commit bae88c5
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 9 deletions.
diff --git a/api/src/main/java/org/jboss/shrinkwrap/api/container/ClassContainer.java b/api/src/main/java/org/jboss/shrinkwrap/api/container/ClassContainer.java
@@ -93,6 +93,15 @@ public interface ClassContainer<T extends Archive<T>> extends ResourceContainer<
     * @see #addPackages(boolean, Package...)
     */
    T addPackage(Package pack) throws IllegalArgumentException;
+
+   /**
+    * Adds all classes in the default {@link Package} to the {@link Archive}.
+    * <br/>
+    * SubPackages are excluded.
+    * 
+    * @return This virtual archive
+    */
+   T addDefaultPackage();
 
    /**
     * Adds all classes in the specified {@link Package}s to the {@link Archive}. 

diff --git a/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/container/ContainerBase.java b/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/container/ContainerBase.java
@@ -76,6 +76,7 @@ public abstract class ContainerBase<T extends Archive<T>> extends AssignableBase
 
    private static final Archive<?>[] CAST = new Archive[]{};
    private static final String DEFAULT_MANIFEST = "DefaultManifest.MF";
+   private static final String DEFAULT_PACKAGE_NAME = "";
 
    //-------------------------------------------------------------------------------------||
    // Instance Members -------------------------------------------------------------------||
@@ -1159,7 +1160,8 @@ public boolean include(ArchivePath path)
                   };
                },
                clazz.getClassLoader(),
-               clazz.getPackage()
+               //Assumes a null package is a class in the default package
+               clazz.getPackage() == null ? DEFAULT_PACKAGE_NAME : clazz.getPackage().getName()
          );
       }
       return covarientReturn();
@@ -1224,6 +1226,16 @@ public T addPackage(String pack) throws IllegalArgumentException
       return addPackages(false, pack);
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.shrinkwrap.api.container.ClassContainer#addDefaultPackage()
+    */
+   @Override
+   public T addDefaultPackage()
+   {
+      return addPackages(false, DEFAULT_PACKAGE_NAME);
+   }
+
    /* (non-Javadoc)
     * @see org.jboss.shrinkwrap.api.container.ClassContainer#addPackages(boolean, java.lang.String[])
     */
@@ -1271,6 +1283,9 @@ private T addPackages(final boolean recursive, final Filter<ArchivePath> filter,
 
    private void addPackage(final boolean recursive, final Filter<ArchivePath> filter, final ClassLoader classLoader, String packageName)
    {
+      //precondition checks
+      Validate.notNull(packageName, "Package doesn't exist");
+
       final URLPackageScanner.Callback callback = new URLPackageScanner.Callback()
       {
          @Override
@@ -1286,8 +1301,7 @@ public void classFound(String className)
             add(asset, location);
          }
       };
-      final URLPackageScanner scanner = packageName == null ? URLPackageScanner.newInstance(recursive,
-            classLoader, callback) : URLPackageScanner.newInstance(recursive, classLoader, callback, packageName);
+      final URLPackageScanner scanner = URLPackageScanner.newInstance(recursive, classLoader, callback, packageName);
       scanner.scanPackage();
    }
 

diff --git a/impl-base/src/test/java/DefaultPackageAddTestCase.java b/impl-base/src/test/java/DefaultPackageAddTestCase.java
@@ -4,7 +4,9 @@
 import org.jboss.shrinkwrap.api.ArchivePaths;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 /*
@@ -45,10 +47,33 @@ public class DefaultPackageAddTestCase
     * Logger
     */
    private static final Logger log = Logger.getLogger(DefaultPackageAddTestCase.class.getName());
+
+   //-------------------------------------------------------------------------------------||
+   // Instance Members -------------------------------------------------------------------||
+   //-------------------------------------------------------------------------------------||
+
+   private ArchivePath classInDefaultPackagePath;
+   private ArchivePath innerClassInDefaultPackagePath;
+
+   //-------------------------------------------------------------------------------------||
+   // Fixtures ---------------------------------------------------------------------------||
+   //-------------------------------------------------------------------------------------||
+
+   @Before
+   public void setupPaths() {
+      classInDefaultPackagePath = ArchivePaths.create("/ClassInDefaultPackage.class");
+      innerClassInDefaultPackagePath = ArchivePaths.create("/ClassInDefaultPackage$InnerClassInDefaultPackage.class");
+   }
+
+   @After
+   public void cleanupPaths() {
+      classInDefaultPackagePath = null;
+      innerClassInDefaultPackagePath = null;
+   }
 
    //-------------------------------------------------------------------------------------||
    // Tests ------------------------------------------------------------------------------||
-   //-------------------------------------------------------------------------------------||
+   //-------------------------------------------------------------------------------------||   
 
    /**
     * Ensures that classes from the default package may be added
@@ -63,14 +88,22 @@ public void canAddClassFromDefaultPackage() throws Exception
             .addClass(ClassInDefaultPackage.class);
       log.info(archive.toString(true));
 
-      // Ensure the classes were added in the paths expected
-      final ArchivePath classInDefaultPackagePath = ArchivePaths.create("/ClassInDefaultPackage.class");
-      final ArchivePath innerClassInDefaultPackagePath = ArchivePaths
-            .create("/ClassInDefaultPackage$InnerClassInDefaultPackage.class");
+      assertClassesWereAdded(archive);
+   }
+
+   @Test
+   public void testAddDefaultPackage() {
+      JavaArchive archive = ShrinkWrap.create(JavaArchive.class);
+      archive.addDefaultPackage();
+
+      assertClassesWereAdded(archive);
+   }
+
+   private void assertClassesWereAdded(JavaArchive archive) {
       Assert.assertTrue("Class in default package was not added to archive", archive
             .contains(classInDefaultPackagePath));
       Assert.assertTrue("Inner class in default package was not added to archive", archive
             .contains(innerClassInDefaultPackagePath));
    }
-
+   
 }
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
@@ -1069,6 +1069,20 @@ public void testAddPackageAsStringRecursiveFiltered() throws Exception
       assertContainsClass(expectedPath);
    }
 
+   /**
+    * SHRINKWRAP-233: Tests adding a non existent package doesn't add any asset to the archive.
+    * 
+    */
+   @Test(expected = IllegalArgumentException.class)
+   public void testAddNonExistentPackage()
+   {
+      final String packageName = "non.existent.package";
+      JavaArchive archive = ShrinkWrap.create(JavaArchive.class);
+
+      //Here the exception should be thrown
+      archive.addPackages(true, Package.getPackage(packageName));
+   }
+
    /**
     * Ensure a package as a String can be added to a container with filter
     *