Repository: incubator-ranger
Updated Branches:
  refs/heads/master e7541c9da -> 53623080a


RANGER-865: Service delete should delete associated service-resources and 
tag-resource-maps

Signed-off-by: Madhan Neethiraj <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/53623080
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/53623080
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/53623080

Branch: refs/heads/master
Commit: 53623080a218954e8ce1464342c13a196d58127f
Parents: e7541c9
Author: Abhay Kulkarni <[email protected]>
Authored: Mon Feb 29 17:41:33 2016 -0800
Committer: Madhan Neethiraj <[email protected]>
Committed: Wed Mar 2 21:12:57 2016 -0800

----------------------------------------------------------------------
 .../apache/ranger/biz/RangerTagDBRetriever.java |  6 +-
 .../java/org/apache/ranger/biz/TagDBStore.java  | 75 ++++++++++++++++++++
 .../ranger/db/XXServiceResourceElementDao.java  | 14 +++-
 .../db/XXServiceResourceElementValueDao.java    | 13 ++++
 .../org/apache/ranger/rest/ServiceREST.java     | 14 ++--
 .../ranger/rest/ServiceTagsProcessor.java       |  2 -
 .../resources/META-INF/jpa_named_queries.xml    | 22 ++++--
 .../org/apache/ranger/rest/TestServiceREST.java |  6 +-
 8 files changed, 137 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java 
b/security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java
index bbdf181..3b7555e 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java
@@ -204,8 +204,8 @@ public class RangerTagDBRetriever {
                        Long serviceId = xService == null ? null : 
xService.getId();
 
                        List<XXServiceResource> xServiceResources = 
daoMgr.getXXServiceResource().findTaggedResourcesInServiceId(serviceId);
-                       List<XXServiceResourceElement> xServiceResourceElements 
= daoMgr.getXXServiceResourceElement().findByServiceId(serviceId);
-                       List<XXServiceResourceElementValue> 
xServiceResourceElementValues = 
daoMgr.getXXServiceResourceElementValue().findByServiceId(serviceId);
+                       List<XXServiceResourceElement> xServiceResourceElements 
= 
daoMgr.getXXServiceResourceElement().findForTaggedResourcesInServiceId(serviceId);
+                       List<XXServiceResourceElementValue> 
xServiceResourceElementValues = 
daoMgr.getXXServiceResourceElementValue().findForTaggedResourcesInServiceId(serviceId);
 
                        this.service = xService;
                        this.iterServiceResource = 
xServiceResources.listIterator();
@@ -318,7 +318,7 @@ public class RangerTagDBRetriever {
                        List<RangerServiceResource> ret = null;
 
                        if (service != null) {
-                               List<XXServiceResource> xServiceResources = 
daoMgr.getXXServiceResource().findByServiceId(service.getId());
+                               List<XXServiceResource> xServiceResources = 
daoMgr.getXXServiceResource().findTaggedResourcesInServiceId(service.getId());
 
                                if 
(CollectionUtils.isNotEmpty(xServiceResources)) {
                                        ret = new 
ArrayList<RangerServiceResource>(xServiceResources.size());

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java
----------------------------------------------------------------------
diff --git a/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java 
b/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java
index bf56afa..bc390b7 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java
@@ -21,8 +21,10 @@ package org.apache.ranger.biz;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
@@ -38,10 +40,13 @@ import org.apache.ranger.entity.XXDBBase;
 import org.apache.ranger.entity.XXResourceDef;
 import org.apache.ranger.entity.XXService;
 import org.apache.ranger.entity.XXServiceDef;
+import org.apache.ranger.entity.XXServiceResource;
+import org.apache.ranger.entity.XXTag;
 import org.apache.ranger.entity.XXTagAttribute;
 import org.apache.ranger.entity.XXTagAttributeDef;
 import org.apache.ranger.entity.XXServiceResourceElement;
 import org.apache.ranger.entity.XXServiceResourceElementValue;
+import org.apache.ranger.entity.XXTagResourceMap;
 import org.apache.ranger.plugin.model.*;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
 import org.apache.ranger.plugin.model.RangerTagDef.RangerTagAttributeDef;
@@ -1089,4 +1094,74 @@ public class TagDBStore extends AbstractTagStore {
                        }
                }
        }
+
+       @Override
+       public void deleteAllTagObjectsForService(String serviceName, boolean 
isResourcePrivateFlag) throws Exception {
+
+               if (LOG.isDebugEnabled()) {
+                       LOG.debug("==> 
TagDBStore.deleteAllTagObjectsForService(" + serviceName + ")");
+               }
+
+               XXService service = 
daoManager.getXXService().findByName(serviceName);
+
+               if (service != null) {
+                       Long serviceId = service.getId();
+
+                       List<XXTagResourceMap> xxTagResourceMaps = 
daoManager.getXXTagResourceMap().findByServiceId(serviceId);
+
+                       if (CollectionUtils.isNotEmpty(xxTagResourceMaps)) {
+                               for (XXTagResourceMap xxTagResourceMap : 
xxTagResourceMaps) {
+                                       try {
+                                               
daoManager.getXXTagResourceMap().remove(xxTagResourceMap);
+                                       } catch (Exception e) {
+                                               LOG.error("Error deleting 
RangerTagResourceMap with id=" + xxTagResourceMap.getId(), e);
+                                               throw e;
+                                       }
+                               }
+                       }
+
+                       List<XXServiceResourceElementValue> 
xxServiceResourceElementValues = 
daoManager.getXXServiceResourceElementValue().findByServiceId(serviceId);
+
+                       if 
(CollectionUtils.isNotEmpty(xxServiceResourceElementValues)) {
+                               for (XXServiceResourceElementValue 
xxServiceResourceElementValue : xxServiceResourceElementValues) {
+                                       try {
+                                               
daoManager.getXXServiceResourceElementValue().remove(xxServiceResourceElementValue);
+                                       } catch (Exception e) {
+                                               LOG.error("Error deleting 
ServiceResourceElementValue with id=" + xxServiceResourceElementValue.getId(), 
e);
+                                               throw e;
+                                       }
+                               }
+                       }
+
+                       List<XXServiceResourceElement> 
xxServiceResourceElements = 
daoManager.getXXServiceResourceElement().findByServiceId(serviceId);
+
+                       if 
(CollectionUtils.isNotEmpty(xxServiceResourceElements)) {
+                               for (XXServiceResourceElement 
xxServiceResourceElement : xxServiceResourceElements) {
+                                       try {
+                                               
daoManager.getXXServiceResourceElement().remove(xxServiceResourceElement);
+                                       } catch (Exception e) {
+                                               LOG.error("Error deleting 
ServiceResourceElement with id=" + xxServiceResourceElement.getId(), e);
+                                               throw e;
+                                       }
+                               }
+                       }
+
+                       List<XXServiceResource> xxServiceResources = 
daoManager.getXXServiceResource().findByServiceId(serviceId);
+
+                       if (CollectionUtils.isNotEmpty(xxServiceResources)) {
+                               for (XXServiceResource xxServiceResource : 
xxServiceResources) {
+                                       try {
+                                               
daoManager.getXXServiceResource().remove(xxServiceResource);
+                                       } catch (Exception e) {
+                                               LOG.error("Error deleting 
RangerServiceResource with id=" + xxServiceResource.getId(), e);
+                                               throw e;
+                                       }
+                               }
+                       }
+               }
+
+               if (LOG.isDebugEnabled()) {
+                       LOG.debug("<== 
TagDBStore.deleteAllTagObjectsForService(" + serviceName + ")");
+               }
+       }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementDao.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementDao.java
 
b/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementDao.java
index 56abeaf..60a95b1 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementDao.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementDao.java
@@ -58,4 +58,16 @@ public class XXServiceResourceElementDao extends 
BaseDao<XXServiceResourceElemen
                        return new ArrayList<XXServiceResourceElement>();
                }
        }
-}
+
+       public List<XXServiceResourceElement> 
findForTaggedResourcesInServiceId(Long serviceId) {
+               if (serviceId == null) {
+                       return new ArrayList<XXServiceResourceElement>();
+               }
+               try {
+                       return 
getEntityManager().createNamedQuery("XXServiceResourceElement.findForTaggedResourcesInServiceId",
 tClass)
+                                       .setParameter("serviceId", serviceId)
+                                       .getResultList();
+               } catch (NoResultException e) {
+                       return new ArrayList<XXServiceResourceElement>();
+               }
+       }}

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementValueDao.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementValueDao.java
 
b/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementValueDao.java
index 48cdbbb..f58b93e 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementValueDao.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceElementValueDao.java
@@ -72,6 +72,19 @@ public class XXServiceResourceElementValueDao extends 
BaseDao<XXServiceResourceE
        }
 
        @SuppressWarnings("unchecked")
+       public List<XXServiceResourceElementValue> 
findForTaggedResourcesInServiceId(Long serviceId) {
+               if (serviceId == null) {
+                       return new ArrayList<XXServiceResourceElementValue>();
+               }
+               try {
+                       return 
getEntityManager().createNamedQuery("XXServiceResourceElementValue.findForTaggedResourcesInServiceId")
+                                       .setParameter("serviceId", 
serviceId).getResultList();
+               } catch (NoResultException e) {
+                       return new ArrayList<XXServiceResourceElementValue>();
+               }
+       }
+
+       @SuppressWarnings("unchecked")
        public List<XXServiceResourceElementValue> findByResourceId(Long 
resourceId) {
                if (resourceId == null) {
                        return new ArrayList<XXServiceResourceElementValue>();

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
index 684f882..7c9c290 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
@@ -50,6 +50,7 @@ import org.apache.ranger.biz.AssetMgr;
 import org.apache.ranger.biz.RangerBizUtil;
 import org.apache.ranger.biz.ServiceDBStore;
 import org.apache.ranger.biz.ServiceMgr;
+import org.apache.ranger.biz.TagDBStore;
 import org.apache.ranger.biz.XUserMgr;
 import org.apache.ranger.common.GUIDUtil;
 import org.apache.ranger.common.MessageEnums;
@@ -154,6 +155,9 @@ public class ServiceREST {
        @Autowired
        RangerDaoManager daoManager;
 
+       @Autowired
+       TagDBStore tagStore;
+
        public ServiceREST() {
        }
 
@@ -539,6 +543,8 @@ public class ServiceREST {
                        XXServiceDef xxServiceDef = 
daoManager.getXXServiceDef().getById(service.getType());
                        bizUtil.hasKMSPermissions("Service", 
xxServiceDef.getImplclassname());
 
+                       
tagStore.deleteAllTagObjectsForService(service.getName(), false);
+
                        svcStore.deleteService(id);
                } catch(WebApplicationException excp) {
                        throw excp;
@@ -760,7 +766,7 @@ public class ServiceREST {
                RangerPerfTracer perf = null;
 
                try {
-                       if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
+                       if (RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
                                perf = RangerPerfTracer.getPerfTracer(PERF_LOG, 
"ServiceREST.validateConfig(serviceName=" + service.getName() + ")");
                        }
                        ret = serviceMgr.validateConfig(service, svcStore);
@@ -794,7 +800,7 @@ public class ServiceREST {
                RangerPerfTracer perf = null;
 
                try {
-                       if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
+                       if (RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
                                perf = RangerPerfTracer.getPerfTracer(PERF_LOG, 
"ServiceREST.lookupResource(serviceName=" + serviceName + ")");
                        }
                        ret = serviceMgr.lookupResource(serviceName, context, 
svcStore);
@@ -1141,7 +1147,7 @@ public class ServiceREST {
                        LOG.debug("==> ServiceREST.updatePolicy(" + policy + 
")");
                }
 
-               RangerPolicy     ret  = null;
+               RangerPolicy ret  = null;
                RangerPerfTracer perf = null;
 
                try {
@@ -1318,7 +1324,7 @@ public class ServiceREST {
                RangerPerfTracer   perf = null;
 
                try {
-                       if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
+                       if (RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) {
                                perf = RangerPerfTracer.getPerfTracer(PERF_LOG, 
"ServiceREST.getPolicies()");
                        }
                        ret = svcStore.getPolicies(filter);

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java 
b/security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java
index f81ee82..ebe71eb 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java
@@ -408,8 +408,6 @@ public class ServiceTagsProcessor {
                        LOG.debug("==> ServiceTagsProcessor.replace()");
                }
 
-               // TODO:
-               // This is an inefficient implementation. Replace by direct 
database deletes
                boolean isResourePrivateTag = 
StringUtils.equals(serviceTags.getTagModel(), 
ServiceTags.TAGMODEL_RESOURCE_PRIVATE) ? true : false;
 
                
tagStore.deleteAllTagObjectsForService(serviceTags.getServiceName(), 
isResourePrivateTag);

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
----------------------------------------------------------------------
diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml 
b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
index 8737460..3c84b62 100644
--- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
+++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml
@@ -698,11 +698,18 @@
                <query>select obj from XXServiceResourceElement obj where 
obj.resourceId = :resourceId order by obj.resourceId, obj.id</query>
        </named-query>
 
+       <named-query 
name="XXServiceResourceElement.findForTaggedResourcesInServiceId">
+               <query>select obj from XXServiceResourceElement obj where 
obj.resourceId in
+                       (select serviceresource.id from XXServiceResource 
serviceresource where serviceresource.serviceId = :serviceId and 
serviceresource.id in
+                       (select tagResMap.resourceId from XXTagResourceMap 
tagResMap)
+                       )
+                       order by obj.resourceId, obj.id
+               </query>
+       </named-query>
+
        <named-query name="XXServiceResourceElement.findByServiceId">
                <query>select obj from XXServiceResourceElement obj where 
obj.resourceId in
-                                       (select serviceresource.id from 
XXServiceResource serviceresource where serviceresource.serviceId = :serviceId 
and serviceresource.id in
-                                               (select tagResMap.resourceId 
from XXTagResourceMap tagResMap)
-                                       )
+                                       (select serviceresource.id from 
XXServiceResource serviceresource where serviceresource.serviceId = :serviceId)
                        order by obj.resourceId, obj.id
                </query>
        </named-query>
@@ -715,7 +722,7 @@
                <query>select obj.value from XXServiceResourceElementValue obj 
where obj.resElementId = :resElementId</query>
        </named-query>
 
-       <named-query name="XXServiceResourceElementValue.findByServiceId">
+       <named-query 
name="XXServiceResourceElementValue.findForTaggedResourcesInServiceId">
                <query>select obj from XXServiceResourceElementValue obj, 
XXServiceResourceElement resElem where obj.resElementId = resElem.id and 
resElem.resourceId in
                                        (select res.id from XXServiceResource 
res where res.serviceId = :serviceId and res.id in
                                                (select tagResMap.resourceId 
from XXTagResourceMap tagResMap)
@@ -724,6 +731,13 @@
                </query>
        </named-query>
 
+       <named-query name="XXServiceResourceElementValue.findByServiceId">
+               <query>select obj from XXServiceResourceElementValue obj, 
XXServiceResourceElement resElem where obj.resElementId = resElem.id and 
resElem.resourceId in
+                       (select res.id from XXServiceResource res where 
res.serviceId = :serviceId)
+                       order by resElem.resourceId, resElem.id
+               </query>
+       </named-query>
+
        <named-query name="XXServiceResourceElementValue.findByResourceId">
                <query>select obj from XXServiceResourceElementValue obj, 
XXServiceResourceElement resElem where obj.resElementId = resElem.id and 
resElem.resourceId = :resourceId
                        order by resElem.resourceId, resElem.id

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/53623080/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java 
b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
index 7d2b98f..881eed9 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
@@ -33,6 +33,7 @@ import org.apache.ranger.admin.client.datatype.RESTResponse;
 import org.apache.ranger.biz.RangerBizUtil;
 import org.apache.ranger.biz.ServiceDBStore;
 import org.apache.ranger.biz.ServiceMgr;
+import org.apache.ranger.biz.TagDBStore;
 import org.apache.ranger.biz.XUserMgr;
 import org.apache.ranger.common.ContextUtil;
 import org.apache.ranger.common.RESTErrorUtil;
@@ -113,6 +114,9 @@ public class TestServiceREST {
        ServiceDBStore svcStore;
 
        @Mock
+       TagDBStore tagStore;
+
+       @Mock
        RangerServiceService svcService;
 
        @Mock
@@ -912,7 +916,7 @@ public class TestServiceREST {
                HttpServletRequest request = 
Mockito.mock(HttpServletRequest.class);
                RangerPolicy rangerPolicy = rangerPolicy();
 
-               PList<RangerPolicy> ret  = Mockito.mock(PList.class);
+               PList<RangerPolicy> ret = Mockito.mock(PList.class);
                SearchFilter filter = new SearchFilter();
                filter.setParam(SearchFilter.POLICY_NAME, "policyName");
                filter.setParam(SearchFilter.SERVICE_NAME, "serviceName");

Reply via email to