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