You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ace.apache.org by ja...@apache.org on 2014/01/31 12:04:41 UTC

svn commit: r1563105 - in /ace/trunk: org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java

Author: jawi
Date: Fri Jan 31 11:04:40 2014
New Revision: 1563105

URL: http://svn.apache.org/r1563105
Log:
ACE-451 - better reporting of installation failures:

- ensure that in case the installation of a DP in an agent fails the originating
  error is logged. This means that we have to have a 'catch all' for exceptions
  that allows us to do this. Catching a throwable is safe in this situation as
  we're aborting the installation anyway and only use it for reporting purposes.


Modified:
    ace/trunk/org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java
    ace/trunk/org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java

Modified: ace/trunk/org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java?rev=1563105&r1=1563104&r2=1563105&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java (original)
+++ ace/trunk/org.apache.ace.agent.itest/src/org/apache/ace/agent/itest/AgentDeploymentTest.java Fri Jan 31 11:04:40 2014
@@ -181,7 +181,7 @@ public class AgentDeploymentTest extends
                     resp.setStatus(416); // content range not satisfiable...
                     return;
                 }
-                
+
                 resp.addHeader("Content-Range", String.format("bytes %d-%d/%d", start, end - 1, fileLength));
                 resp.setStatus(206); // partial
             }
@@ -337,6 +337,7 @@ public class AgentDeploymentTest extends
     private TestPackage m_package4;
     private TestPackage m_package5;
     private TestPackage m_package6;
+    private TestPackage m_package7;
 
     /**
      * Test case for ACE-323: when a version of a DP was downloaded correctly, but did not install correctly, we should
@@ -438,25 +439,25 @@ public class AgentDeploymentTest extends
 
     /**
      * Tests the deployment of "non-streamed" deployment packages in various situations.
+     * <p>
+     * This test simulates a DP that is already downloaded, but not yet installed as reported in ACE-413.
+     * </p>
      */
-    public void testNonStreamingDeployment_ChunkedContentRange() throws Exception {
+    public void testNonStreamingDeployment_ChunkedContentAlreadyCompletelyDownloaded() throws Exception {
         setupAgentForNonStreamingDeployment();
 
+        // Simulate that the DP is already downloaded...
+        simulateDPDownloadComplete(m_package6);
+
         expectSuccessfulDeployment(m_package6, Failure.CONTENT_RANGE);
     }
 
     /**
      * Tests the deployment of "non-streamed" deployment packages in various situations.
-     * <p>
-     * This test simulates a DP that is already downloaded, but not yet installed as reported in ACE-413.
-     * </p>
      */
-    public void testNonStreamingDeployment_ChunkedContentAlreadyCompletelyDownloaded() throws Exception {
+    public void testNonStreamingDeployment_ChunkedContentRange() throws Exception {
         setupAgentForNonStreamingDeployment();
 
-        // Simulate that the DP is already downloaded...
-        simulateDPDownloadComplete(m_package6);
-
         expectSuccessfulDeployment(m_package6, Failure.CONTENT_RANGE);
     }
 
@@ -488,6 +489,16 @@ public class AgentDeploymentTest extends
     }
 
     /**
+     * ACE-451: Tests the deployment of an invalid "non-streamed" deployment packages, which should cause the
+     * installation to be aborted.
+     */
+    public void testNonStreamingDeployment_InvalidDeploymentPackage() throws Exception {
+        setupAgentForNonStreamingDeployment();
+
+        expectFailedDeployment(m_package7, null);
+    }
+
+    /**
      * Tests the deployment of "non-streamed" deployment packages in various situations.
      */
     public void testNonStreamingDeployment_VersionsRetryAfter() throws Exception {
@@ -551,6 +562,16 @@ public class AgentDeploymentTest extends
     }
 
     /**
+     * ACE-451: Tests the deployment of an invalid "streamed" deployment packages, which should cause the installation
+     * to be aborted.
+     */
+    public void testStreamingDeployment_InvalidDeploymentPackage() throws Exception {
+        setupAgentForStreamingDeployment();
+
+        expectFailedDeployment(m_package7, null);
+    }
+
+    /**
      * Tests the deployment of "streamed" deployment packages in various situations.
      */
     public void testStreamingDeployment_VersionsRetryAfter() throws Exception {
@@ -597,6 +618,8 @@ public class AgentDeploymentTest extends
         m_package4 = new TestPackage(AGENT_ID, V4_0_0, bundle1v2, bundle2v2);
         m_package5 = new TestPackage(AGENT_ID, V5_0_0, bundle1v2, bundle2v2, bundle3v1);
         m_package6 = new TestPackage(AGENT_ID, V6_0_0, bundle1v2, bundle2v2, bundle3v2);
+        // This leads to an *incorrect* DP, as it contains two bundles with the same BSN...
+        m_package7 = new TestPackage(AGENT_ID, V1_0_0, bundle1v1, bundle1v2);
 
         m_servlet = new TestDeploymentServlet(AGENT_ID);
 

Modified: ace/trunk/org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java?rev=1563105&r1=1563104&r2=1563105&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java (original)
+++ ace/trunk/org.apache.ace.agent/src/org/apache/ace/agent/impl/DefaultController.java Fri Jan 31 11:04:40 2014
@@ -113,12 +113,16 @@ public class DefaultController extends C
                     }
                 }
             }
+            catch (RetryAfterException ex) {
+                // We aren't ready yet...
+                throw ex;
+            }
             catch (InstallationFailedException exception) {
-                // Installation failed...
                 installationFailed(updateInfo, exception);
             }
-            catch (IOException exception) {
-                // I/O exception causes the installation to fail...
+            catch (Throwable exception) {
+                // ACE-451, catch all exceptions, including runtime exceptions to ensure proper handling and logging
+                // takes place...
                 installationFailed(updateInfo, exception);
             }
         }
@@ -165,10 +169,10 @@ public class DefaultController extends C
             }
             catch (InstallationFailedException ex) {
                 installationFailed(updateInfo, ex);
-                
-                controller.logError("Deployment failed: %s %s", ex.getMessage(), (ex.getCause() != null) ? ex.getCause().getMessage() : "");
             }
-            catch (IOException ex) {
+            catch (Throwable ex) {
+                // ACE-451, catch all exceptions, including runtime exceptions to ensure proper handling and logging
+                // takes place...
                 installationFailed(updateInfo, ex);
             }
             finally {
@@ -298,7 +302,7 @@ public class DefaultController extends C
          */
         protected final void installationFailed(UpdateInfo updateInfo, InstallationFailedException exception) {
             // InstallationFailedException is a catch-all wrapper exception, so use its cause directly...
-            getController().logWarning("Installation of %s update failed: %s!", exception.getCause(), updateInfo.m_type, exception.getReason());
+            getController().logError("Installation of %s update failed: %s!", exception.getCause(), updateInfo.m_type, exception.getReason());
 
             m_lastVersionSuccessful = false;
             m_failureCount++;
@@ -313,8 +317,8 @@ public class DefaultController extends C
          * @param cause
          *            the (optional) cause why the installation failed.
          */
-        protected final void installationFailed(UpdateInfo updateInfo, IOException cause) {
-            getController().logWarning("Installation of %s update failed: generic I/O exception.", cause, updateInfo.m_type);
+        protected final void installationFailed(UpdateInfo updateInfo, Throwable cause) {
+            getController().logError("Installation of %s update failed: %s!", cause, updateInfo.m_type, cause.getMessage());
 
             m_lastVersionSuccessful = false;
             m_failureCount++;