----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74953/#review226474 -----------------------------------------------------------
Fix it, then Ship it! security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java Lines 187 (patched) <https://reviews.apache.org/r/74953/#comment314680> Consider rewriting this as if (StringUtils.isBlank(value) && (action == OPERATION_CREATE_CONTEXT || action == OPERATION_DELETE_CONTEXT) { return null; } else if (skipTrxLogForAttribute(obj, oldObj, trxLogAttr)) { return null; } else if (StringUtils.equals(prevValue, newValue)) { return null; } else { ret = new XXTrxLog(classType, obj.getId(), objName, actionString, trxLogAttr.getAttribUserFriendlyName(), prevValue, newValue); ret.setParentObjectClassType(parentClassType); ret.setParentObjectId(parentObjId); ret.setParentObjectName(parentObjName); } security-admin/src/main/java/org/apache/ranger/service/RangerAuditedModelService.java Lines 109 (patched) <https://reviews.apache.org/r/74953/#comment314681> This class duplicates many methods in AbstractAuditedResourceService class. Please review and see if the common methods can be moved to a base class. - Abhay Kulkarni On April 5, 2024, 10:56 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74953/ > ----------------------------------------------------------- > > (Updated April 5, 2024, 10:56 p.m.) > > > Review request for ranger, Anand Nadar, Asit Vadhavkar, Abhay Kulkarni, Mehul > Parikh, Ramesh Mani, Siddhesh Phatak, Sailaja Polavarapu, Subhrat Chaudhary, > and Velmurugan Periasamy. > > > Bugs: RANGER-4769 > https://issues.apache.org/jira/browse/RANGER-4769 > > > Repository: ranger > > > Description > ------- > > - replaced getTransactionLog() implementation in 13 classes with > implementations in following base classes > -- AbstractAuditedResourceService.createTransactionLog() > -- RangerAuditedModelService.createTransactionLog() > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java > 40d4c4043 > security-admin/src/main/java/org/apache/ranger/biz/RoleDBStore.java > 0aa03e7c2 > security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java > 9e2f28716 > security-admin/src/main/java/org/apache/ranger/biz/SecurityZoneDBStore.java > 42943946c > > security-admin/src/main/java/org/apache/ranger/biz/SecurityZoneRefUpdater.java > 10bbfcd32 > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > cccc47fbe > security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java d5393603e > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java d202b2184 > security-admin/src/main/java/org/apache/ranger/common/view/VTrxLogAttr.java > 8c86a92c6 > security-admin/src/main/java/org/apache/ranger/entity/XXTrxLog.java > c308fba7b > security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4bfaa862c > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > a6c759234 > > security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/AbstractBaseResourceService.java > 1f2659d4b > > security-admin/src/main/java/org/apache/ranger/service/RangerAuditedModelService.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsBaseModelService.java > 421646d86 > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsDataShareInDatasetService.java > a76cb2a99 > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsDataShareService.java > 55c4b3633 > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsDatasetInProjectService.java > 62074cfd7 > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsDatasetService.java > a82672a37 > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsProjectService.java > d70da154e > > security-admin/src/main/java/org/apache/ranger/service/RangerGdsSharedResourceService.java > b1b5a841a > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java > b59b94f43 > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyServiceBase.java > a2929f3c1 > > security-admin/src/main/java/org/apache/ranger/service/RangerRoleService.java > 7e0b29c89 > > security-admin/src/main/java/org/apache/ranger/service/RangerRoleServiceBase.java > 39755da1d > > security-admin/src/main/java/org/apache/ranger/service/RangerSecurityZoneServiceBase.java > 586a6b705 > > security-admin/src/main/java/org/apache/ranger/service/RangerSecurityZoneServiceService.java > a6cb2ae74 > > security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java > 3acbfd55d > > security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java > fa23b96d7 > security-admin/src/main/java/org/apache/ranger/service/XAssetService.java > b937cff04 > > security-admin/src/main/java/org/apache/ranger/service/XAssetServiceBase.java > 6855a0b63 > > security-admin/src/main/java/org/apache/ranger/service/XAuditMapService.java > 3fe0832e2 > > security-admin/src/main/java/org/apache/ranger/service/XAuditMapServiceBase.java > 0eb7d9bc7 > security-admin/src/main/java/org/apache/ranger/service/XGroupService.java > 1f033b33d > > security-admin/src/main/java/org/apache/ranger/service/XGroupServiceBase.java > 3b50ed5fe > > security-admin/src/main/java/org/apache/ranger/service/XGroupUserService.java > 5cfcb785c > > security-admin/src/main/java/org/apache/ranger/service/XGroupUserServiceBase.java > feaaa20dd > security-admin/src/main/java/org/apache/ranger/service/XPermMapService.java > 2fa22fa68 > > security-admin/src/main/java/org/apache/ranger/service/XPermMapServiceBase.java > d3d2e9a8c > security-admin/src/main/java/org/apache/ranger/service/XPolicyService.java > 00b902f1a > > security-admin/src/main/java/org/apache/ranger/service/XPortalUserService.java > eb97bab9f > > security-admin/src/main/java/org/apache/ranger/service/XPortalUserServiceBase.java > 4d2e9d74a > > security-admin/src/main/java/org/apache/ranger/service/XResourceService.java > 57b20b7e7 > > security-admin/src/main/java/org/apache/ranger/service/XResourceServiceBase.java > c82fda060 > > security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java > bc6c21eae > security-admin/src/main/java/org/apache/ranger/service/XUserService.java > 166efe82c > > security-admin/src/main/java/org/apache/ranger/service/XUserServiceBase.java > ffeb19c73 > security-admin/src/test/java/org/apache/ranger/biz/TestRoleDBStore.java > 6df1f7369 > > security-admin/src/test/java/org/apache/ranger/biz/TestSecurityZoneDBStore.java > f5d60dfdd > security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java > e298eeab1 > security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java > b6c43133b > security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java > 1c90cb18e > > security-admin/src/test/java/org/apache/ranger/service/TestRangerPolicyService.java > dfb78b683 > > security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java > 163e4169f > > security-admin/src/test/java/org/apache/ranger/service/TestXAssetService.java > 2ecae9831 > > security-admin/src/test/java/org/apache/ranger/service/TestXAuditMapService.java > 2b69f60fb > > security-admin/src/test/java/org/apache/ranger/service/TestXGroupService.java > 7624345cc > > security-admin/src/test/java/org/apache/ranger/service/TestXGroupUserService.java > 4c513a8c8 > > security-admin/src/test/java/org/apache/ranger/service/TestXPermMapService.java > d74058ded > > > Diff: https://reviews.apache.org/r/74953/diff/1/ > > > Testing > ------- > > - verified that all existing tests pass successfully > > > Thanks, > > Madhan Neethiraj > >
