From ac2041bbd4f3f7212f91b75bc72d3735b6bc3642 Mon Sep 17 00:00:00 2001 From: pkaras Date: Fri, 31 May 2019 12:42:52 +0200 Subject: [PATCH] AafPermissionService refactor Change-Id: I7c1fecf232f86b4352d24b26ae3b8da47559c428 Issue-ID: DMAAP-1211 Signed-off-by: piotr.karas --- .../dmaap/dbcapi/service/AafPermissionService.java | 122 +++++++++++---------- .../dbcapi/service/AafPermissionServiceTest.java | 30 ++--- 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); } -- 2.16.6