-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72670/#review221204
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
Lines 1762 (patched)
<https://reviews.apache.org/r/72670/#comment309989>

    Consider making line# #1762 - #1764 a little easier to read. Is this about 
checking value of PARAM_MERGE_IF_EXISTS for "true" vs "false"?


- Madhan Neethiraj


On July 14, 2020, 12:24 a.m., Abhay Kulkarni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72670/
> -----------------------------------------------------------
> 
> (Updated July 14, 2020, 12:24 a.m.)
> 
> 
> Review request for ranger, Dineshkumar Yadav, Madhan Neethiraj, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2772
>     https://issues.apache.org/jira/browse/RANGER-2772
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> applyPolicy() semantics implies that it is equivalent to createPolicy() with 
> 'mergeIfExists' set to value "true". However, a regression was introduced by 
> a patch for RANGER-2772 which violates this implication; 'mergeIfExists' is 
> presumed to be set to "false" by default. This leads to applyPolicy() to 
> implement updatePolicy() semantics when it's target is an existing policy.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> 3422e436b 
> 
> 
> Diff: https://reviews.apache.org/r/72670/diff/1/
> 
> 
> Testing
> -------
> 
> Verified that default value of 'mergeIfExists' is set up correctly in all 
> (two) contexts.
> 1. When it is used from createPolicy(), and
> 2. When it is invoked through REST API.
> 
> Verified that legacy code using applyPolicy REST API is not broken.
> 
> 
> Thanks,
> 
> Abhay Kulkarni
> 
>

Reply via email to