This is an automated email from the ASF dual-hosted git repository.

pradeep pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git


The following commit(s) were added to refs/heads/master by this push:
     new 65020a7  RANGER-2535 : Good coding practices for storing and 
retrieving data history in ranger
65020a7 is described below

commit 65020a7bf1cf3b38b9ed3e558c88c7c3574ebb79
Author: Nikhil P <[email protected]>
AuthorDate: Wed Aug 21 11:13:26 2019 +0530

    RANGER-2535 : Good coding practices for storing and retrieving data history 
in ranger
    
    Signed-off-by: Pradeep <[email protected]>
---
 .../apache/ranger/plugin/store/ServiceStore.java   |  2 +-
 .../java/org/apache/ranger/biz/ServiceDBStore.java | 14 +++---
 .../java/org/apache/ranger/rest/ServiceREST.java   |  2 +-
 .../ranger/service/RangerDataHistService.java      | 11 +----
 ...lsService.java => RangerPolicyLabelHelper.java} | 50 +++-------------------
 .../ranger/service/RangerPolicyLabelsService.java  | 35 +--------------
 .../org/apache/ranger/biz/TestServiceDBStore.java  |  5 +--
 7 files changed, 20 insertions(+), 99 deletions(-)

diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
b/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java
index 2af5845..ba7407f 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java
@@ -71,7 +71,7 @@ public interface ServiceStore {
 
        void deletePolicy(RangerPolicy policy, RangerService service) throws 
Exception;
 
-       void deletePolicy(Long id) throws Exception;
+       void deletePolicy(RangerPolicy policy) throws Exception;
 
        RangerPolicy getPolicy(Long id) throws Exception;
 
diff --git 
a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
index cdda7bd..fc4b40d 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
@@ -136,6 +136,7 @@ import org.apache.ranger.rest.ServiceREST;
 import org.apache.ranger.rest.TagREST;
 import org.apache.ranger.service.RangerAuditFields;
 import org.apache.ranger.service.RangerDataHistService;
+import org.apache.ranger.service.RangerPolicyLabelHelper;
 import org.apache.ranger.service.RangerPolicyLabelsService;
 import org.apache.ranger.service.RangerPolicyService;
 import org.apache.ranger.service.RangerPolicyWithAssignedIdService;
@@ -242,6 +243,9 @@ public class ServiceDBStore extends AbstractServiceStore {
        @Autowired
         RangerPolicyLabelsService<XXPolicyLabel, ?> policyLabelsService;
 
+       @Autowired
+       RangerPolicyLabelHelper policyLabelsHelper;
+
         @Autowired
        XUserService xUserService;
        
@@ -1934,7 +1938,7 @@ public class ServiceDBStore extends AbstractServiceStore {
                        XXPolicyLabel xxPolicyLabel = 
daoMgr.getXXPolicyLabels().findByName(policyLabel);
                        if(xxPolicyLabel == null) {
                                synchronized(this) {
-                                       xxPolicyLabel  = 
policyLabelsService.createNewOrGetLabel(policyLabel, xPolicy);
+                                       xxPolicyLabel  = 
policyLabelsHelper.createNewOrGetLabel(policyLabel, xPolicy);
                                }
                        }
                        //label mapping with policy
@@ -2051,15 +2055,13 @@ public class ServiceDBStore extends 
AbstractServiceStore {
        }
 
        @Override
-       public void deletePolicy(Long policyId) throws Exception {
+       public void deletePolicy(RangerPolicy policy) throws Exception {
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> ServiceDBStore.deletePolicy(" + policyId 
+ ")");
+                       LOG.debug("==> ServiceDBStore.deletePolicy(" + policy + 
")");
                }
 
-               RangerPolicy policy = getPolicy(policyId);
-
                if(policy == null) {
-                       throw new Exception("no policy exists with ID=" + 
policyId);
+                       throw new Exception("No such policy exists");
                }
 
                String policyName = policy.getName();
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 bae04fe..bb825b8 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
@@ -1786,7 +1786,7 @@ public class ServiceREST {
 
                        ensureAdminAccess(policy);
                         bizUtil.blockAuditorRoleUser();
-                       svcStore.deletePolicy(id);
+                       svcStore.deletePolicy(policy);
                } catch(WebApplicationException excp) {
                        throw excp;
                } catch(Throwable excp) {
diff --git 
a/security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
 
b/security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
index 7bd0681..ced9ea8 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
@@ -26,8 +26,6 @@ import org.apache.ranger.common.MessageEnums;
 import org.apache.ranger.common.RESTErrorUtil;
 import org.apache.ranger.db.RangerDaoManager;
 import org.apache.ranger.entity.XXDataHist;
-import org.apache.ranger.entity.XXService;
-import org.apache.ranger.entity.XXServiceDef;
 import org.apache.ranger.plugin.model.RangerBaseModelObject;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerService;
@@ -94,14 +92,7 @@ public class RangerDataHistService {
                        RangerPolicy policy = (RangerPolicy) baseModelObj;
                        objectName = policy.getName();
                        classType = AppConstants.CLASS_TYPE_RANGER_POLICY;
-                        XXService xXService = 
daoMgr.getXXService().findByName(policy.getService());
-                        XXServiceDef xxServiceDef = null;
-                        if(xXService != null){
-                                xxServiceDef = 
daoMgr.getXXServiceDef().getById(xXService.getType());
-                        }
-                        if(xxServiceDef != null){
-                                policy.setServiceType(xxServiceDef.getName());
-                        }
+                       policy.setServiceType(policy.getServiceType());
                        content = jsonUtil.writeObjectAsString(policy);
                }
                
diff --git 
a/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
 
b/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
similarity index 61%
copy from 
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
copy to 
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
index 1a1b56e..2e17092 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
@@ -21,15 +21,9 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.ranger.biz.ServiceDBStore;
-import org.apache.ranger.common.SearchField;
-import org.apache.ranger.common.SortField;
-import org.apache.ranger.common.SearchField.DATA_TYPE;
-import org.apache.ranger.common.SearchField.SEARCH_TYPE;
-import org.apache.ranger.common.SortField.SORT_ORDER;
+import org.apache.ranger.db.RangerDaoManager;
 import org.apache.ranger.entity.XXPolicy;
 import org.apache.ranger.entity.XXPolicyLabel;
-import org.apache.ranger.plugin.model.RangerPolicy;
-import org.apache.ranger.plugin.util.SearchFilter;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Service;
@@ -38,50 +32,20 @@ import 
org.springframework.transaction.annotation.Transactional;
 
 @Service
 @Scope("singleton")
-public class RangerPolicyLabelsService<T extends XXPolicyLabel, V extends 
RangerPolicy>
-               extends RangerBaseModelService<T, V> {
-
+public class RangerPolicyLabelHelper {
        private static final Log LOG = LogFactory.getLog(ServiceDBStore.class);
 
        @Autowired
-       RangerAuditFields<?> rangerAuditFields;
-
-       public RangerPolicyLabelsService() {
-               super();
-               searchFields.add(
-                               new SearchField(SearchFilter.POLICY_LABEL, 
"obj.policyLabel", DATA_TYPE.STRING, SEARCH_TYPE.PARTIAL));
-               sortFields.add(new SortField(SearchFilter.POLICY_LABEL_ID, 
"obj.id", true, SORT_ORDER.ASC));
-       }
-
-       @Override
-       protected T mapViewToEntityBean(V viewBean, T t, int OPERATION_CONTEXT) 
{
-               // TODO Auto-generated method stub
-               return null;
-       }
-
-       @Override
-       protected V mapEntityToViewBean(V viewBean, T t) {
-               // TODO Auto-generated method stub
-               return null;
-       }
+       protected RangerDaoManager daoMgr;
 
-       @Override
-       protected void validateForCreate(V vObj) {
-               // TODO Auto-generated method stub
-
-       }
-
-       @Override
-       protected void validateForUpdate(V vObj, T entityObj) {
-               // TODO Auto-generated method stub
-
-       }
+       @Autowired
+       RangerAuditFields<?> rangerAuditFields;
 
        @Transactional(propagation = Propagation.REQUIRES_NEW)
        public XXPolicyLabel createNewOrGetLabel(String policyLabel, XXPolicy 
xPolicy) {
 
                if (LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerPolicyLabelService.createNewOrGetLabel()");
+                       LOG.debug("==> 
RangerPolicyLabelHelper.createNewOrGetLabel()");
                }
 
                XXPolicyLabel xxPolicyLabel = 
daoMgr.getXXPolicyLabels().findByName(policyLabel);
@@ -95,7 +59,7 @@ public class RangerPolicyLabelsService<T extends 
XXPolicyLabel, V extends Ranger
                }
 
                if (LOG.isDebugEnabled()) {
-                       LOG.debug("<== 
RangerPolicyLabelService.createNewOrGetLabel(), xxPolicyLabel = " + 
xxPolicyLabel);
+                       LOG.debug("<== 
RangerPolicyLabelHelper.createNewOrGetLabel(), xxPolicyLabel = " + 
xxPolicyLabel);
                }
 
                return xxPolicyLabel;
diff --git 
a/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
 
b/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
index 1a1b56e..ece9997 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
@@ -17,32 +17,23 @@
 
 package org.apache.ranger.service;
 
-import org.apache.commons.lang.StringUtils;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.ranger.biz.ServiceDBStore;
 import org.apache.ranger.common.SearchField;
-import org.apache.ranger.common.SortField;
 import org.apache.ranger.common.SearchField.DATA_TYPE;
 import org.apache.ranger.common.SearchField.SEARCH_TYPE;
+import org.apache.ranger.common.SortField;
 import org.apache.ranger.common.SortField.SORT_ORDER;
-import org.apache.ranger.entity.XXPolicy;
 import org.apache.ranger.entity.XXPolicyLabel;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.util.SearchFilter;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Service;
-import org.springframework.transaction.annotation.Propagation;
-import org.springframework.transaction.annotation.Transactional;
 
 @Service
 @Scope("singleton")
 public class RangerPolicyLabelsService<T extends XXPolicyLabel, V extends 
RangerPolicy>
                extends RangerBaseModelService<T, V> {
 
-       private static final Log LOG = LogFactory.getLog(ServiceDBStore.class);
-
        @Autowired
        RangerAuditFields<?> rangerAuditFields;
 
@@ -77,28 +68,4 @@ public class RangerPolicyLabelsService<T extends 
XXPolicyLabel, V extends Ranger
 
        }
 
-       @Transactional(propagation = Propagation.REQUIRES_NEW)
-       public XXPolicyLabel createNewOrGetLabel(String policyLabel, XXPolicy 
xPolicy) {
-
-               if (LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerPolicyLabelService.createNewOrGetLabel()");
-               }
-
-               XXPolicyLabel xxPolicyLabel = 
daoMgr.getXXPolicyLabels().findByName(policyLabel);
-               if (xxPolicyLabel == null) {
-                       xxPolicyLabel = new XXPolicyLabel();
-                       if (StringUtils.isNotEmpty(policyLabel)) {
-                               xxPolicyLabel.setPolicyLabel(policyLabel);
-                               xxPolicyLabel = 
rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabel);
-                               xxPolicyLabel = 
daoMgr.getXXPolicyLabels().create(xxPolicyLabel);
-                       }
-               }
-
-               if (LOG.isDebugEnabled()) {
-                       LOG.debug("<== 
RangerPolicyLabelService.createNewOrGetLabel(), xxPolicyLabel = " + 
xxPolicyLabel);
-               }
-
-               return xxPolicyLabel;
-       }
-
 }
diff --git 
a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
index 2be3718..69c8a4c 100644
--- a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
+++ b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
@@ -1975,9 +1975,6 @@ public class TestServiceDBStore {
                serviceConfigDefObj.setId(Id);
                xServiceConfigDefList.add(serviceConfigDefObj);
 
-               
Mockito.when(policyService.read(rangerPolicy.getId())).thenReturn(
-                               rangerPolicy);
-
                Mockito.when(daoManager.getXXService()).thenReturn(xServiceDao);
                Mockito.when(xServiceDao.findByName(name)).thenReturn(xService);
                
Mockito.when(svcService.getPopulatedViewObject(xService)).thenReturn(
@@ -1995,7 +1992,7 @@ public class TestServiceDBStore {
                Mockito.when(!bizUtil.hasAccess(xService, 
null)).thenReturn(true);
         
Mockito.when(policyRefUpdater.cleanupRefTables(rangerPolicy)).thenReturn(true);
 
-               serviceDBStore.deletePolicy(Id);
+               serviceDBStore.deletePolicy(rangerPolicy);
        }
 
        @Test

Reply via email to