You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Zsombor Gegesy <gz...@gmail.com> on 2017/02/12 14:36:30 UTC

Review Request 56579: [RANGER-1377] - code cleanup in agents-common module

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

Review request for ranger.


Bugs: RANGER-1377
    https://issues.apache.org/jira/browse/RANGER-1377


Repository: ranger


Description
-------

* use collection methods properly - addAll/removeAll/isEmpty,
* string comparison methods
* and simplify the conditions, when multiple ifs are used


Diffs
-----

  agents-common/src/main/java/org/apache/hadoop/security/SecureClientLogin.java e8516cd 
  agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java 4966a78 
  agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerLegacyConfigBuilder.java 2d1a56c 
  agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java 57570c2 
  agents-common/src/main/java/org/apache/ranger/plugin/audit/RangerDefaultAuditHandler.java b7a6f6e 
  agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java bd7d770 
  agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesAccessedTogetherCondition.java 598cd0c 
  agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesNotAccessedTogetherCondition.java a166fe1 
  agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerScriptExecutionContext.java b8ee2f3 
  agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerTimeOfDayMatcher.java b561142 
  agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java 0f681f8 
  agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerFileBasedTagRetriever.java 031a59f 
  agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java ec10524 
  agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationDatabase.java e4a7dcc 
  agents-common/src/main/java/org/apache/ranger/plugin/geo/ValuePrinter.java 62d6891 
  agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java 40667ee 
  agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java b3e0964 
  agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b401c10 
  agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java 89f36cd 
  agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java d296879 
  agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java 14b5402 
  agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceAccessInfo.java 6a00ce3 
  agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java 67fb09e 
  agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java cfee884 
  agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java 86a18ba 
  agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java d1c21ff 
  agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java acfbf02 
  agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java 2df0201 
  agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java df216a7 
  agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 023b4f8 
  agents-common/src/main/java/org/apache/ranger/plugin/store/EmbeddedServiceDefsUtil.java 07c7a1e 
  agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java f3f7ffc 
  agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRequestedResources.java c73d68a 
  agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java bb4d145 
  agents-common/src/main/java/org/apache/ranger/services/tag/RangerServiceTag.java 859cdd9 
  agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerIpMatcherTest.java 10780cf 
  agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerSimpleMatcher.java ed96773 
  agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java 1728c22 

Diff: https://reviews.apache.org/r/56579/diff/


Testing
-------


Thanks,

Zsombor Gegesy


Re: Review Request 56579: [RANGER-1377] - code cleanup in agents-common module

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56579/#review165314
-----------------------------------------------------------



Looks good thanks, I just took one change out that I was unsure of (removing Integer.toString() in RangerDefaultAuditHandler)

- Colm O hEigeartaigh


On Feb. 12, 2017, 2:36 p.m., Zsombor Gegesy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56579/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2017, 2:36 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1377
>     https://issues.apache.org/jira/browse/RANGER-1377
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> * use collection methods properly - addAll/removeAll/isEmpty,
> * string comparison methods
> * and simplify the conditions, when multiple ifs are used
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/hadoop/security/SecureClientLogin.java e8516cd 
>   agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java 4966a78 
>   agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerLegacyConfigBuilder.java 2d1a56c 
>   agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java 57570c2 
>   agents-common/src/main/java/org/apache/ranger/plugin/audit/RangerDefaultAuditHandler.java b7a6f6e 
>   agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java bd7d770 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesAccessedTogetherCondition.java 598cd0c 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesNotAccessedTogetherCondition.java a166fe1 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerScriptExecutionContext.java b8ee2f3 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerTimeOfDayMatcher.java b561142 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java 0f681f8 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerFileBasedTagRetriever.java 031a59f 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java ec10524 
>   agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationDatabase.java e4a7dcc 
>   agents-common/src/main/java/org/apache/ranger/plugin/geo/ValuePrinter.java 62d6891 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java 40667ee 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java b3e0964 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b401c10 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java 89f36cd 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java d296879 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java 14b5402 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceAccessInfo.java 6a00ce3 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java 67fb09e 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java cfee884 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java 86a18ba 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java d1c21ff 
>   agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java acfbf02 
>   agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java 2df0201 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java df216a7 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 023b4f8 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/EmbeddedServiceDefsUtil.java 07c7a1e 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java f3f7ffc 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRequestedResources.java c73d68a 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java bb4d145 
>   agents-common/src/main/java/org/apache/ranger/services/tag/RangerServiceTag.java 859cdd9 
>   agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerIpMatcherTest.java 10780cf 
>   agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerSimpleMatcher.java ed96773 
>   agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java 1728c22 
> 
> Diff: https://reviews.apache.org/r/56579/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>


Re: Review Request 56579: [RANGER-1377] - code cleanup in agents-common module

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56579/#review165315
-----------------------------------------------------------


Ship it!




Ship It!

- Colm O hEigeartaigh


On Feb. 12, 2017, 2:36 p.m., Zsombor Gegesy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56579/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2017, 2:36 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1377
>     https://issues.apache.org/jira/browse/RANGER-1377
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> * use collection methods properly - addAll/removeAll/isEmpty,
> * string comparison methods
> * and simplify the conditions, when multiple ifs are used
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/hadoop/security/SecureClientLogin.java e8516cd 
>   agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java 4966a78 
>   agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerLegacyConfigBuilder.java 2d1a56c 
>   agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java 57570c2 
>   agents-common/src/main/java/org/apache/ranger/plugin/audit/RangerDefaultAuditHandler.java b7a6f6e 
>   agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java bd7d770 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesAccessedTogetherCondition.java 598cd0c 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerHiveResourcesNotAccessedTogetherCondition.java a166fe1 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerScriptExecutionContext.java b8ee2f3 
>   agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerTimeOfDayMatcher.java b561142 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java 0f681f8 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerFileBasedTagRetriever.java 031a59f 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java ec10524 
>   agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationDatabase.java e4a7dcc 
>   agents-common/src/main/java/org/apache/ranger/plugin/geo/ValuePrinter.java 62d6891 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java 40667ee 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java b3e0964 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b401c10 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java 89f36cd 
>   agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java d296879 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java 14b5402 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceAccessInfo.java 6a00ce3 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java 67fb09e 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java cfee884 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java 86a18ba 
>   agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java d1c21ff 
>   agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java acfbf02 
>   agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java 2df0201 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java df216a7 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java 023b4f8 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/EmbeddedServiceDefsUtil.java 07c7a1e 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java f3f7ffc 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRequestedResources.java c73d68a 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java bb4d145 
>   agents-common/src/main/java/org/apache/ranger/services/tag/RangerServiceTag.java 859cdd9 
>   agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerIpMatcherTest.java 10780cf 
>   agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerSimpleMatcher.java ed96773 
>   agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java 1728c22 
> 
> Diff: https://reviews.apache.org/r/56579/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>