From 392df8b9b82da9a700e1b7368e4d5b03de2a6067 Mon Sep 17 00:00:00 2001 From: pkaras Date: Thu, 6 Jun 2019 10:06:05 +0200 Subject: [PATCH] use delete level property in AafTopicSetupService Change-Id: I68a8ddaeb40fa25ccb9345266682a0dfbc0230c0 Issue-ID: DMAAP-1221 Signed-off-by: piotr.karas --- .../java/org/onap/dmaap/dbcapi/aaf/AafService.java | 2 - .../org/onap/dmaap/dbcapi/aaf/AafServiceImpl.java | 6 --- .../dmaap/dbcapi/service/AafPermissionService.java | 14 ------- .../dmaap/dbcapi/service/AafTopicSetupService.java | 43 ++++++++++++++------- .../dmaap/dbcapi/service/MR_ClientService.java | 7 ---- .../onap/dmaap/dbcapi/service/TopicService.java | 4 +- .../onap/dmaap/dbcapi/aaf/AafServiceImplTest.java | 45 +++++++++++----------- .../dbcapi/service/AafPermissionServiceTest.java | 26 ------------- .../dbcapi/service/AafTopicSetupServiceTest.java | 40 +++++++++++++++---- 9 files changed, 84 insertions(+), 103 deletions(-) diff --git a/src/main/java/org/onap/dmaap/dbcapi/aaf/AafService.java b/src/main/java/org/onap/dmaap/dbcapi/aaf/AafService.java index c49ffb6..3f009f8 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/aaf/AafService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/aaf/AafService.java @@ -39,8 +39,6 @@ public interface AafService { int addUserRole(AafUserRole ur); - int delGrant(DmaapGrant grant); - int addRole(AafRole role); int addNamespace(AafNamespace ns); diff --git a/src/main/java/org/onap/dmaap/dbcapi/aaf/AafServiceImpl.java b/src/main/java/org/onap/dmaap/dbcapi/aaf/AafServiceImpl.java index 4848a69..1491818 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/aaf/AafServiceImpl.java +++ b/src/main/java/org/onap/dmaap/dbcapi/aaf/AafServiceImpl.java @@ -73,12 +73,6 @@ public class AafServiceImpl extends BaseLoggingClass implements AafService { return doPost(ur, "authz/userRole", CREATED); } - @Override - public int delGrant(DmaapGrant grant) { - logger.info("entry: delGrant() "); - return doDelete(grant, "authz/role/:" + grant.getRole() + "/perm", OK); - } - @Override public int addRole(AafRole role) { logger.info("entry: addRole() "); 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 51941d9..1997633 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/service/AafPermissionService.java @@ -56,10 +56,6 @@ class AafPermissionService extends BaseLoggingClass { return forEachClientAction(client, this::grantPermForClientRole); } - ApiError revokeClientPerms(MR_Client client) { - return forEachClientAction(client, this::revokePermForClientRole); - } - private ApiError forEachClientAction(MR_Client client, PermissionUpdate permissionUpdate) { try { String instance = INSTANCE_PREFIX + client.getFqtn(); @@ -88,16 +84,6 @@ class AafPermissionService extends BaseLoggingClass { } } - 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); - 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 ApiError handleErrorStatus(int code, MR_Client client, String message) { ApiError apiError = new ApiError(code, message); client.setStatus(DmaapObject_Status.INVALID); diff --git a/src/main/java/org/onap/dmaap/dbcapi/service/AafTopicSetupService.java b/src/main/java/org/onap/dmaap/dbcapi/service/AafTopicSetupService.java index 9480b6a..29389aa 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/service/AafTopicSetupService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/service/AafTopicSetupService.java @@ -27,19 +27,21 @@ import org.onap.dmaap.dbcapi.aaf.DmaapPerm; import org.onap.dmaap.dbcapi.logging.BaseLoggingClass; import org.onap.dmaap.dbcapi.model.ApiError; import org.onap.dmaap.dbcapi.model.Topic; +import org.onap.dmaap.dbcapi.util.DmaapConfig; import static java.lang.String.format; +import static org.apache.commons.lang3.StringUtils.isNumeric; class AafTopicSetupService extends BaseLoggingClass { private final AafService aafService; private final DmaapService dmaapService; - private final boolean createTopicRoles; + private final DmaapConfig dmaapConfig; - AafTopicSetupService(AafService aafService, DmaapService dmaapService, boolean createTopicRoles) { + AafTopicSetupService(AafService aafService, DmaapService dmaapService, DmaapConfig dmaapConfig) { this.aafService = aafService; this.dmaapService = dmaapService; - this.createTopicRoles = createTopicRoles; + this.dmaapConfig = dmaapConfig; } ApiError aafTopicSetup(Topic topic) { @@ -55,7 +57,7 @@ class AafTopicSetupService extends BaseLoggingClass { // For backwards compatibility, only do this if the feature is enabled. // Also, if the namespace of the topic is a foreign namespace, (i.e. not the same as our root ns) // then we likely don't have permission to create sub-ns and Roles so don't try. - if (createTopicRoles && topic.getFqtn().startsWith(getTopicsNsRoot())) { + if (createTopicRoles() && topic.getFqtn().startsWith(getTopicsNsRoot())) { createNamespace(topic); AafRole pubRole = createRole(topic, "publisher"); @@ -79,17 +81,17 @@ class AafTopicSetupService extends BaseLoggingClass { ApiError aafTopicCleanup(Topic topic) { try { - - String instance = ":topic." + topic.getFqtn(); - String topicPerm = dmaapService.getTopicPerm(); - removePermission(topicPerm, instance, "pub"); - removePermission(topicPerm, instance, "sub"); - removePermission(topicPerm, instance, "view"); - - if (createTopicRoles && topic.getFqtn().startsWith(getTopicsNsRoot())) { - removeNamespace(topic); + if (performCleanup()) { + String instance = ":topic." + topic.getFqtn(); + String topicPerm = dmaapService.getTopicPerm(); + removePermission(topicPerm, instance, "pub"); + removePermission(topicPerm, instance, "sub"); + removePermission(topicPerm, instance, "view"); + + if (createTopicRoles() && topic.getFqtn().startsWith(getTopicsNsRoot())) { + removeNamespace(topic); + } } - } catch (TopicSetupException ex) { return new ApiError(ex.getCode(), ex.getMessage(), ex.getFields()); } @@ -171,7 +173,20 @@ class AafTopicSetupService extends BaseLoggingClass { return new ApiError(200, "OK"); } + private boolean createTopicRoles() { + return "true".equalsIgnoreCase(dmaapConfig.getProperty("aaf.CreateTopicRoles", "true")); + } + + private boolean performCleanup() { + String deleteLevel = dmaapConfig.getProperty("MR.ClientDeleteLevel", "0"); + if (!isNumeric(deleteLevel)) { + return false; + } + return Integer.valueOf(deleteLevel) >= 2; + } + private class TopicSetupException extends Exception { + private final int code; private final String message; private final String fields; diff --git a/src/main/java/org/onap/dmaap/dbcapi/service/MR_ClientService.java b/src/main/java/org/onap/dmaap/dbcapi/service/MR_ClientService.java index d3278f5..bcf5408 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/service/MR_ClientService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/service/MR_ClientService.java @@ -220,13 +220,6 @@ public class MR_ClientService extends BaseLoggingClass { } - // remove from AAF - if (deleteLevel >= 2) { - updateApiError(apiError, aafPermissionService.revokeClientPerms(client)); - if (!apiError.is2xx()) { - return; - } - } // remove from DB if (deleteLevel >= 1) { mr_clients.remove(key); diff --git a/src/main/java/org/onap/dmaap/dbcapi/service/TopicService.java b/src/main/java/org/onap/dmaap/dbcapi/service/TopicService.java index c432254..009b745 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/service/TopicService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/service/TopicService.java @@ -72,9 +72,7 @@ public class TopicService extends BaseLoggingClass { new MR_ClusterService(), new DcaeLocationService(), new MirrorMakerService(), new AafTopicSetupService( new AafServiceFactory().initAafService(ServiceType.AAF_TopicMgr), - dmaapSvc, - "true".equalsIgnoreCase(DmaapConfig.getConfig().getProperty("aaf.CreateTopicRoles", "true")))); - + dmaapSvc, (DmaapConfig) DmaapConfig.getConfig())); } TopicService(Map mr_topics, MR_ClientService clientService, DmaapConfig p, diff --git a/src/test/java/org/onap/dmaap/dbcapi/aaf/AafServiceImplTest.java b/src/test/java/org/onap/dmaap/dbcapi/aaf/AafServiceImplTest.java index 69b320a..efce4a2 100644 --- a/src/test/java/org/onap/dmaap/dbcapi/aaf/AafServiceImplTest.java +++ b/src/test/java/org/onap/dmaap/dbcapi/aaf/AafServiceImplTest.java @@ -113,16 +113,6 @@ public class AafServiceImplTest { assertEquals(CREATED, status); } - @Test - public void shouldDeleteDmaapGrant() { - DmaapGrant grant = new DmaapGrant(new DmaapPerm("perm", "type", "action"), "roles"); - - int status = aafService.delGrant(grant); - - then(aafConnection).should().delAaf(grant, AAF_URL + "authz/role/:" + grant.getRole() + "/perm"); - assertEquals(OK, status); - } - @Test public void shouldNotConnectToAafDuringCreate() { aafService = new AafServiceImpl(false, AAF_URL, IDENTITY, aafConnection); @@ -134,17 +124,6 @@ public class AafServiceImplTest { assertEquals(CREATED, status); } - @Test - public void shouldNotConnectToAafDuringDelete() { - aafService = new AafServiceImpl(false, AAF_URL, IDENTITY, aafConnection); - DmaapGrant grant = new DmaapGrant(new DmaapPerm("perm", "type", "action"), "roles"); - - int status = aafService.delGrant(grant); - - verifyZeroInteractions(aafConnection); - assertEquals(OK, status); - } - @Test @Parameters({"401", "403", "409", "200", "500"}) public void shouldHandleErrorDuringCreate(int aafServiceReturnedCode) { @@ -160,9 +139,9 @@ public class AafServiceImplTest { @Parameters({"401", "403", "404", "200", "500"}) public void shouldHandleErrorDuringDelete(int aafServiceReturnedCode) { given(aafConnection.delAaf(any(AafObject.class), anyString())).willReturn(aafServiceReturnedCode); - DmaapGrant grant = new DmaapGrant(new DmaapPerm("perm", "type", "action"), "roles"); + DmaapPerm perm = new DmaapPerm("perm", "type", "action"); - int status = aafService.delGrant(grant); + int status = aafService.delPerm(perm, false); assertEquals(aafServiceReturnedCode, status); } @@ -206,4 +185,24 @@ public class AafServiceImplTest { then(aafConnection).should().delAaf(any(AafEmpty.class), eq(AAF_URL + "authz/ns/nsName?force=true")); assertEquals(OK, status); } + + @Test + public void shouldReturnExpectedCodeDuringPostWhenUseAffIsSetToFalse() { + aafService = new AafServiceImpl(false, AAF_URL, IDENTITY, aafConnection); + DmaapPerm perm = new DmaapPerm("perm", "type", "action"); + + int status = aafService.addPerm(perm); + + assertEquals(CREATED, status); + } + + @Test + public void shouldReturnExpectedCodeDuringDeleteWhenUseAffIsSetToFalse() { + aafService = new AafServiceImpl(false, AAF_URL, IDENTITY, aafConnection); + DmaapPerm perm = new DmaapPerm("perm", "type", "action"); + + int status = aafService.delPerm(perm, false); + + assertEquals(OK, status); + } } \ No newline at end of file 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 bad66b8..716736e 100644 --- a/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java +++ b/src/test/java/org/onap/dmaap/dbcapi/service/AafPermissionServiceTest.java @@ -130,32 +130,6 @@ public class AafPermissionServiceTest { assertOkStatus(apiError); } - @Test - @Parameters({"200", "404"}) - public void shouldRevokeActionPermissionForClientRole(int aafServiceReturnedCode) { - DmaapGrant grant = new DmaapGrant(new DmaapPerm(TOPIC_PERM, ":topic." + FQTN, PUB_ACTION), ROLE); - given(mrClient.getClientRole()).willReturn(ROLE); - given(aafService.delGrant(grant)).willReturn(aafServiceReturnedCode); - - ApiError apiError = aafPermissionService.revokeClientPerms(mrClient); - - then(aafService).should().delGrant(grant); - then(mrClient).should().setStatus(VALID); - assertOkStatus(apiError); - } - - @Test - public void shouldReturnErrorStatusWhenPermissionWasNotRevokedFromRole() { - 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); - - ApiError apiError = aafPermissionService.revokeClientPerms(mrClient); - - then(mrClient).should().setStatus(INVALID); - assertErrorStatus(apiError, INTERNAL_SERVER_ERROR); - } - private void assertErrorStatus(ApiError apiError, int code) { assertEquals(code, apiError.getCode()); } diff --git a/src/test/java/org/onap/dmaap/dbcapi/service/AafTopicSetupServiceTest.java b/src/test/java/org/onap/dmaap/dbcapi/service/AafTopicSetupServiceTest.java index 8fd8c6f..0ca406a 100644 --- a/src/test/java/org/onap/dmaap/dbcapi/service/AafTopicSetupServiceTest.java +++ b/src/test/java/org/onap/dmaap/dbcapi/service/AafTopicSetupServiceTest.java @@ -35,6 +35,7 @@ import org.onap.dmaap.dbcapi.aaf.DmaapPerm; import org.onap.dmaap.dbcapi.model.ApiError; import org.onap.dmaap.dbcapi.model.Dmaap; import org.onap.dmaap.dbcapi.model.Topic; +import org.onap.dmaap.dbcapi.util.DmaapConfig; import java.util.List; @@ -58,6 +59,8 @@ public class AafTopicSetupServiceTest { private AafServiceStub aafService = new AafServiceStub(); @Mock private DmaapService dmaapService; + @Mock + private DmaapConfig dmaapConfig; private AafTopicSetupService aafTopicSetupService; @Before @@ -67,7 +70,9 @@ public class AafTopicSetupServiceTest { dmaap.setTopicNsRoot(TOPIC_NS_ROOT); given(dmaapService.getDmaap()).willReturn(dmaap); given(dmaapService.getTopicPerm()).willReturn(TOPIC_PERM); - aafTopicSetupService = new AafTopicSetupService(aafService, dmaapService, true); + given(dmaapConfig.getProperty("aaf.CreateTopicRoles", "true")).willReturn("true"); + given(dmaapConfig.getProperty("MR.ClientDeleteLevel", "0")).willReturn("2"); + aafTopicSetupService = new AafTopicSetupService(aafService, dmaapService, dmaapConfig); } @Test @@ -159,7 +164,7 @@ public class AafTopicSetupServiceTest { @Test public void shouldCreateOnlyPermissionsWhenCreateTopicRolesIsFalse() { - aafTopicSetupService = new AafTopicSetupService(aafService, dmaapService, false); + given(dmaapConfig.getProperty("aaf.CreateTopicRoles", "true")).willReturn("false"); aafTopicSetupService.aafTopicSetup(givenTopic(TOPIC_FQTN)); @@ -254,7 +259,7 @@ public class AafTopicSetupServiceTest { @Test public void shouldRemoveOnlyPermissionsWhenCreateTopicRolesIsFalse() { - aafTopicSetupService = new AafTopicSetupService(aafService, dmaapService, false); + given(dmaapConfig.getProperty("aaf.CreateTopicRoles", "true")).willReturn("false"); aafTopicSetupService.aafTopicCleanup(givenTopic(TOPIC_FQTN)); @@ -294,6 +299,26 @@ public class AafTopicSetupServiceTest { assertErrorStatus(apiError, INTERNAL_SERVER_ERROR); } + @Test + public void shouldNotPerformCleanupWhenDeleteLevelIsLessThanTwo() { + given(dmaapConfig.getProperty("MR.ClientDeleteLevel", "0")).willReturn("0"); + + ApiError apiError = aafTopicSetupService.aafTopicCleanup(givenTopic(TOPIC_FQTN)); + + aafService.shouldNotPerformCleanup(); + assertOkStatus(apiError); + } + + @Test + public void shouldNotPerformCleanupWhenDeleteLevelIsNotNumericValue() { + given(dmaapConfig.getProperty("MR.ClientDeleteLevel", "0")).willReturn("not number"); + + ApiError apiError = aafTopicSetupService.aafTopicCleanup(givenTopic(TOPIC_FQTN)); + + aafService.shouldNotPerformCleanup(); + assertOkStatus(apiError); + } + private Topic givenTopic(String topicFqtn) { Topic topic = new Topic(); topic.setFqtn(topicFqtn); @@ -352,11 +377,6 @@ public class AafTopicSetupServiceTest { throw new UnsupportedOperationException(); } - @Override - public int delGrant(DmaapGrant grant) { - throw new UnsupportedOperationException(); - } - @Override public int addRole(AafRole role) { this.addedRoles.add(role); @@ -442,5 +462,9 @@ public class AafTopicSetupServiceTest { assertNull(this.removedNamespace); } + void shouldNotPerformCleanup() { + shouldNotRemoveNamespace(); + assertTrue(removedPerms.isEmpty()); + } } } \ No newline at end of file -- 2.16.6