----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50511/#review143827 -----------------------------------------------------------
credentialbuilder/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java (line 60) <https://reviews.apache.org/r/50511/#comment209759> Consider moving this within the 'for' loop below, to line #66. security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 883) <https://reviews.apache.org/r/50511/#comment209826> Why is it necessary to add "CollectionUtils.isNotEmpty(xxEnums)" here? security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 948) <https://reviews.apache.org/r/50511/#comment209823> Consider keeping this at line #950. security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 1953) <https://reviews.apache.org/r/50511/#comment209821> Consider moving retTemp to line #1963 security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2039) <https://reviews.apache.org/r/50511/#comment209819> catch exception in .close() and ignore. security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2043) <https://reviews.apache.org/r/50511/#comment209820> catch exception in .flush and .close() and ignore. security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 3288) <https://reviews.apache.org/r/50511/#comment209818> policyItem0, policyItem1, policyItem2 - these variables should not be needed. Please review and remove these - it should make it easier to read the code. security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java (line 746) <https://reviews.apache.org/r/50511/#comment209768> Consider replacing this with: "if (resultSize == 0)"; it looks simper to read. security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java (line 42) <https://reviews.apache.org/r/50511/#comment209777> Please add "static" here. security-admin/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java (line 60) <https://reviews.apache.org/r/50511/#comment209782> Consider moving this to line #65, within "for" loop. security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java (line 126) <https://reviews.apache.org/r/50511/#comment209785> "userId!=null &&" - this should not be needed. Please review. security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java (line 138) <https://reviews.apache.org/r/50511/#comment209786> "groupId!=null &&" - this should not be needed. Please review. security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java (line 505) <https://reviews.apache.org/r/50511/#comment209807> Consider moving this to line #508. security-admin/src/main/java/org/apache/ranger/rest/PublicAPIs.java (line 354) <https://reviews.apache.org/r/50511/#comment209810> Consider moving this to line #359. security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java (line 1501) <https://reviews.apache.org/r/50511/#comment209816> changes in #1501 and #1502 should not be needed. Please review. security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java (line 1507) <https://reviews.apache.org/r/50511/#comment209813> This fix does not look right; it changes the logic. Please review. security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java <https://reviews.apache.org/r/50511/#comment209817> This fix changes the flow/logic and does not look right. Please review. security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 95) <https://reviews.apache.org/r/50511/#comment209765> Consider moving this to line #107. security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 103) <https://reviews.apache.org/r/50511/#comment209764> Consider moving this to line #106 security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 321) <https://reviews.apache.org/r/50511/#comment209766> Consider moving these inside following 'for'. - Madhan Neethiraj On July 27, 2016, 5:26 p.m., Pradeep Agrawal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50511/ > ----------------------------------------------------------- > > (Updated July 27, 2016, 5:26 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-1124 > https://issues.apache.org/jira/browse/RANGER-1124 > > > Repository: ranger > > > Description > ------- > > Code changes to guard against potential NPEs and other potential run-time > issues; good coding practices. > > > Diffs > ----- > > > credentialbuilder/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java > ecede34 > > credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java > d8ffe2c > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java > a74f8d1 > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/StopEmbeddedServer.java > ef80f43 > jisql/src/main/java/org/apache/util/sql/Jisql.java 36c4340 > jisql/src/main/java/org/apache/util/sql/MySQLPLRunner.java b2eda30 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerHSM.java 606706b > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java d70ec4e > > kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSJSONWriter.java > 3674e7a > kms/src/main/java/org/apache/ranger/entity/XXDBBase.java a0d0120 > > plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSClient.java > 5337ee3 > > plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java > e61d0bc > security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java 5ecae8d > security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959 > security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java > e0a9840 > > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyRetriever.java > 3ba33d4 > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 7947d4b > security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884 > security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5 > security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926 > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96 > security-admin/src/main/java/org/apache/ranger/common/ErrorMessageUtil.java > 582580c > security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java > 72fde46 > > security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java > a68b215 > > security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java > 66c1562 > security-admin/src/main/java/org/apache/ranger/common/SearchField.java > 1891edb > security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java > c1baae8 > security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java > 13607d3 > > security-admin/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java > 429be27 > security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b > > security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java > 5291045 > > security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java > 380cca0 > > security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java > 54148f1 > > security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java > 3a3bed2 > security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIs.java 1f465d5 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 1185e45 > security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java > e84a1aa > security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250 > security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java b07bf9c > security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 226d3c7 > > security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java > 3fa3436 > > security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java > 6496698 > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java > d431bc1 > > security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java > 7314782 > > security-admin/src/main/java/org/apache/ranger/security/web/filter/SSOAuthentication.java > 6fcadb7 > > security-admin/src/main/java/org/apache/ranger/service/AbstractBaseResourceService.java > 6c7e1f0 > > security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java > 65ad5ad > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java > 4b792de > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyServiceBase.java > 2649ff3 > > security-admin/src/main/java/org/apache/ranger/service/XResourceService.java > 839bf59 > security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java > f28ccca > security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f > security-admin/src/main/java/org/apache/ranger/solr/SolrUtil.java e912cfb > security-admin/src/main/java/org/apache/ranger/view/VXAuditRecordList.java > 42ff4d1 > security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java > 637e43f > > Diff: https://reviews.apache.org/r/50511/diff/ > > > Testing > ------- > > Built clean and ran unit tests > installed ranger and ranger-kms. > Tested create/update/delete operation on service, policy, users and groups. > > > Thanks, > > Pradeep Agrawal > >
