----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32007/#review76309 -----------------------------------------------------------
@Abhay - Sending the comments for the review done so far. I will contine review later today. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123836> Please check for policyRepository == null. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123837> Please check for policyRepository == null. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123838> To keep consistent with other debug messages, please use ==> for enter and <== for exit. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123839> auditHandler can continue to remain in policyEngine, instead of moving to policyRepository. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123840> Please check for policyRepository == null. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123841> Please check for policyRepository == null. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123842> Please check for policyRepository == null. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java <https://reviews.apache.org/r/32007/#comment123843> Please check for policyRepository == null. Please review all references to policyRepository. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java <https://reviews.apache.org/r/32007/#comment123848> Instead of returning false, this should forward to delegate. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java <https://reviews.apache.org/r/32007/#comment123849> Instead of returning false, this should forward to delegate. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java <https://reviews.apache.org/r/32007/#comment123850> Please verify and handle cases where getConditionEvaluators() is null. Also please use "Integer.compare(lhs, rhs)", instead of using minus operator. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java <https://reviews.apache.org/r/32007/#comment123851> A better one would be to use "return Integer.compare(lhs, hrs)" agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java <https://reviews.apache.org/r/32007/#comment123853> This should come from a configuration - RangerConfiguration.get("ranger.plugin..."). - Madhan Neethiraj On March 12, 2015, 11:37 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32007/ > ----------------------------------------------------------- > > (Updated March 12, 2015, 11:37 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-307 > https://issues.apache.org/jira/browse/RANGER-307 > > > Repository: ranger > > > Description > ------- > > RANGER-307: Improvement to make policy execution more efficient by sorting > policies in a way that a policy more likely to allow access is evaluated > first. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/CacheMap.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessData.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessResult.java > 2eaec16 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > 51787ac > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 0ac5eed > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java > PRE-CREATION > > agents-common/src/test/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluatorTest.java > 9256995 > agents-common/src/test/resources/policyengine/test_policyengine_hdfs.json > 943fe80 > pom.xml 7df033d > src/main/assembly/ranger-src.xml cf6b1da > > Diff: https://reviews.apache.org/r/32007/diff/ > > > Testing > ------- > > Verified that the code builds cleanly and all existing unit testcases pass. > > > Thanks, > > Madhan Neethiraj > >
