[ 
https://issues.apache.org/jira/browse/RANGER-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ramachandran reassigned RANGER-3903:
------------------------------------

    Assignee: Ramachandran

> Improvements in RangerPolicyDeltaUtil--> applyDeltas method
> -----------------------------------------------------------
>
>                 Key: RANGER-3903
>                 URL: https://issues.apache.org/jira/browse/RANGER-3903
>             Project: Ranger
>          Issue Type: Improvement
>          Components: Ranger
>    Affects Versions: 3.0.0
>            Reporter: Ramachandran
>            Assignee: Ramachandran
>            Priority: Major
>
>  
> {code:java}
>  while (iter.hasNext()) {
>                             RangerPolicy policy = iter.next();
>                             if (policyId.equals(policy.getId()) && 
> (changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == 
> RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
>                                 deletedPolicies.add(policy);
>                                 iter.remove();
>                             }
>                         }                        switch(changeType) {
>                             case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE: 
> {
>                                 if 
> (CollectionUtils.isNotEmpty(deletedPolicies)) {
>                                     LOG.warn("Unexpected: found existing 
> policy for CHANGE_TYPE_POLICY_CREATE: " + 
> Arrays.toString(deletedPolicies.toArray()));
>                                 }
>                                 break;
>                             }
>                             case RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE: 
> {
>                                 if (CollectionUtils.isEmpty(deletedPolicies) 
> || deletedPolicies.size() > 1) {
>                                     LOG.warn("Unexpected: found no policy or 
> multiple policies for CHANGE_TYPE_POLICY_UPDATE: " + 
> Arrays.toString(deletedPolicies.toArray()));
>                                 }
>                                 break;
>                             }
>                             case RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE: 
> {
>                                 if (CollectionUtils.isEmpty(deletedPolicies) 
> || deletedPolicies.size() > 1) {
>                                     LOG.warn("Unexpected: found no policy or 
> multiple policies for CHANGE_TYPE_POLICY_DELETE: " + 
> Arrays.toString(deletedPolicies.toArray()));
>                                 }
>                                 break;
>                             }
>                             default:
>                                 break;
>                         } {code}
> From the above code, we iterate delta policies and check if this policy 
> exists in the existing policy, we add that to deletePolicies list.
> If a delta change type is created, we expect that it should not be in the 
> existing old policy which should not have happened. So the below block code 
> is not needed
> {code:java}
>  case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE: {
>                                 if 
> (CollectionUtils.isNotEmpty(deletedPolicies)) {
>                                     LOG.warn("Unexpected: found existing 
> policy for CHANGE_TYPE_POLICY_CREATE: " + 
> Arrays.toString(deletedPolicies.toArray()));
>                                 }
>                                 break;
>                             }{code}
> Proposal :
> The same id can be for multiple policies here.?
> If policy Id is uniqiue and found a match, why are still we iterating here 
> instead of adding a break 
>  
> {code:java}
> while (iter.hasNext()) {
>     RangerPolicy policy = iter.next();
>     if (policyId.equals(policy.getId()) && (changeType == 
> RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == 
> RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
>         deletedPolicies.add(policy);
>         iter.remove();
>     }
> } {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to