[jboss-svn-commits] JBoss Common SVN: r4278 - in shrinkwrap/trunk: impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter and 2 other directories.

jboss-svn-commits at lists.jboss.org jboss-svn-commits at lists.jboss.org
Tue Apr 20 18:49:18 EDT 2010


Author: ALRubinger
Date: 2010-04-20 18:49:18 -0400 (Tue, 20 Apr 2010)
New Revision: 4278

Modified:
   shrinkwrap/trunk/api/src/main/java/org/jboss/shrinkwrap/api/ConfigurationBuilder.java
   shrinkwrap/trunk/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter/JdkZipExporterDelegate.java
   shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/ConfigurationBuilderTestCase.java
   shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/exporter/ZipExporterTestCase.java
Log:
[SHRINKWRAP-154] Honor the configured ExecutorService during ZIP export

Modified: shrinkwrap/trunk/api/src/main/java/org/jboss/shrinkwrap/api/ConfigurationBuilder.java
===================================================================
--- shrinkwrap/trunk/api/src/main/java/org/jboss/shrinkwrap/api/ConfigurationBuilder.java	2010-04-20 16:40:50 UTC (rev 4277)
+++ shrinkwrap/trunk/api/src/main/java/org/jboss/shrinkwrap/api/ConfigurationBuilder.java	2010-04-20 22:49:18 UTC (rev 4278)
@@ -17,7 +17,6 @@
 package org.jboss.shrinkwrap.api;
 
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -26,7 +25,7 @@
  * Provides defaults for each property if not specified (null) according to the following:
  * 
  * <ul>
- *   <li><code>executorService</code> - A new {@link Executors#newCachedThreadPool()}</li>
+ *   <li><code>executorService</code> - Stay null, none is required and ShrinkWrap will create its own and destroy it when done as needed</li>
  *   <li><code>extensionLoader</code> - A new instance of the service extension loader from shrinkwrap-impl</li>
  * </ul>
  * 
@@ -52,15 +51,6 @@
     */
    private static final String EXTENSION_LOADER_IMPL = "org.jboss.shrinkwrap.impl.base.ServiceExtensionLoader";
 
-   /**
-    * Obtains the default {@link ExecutorService} to be used if none is specified
-    * @return
-    */
-   private static ExecutorService createDefaultExecutorService()
-   {
-      return Executors.newCachedThreadPool();
-   }
-
    //-------------------------------------------------------------------------------------||
    // Instance Members -------------------------------------------------------------------||
    //-------------------------------------------------------------------------------------||
@@ -167,18 +157,6 @@
          }
          this.extensionLoader(loader);
       }
-
-      // If no executor service is present, create one
-      if (getExecutorService() == null)
-      {
-         final ExecutorService service = createDefaultExecutorService();
-         if (log.isLoggable(Level.FINER))
-         {
-            log.finer("User has not defined an explicit " + ExecutorService.class.getSimpleName() + "; defaulting to "
-                  + service);
-         }
-         this.executorService(service);
-      }
    }
 
    /**

Modified: shrinkwrap/trunk/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter/JdkZipExporterDelegate.java
===================================================================
--- shrinkwrap/trunk/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter/JdkZipExporterDelegate.java	2010-04-20 16:40:50 UTC (rev 4277)
+++ shrinkwrap/trunk/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter/JdkZipExporterDelegate.java	2010-04-20 22:49:18 UTC (rev 4278)
@@ -42,6 +42,7 @@
 import org.jboss.shrinkwrap.impl.base.io.StreamErrorHandler;
 import org.jboss.shrinkwrap.impl.base.io.StreamTask;
 import org.jboss.shrinkwrap.impl.base.path.PathUtil;
+import org.jboss.shrinkwrap.spi.Configurable;
 
 /**
  * JDK-based implementation of a ZIP exporter.  Cannot handle archives
@@ -174,12 +175,27 @@
          }
       };
 
+      // Get an ExecutorService to which we may submit jobs.  This is either supplied by the user
+      // in a custom domain, or if one has not been specified, we'll make one and shut it down right
+      // here.  ExecutorServices supplied by the user are under the user's lifecycle, therefore it's
+      // user responsibility to shut it down appropriately.
+      boolean executorServiceIsOurs = false;
+      ExecutorService service = this.getArchive().as(Configurable.class).getConfiguration().getExecutorService();
+      if (service == null)
+      {
+         service = Executors.newSingleThreadExecutor();
+         executorServiceIsOurs = true;
+      }
+
       // Get a handle and return it to the caller
-      final ExecutorService service = Executors.newSingleThreadExecutor();
       final Future<Void> job = service.submit(exportTask);
 
-      // Tell the service to shut down after the job has completed, and accept no new jobs
-      service.shutdown();
+      // If we've created the ES
+      if (executorServiceIsOurs)
+      {
+         // Tell the service to shut down after the job has completed, and accept no new jobs
+         service.shutdown();
+      }
 
       /*
        * At this point the job will start, but hit the latch until we set up the streams

Modified: shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/ConfigurationBuilderTestCase.java
===================================================================
--- shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/ConfigurationBuilderTestCase.java	2010-04-20 16:40:50 UTC (rev 4277)
+++ shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/ConfigurationBuilderTestCase.java	2010-04-20 22:49:18 UTC (rev 4278)
@@ -67,15 +67,12 @@
    //-------------------------------------------------------------------------------------||
 
    /**
-    * Ensures that the {@link ExecutorService} is defaulted
-    * as contracted to {@link Executors#newCachedThreadPool()}
+    * Ensures that a null {@link ExecutorService} is not defaulted
+    * to any value, and may remain null
     */
    @Test
    public void defaultsExecutorService()
    {
-      // Define the expected type as contracted
-      final Class<?> expectedType = Executors.newCachedThreadPool().getClass();
-
       // Build and default
       builder.build();
 
@@ -83,9 +80,7 @@
       final ExecutorService service = builder.getExecutorService();
 
       // Test
-      Assert.assertNotNull("The builder should default an " + ExecutorService.class.getSimpleName(), service);
-      Assert.assertEquals("The default " + ExecutorService.class.getSimpleName() + " was not of contracted type",
-            expectedType, service.getClass());
+      Assert.assertNull("The builder should not default an " + ExecutorService.class.getSimpleName(), service);
    }
 
    /**

Modified: shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/exporter/ZipExporterTestCase.java
===================================================================
--- shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/exporter/ZipExporterTestCase.java	2010-04-20 16:40:50 UTC (rev 4277)
+++ shrinkwrap/trunk/impl-base/src/test/java/org/jboss/shrinkwrap/impl/base/exporter/ZipExporterTestCase.java	2010-04-20 22:49:18 UTC (rev 4278)
@@ -21,6 +21,15 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Logger;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
@@ -31,6 +40,9 @@
 import org.jboss.shrinkwrap.api.ArchivePath;
 import org.jboss.shrinkwrap.api.Archives;
 import org.jboss.shrinkwrap.api.Asset;
+import org.jboss.shrinkwrap.api.ConfigurationBuilder;
+import org.jboss.shrinkwrap.api.Domain;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.exporter.ArchiveExportException;
 import org.jboss.shrinkwrap.api.exporter.FileExistsException;
 import org.jboss.shrinkwrap.api.exporter.ZipExporter;
@@ -90,6 +102,43 @@
    }
 
    /**
+    * Ensures that the ZIP export write task uses the {@link ExecutorService}
+    * that we've configured, and leaves it running (does not shut it down)
+    * @throws Exception
+    */
+   @Test
+   public void exportUsesOurExecutorService() throws Exception
+   {
+      // Make a custom ES
+      final CountingExecutorService service = new CountingExecutorService();
+
+      // Create a custom configuration
+      final Domain domain = ShrinkWrap.createDomain(new ConfigurationBuilder().executorService(service).build());
+
+      // Make an archive using the new configuration
+      final Archive<?> archive = domain.getArchiveFactory().create("test.jar", JavaArchive.class).addClass(
+            ZipExporterTestCase.class);
+
+      // Fully export by reading all content (export is on-demand)
+      final InputStream zip = archive.as(ZipExporter.class).exportZip();
+      while (zip.read() != -1)
+      {
+
+      }
+
+      // Ensure the ES was used (one job was submitted to it)
+      TestCase.assertEquals("Custom " + ExecutorService.class.getSimpleName() + " was not used by ZIP export", 1,
+            service.counter);
+
+      // Ensure the ES was not shut down by the export process
+      TestCase.assertFalse("ZIP Export should not shut down a user-supplied " + ExecutorService.class.getName(),
+            service.isShutdown());
+
+      // Shut down the ES (clean up)
+      service.shutdown();
+   }
+
+   /**
     * Test to ensure that the {@link JdkZipExporterDelegate} does not accept 
     * an empty archive as input
     * 
@@ -103,7 +152,7 @@
       // Attempt to export an empty archive, should fail
       Archives.create(NAME_ARCHIVE, JavaArchive.class).as(ZipExporter.class).exportZip();
    }
-   
+
    /**
     * Test to ensure that the {@link JdkZipExporterDelegate} accepts an archive
     * with only directories, no assets.
@@ -111,7 +160,7 @@
     * @throws Exception
     */
    @Test
-   public void testExportArchiveWithOnlyDirectories() 
+   public void testExportArchiveWithOnlyDirectories()
    {
       // Attempt to export an archive with some directories, should pass
       Archives.create(NAME_ARCHIVE, JavaArchive.class).addDirectories("/test/game").as(ZipExporter.class).exportZip();
@@ -382,4 +431,110 @@
       ZipEntry rootEntry = expectedZip.getEntry("/");
       Assert.assertNull("ZIP should not have explicit root path written (SHRINKWRAP-96)", rootEntry);
    }
+
+   /**
+    * Test implementation of an {@link ExecutorService} which 
+    * counts all jobs submitted.
+    * 
+    * @author <a href="mailto:andrew.rubinger at jboss.org">ALR</a>
+    * @version $Revision: $
+    */
+   private static class CountingExecutorService implements ExecutorService
+   {
+
+      private final ExecutorService delegate;
+
+      int counter = 0;
+
+      public CountingExecutorService()
+      {
+         delegate = Executors.newSingleThreadExecutor();
+      }
+
+      @Override
+      public boolean awaitTermination(final long timeout, final TimeUnit unit) throws InterruptedException
+      {
+         return delegate.awaitTermination(timeout, unit);
+      }
+
+      @Override
+      public <T> List<Future<T>> invokeAll(final Collection<? extends Callable<T>> tasks) throws InterruptedException
+      {
+         return delegate.invokeAll(tasks);
+      }
+
+      @Override
+      public <T> List<Future<T>> invokeAll(final Collection<? extends Callable<T>> tasks, final long timeout,
+            final TimeUnit unit) throws InterruptedException
+      {
+         return delegate.invokeAll(tasks, timeout, unit);
+      }
+
+      @Override
+      public <T> T invokeAny(final Collection<? extends Callable<T>> tasks) throws InterruptedException,
+            ExecutionException
+      {
+         return delegate.invokeAny(tasks);
+      }
+
+      @Override
+      public <T> T invokeAny(final Collection<? extends Callable<T>> tasks, final long timeout, final TimeUnit unit)
+            throws InterruptedException, ExecutionException, TimeoutException
+      {
+         return delegate.invokeAny(tasks, timeout, unit);
+      }
+
+      @Override
+      public boolean isShutdown()
+      {
+         return delegate.isShutdown();
+      }
+
+      @Override
+      public boolean isTerminated()
+      {
+         return delegate.isTerminated();
+      }
+
+      @Override
+      public void shutdown()
+      {
+         delegate.shutdown();
+      }
+
+      @Override
+      public List<Runnable> shutdownNow()
+      {
+         return delegate.shutdownNow();
+      }
+
+      @Override
+      public <T> Future<T> submit(final Callable<T> task)
+      {
+         counter++;
+         return delegate.submit(task);
+      }
+
+      @Override
+      public Future<?> submit(final Runnable task)
+      {
+         counter++;
+         return delegate.submit(task);
+      }
+
+      @Override
+      public <T> Future<T> submit(final Runnable task, final T result)
+      {
+         counter++;
+         return delegate.submit(task, result);
+      }
+
+      @Override
+      public void execute(final Runnable command)
+      {
+         counter++;
+         delegate.execute(command);
+      }
+
+   }
 }



More information about the jboss-svn-commits mailing list