[HQ-2038] Improve error handling when resource group membership is be…

…ing updated while scheduled downtime is in progress.

ChangeLog

@@ -1,6 +1,9 @@
 
 Changes in HQApi 3.2
 
+ *) [HQ-2038] Improve error handling when resource group membership 
+ 	is being updated while scheduled downtime is in progress.
+
 Changes in HQApi 3.1
 
  *) Update Spring to 3.0.1.RELEASE.

hqu/hqapi1/app/GroupController.groovy

@@ -1,5 +1,6 @@
 import org.hyperic.hq.hqapi1.ErrorCode
 import org.hyperic.hq.authz.shared.PermissionException
+import org.hyperic.hq.common.VetoException
 
 class GroupController extends ApiController {
 
@@ -198,13 +199,18 @@ class GroupController extends ApiController {
             if (!failureXml) {
                 if (existing) {
                     // TODO: This needs to be moved out to the Manager.
-                    existing.setRoles(roles)
-                    existing.setResources(user, resources)
-                    existing.updateGroup(user,
-                                         xmlGroup.'@name',
-                                         xmlGroup.'@description',
-                                         xmlGroup.'@location')
-                    groups << existing
+                    try {
+                    	existing.setResources(user, resources)
+                    	existing.setRoles(roles)
+                    	existing.updateGroup(user,
+                                         	 xmlGroup.'@name',
+                                         	 xmlGroup.'@description',
+                                         	 xmlGroup.'@location')
+                    	groups << existing
+                    } catch (VetoException ve) {
+                    	failureXml = getFailureXML(ErrorCode.OPERATION_DENIED,
+                    							   ve.getMessage())
+                    }
                 } else {
                     // TODO: private groups
                     def group = resourceHelper.createGroup(xmlGroup.'@name',

src/org/hyperic/hq/hqapi1/ErrorCode.java

@@ -7,7 +7,7 @@
  * normal use of the program, and does *not* fall under the heading of
  * "derived work".
  * 
- * Copyright (C) [2008, 2009], Hyperic, Inc.
+ * Copyright (C) [2008-2010], Hyperic, Inc.
  * This file is part of HQ.
  * 
  * HQ is free software; you can redistribute it and/or modify
@@ -59,6 +59,11 @@ public enum ErrorCode {
     PERMISSION_DENIED("PermissionDenied",
                       "The current user does not have permission for this operation"),
 
+    /**
+     * The requested operation was denied.
+     */
+    OPERATION_DENIED("OperationDenied",
+                     "The request operation was denied"),
     /**
      * The requested API is not implemented.
      */

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

@@ -54,6 +54,7 @@
 import org.hyperic.hq.hqapi1.types.ResponseStatus;
 import org.hyperic.hq.hqapi1.types.Role;
 import org.hyperic.hq.hqapi1.types.RoleResponse;
+import org.hyperic.hq.hqapi1.types.RolesResponse;
 import org.hyperic.hq.hqapi1.types.User;
 import org.hyperic.hq.hqapi1.types.UserResponse;
 import org.hyperic.hq.hqapi1.types.StatusResponse;
@@ -392,7 +393,7 @@ protected Role createRole(List<User> users, List<Operation> operations)
         hqAssertSuccess(roleResponse);
         Role createdRole = roleResponse.getRole();
 
-        assertEquals("The role should have one user",
+        assertEquals("The role should have " + users.size() + " users",
                      users.size(), createdRole.getUser().size());
 
         for (Operation o : operations) {
@@ -409,6 +410,17 @@ protected void cleanupRole(Role r) throws Exception {
         hqAssertSuccess(response);
     }
 
+    protected void cleanupRoles() throws Exception {
+        RoleApi api = getApi().getRoleApi();
+        RolesResponse response = api.getRoles();
+
+        for (Role r : response.getRole()) {
+            if (r.getName().startsWith(TESTROLE_NAME_PREFIX)) {
+                api.deleteRole(r.getId());
+            }
+        }
+    }
+
     protected void cleanupGroup(Group g) throws Exception {
         cleanupGroup(g, false);
     }
@@ -531,6 +543,13 @@ void hqAssertFailurePermissionDenied(Response response) {
                      response.getError().getErrorCode());
     }
 
+    void hqAssertFailureOperationDenied(Response response) {
+        assertEquals(ResponseStatus.FAILURE, response.getStatus());
+        assertEquals(response.getError().getReasonText(),
+                     ErrorCode.OPERATION_DENIED.getErrorCode(),
+                     response.getError().getErrorCode());
+    }
+
     void hqAssertFailureNotImplemented(Response response) {
         assertEquals(ResponseStatus.FAILURE, response.getStatus());
         assertEquals(response.getError().getReasonText(),

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

@@ -33,6 +33,7 @@
 import org.hyperic.hq.hqapi1.types.Alert;
 import org.hyperic.hq.hqapi1.types.AlertDefinition;
 import org.hyperic.hq.hqapi1.types.DataPoint;
+import org.hyperic.hq.hqapi1.types.GroupsResponse;
 import org.hyperic.hq.hqapi1.types.LastMetricDataResponse;
 import org.hyperic.hq.hqapi1.types.MaintenanceEvent;
 import org.hyperic.hq.hqapi1.types.MaintenanceResponse;
@@ -47,6 +48,7 @@
 import org.hyperic.hq.hqapi1.types.StatusResponse;
 import org.hyperic.hq.hqapi1.types.User;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Random;
@@ -158,6 +160,9 @@ public void testFireAlertsBeforeSchedulingCompatibleGroup()
         // wait for maintenance to start
         waitForMaintenanceStateChange(maintGroup, MaintenanceState.RUNNING);
 
+        // try to modify group membership while downtime is in progress
+        modifyGroupMembershipDuringDowntime(maintGroup);
+
         // validate alert definitions during the maintenance
         // the internal enabled flag should be false for all alert definitions
         alertDefFireOnce = getAlertDefinition(alertDefFireOnce.getId());
@@ -198,6 +203,29 @@ public void testFireAlertsBeforeSchedulingCompatibleGroup()
 
         cleanupGroup(maintGroup, true);         
     }
+
+    /**
+     * To validate HQ-2038
+     */
+    private void modifyGroupMembershipDuringDowntime(Group g) 
+        throws Exception {
+
+        GroupApi groupApi = getApi().getGroupApi();
+
+        List<Resource> existingResources = new ArrayList<Resource>();
+        existingResources.addAll(g.getResource());
+        assertTrue(existingResources.size() > 0);
+
+        // remove resources from group
+        g.getResource().clear();
+
+        GroupsResponse syncResponse = groupApi.syncGroups(Collections.singletonList(g));
+        hqAssertFailureOperationDenied(syncResponse);
+
+        // reset by adding resources back to group
+        g.getResource().addAll(existingResources);
+        assertTrue(g.getResource().size() > 0);
+    }
 
     public void testScheduleNoGroupPermission() throws Exception {
 

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

@@ -89,14 +89,7 @@ public RoleApi getRoleApi(String name, String password){
      */
     public void tearDown() throws Exception {
 
-        RoleApi api = getRoleApi();
-        RolesResponse response = api.getRoles();
-
-        for (Role r : response.getRole()) {
-            if (r.getName().startsWith(TESTROLE_NAME_PREFIX)) {
-                api.deleteRole(r.getId());
-            }
-        }
+        cleanupRoles();
 
         super.tearDown();
     }