You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Pradeep Agrawal <pr...@freestoneinfotech.com> on 2016/04/12 15:52:01 UTC
Review Request 46089: RANGER-921 : Improve implementation of internal
SQL calls and make it more generic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46089/
-----------------------------------------------------------
Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
Bugs: RANGER-921
https://issues.apache.org/jira/browse/RANGER-921
Repository: ranger
Description
-------
**Problem Statement :**
Currently implementation of XXDataHistDao.findObjByEventTimeClassTypeAndId() and XXPortalUserDao.findXPortalUserRolebyXPortalUserId() usage native query which can be converted to JPA named query.
**Proposed Solution :**
Added a jpa named query to filter XXDataHist entries based on eventTime, ClassType and Id field. Have removed SQL statement rewritting part and replaced with parameterised named query. similarly list of user's role can be fetched from x_portal_user_role table by using parameterised named query hence implementation has been added in XXPortalUserDao class.
Diffs
-----
security-admin/src/main/java/org/apache/ranger/common/DateUtil.java 92150ee
security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java 07db458
security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java 393252c
security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java 99d0fe2
security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 804d08e
security-admin/src/main/resources/META-INF/jpa_named_queries.xml 469a400
Diff: https://reviews.apache.org/r/46089/diff/
Testing
-------
Steps Performed without patch :
1. After starting Ranger service, enabled HDFS plugin and created audit logs.
2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
3. A screen pop-up with Policy details.
Steps Performed with patch :
1. After starting Ranger service, enabled HDFS plugin and created audit logs.
2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
3. A screen pop-up with Policy details.
Expected Behavior :
Policy details page should come and policy details page appeared in case of with and without patch should match.
Actual Behavior :
Policy details page in both cases(with and without patch) was same.
Thanks,
Pradeep Agrawal
Re: Review Request 46089: RANGER-921 : Improve implementation of
internal SQL calls and make it more generic
Posted by Pradeep Agrawal <pr...@freestoneinfotech.com>.
> On April 12, 2016, 5:10 p.m., Ramesh Mani wrote:
> > security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java, line 60
> > <https://reviews.apache.org/r/46089/diff/1/?file=1341315#file1341315line60>
> >
> > - please consider refactoring the method to have one return
There are other methods also in the same class which are having more than one return, so fix for this can be tracked separatly as part of code refactoring and can be done later.
> On April 12, 2016, 5:10 p.m., Ramesh Mani wrote:
> > security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java, line 68
> > <https://reviews.apache.org/r/46089/diff/1/?file=1341317#file1341317line68>
> >
> > - please consider refactoring the method to have one return
There are other methods also in the same class which are having more than one return, so fix for this can be tracked separatly as part of code refactoring and can be done later.
- Pradeep
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46089/#review128469
-----------------------------------------------------------
On April 12, 2016, 1:52 p.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46089/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 1:52 p.m.)
>
>
> Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-921
> https://issues.apache.org/jira/browse/RANGER-921
>
>
> Repository: ranger
>
>
> Description
> -------
>
> **Problem Statement :**
> Currently implementation of XXDataHistDao.findObjByEventTimeClassTypeAndId() and XXPortalUserDao.findXPortalUserRolebyXPortalUserId() usage native query which can be converted to JPA named query.
>
> **Proposed Solution :**
> Added a jpa named query to filter XXDataHist entries based on eventTime, ClassType and Id field. Have removed SQL statement rewritting part and replaced with parameterised named query. similarly list of user's role can be fetched from x_portal_user_role table by using parameterised named query hence implementation has been added in XXPortalUserDao class.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/common/DateUtil.java 92150ee
> security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java 07db458
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java 393252c
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java 99d0fe2
> security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 804d08e
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 469a400
>
> Diff: https://reviews.apache.org/r/46089/diff/
>
>
> Testing
> -------
>
> Steps Performed without patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Steps Performed with patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Expected Behavior :
> Policy details page should come and policy details page appeared in case of with and without patch should match.
>
> Actual Behavior :
> Policy details page in both cases(with and without patch) was same.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 46089: RANGER-921 : Improve implementation of
internal SQL calls and make it more generic
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46089/#review128469
-----------------------------------------------------------
Please fix the comments and ship it!
security-admin/src/main/java/org/apache/ranger/common/DateUtil.java (line 126)
<https://reviews.apache.org/r/46089/#comment191889>
- please consider refactoring the method to have one return
e.g Date ret = null;
try {
ret = simpleDateFormat.parse(dateString);
} catch ( Exception ex) {
//noop
}
return ret;
security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java (line 56)
<https://reviews.apache.org/r/46089/#comment191890>
- please consider refactoring the method to have one return
security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java (line 68)
<https://reviews.apache.org/r/46089/#comment191891>
- please consider refactoring the method to have one return
PEase
- Ramesh Mani
On April 12, 2016, 1:52 p.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46089/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 1:52 p.m.)
>
>
> Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-921
> https://issues.apache.org/jira/browse/RANGER-921
>
>
> Repository: ranger
>
>
> Description
> -------
>
> **Problem Statement :**
> Currently implementation of XXDataHistDao.findObjByEventTimeClassTypeAndId() and XXPortalUserDao.findXPortalUserRolebyXPortalUserId() usage native query which can be converted to JPA named query.
>
> **Proposed Solution :**
> Added a jpa named query to filter XXDataHist entries based on eventTime, ClassType and Id field. Have removed SQL statement rewritting part and replaced with parameterised named query. similarly list of user's role can be fetched from x_portal_user_role table by using parameterised named query hence implementation has been added in XXPortalUserDao class.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/common/DateUtil.java 92150ee
> security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java 07db458
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java 393252c
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java 99d0fe2
> security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 804d08e
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 469a400
>
> Diff: https://reviews.apache.org/r/46089/diff/
>
>
> Testing
> -------
>
> Steps Performed without patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Steps Performed with patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Expected Behavior :
> Policy details page should come and policy details page appeared in case of with and without patch should match.
>
> Actual Behavior :
> Policy details page in both cases(with and without patch) was same.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 46089: RANGER-921 : Improve implementation of
internal SQL calls and make it more generic
Posted by Selvamohan Neethiraj <sn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46089/#review128586
-----------------------------------------------------------
Ship it!
Ship It!
- Selvamohan Neethiraj
On April 12, 2016, 9:52 a.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46089/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 9:52 a.m.)
>
>
> Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-921
> https://issues.apache.org/jira/browse/RANGER-921
>
>
> Repository: ranger
>
>
> Description
> -------
>
> **Problem Statement :**
> Currently implementation of XXDataHistDao.findObjByEventTimeClassTypeAndId() and XXPortalUserDao.findXPortalUserRolebyXPortalUserId() usage native query which can be converted to JPA named query.
>
> **Proposed Solution :**
> Added a jpa named query to filter XXDataHist entries based on eventTime, ClassType and Id field. Have removed SQL statement rewritting part and replaced with parameterised named query. similarly list of user's role can be fetched from x_portal_user_role table by using parameterised named query hence implementation has been added in XXPortalUserDao class.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/common/DateUtil.java 92150ee
> security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java 07db458
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java 393252c
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java 99d0fe2
> security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 804d08e
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 469a400
>
> Diff: https://reviews.apache.org/r/46089/diff/
>
>
> Testing
> -------
>
> Steps Performed without patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Steps Performed with patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Expected Behavior :
> Policy details page should come and policy details page appeared in case of with and without patch should match.
>
> Actual Behavior :
> Policy details page in both cases(with and without patch) was same.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 46089: RANGER-921 : Improve implementation of
internal SQL calls and make it more generic
Posted by Velmurugan Periasamy <vp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46089/#review128473
-----------------------------------------------------------
Ship it!
Ship It!
- Velmurugan Periasamy
On April 12, 2016, 1:52 p.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46089/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 1:52 p.m.)
>
>
> Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-921
> https://issues.apache.org/jira/browse/RANGER-921
>
>
> Repository: ranger
>
>
> Description
> -------
>
> **Problem Statement :**
> Currently implementation of XXDataHistDao.findObjByEventTimeClassTypeAndId() and XXPortalUserDao.findXPortalUserRolebyXPortalUserId() usage native query which can be converted to JPA named query.
>
> **Proposed Solution :**
> Added a jpa named query to filter XXDataHist entries based on eventTime, ClassType and Id field. Have removed SQL statement rewritting part and replaced with parameterised named query. similarly list of user's role can be fetched from x_portal_user_role table by using parameterised named query hence implementation has been added in XXPortalUserDao class.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/common/DateUtil.java 92150ee
> security-admin/src/main/java/org/apache/ranger/db/XXDataHistDao.java 07db458
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java 393252c
> security-admin/src/main/java/org/apache/ranger/db/XXPortalUserRoleDao.java 99d0fe2
> security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 804d08e
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 469a400
>
> Diff: https://reviews.apache.org/r/46089/diff/
>
>
> Testing
> -------
>
> Steps Performed without patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Steps Performed with patch :
>
> 1. After starting Ranger service, enabled HDFS plugin and created audit logs.
> 2. in Ranger Admin UI -> Access Audit logs : clicked on policy id link of generated audit logs.
> 3. A screen pop-up with Policy details.
>
> Expected Behavior :
> Policy details page should come and policy details page appeared in case of with and without patch should match.
>
> Actual Behavior :
> Policy details page in both cases(with and without patch) was same.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>