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

madhan 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 105f6f5ce RANGER-4485: refactored condition instantiation
105f6f5ce is described below

commit 105f6f5ce0e89fe68292798650c3141e40fd2d6a
Author: Madhan Neethiraj <mad...@apache.org>
AuthorDate: Thu Oct 19 10:02:41 2023 -0700

    RANGER-4485: refactored condition instantiation
---
 .../RangerCustomConditionEvaluator.java            | 182 +++++++++------------
 .../RangerDefaultPolicyEvaluator.java              |  18 +-
 .../RangerDefaultPolicyItemEvaluator.java          |  52 +-----
 .../apache/ranger/plugin/util/ServiceDefUtil.java  |  48 ++++++
 .../org/apache/ranger/biz/PolicyRefUpdater.java    |   4 +-
 .../service/RangerServiceDefServiceBase.java       |  28 +---
 .../service/TestRangerServiceDefService.java       |   3 +-
 7 files changed, 140 insertions(+), 195 deletions(-)

diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerCustomConditionEvaluator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerCustomConditionEvaluator.java
index 6f15eed8e..271edf97f 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerCustomConditionEvaluator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerCustomConditionEvaluator.java
@@ -20,161 +20,129 @@
 package org.apache.ranger.plugin.policyevaluator;
 
 import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang.StringUtils;
-import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemCondition;
 import org.apache.ranger.plugin.conditionevaluator.RangerConditionEvaluator;
 import org.apache.ranger.plugin.model.RangerPolicy;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemCondition;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
 import org.apache.ranger.plugin.model.RangerServiceDef;
+import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerPolicyConditionDef;
 import org.apache.ranger.plugin.policyengine.RangerPolicyEngineOptions;
 import org.apache.ranger.plugin.util.RangerPerfTracer;
+import org.apache.ranger.plugin.util.ServiceDefUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
-
+//
+// this class should have been named RangerConditionEvaluatorFactory
+//
 public class RangerCustomConditionEvaluator {
-
-    private static final Logger LOG = 
LoggerFactory.getLogger(RangerCustomConditionEvaluator.class);
-    private static final Logger PERF_POLICY_INIT_LOG = 
RangerPerfTracer.getPerfLogger("policy.init");
-    private static final Logger PERF_POLICYITEM_INIT_LOG = 
RangerPerfTracer.getPerfLogger("policyitem.init");
+    private static final Logger LOG                           = 
LoggerFactory.getLogger(RangerCustomConditionEvaluator.class);
+    private static final Logger PERF_POLICY_INIT_LOG          = 
RangerPerfTracer.getPerfLogger("policy.init");
+    private static final Logger PERF_POLICYITEM_INIT_LOG      = 
RangerPerfTracer.getPerfLogger("policyitem.init");
     private static final Logger PERF_POLICYCONDITION_INIT_LOG = 
RangerPerfTracer.getPerfLogger("policycondition.init");
 
-    public List<RangerConditionEvaluator> 
getRangerPolicyConditionEvaluator(RangerPolicy policy,
-                                                                               
   RangerServiceDef serviceDef,
-                                                                               
   RangerPolicyEngineOptions options) {
-        List<RangerConditionEvaluator> conditionEvaluators = new ArrayList<>();
-
-        if (!getConditionsDisabledOption(options) && 
CollectionUtils.isNotEmpty(policy.getConditions())) {
-
-            RangerPerfTracer perf = null;
-
-            long policyId = policy.getId();
-
-            if(RangerPerfTracer.isPerfTraceEnabled(PERF_POLICY_INIT_LOG)) {
-                perf = RangerPerfTracer.getPerfTracer(PERF_POLICY_INIT_LOG, 
"RangerCustomConditionEvaluator.init(policyId=" + policyId + ")");
-            }
-
-            for (RangerPolicy.RangerPolicyItemCondition condition : 
policy.getConditions()) {
-                RangerServiceDef.RangerPolicyConditionDef conditionDef = 
getConditionDef(condition.getType(),serviceDef);
-
-                if (conditionDef == null) {
-                    
LOG.error("RangerCustomConditionEvaluator.getRangerPolicyConditionEvaluator(policyId="
 + policyId + "): conditionDef '" + condition.getType() + "' not found. 
Ignoring the condition");
-
-                    continue;
-                }
-
-                RangerConditionEvaluator conditionEvaluator = 
newConditionEvaluator(conditionDef.getEvaluator());
 
-                if (conditionEvaluator != null) {
-                    conditionEvaluator.setServiceDef(serviceDef);
-                    conditionEvaluator.setConditionDef(conditionDef);
-                    conditionEvaluator.setPolicyItemCondition(condition);
+    public static RangerCustomConditionEvaluator getInstance() {
+        return RangerCustomConditionEvaluator.SingletonHolder.s_instance;
+    }
 
-                    RangerPerfTracer perfConditionInit = null;
+    private RangerCustomConditionEvaluator() {
+    }
 
-                    if 
(RangerPerfTracer.isPerfTraceEnabled(PERF_POLICYCONDITION_INIT_LOG)) {
-                        perfConditionInit = 
RangerPerfTracer.getPerfTracer(PERF_POLICYCONDITION_INIT_LOG, 
"RangerConditionEvaluator.init(policyId=" + policyId + "policyConditionType=" + 
condition.getType() + ")");
-                    }
+    public List<RangerConditionEvaluator> 
getPolicyConditionEvaluators(RangerPolicy policy, RangerServiceDef serviceDef, 
RangerPolicyEngineOptions options) {
+        RangerPerfTracer perf     = null;
+        String           parentId = "policyId=" + policy.getId() ;
 
-                    conditionEvaluator.init();
+        if (RangerPerfTracer.isPerfTraceEnabled(PERF_POLICY_INIT_LOG)) {
+            perf = RangerPerfTracer.getPerfTracer(PERF_POLICY_INIT_LOG, 
"RangerPolicyEvaluator.getPolicyConditionEvaluators(" + parentId + ")");
+        }
 
-                    RangerPerfTracer.log(perfConditionInit);
+        List<RangerConditionEvaluator> ret = getConditionEvaluators(parentId, 
policy.getConditions(), serviceDef, options);
 
-                    conditionEvaluators.add(conditionEvaluator);
-                } else {
-                    
LOG.error("RangerCustomConditionEvaluator.getRangerPolicyConditionEvaluator(policyId="
 + policyId + "): failed to init Policy ConditionEvaluator '" + 
condition.getType() + "'; evaluatorClassName='" + conditionDef.getEvaluator() + 
"'");
-                }
-            }
+        RangerPerfTracer.log(perf);
 
-            RangerPerfTracer.log(perf);
-        }
-        return conditionEvaluators;
+        return ret;
     }
 
+    public List<RangerConditionEvaluator> 
getPolicyItemConditionEvaluators(RangerPolicy policy, RangerPolicyItem 
policyItem, RangerServiceDef serviceDef, RangerPolicyEngineOptions options, int 
policyItemIndex) {
+        RangerPerfTracer perf     = null;
+        String           parentId = "policyId=" + policy.getId() + ", 
policyItemIndex=" + policyItemIndex;
 
-    public List<RangerConditionEvaluator> 
getPolicyItemConditionEvaluator(RangerPolicy policy,
-                                                                           
RangerPolicyItem policyItem,
-                                                                           
RangerServiceDef serviceDef,
-                                                                           
RangerPolicyEngineOptions options,
-                                                                           int 
policyItemIndex) {
+        if (RangerPerfTracer.isPerfTraceEnabled(PERF_POLICYITEM_INIT_LOG)) {
+            perf = RangerPerfTracer.getPerfTracer(PERF_POLICYITEM_INIT_LOG, 
"RangerPolicyItemEvaluator.getPolicyItemConditionEvaluators(" + parentId + ")");
+        }
 
-        List<RangerConditionEvaluator> conditionEvaluators = new ArrayList<>();
+        List<RangerConditionEvaluator> ret = getConditionEvaluators(parentId, 
policyItem.getConditions(), serviceDef, options);
 
-        if (!getConditionsDisabledOption(options) && 
CollectionUtils.isNotEmpty(policyItem.getConditions())) {
+        RangerPerfTracer.log(perf);
 
-            RangerPerfTracer perf = null;
+        return ret;
+    }
 
-            Long policyId = policy.getId();
+    public List<RangerConditionEvaluator> getConditionEvaluators(String 
parentId, List<RangerPolicyItemCondition> conditions, RangerServiceDef 
serviceDef, RangerPolicyEngineOptions options) {
+        final List<RangerConditionEvaluator> ret;
 
-            if(RangerPerfTracer.isPerfTraceEnabled(PERF_POLICYITEM_INIT_LOG)) {
-                perf = 
RangerPerfTracer.getPerfTracer(PERF_POLICYITEM_INIT_LOG, 
"RangerPolicyItemEvaluator.getRangerPolicyConditionEvaluator(policyId=" + 
policyId + ",policyItemIndex=" + policyItemIndex + ")");
-            }
+        if (!getConditionsDisabledOption(options) && 
CollectionUtils.isNotEmpty(conditions)) {
+            ret = new ArrayList<>(conditions.size());
 
-            for (RangerPolicyItemCondition condition : 
policyItem.getConditions()) {
-                RangerServiceDef.RangerPolicyConditionDef conditionDef = 
getConditionDef(condition.getType(), serviceDef);
+            for (RangerPolicyItemCondition condition : conditions) {
+                RangerPolicyConditionDef conditionDef = 
ServiceDefUtil.getConditionDef(serviceDef, condition.getType());
 
                 if (conditionDef == null) {
-                    
LOG.error("RangerCustomConditionEvaluator.getPolicyItemConditionEvaluator(policyId="
 + policyId + "): conditionDef '" + condition.getType() + "' not found. 
Ignoring the condition");
+                    
LOG.error("RangerCustomConditionEvaluator.getConditionEvaluators(" + parentId + 
"): conditionDef '" + condition.getType() + "' not found. Ignoring the 
condition");
 
                     continue;
                 }
 
-                RangerConditionEvaluator conditionEvaluator = 
newConditionEvaluator(conditionDef.getEvaluator());
+                RangerConditionEvaluator conditionEvaluator = 
getConditionEvaluator(parentId, condition, conditionDef, serviceDef, options);
 
                 if (conditionEvaluator != null) {
-                    conditionEvaluator.setServiceDef(serviceDef);
-                    conditionEvaluator.setConditionDef(conditionDef);
-                    conditionEvaluator.setPolicyItemCondition(condition);
-
-                    RangerPerfTracer perfConditionInit = null;
-
-                    
if(RangerPerfTracer.isPerfTraceEnabled(PERF_POLICYCONDITION_INIT_LOG)) {
-                        perfConditionInit = 
RangerPerfTracer.getPerfTracer(PERF_POLICYCONDITION_INIT_LOG, 
"RangerConditionEvaluator.init(policyId=" + policyId + ",policyItemIndex=" + 
policyItemIndex + ",policyConditionType=" + condition.getType() + ")");
-                    }
-
-                    conditionEvaluator.init();
-
-                    RangerPerfTracer.log(perfConditionInit);
-
-                    conditionEvaluators.add(conditionEvaluator);
-                } else {
-                    
LOG.error("RangerCustomConditionEvaluator.getPolicyItemConditionEvaluator(policyId="
 + policyId + "): failed to init PolicyItem ConditionEvaluator '" + 
condition.getType() + "'; evaluatorClassName='" + conditionDef.getEvaluator() + 
"'");
+                    ret.add(conditionEvaluator);
                 }
             }
-            RangerPerfTracer.log(perf);
+        } else {
+            ret = Collections.emptyList();
         }
-        return  conditionEvaluators;
+
+        return  ret;
     }
 
-    private RangerServiceDef.RangerPolicyConditionDef getConditionDef(String 
conditionName, RangerServiceDef serviceDef) {
-        if(LOG.isDebugEnabled()) {
-            LOG.debug("==> RangerCustomConditionEvaluator.getConditionDef(" + 
conditionName + ")");
-        }
+    public RangerConditionEvaluator getConditionEvaluator(String parentId, 
RangerPolicyItemCondition condition, RangerPolicyConditionDef conditionDef, 
RangerServiceDef serviceDef, RangerPolicyEngineOptions options) {
+        final RangerConditionEvaluator ret;
+
+        if (condition != null && conditionDef != null && 
!getConditionsDisabledOption(options)) {
+            ret = newConditionEvaluator(conditionDef.getEvaluator());
 
-        RangerServiceDef.RangerPolicyConditionDef ret = null;
+            if (ret != null) {
+                ret.setServiceDef(serviceDef);
+                ret.setConditionDef(conditionDef);
+                ret.setPolicyItemCondition(condition);
 
-        if (serviceDef != null && 
CollectionUtils.isNotEmpty(serviceDef.getPolicyConditions())) {
-            for(RangerServiceDef.RangerPolicyConditionDef conditionDef : 
serviceDef.getPolicyConditions()) {
-                if(StringUtils.equals(conditionName, conditionDef.getName())) {
-                    ret = conditionDef;
-                    break;
+                RangerPerfTracer perf = null;
+
+                if 
(RangerPerfTracer.isPerfTraceEnabled(PERF_POLICYCONDITION_INIT_LOG)) {
+                    perf = 
RangerPerfTracer.getPerfTracer(PERF_POLICYCONDITION_INIT_LOG, 
"RangerConditionEvaluator.init(" + parentId + ", policyConditionType=" + 
condition.getType() + ")");
                 }
-            }
-        }
 
-        if(LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerCustomConditionEvaluator.getConditionDef(" + 
conditionName + "): " + ret);
+                ret.init();
+
+                RangerPerfTracer.log(perf);
+            } else {
+                
LOG.error("RangerCustomConditionEvaluator.getConditionEvaluator(" + parentId + 
"): failed to init ConditionEvaluator '" + condition.getType() + "'; 
evaluatorClassName='" + conditionDef.getEvaluator() + "'");
+            }
+        } else {
+            ret = null;
         }
 
         return ret;
     }
 
-
     private RangerConditionEvaluator newConditionEvaluator(String className) {
-        if(LOG.isDebugEnabled()) {
+        if (LOG.isDebugEnabled()) {
             LOG.debug("==> 
RangerCustomConditionEvaluator.newConditionEvaluator(" + className + ")");
         }
 
@@ -182,14 +150,14 @@ public class RangerCustomConditionEvaluator {
 
         try {
             @SuppressWarnings("unchecked")
-            
Class<org.apache.ranger.plugin.conditionevaluator.RangerConditionEvaluator> 
matcherClass = 
(Class<org.apache.ranger.plugin.conditionevaluator.RangerConditionEvaluator>)Class.forName(className);
+            Class<RangerConditionEvaluator> evaluatorClass = 
(Class<RangerConditionEvaluator>)Class.forName(className);
 
-            evaluator = matcherClass.newInstance();
-        } catch(Throwable t) {
+            evaluator = evaluatorClass.newInstance();
+        } catch (Throwable t) {
             LOG.error("RangerCustomConditionEvaluator.newConditionEvaluator(" 
+ className + "): error instantiating evaluator", t);
         }
 
-        if(LOG.isDebugEnabled()) {
+        if (LOG.isDebugEnabled()) {
             LOG.debug("<== 
RangerCustomConditionEvaluator.newConditionEvaluator(" + className + "): " + 
evaluator);
         }
 
@@ -199,4 +167,8 @@ public class RangerCustomConditionEvaluator {
     private boolean getConditionsDisabledOption(RangerPolicyEngineOptions 
options) {
         return options != null && options.disableCustomConditions;
     }
+
+    private static class SingletonHolder {
+        private static final RangerCustomConditionEvaluator s_instance = new 
RangerCustomConditionEvaluator();
+    }
 }
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
index 8e908f6a9..bc627adf5 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
@@ -162,7 +162,7 @@ public class RangerDefaultPolicyEvaluator extends 
RangerAbstractPolicyEvaluator
 
                        dataMaskEvaluators  = 
createDataMaskPolicyItemEvaluators(policy, serviceDef, options, 
policy.getDataMaskPolicyItems());
                        rowFilterEvaluators = 
createRowFilterPolicyItemEvaluators(policy, serviceDef, options, 
policy.getRowFilterPolicyItems());
-                       conditionEvaluators = 
createRangerPolicyConditionEvaluator(policy, serviceDef, options);
+                       conditionEvaluators = 
createPolicyConditionEvaluators(policy, serviceDef, options);
                } else {
                        validityScheduleEvaluators = 
Collections.<RangerValidityScheduleEvaluator>emptyList();
                        allowEvaluators            = 
Collections.<RangerPolicyItemEvaluator>emptyList();
@@ -1542,20 +1542,12 @@ public class RangerDefaultPolicyEvaluator extends 
RangerAbstractPolicyEvaluator
                return ret;
        }
 
-       private List<RangerConditionEvaluator> 
createRangerPolicyConditionEvaluator(RangerPolicy policy,
-                                                                               
                                                                                
RangerServiceDef serviceDef,
-                                                                               
                                                                                
RangerPolicyEngineOptions options) {
-               List<RangerConditionEvaluator> rangerConditionEvaluators = null;
+       private List<RangerConditionEvaluator> 
createPolicyConditionEvaluators(RangerPolicy policy, RangerServiceDef 
serviceDef, RangerPolicyEngineOptions options) {
+               List<RangerConditionEvaluator> ret = 
RangerCustomConditionEvaluator.getInstance().getPolicyConditionEvaluators(policy,
 serviceDef, options);
 
-               RangerCustomConditionEvaluator rangerConditionEvaluator = new 
RangerCustomConditionEvaluator();
+               customConditionsCount += ret.size();
 
-               rangerConditionEvaluators = 
rangerConditionEvaluator.getRangerPolicyConditionEvaluator(policy,serviceDef,options);
-
-               if (rangerConditionEvaluators != null) {
-                       customConditionsCount += 
rangerConditionEvaluators.size();
-               }
-
-               return rangerConditionEvaluators;
+               return ret;
        }
 
 }
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
index 2528aeafa..9ed0249ef 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
@@ -30,7 +30,6 @@ import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerServiceDef;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemAccess;
-import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerPolicyConditionDef;
 import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
 import org.apache.ranger.plugin.policyengine.RangerAccessResource;
 import org.apache.ranger.plugin.policyengine.RangerAccessResult;
@@ -61,9 +60,7 @@ public class RangerDefaultPolicyItemEvaluator extends 
RangerAbstractPolicyItemEv
                        LOG.debug("==> 
RangerDefaultPolicyItemEvaluator(policyId=" + policyId + ", policyItem=" + 
policyItem + ", serviceType=" + getServiceType() + ", conditionsDisabled=" + 
getConditionsDisabledOption() + ")");
                }
 
-               RangerCustomConditionEvaluator rangerCustomConditionEvaluator = 
new RangerCustomConditionEvaluator();
-
-               conditionEvaluators = 
rangerCustomConditionEvaluator.getPolicyItemConditionEvaluator(policy, 
policyItem, serviceDef, options, policyItemIndex);
+               conditionEvaluators = 
RangerCustomConditionEvaluator.getInstance().getPolicyItemConditionEvaluators(policy,
 policyItem, serviceDef, options, policyItemIndex);
 
                List<String> users = policyItem.getUsers();
                this.hasCurrentUser = CollectionUtils.isNotEmpty(users) && 
users.contains(RangerPolicyEngine.USER_CURRENT);
@@ -291,51 +288,4 @@ public class RangerDefaultPolicyItemEvaluator extends 
RangerAbstractPolicyItemEv
        public void updateAccessResult(RangerPolicyEvaluator policyEvaluator, 
RangerAccessResult result, RangerPolicyResourceMatcher.MatchType matchType) {
                policyEvaluator.updateAccessResult(result, matchType, 
getPolicyItemType() != RangerPolicyItemEvaluator.POLICY_ITEM_TYPE_DENY, 
getComments());
        }
-
-       RangerPolicyConditionDef getConditionDef(String conditionName) {
-               if (LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerDefaultPolicyItemEvaluator.getConditionDef(" + conditionName + ")");
-               }
-
-               RangerPolicyConditionDef ret = null;
-
-               if (serviceDef != null && 
CollectionUtils.isNotEmpty(serviceDef.getPolicyConditions())) {
-                       for (RangerPolicyConditionDef conditionDef : 
serviceDef.getPolicyConditions()) {
-                               if (StringUtils.equals(conditionName, 
conditionDef.getName())) {
-                                       ret = conditionDef;
-
-                                       break;
-                               }
-                       }
-               }
-
-               if (LOG.isDebugEnabled()) {
-                       LOG.debug("<== 
RangerDefaultPolicyItemEvaluator.getConditionDef(" + conditionName + "): " + 
ret);
-               }
-
-               return ret;
-       }
-
-       RangerConditionEvaluator newConditionEvaluator(String className) {
-               if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerDefaultPolicyItemEvaluator.newConditionEvaluator(" + className + ")");
-               }
-
-               RangerConditionEvaluator evaluator = null;
-
-               try {
-                       @SuppressWarnings("unchecked")
-                       Class<RangerConditionEvaluator> matcherClass = 
(Class<RangerConditionEvaluator>)Class.forName(className);
-
-                       evaluator = matcherClass.newInstance();
-               } catch(Throwable t) {
-                       
LOG.error("RangerDefaultPolicyItemEvaluator.newConditionEvaluator(" + className 
+ "): error instantiating evaluator", t);
-               }
-
-               if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== 
RangerDefaultPolicyItemEvaluator.newConditionEvaluator(" + className + "): " + 
evaluator);
-               }
-
-               return evaluator;
-       }
 }
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java 
b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java
index 53eb0f81e..d78674d51 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java
@@ -23,6 +23,7 @@ import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
+import 
org.apache.ranger.plugin.conditionevaluator.RangerScriptConditionEvaluator;
 import org.apache.ranger.plugin.contextenricher.RangerUserStoreEnricher;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
@@ -34,6 +35,7 @@ import 
org.apache.ranger.plugin.model.RangerPolicy.RangerRowFilterPolicyItem;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerDataMaskPolicyItem;
 import org.apache.ranger.plugin.model.RangerPolicyDelta;
 import org.apache.ranger.plugin.model.RangerServiceDef;
+import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerPolicyConditionDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef;
 import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerContextEnricherDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerDataMaskTypeDef;
@@ -57,6 +59,11 @@ import java.util.Map;
 public class ServiceDefUtil {
     private static final Logger LOG = 
LoggerFactory.getLogger(ServiceDefUtil.class);
 
+    public static final String IMPLICIT_CONDITION_EXPRESSION_EVALUATOR = 
RangerScriptConditionEvaluator.class.getCanonicalName();
+    public static final String IMPLICIT_CONDITION_EXPRESSION_NAME      = 
"_expression";
+    public static final String IMPLICIT_CONDITION_EXPRESSION_LABEL     = 
"Enter boolean expression";
+    public static final String IMPLICIT_CONDITION_EXPRESSION_DESC      = 
"Boolean expression";
+
     private static final String USER_STORE_ENRICHER = 
RangerUserStoreEnricher.class.getCanonicalName();
 
     public static boolean 
getOption_enableDenyAndExceptionsInPolicies(RangerServiceDef serviceDef, 
RangerPluginContext pluginContext) {
@@ -99,6 +106,22 @@ public class ServiceDefUtil {
         return serviceDef;
     }
 
+    public static RangerPolicyConditionDef getConditionDef(RangerServiceDef 
serviceDef, String conditionName) {
+        RangerPolicyConditionDef ret = null;
+
+        if (serviceDef != null && serviceDef.getPolicyConditions() != null) {
+            for (RangerPolicyConditionDef conditionDef : 
serviceDef.getPolicyConditions()) {
+                if (StringUtils.equals(conditionDef.getName(), conditionName)) 
{
+                    ret = conditionDef;
+
+                    break;
+                }
+            }
+        }
+
+        return ret;
+    }
+
     public static RangerResourceDef getResourceDef(RangerServiceDef 
serviceDef, String resource) {
         RangerResourceDef ret = null;
 
@@ -589,6 +612,31 @@ public class ServiceDefUtil {
         return ret;
     }
 
+    public static RangerPolicyConditionDef 
createImplicitExpressionConditionDef(Long itemId) {
+        RangerPolicyConditionDef ret = new RangerPolicyConditionDef(itemId, 
IMPLICIT_CONDITION_EXPRESSION_NAME, IMPLICIT_CONDITION_EXPRESSION_EVALUATOR, 
new HashMap<>());
+
+        ret.getEvaluatorOptions().put("ui.isMultiline", "true");
+        ret.setLabel(IMPLICIT_CONDITION_EXPRESSION_LABEL);
+        ret.setDescription(IMPLICIT_CONDITION_EXPRESSION_DESC);
+        ret.setUiHint("{ \"isMultiline\":true }");
+
+        return ret;
+    }
+
+    public static long getConditionsMaxItemId(List<RangerPolicyConditionDef> 
conditions) {
+        long ret = 0;
+
+        if (conditions != null) {
+            for (RangerPolicyConditionDef condition : conditions) {
+                if (condition != null && condition.getItemId() != null && ret 
< condition.getItemId()) {
+                    ret = condition.getItemId();
+                }
+            }
+        }
+
+        return ret;
+    }
+
     private static boolean 
anyPolicyHasUserGroupAttributeExpression(List<RangerPolicy> policies) {
         boolean ret = false;
 
diff --git 
a/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java 
b/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java
index 83f662518..dead32e87 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java
@@ -53,8 +53,8 @@ import 
org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemAccess;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemCondition;
 import 
org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemDataMaskInfo;
 import org.apache.ranger.plugin.model.RangerRole;
+import org.apache.ranger.plugin.util.ServiceDefUtil;
 import org.apache.ranger.service.RangerAuditFields;
-import org.apache.ranger.service.RangerServiceDefService;
 import org.apache.ranger.service.XGroupService;
 import org.apache.ranger.view.VXGroup;
 import org.apache.ranger.view.VXResponse;
@@ -253,7 +253,7 @@ public class PolicyRefUpdater {
                        XXPolicyConditionDef xPolCondDef = 
daoMgr.getXXPolicyConditionDef().findByServiceDefIdAndName(xServiceDef.getId(), 
condition);
 
                        if (xPolCondDef == null) {
-                               if (StringUtils.equalsIgnoreCase(condition, 
RangerServiceDefService.IMPLICIT_CONDITION_EXPRESSION_NAME)) {
+                               if (StringUtils.equalsIgnoreCase(condition, 
ServiceDefUtil.IMPLICIT_CONDITION_EXPRESSION_NAME)) {
                                        continue;
                                }
 
diff --git 
a/security-admin/src/main/java/org/apache/ranger/service/RangerServiceDefServiceBase.java
 
b/security-admin/src/main/java/org/apache/ranger/service/RangerServiceDefServiceBase.java
index dcbfbfdc2..e820c8b61 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/service/RangerServiceDefServiceBase.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/service/RangerServiceDefServiceBase.java
@@ -37,7 +37,6 @@ 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.entity.*;
-import 
org.apache.ranger.plugin.conditionevaluator.RangerScriptConditionEvaluator;
 import org.apache.ranger.plugin.model.RangerServiceDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef;
 import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerContextEnricherDef;
@@ -56,6 +55,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import static 
org.apache.ranger.plugin.util.ServiceDefUtil.IMPLICIT_CONDITION_EXPRESSION_EVALUATOR;
+
 public abstract class RangerServiceDefServiceBase<T extends XXServiceDefBase, 
V extends RangerServiceDef>
                extends RangerBaseModelService<T, V> {
        private static final Logger LOG = 
LoggerFactory.getLogger(RangerServiceDefServiceBase.class);
@@ -63,10 +64,6 @@ public abstract class RangerServiceDefServiceBase<T extends 
XXServiceDefBase, V
        private static final String OPTION_RESOURCE_ACCESS_TYPE_RESTRICTIONS = 
"__accessTypeRestrictions";
        private static final String OPTION_RESOURCE_IS_VALID_LEAF            = 
"__isValidLeaf";
        public static final String PROP_ENABLE_IMPLICIT_CONDITION_EXPRESSION = 
"ranger.servicedef.enableImplicitConditionExpression";
-       public static final String IMPLICIT_CONDITION_EXPRESSION_EVALUATOR   = 
RangerScriptConditionEvaluator.class.getCanonicalName();
-       public static final String IMPLICIT_CONDITION_EXPRESSION_NAME        = 
"_expression";
-       public static final String IMPLICIT_CONDITION_EXPRESSION_LABEL       = 
"Enter boolean expression";
-       public static final String IMPLICIT_CONDITION_EXPRESSION_DESC        = 
"Boolean expression";
 
        @Autowired
        RangerAuditFields<?> rangerAuditFields;
@@ -724,7 +721,6 @@ public abstract class RangerServiceDefServiceBase<T extends 
XXServiceDefBase, V
 
                if (implicitConditionEnabled) {
                        boolean                        exists        = false;
-                       Long                           maxItemId     = 0L;
                        List<RangerPolicyConditionDef> conditionDefs = 
serviceDef.getPolicyConditions();
 
                        if (conditionDefs == null) {
@@ -737,26 +733,12 @@ public abstract class RangerServiceDefServiceBase<T 
extends XXServiceDefBase, V
 
                                        break;
                                }
-
-                               if (conditionDef.getItemId() != null && 
maxItemId < conditionDef.getItemId()) {
-                                       maxItemId = conditionDef.getItemId();
-                               }
                        }
 
                        if (!exists) {
-                               RangerPolicyConditionDef conditionDef = new 
RangerPolicyConditionDef();
-                               Map<String, String>      options      = new 
HashMap<>();
-
-                               options.put("ui.isMultiline", "true");
-
-                               conditionDef.setItemId(maxItemId + 1);
-                               
conditionDef.setName(IMPLICIT_CONDITION_EXPRESSION_NAME);
-                               
conditionDef.setLabel(IMPLICIT_CONDITION_EXPRESSION_LABEL);
-                               
conditionDef.setDescription(IMPLICIT_CONDITION_EXPRESSION_DESC);
-                               
conditionDef.setEvaluator(IMPLICIT_CONDITION_EXPRESSION_EVALUATOR);
-                               conditionDef.setEvaluatorOptions(options);
-                               conditionDef.setUiHint("{ \"isMultiline\":true 
}");
-                               conditionDefs.add(conditionDef);
+                               long maxItemId = 
ServiceDefUtil.getConditionsMaxItemId(conditionDefs);
+
+                               
conditionDefs.add(ServiceDefUtil.createImplicitExpressionConditionDef(maxItemId 
+ 1));
 
                                serviceDef.setPolicyConditions(conditionDefs);
 
diff --git 
a/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceDefService.java
 
b/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceDefService.java
index 31f698292..789428854 100644
--- 
a/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceDefService.java
+++ 
b/security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceDefService.java
@@ -38,6 +38,7 @@ import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerEnumDef;
 import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerPolicyConditionDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerServiceConfigDef;
+import org.apache.ranger.plugin.util.ServiceDefUtil;
 import org.apache.ranger.security.context.RangerContextHolder;
 import org.apache.ranger.security.context.RangerSecurityContext;
 import org.junit.Assert;
@@ -768,7 +769,7 @@ public class TestRangerServiceDefService {
                boolean exists = false;
 
                for (RangerPolicyConditionDef conditionDef : 
serviceDef.getPolicyConditions()) {
-                       if (StringUtils.equals(conditionDef.getEvaluator(), 
RangerServiceDefService.IMPLICIT_CONDITION_EXPRESSION_EVALUATOR)) {
+                       if (StringUtils.equals(conditionDef.getEvaluator(), 
ServiceDefUtil.IMPLICIT_CONDITION_EXPRESSION_EVALUATOR)) {
                                exists = true;
 
                                break;

Reply via email to