[
https://issues.apache.org/jira/browse/RANGER-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ramachandran updated RANGER-3903:
---------------------------------
Description:
{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();
break;
}
} {code}
was:
{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}
>From the above code, we iterate delta policies and check if this policy exists
>in the existing policy, we add that to deletePolicies.
If a delta change type is created, we expect that it should not be in the
existing old policy which should not have happened. If it happens to be found,
we log a warning.
And if the change type is updated, then it should be in deleted Policy, so if
it is empty, we log a warning that is not expected here. Same for delete.
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
> 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
> 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();
> break;
> }
> } {code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
