> On May 21, 2024, 4:16 p.m., Abhay Kulkarni wrote:
> > security-admin/src/main/java/org/apache/ranger/service/RangerAuditedModelService.java
> > Lines 109 (patched)
> > <https://reviews.apache.org/r/74953/diff/1/?file=2287002#file2287002line109>
> >
> >     This class duplicates many methods in AbstractAuditedResourceService 
> > class. Please review and see if the common methods can be moved to a base 
> > class.

Abhay - both AbstractAuditedResourceService and RangerAuditedModelService deal 
with persisting entity objects (derived from XXDBBase). However 
AbstractAuditedResourceService deals with view classes deriving from 
VXDataObject, while RangerAuditedModelService deals with view classes deriving 
from  RangerBaseModelObject. Given this, carving out few common methods into a 
separate class will likely result in many more classes to be updated. If 
necessary, I suggest tracking that via a separate JIRA.


- Madhan


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


On May 21, 2024, 7:03 p.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74953/
> -----------------------------------------------------------
> 
> (Updated May 21, 2024, 7:03 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 
> 38b95543e 
>   security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java d5393603e 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java e0623e0c2 
>   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 
> d0649ab1a 
>   
> 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
>  a4b7616e1 
>   
> 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/XAccessAuditService.java
>  4d96df6ea 
>   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/XUgsyncAuditInfoService.java
>  7fa96fbd0 
>   
> 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/3/
> 
> 
> Testing
> -------
> 
> - verified that all existing tests pass successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>

Reply via email to