use delete level property in AafTopicSetupService

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

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
@@ -74,12 +74,6 @@ public class AafServiceImpl extends BaseLoggingClass implements AafService {
     }
 
     @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() ");
         return doPost(role, "authz/role", CREATED);
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<String, Topic> 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
@@ -114,16 +114,6 @@ public class AafServiceImplTest {
     }
 
     @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);
         DmaapPerm perm = new DmaapPerm("perm", "type", "action");
@@ -135,17 +125,6 @@ public class AafServiceImplTest {
     }
 
     @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) {
         given(aafConnection.postAaf(any(AafObject.class), anyString())).willReturn(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);
@@ -353,11 +378,6 @@ public class AafTopicSetupServiceTest {
         }
 
         @Override
-        public int delGrant(DmaapGrant grant) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
         public int addRole(AafRole role) {
             this.addedRoles.add(role);
             return addRoleStatus;
@@ -442,5 +462,9 @@ public class AafTopicSetupServiceTest {
             assertNull(this.removedNamespace);
         }
 
+        void shouldNotPerformCleanup() {
+            shouldNotRemoveNamespace();
+            assertTrue(removedPerms.isEmpty());
+        }
     }
 }
\ No newline at end of file