----------------------------------------------------------- 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 > >