----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42287/#review117353 -----------------------------------------------------------
Thank Colin for the refactor, I think it's an improtant improvment. We're better to reuse authorizable and BitFieldAction, here are my comments. sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/ImplyMethodType.java (line 19) <https://reviews.apache.org/r/42287/#comment178522> I think action should be a part of Model(PrivilegeModel), we can reuse the existing action interface. For the URL type privilege, we can add a interface like PathAuthorizable or URLAuthorizable for URL type privilege, so that CommonPrvilege could handle it by URL rule. sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Model.java (line 22) <https://reviews.apache.org/r/42287/#comment178505> Nit: how about PrivilegeModel. sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Resource.java (line 20) <https://reviews.apache.org/r/42287/#comment178519> Hi Colin, I found Resource is target to distinguish URI_type_privilege and common_type_privilege, how about reuse authorizable. sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java (line 53) <https://reviews.apache.org/r/42287/#comment178524> I'm strongly agreed with to reuse the method **implies**. And I think we can use Authorizable and BitFieldAction in it. - Dapeng Sun On 一月 29, 2016, 4:41 p.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42287/ > ----------------------------------------------------------- > > (Updated 一月 29, 2016, 4:41 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Create CommonPrivilege for every component. > The main change is for the interface Privilege, implies(Privilege p) ---> > implies(Privilege privilege, Map<String, String> implyMethodMap, Map<String, > Integer> actionMap) > The actionMap is stored the mapping between action name and action value, eg: > for Hive: select ---> 1 (binary: 0000001) > insert ---> 3 (binary: 0000011) > create ---> 4 (binary: 0000100) > all ---> 7 (binary: 0000111) > when compare the action, use the action value with the & operator > The implyMethodMap is stored the mapping between resource type and imply > method, eg: > for Hive: db ---> string (imply as string value) > url ---> url (imply as url value) > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/ImplyMethodType.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Model.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Resource.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyConstants.java > 0bad8c1 > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/ColumnResourceForTest.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/DbResourceForTest.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/ModelForTest.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/ServerResourceForTest.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TableResourceForTest.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestKeyValue.java > 2dfc7c5 > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/UrlResourceForTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/42287/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
