Fix handling of duplicate resources when creating or updating an Appl…

…ication

hqu/hqapi1/app/ApplicationController.groovy

@@ -280,34 +280,46 @@ class ApplicationController extends ApiController {
     }
 
     private updateAppServices(app, resources) {
-        def svcList = []
+        def svcList = [] // List of AppdefEntityID's to add to the application
 
         if (resources) {
             resources.each { res ->
                 def rid = res.'@id'?.toInteger()
                 def sid = resMan.findResourceById(rid)?.instanceId
-                def svcAeid = AppdefEntityID.newServiceID(sid)
-                svcList.add(svcAeid)
+                def entId = AppdefEntityID.newServiceID(sid)
+                if (!svcList.contains(entId)) {
+                    svcList.add(entId)
+                }
             }
         }
 
-        appMan.setApplicationServices(user, app.id, svcList)
-
         // Setting the application services does not remove any app services
-        // that may have been removed from the application.  We must look up
-        // the tree and remove one-by-one.
-        // TODO: Fix me - Need manager APIs for Service -> AppService mappings
+        // that may have been removed from the application.  It will also add
+        // duplicates, so we need to iterate the list, first removing services
+        // not present in the list, then adding the new entries.
         def sessionId = SessionManager.instance.put(user)
         def dao = new AppServiceDAO(DAOFactory.getDAOFactory());
+
+        def svcListExisting = [] // List of AppdefEntityID's existing in the Application
+        def svcListToRemove = [] // List of app service id's to remove
         for (appService in aBoss.findServiceInventoryByApplication(sessionId, app.id, PageControl.PAGE_ALL)) {
             if (appService instanceof ServiceValue) {
                 def entId = AppdefEntityID.newServiceID(appService.id)
+                svcListExisting << entId
                 if (!svcList.contains(entId)) {
                     def appSvc = dao.findByAppAndService(app.id, appService.id)
-                    appMan.removeAppService(user, app.id, appSvc.id)
+                    svcListToRemove << appSvc.id
                 }
             }
         }
+
+        def toAdd = svcList - svcListExisting
+        appMan.setApplicationServices(user, app.id, toAdd)
+
+        // Remove all deleted services
+        for (appSvcId in svcListToRemove) {
+            appMan.removeAppService(user, app.id, appSvcId)
+        }
     }
 
     private getApplication(id) {

src/org/hyperic/hq/hqapi1/test/ApplicationCreate_test.java

@@ -5,6 +5,9 @@
 import org.hyperic.hq.hqapi1.ResourceApi;
 import org.hyperic.hq.hqapi1.types.*;
 
+import java.util.List;
+import java.util.ArrayList;
+
 public class ApplicationCreate_test extends ApplicationTestBase {
 
     public ApplicationCreate_test(String name) {
@@ -14,7 +17,8 @@ public ApplicationCreate_test(String name) {
     public void testApplicationCreateNoServices() throws Exception {
         ApplicationApi api = getApi().getApplicationApi();
         Application a = createTestApplication(null);
-
+        assertEquals("Wrong number of application services",
+                     0, a.getResource().size());
         StatusResponse response = api.deleteApplication(a.getId());
         hqAssertSuccess(response);
     }
@@ -34,6 +38,35 @@ public void testApplicationCreateWithServices() throws Exception {
         hqAssertSuccess(cpusResponse);
 
         Application a = createTestApplication(cpusResponse.getResource());
+        assertEquals(a.getResource().size(), cpusResponse.getResource().size());
+
+        StatusResponse deleteResponse = appApi.deleteApplication(a.getId());
+        hqAssertSuccess(deleteResponse);
+    }
+
+    public void testApplicationCreateDuplicateServices() throws Exception {
+        HQApi api = getApi();
+        ResourceApi rApi = api.getResourceApi();
+        ApplicationApi appApi = api.getApplicationApi();
+
+        ResourcePrototypeResponse protoResponse =
+                rApi.getResourcePrototype("CPU");
+        hqAssertSuccess(protoResponse);
+
+        ResourcesResponse cpusResponse =
+                rApi.getResources(protoResponse.getResourcePrototype(),
+                                  false, false);
+        hqAssertSuccess(cpusResponse);
+
+        List<Resource> resources = new ArrayList<Resource>();
+        resources.addAll(cpusResponse.getResource());
+        resources.addAll(cpusResponse.getResource());
+        resources.addAll(cpusResponse.getResource());
+
+        Application a = createTestApplication(resources);
+        assertEquals("Wrong number of application services",
+                     cpusResponse.getResource().size(),
+                     a.getResource().size());
 
         StatusResponse deleteResponse = appApi.deleteApplication(a.getId());
         hqAssertSuccess(deleteResponse);

src/org/hyperic/hq/hqapi1/test/ApplicationTestBase.java

@@ -76,10 +76,6 @@ protected Application createTestApplication(List<Resource> services)
         assertEquals(createdApp.getBizContact(), APP_BIZ_CONTACT);
         assertEquals(createdApp.getOpsContact(), APP_OPS_CONTACT);
 
-        if (services != null) {
-            assertEquals(createdApp.getResource().size(), services.size());
-        }
-
         return response.getApplication();
     }
 }

src/org/hyperic/hq/hqapi1/test/ApplicationUpdate_test.java

@@ -184,4 +184,107 @@ public void testUpdateExistingApplicationName() throws Exception {
         deleteResponse = appApi.deleteApplication(app2.getId());
         hqAssertSuccess(deleteResponse);
     }
+
+    public void testApplicationUpdateDuplicateServices() throws Exception {
+        HQApi api = getApi();
+        ResourceApi rApi = api.getResourceApi();
+        ApplicationApi appApi = api.getApplicationApi();
+
+        ResourcePrototypeResponse protoResponse =
+                rApi.getResourcePrototype("CPU");
+        hqAssertSuccess(protoResponse);
+
+        ResourcesResponse cpusResponse =
+                rApi.getResources(protoResponse.getResourcePrototype(),
+                                  false, false);
+        hqAssertSuccess(cpusResponse);
+
+        // Create empty application
+        Application a = createTestApplication(null);
+
+        // Add CPU services
+        a.getResource().addAll(cpusResponse.getResource());
+        ApplicationResponse response = appApi.updateApplication(a);
+        hqAssertSuccess(response);
+
+        assertEquals("Wrong number of application services!",
+                     cpusResponse.getResource().size(),
+                     response.getApplication().getResource().size());
+
+        // Add CPUs again
+        a = response.getApplication();
+        a.getResource().addAll(cpusResponse.getResource());
+        response = appApi.updateApplication(a);
+        hqAssertSuccess(response);
+
+        assertEquals("Wrong number of application services!",
+                     cpusResponse.getResource().size(),
+                     response.getApplication().getResource().size());
+
+        StatusResponse deleteResponse = appApi.deleteApplication(a.getId());
+        hqAssertSuccess(deleteResponse);
+    }
+
+    public void testApplicationUpdateDuplicateServicesSingleCall()
+            throws Exception
+    {
+        HQApi api = getApi();
+        ResourceApi rApi = api.getResourceApi();
+        ApplicationApi appApi = api.getApplicationApi();
+
+        ResourcePrototypeResponse protoResponse =
+                rApi.getResourcePrototype("CPU");
+        hqAssertSuccess(protoResponse);
+
+        ResourcesResponse cpusResponse =
+                rApi.getResources(protoResponse.getResourcePrototype(),
+                                  false, false);
+        hqAssertSuccess(cpusResponse);
+
+        // Create empty application
+        Application a = createTestApplication(null);
+
+        // Add duplicate CPU services
+        a.getResource().addAll(cpusResponse.getResource());
+        a.getResource().addAll(cpusResponse.getResource());
+        a.getResource().addAll(cpusResponse.getResource());
+        ApplicationResponse response = appApi.updateApplication(a);
+        hqAssertSuccess(response);
+
+        assertEquals("Wrong number of application services!",
+                     cpusResponse.getResource().size(),
+                     response.getApplication().getResource().size());
+
+        StatusResponse deleteResponse = appApi.deleteApplication(a.getId());
+        hqAssertSuccess(deleteResponse);
+    }
+
+    public void testApplicationUpdateNoop() throws Exception {
+        HQApi api = getApi();
+        ResourceApi rApi = api.getResourceApi();
+        ApplicationApi appApi = api.getApplicationApi();
+
+        ResourcePrototypeResponse protoResponse =
+                rApi.getResourcePrototype("CPU");
+        hqAssertSuccess(protoResponse);
+
+        ResourcesResponse cpusResponse =
+                rApi.getResources(protoResponse.getResourcePrototype(),
+                                  false, false);
+        hqAssertSuccess(cpusResponse);
+
+        // Create Application w/ CPU resources
+        Application a = createTestApplication(cpusResponse.getResource());
+
+        // Issue update with no changes
+        ApplicationResponse response = appApi.updateApplication(a);
+        hqAssertSuccess(response);
+
+        assertEquals("Wrong number of application services!",
+                     cpusResponse.getResource().size(),
+                     response.getApplication().getResource().size());
+
+        StatusResponse deleteResponse = appApi.deleteApplication(a.getId());
+        hqAssertSuccess(deleteResponse);
+    }
 }