AafPermissionService refactor

Change-Id: I7c1fecf232f86b4352d24b26ae3b8da47559c428
Issue-ID: DMAAP-1211
Signed-off-by: piotr.karas <piotr.karas@nokia.com>

2 files changed, 81 insertions, 71 deletions

diff --git a/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java b/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java
index 857b695..df60448 100644
--- a/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java
+++ b/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java
@@ -29,8 +29,11 @@ import org.onap.dmaap.dbcapi.model.ApiError;
 import org.onap.dmaap.dbcapi.model.DmaapObject.DmaapObject_Status;
 import org.onap.dmaap.dbcapi.model.MR_Client;
 
+import static java.lang.String.format;
+
 public class AafPermissionService extends BaseLoggingClass {
 
+    private static final String INSTANCE_PREFIX = ":topic.";
     private final AafService aafService;
     private final DmaapService dmaapService;
 
@@ -43,91 +46,96 @@ public class AafPermissionService extends BaseLoggingClass {
         this.dmaapService = dmaapService;
     }
 
-    void assignIdentityToRole(MR_Client client, String role, ApiError err) {
-        okStatus(err);
+    ApiError assignClientToRole(MR_Client client, String role) {
         AafUserRole ur = new AafUserRole(client.getClientIdentity(), role);
-        client.setStatus(DmaapObject_Status.VALID);
         int rc = aafService.addUserRole(ur);
         if (rc != 201 && rc != 409) {
-            client.setStatus(DmaapObject_Status.INVALID);
-            assignClientToRoleError(err, rc, client.getClientIdentity(), role);
+            return handleErrorStatus(rc, client,
+                    format("Failed to add user %s to role %s", client.getClientIdentity(), role));
         }
+        return handleOkStatus(client);
     }
 
-    void grantClientRolePerms(MR_Client client, ApiError err) {
-
-        okStatus(err);
-        String instance = ":topic." + client.getFqtn();
-        client.setStatus(DmaapObject_Status.VALID);
-
-        for (String action : client.getAction()) {
-            if (client.getClientRole() != null) {
-                int rc = grantPermForClientRole(client.getClientRole(), instance, action);
-                if (rc != 201 && rc != 409) {
-                    client.setStatus(DmaapObject_Status.INVALID);
-                    grantPermsError(err, rc, dmaapService.getTopicPerm(), instance, action, client.getClientRole());
-                }
+    ApiError grantClientRolePerms(MR_Client client) {
+        try {
+            String instance = INSTANCE_PREFIX + client.getFqtn();
 
-            } else {
-                logger.warn("No Grant of " + permissionFullName(dmaapService.getTopicPerm(), instance, action) + " because role is null ");
+            for (String action : client.getAction()) {
+                grantPermForClientRole(client.getClientRole(), instance, action);
             }
+
+        } catch (PermissionServiceException e) {
+            return handleErrorStatus(e.getCode(), client, e.getMessage());
         }
+        return handleOkStatus(client);
     }
 
-    void revokeClientPerms(MR_Client client, ApiError err) {
-        okStatus(err);
-        String instance = ":topic." + client.getFqtn();
-        client.setStatus(DmaapObject_Status.VALID);
+    ApiError revokeClientPerms(MR_Client client) {
+        try {
+            String instance = INSTANCE_PREFIX + client.getFqtn();
 
-        for (String action : client.getAction()) {
+            for (String action : client.getAction()) {
+                revokePermForClientRole(client.getClientRole(), instance, action);
+            }
 
-            int rc = revokePermForClientRole(client.getClientRole(), instance, action);
+        } catch (PermissionServiceException e) {
+            return handleErrorStatus(e.getCode(), client, e.getMessage());
+        }
+        return handleOkStatus(client);
+    }
 
-            if (rc != 200 && rc != 404) {
-                client.setStatus(DmaapObject_Status.INVALID);
-                revokePermsError(err, rc, dmaapService.getTopicPerm(), instance, action, client.getClientRole());
+    private void grantPermForClientRole(String clientRole, String instance, String action) throws PermissionServiceException {
+        if (clientRole != null) {
+            DmaapPerm perm = new DmaapPerm(dmaapService.getTopicPerm(), instance, action);
+            DmaapGrant g = new DmaapGrant(perm, clientRole);
+            int code = aafService.addGrant(g);
+            if (code != 201 && code != 409) {
+                throw new PermissionServiceException(code, format("Grant of %s|%s|%s failed for %s",
+                        dmaapService.getTopicPerm(), instance, action, clientRole));
             }
+        } else {
+            logger.warn("No Grant of {}|{}|{} because role is null ", dmaapService.getTopicPerm(), instance, action);
         }
-
     }
 
-    private int grantPermForClientRole(String clientRole, String instance, String action) {
+    private void revokePermForClientRole(String clientRole, String instance, String action) throws PermissionServiceException {
         DmaapPerm perm = new DmaapPerm(dmaapService.getTopicPerm(), instance, action);
         DmaapGrant g = new DmaapGrant(perm, clientRole);
-        return aafService.addGrant(g);
+        int code = aafService.delGrant(g);
+        if (code != 200 && code != 404) {
+            throw new PermissionServiceException(code, format("Revoke of %s|%s|%s failed for %s",
+                    dmaapService.getTopicPerm(), instance, action, clientRole));
+        }
     }
 
-    private int revokePermForClientRole(String clientRole, String instance, String action) {
-        DmaapPerm perm = new DmaapPerm(dmaapService.getTopicPerm(), instance, action);
-        DmaapGrant g = new DmaapGrant(perm, clientRole);
-        return aafService.delGrant(g);
+    private ApiError handleErrorStatus(int code, MR_Client client, String message) {
+        ApiError apiError = new ApiError(code, message);
+        client.setStatus(DmaapObject_Status.INVALID);
+        logger.warn(apiError.getMessage());
+        return apiError;
     }
 
-    private void assignClientToRoleError(ApiError err, int code, String clientIdentity, String role) {
-        err.setCode(code);
-        err.setMessage("Failed to add user " + clientIdentity + "  to " + role);
-        logger.warn(err.getMessage());
+    private ApiError handleOkStatus(MR_Client client) {
+        client.setStatus(DmaapObject_Status.VALID);
+        return new ApiError(200, "OK");
     }
 
-    private void grantPermsError(ApiError err, int code, String permission, String instance, String action, String role) {
-        err.setCode(code);
-        err.setMessage("Grant of " + permissionFullName(permission, instance, action) + " failed for " + role);
-        logger.warn(err.getMessage());
-    }
+    private class PermissionServiceException extends Exception {
+        private final int code;
+        private final String message;
 
-    private void revokePermsError(ApiError err, int code, String permission, String instance, String action, String role) {
-        err.setCode(code);
-        err.setMessage("Revoke of " + permissionFullName(permission, instance, action) + " failed for " + role);
-        logger.warn(err.getMessage());
-    }
+        PermissionServiceException(int code, String message) {
+            this.code = code;
+            this.message = message;
+        }
 
-    private String permissionFullName(String permission, String instance, String action) {
-        return permission + "|" + instance + "|" + action;
-    }
+        public int getCode() {
+            return code;
+        }
 
-    private void okStatus(ApiError err) {
-        err.setCode(200);
-        err.setMessage("OK");
+        @Override
+        public String getMessage() {
+            return message;
+        }
     }
-
 }
diff --git a/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java b/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java
index 1bba2bf..bad66b8 100644
--- a/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java
+++ b/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java
@@ -39,6 +39,8 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.BDDMockito.then;
 import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.onap.dmaap.dbcapi.model.DmaapObject.DmaapObject_Status.INVALID;
+import static org.onap.dmaap.dbcapi.model.DmaapObject.DmaapObject_Status.VALID;
 
 @RunWith(JUnitParamsRunner.class)
 public class AafPermissionServiceTest {
@@ -70,87 +72,87 @@ public class AafPermissionServiceTest {
     @Test
     @Parameters({"201", "409"})
     public void shouldAssignClientToRole(int aafServiceReturnedCode) {
-        ApiError apiError = new ApiError();
         AafUserRole userRole = new AafUserRole(IDENTITY, ROLE);
         given(aafService.addUserRole(userRole)).willReturn(aafServiceReturnedCode);
 
-        aafPermissionService.assignIdentityToRole(mrClient, ROLE, apiError);
+        ApiError apiError = aafPermissionService.assignClientToRole(mrClient, ROLE);
 
         then(aafService).should().addUserRole(userRole);
+        then(mrClient).should().setStatus(VALID);
         assertOkStatus(apiError);
     }
 
     @Test
     public void shouldReturnErrorStatusWhenClientWasNotAssignedToRole() {
-        ApiError apiError = new ApiError();
         AafUserRole userRole = new AafUserRole(IDENTITY, ROLE);
         given(aafService.addUserRole(userRole)).willReturn(INTERNAL_SERVER_ERROR);
 
-        aafPermissionService.assignIdentityToRole(mrClient, ROLE, apiError);
+        ApiError apiError = aafPermissionService.assignClientToRole(mrClient, ROLE);
 
+        then(mrClient).should().setStatus(INVALID);
         assertErrorStatus(apiError, INTERNAL_SERVER_ERROR);
     }
 
     @Test
     @Parameters({"201", "409"})
     public void shouldGrantActionPermissionForClientRole(int aafServiceReturnedCode) {
-        ApiError apiError = new ApiError();
         DmaapGrant grant = new DmaapGrant(new DmaapPerm(TOPIC_PERM, ":topic." + FQTN, PUB_ACTION), ROLE);
         given(mrClient.getClientRole()).willReturn(ROLE);
         given(aafService.addGrant(grant)).willReturn(aafServiceReturnedCode);
 
-        aafPermissionService.grantClientRolePerms(mrClient, apiError);
+        ApiError apiError = aafPermissionService.grantClientRolePerms(mrClient);
 
         then(aafService).should().addGrant(grant);
+        then(mrClient).should().setStatus(VALID);
         assertOkStatus(apiError);
     }
 
     @Test
     public void shouldReturnErrorStatusWhenPermissionWasNotGrantToRole() {
-        ApiError apiError = new ApiError();
         DmaapGrant grant = new DmaapGrant(new DmaapPerm(TOPIC_PERM, ":topic." + FQTN, PUB_ACTION), ROLE);
         given(mrClient.getClientRole()).willReturn(ROLE);
         given(aafService.addGrant(grant)).willReturn(INTERNAL_SERVER_ERROR);
 
-        aafPermissionService.grantClientRolePerms(mrClient, apiError);
+        ApiError apiError = aafPermissionService.grantClientRolePerms(mrClient);
 
+        then(mrClient).should().setStatus(INVALID);
         assertErrorStatus(apiError, INTERNAL_SERVER_ERROR);
     }
 
     @Test
     public void shouldReturnOkStatusWhenClientRoleIsNull() {
-        ApiError apiError = new ApiError();
         given(mrClient.getClientRole()).willReturn(null);
 
-        aafPermissionService.grantClientRolePerms(mrClient, apiError);
+        ApiError apiError = aafPermissionService.grantClientRolePerms(mrClient);
 
         verifyZeroInteractions(aafService);
+        then(mrClient).should().setStatus(VALID);
         assertOkStatus(apiError);
     }
 
     @Test
     @Parameters({"200", "404"})
     public void shouldRevokeActionPermissionForClientRole(int aafServiceReturnedCode) {
-        ApiError apiError = new ApiError();
         DmaapGrant grant = new DmaapGrant(new DmaapPerm(TOPIC_PERM, ":topic." + FQTN, PUB_ACTION), ROLE);
         given(mrClient.getClientRole()).willReturn(ROLE);
         given(aafService.delGrant(grant)).willReturn(aafServiceReturnedCode);
 
-        aafPermissionService.revokeClientPerms(mrClient, apiError);
+        ApiError apiError = aafPermissionService.revokeClientPerms(mrClient);
 
         then(aafService).should().delGrant(grant);
+        then(mrClient).should().setStatus(VALID);
         assertOkStatus(apiError);
     }
 
     @Test
     public void shouldReturnErrorStatusWhenPermissionWasNotRevokedFromRole() {
-        ApiError apiError = new ApiError();
         DmaapGrant grant = new DmaapGrant(new DmaapPerm(TOPIC_PERM, ":topic." + FQTN, PUB_ACTION), ROLE);
         given(mrClient.getClientRole()).willReturn(ROLE);
         given(aafService.delGrant(grant)).willReturn(INTERNAL_SERVER_ERROR);
 
-        aafPermissionService.revokeClientPerms(mrClient, apiError);
+        ApiError apiError = aafPermissionService.revokeClientPerms(mrClient);
 
+        then(mrClient).should().setStatus(INVALID);
         assertErrorStatus(apiError, INTERNAL_SERVER_ERROR);
     }