You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by Guido Wimmel <gu...@gmx.net> on 2015/04/16 11:00:52 UTC
getAdminRolesFilter-query
Hi,
minor detail, but I recently stubled upon the query generated by SubjectSearchDAOImpl.getAdminRolesFilter(),
used by search() and count(). Maybe it could be simplified.
It seems to determine all users which have roles not in adminRoles (to be able to filter them out
by WHERE subject_id NOT IN ... ).
The generated query looks like (for type==USER and adminRoles={1,2}):
(SELECT syncopeUser_id AS subject_id
FROM Membership M1
WHERE syncopeRole_id IN
(SELECT syncopeRole_id
FROM Membership M2
WHERE M2.syncopeUser_id=M1.syncopeUser_id AND syncopeRole_id NOT IN
(SELECT id AS syncopeRole_id
FROM SyncopeRole
WHERE id=1 OR id=2
)
)
I don't really understand why two (inter-dependent) subqueries on membership are needed.
I don't see a difference to
WHERE subject_id NOT IN (
SELECT syncopeUser_id AS subject_id FROM Membership M2 WHERE syncopeRole_id NOT IN (
SELECT id AS syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
)
)
... but probably I'm overlooking something?
Cheers, Guido
Re: getAdminRolesFilter-query
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 12/05/2015 17:20, Guido Wimmel wrote:
> Hi Francesco,
>
> finally I had some time to investigate further and create a branch.
> See https://issues.apache.org/jira/browse/SYNCOPE-667
Cool: I've commented there.
Thanks for taking this further.
Regards.
>> Gesendet: Freitag, 17. April 2015 um 11:06 Uhr
>> Von: "Francesco Chicchiriccò" <il...@apache.org>
>> An: dev@syncope.apache.org
>> Betreff: Re: getAdminRolesFilter-query
>>
>> On 17/04/2015 10:41, Guido Wimmel wrote:
>>> Hi Francesco,
>>>
>>> ok, thanks for the clarification. I can see if I can investigate further and provide a patch.
>>>
>>> I did run the integration tests but I do not think the admin roles filtering is covered too well.
>>> It seems that SearchTestITCase performs the rest calls with the global admin user, who owns all entitlements
>>> (so nothing is filtered out).
>> It seems that there is some test coverage, but not in SearchTestITCase:
>>
>> https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L217
>>
>> https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L229
>>
>> Regards.
>>
>>>> Gesendet: Freitag, 17. April 2015 um 09:05 Uhr
>>>> Von: "Francesco Chicchiriccò" <il...@apache.org>
>>>> An: dev@syncope.apache.org
>>>> Betreff: Re: getAdminRolesFilter-query
>>>>
>>>> On 16/04/2015 21:20, Guido Wimmel wrote:
>>>>> Hi Francesco,
>>>>>
>>>>> I'm only talking about the part of the query generated by
>>>>> SubjectSearchDAOImpl.getAdminRolesFilter().
>>>>> The FIQL string is not used for generating this part, only type and
>>>>> adminRoles.
>>>>> The part I wrote about is only used when type==SubjectType.USER.
>>>>>
>>>>> I was considering if the original query
>>>>>
>>>>> SELECT syncopeUser_id AS subject_id
>>>>> FROM Membership M1 WHERE syncopeRole_id IN
>>>>> (SELECT syncopeRole_id FROM Membership M2
>>>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
>>>>> syncopeRole_id NOT IN
>>>>> (SELECT id AS syncopeRole_id FROM
>>>>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
>>>>> )
>>>>>
>>>>> can be simplified to
>>>>>
>>>>> SELECT syncopeUser_id AS subject_id FROM
>>>>> Membership M2 WHERE syncopeRole_id NOT IN
>>>>> (SELECT id AS syncopeRole_id FROM
>>>>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
>>>>>
>>>>>
>>>>> If the original query works with all supported databases, the
>>>>> simplified version should work as well.
>>>> I might agree with you, but you need to provide a proof - e.g. a patch,
>>>> and consider that the same getAdminRolesFilter() method is invoked when
>>>> searching for groups (besides users) as well.
>>>>
>>>>> I changed the code accordingly and ran the normal build with HSQL, and
>>>>> there are no errors.
>>>>> But it seems that the use cases of getAdminRolesFilter() are not
>>>>> covered too well, as the build also seems to run
>>>>> successfully if I replace the query by "SELECT syncopeUser_id WHERE
>>>>> 0=1" (i.e. just return nothing).
>>>> Entitlements and then consequently real admin filters come into the game
>>>> only when Spring Security is involved, e.g. not during unit tests, but
>>>> only with integration tests.
>>>>
>>>> If you really want to check your modifications, you'll need to follow
>>>> the steps in
>>>>
>>>> http://syncope.apache.org/building.html
>>>>
>>>> e.g.
>>>>
>>>> "mvn -PskipTests" from root then "mvn clean verify" from core/
>>>> (this for checking with H2, for other RDBMS see [3] as said below).
>>>>
>>>> FYI, the test class with particular reference to search features is
>>>> SearchTestITCase.
>>>>
>>>>> I can try and investigate further when I have time.
>>>>> I only wanted to know first if somebody maybe still can reproduce why
>>>>> the query with M1 and M2 was designed the way it is.
>>>> Not really: I can only argue that it reached the current form as
>>>> consequence of several incremental changes, with reference to RDBMS
>>>> support and different subject (users, roles).
>>>>
>>>> This is why I believe it is definitely not the best option, but anyway
>>>> working: if you are able to improve such query, while keeping all
>>>> integration tests happy with all supported RDBMS, that would be great.
>>>>
>>>> Regards.
>>>>
>>>>> On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
>>>>>> On 16/04/2015 11:00, Guido Wimmel wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> minor detail, but I recently stubled upon the query generated by
>>>>>>> SubjectSearchDAOImpl.getAdminRolesFilter(),
>>>>>>> used by search() and count(). Maybe it could be simplified.
>>>>>>>
>>>>>>> It seems to determine all users which have roles not in adminRoles
>>>>>>> (to be able to filter them out
>>>>>>> by WHERE subject_id NOT IN ... ).
>>>>>>>
>>>>>>> The generated query looks like (for type==USER and adminRoles={1,2}):
>>>>>>>
>>>>>>> (SELECT syncopeUser_id AS subject_id
>>>>>>> FROM Membership M1
>>>>>>> WHERE syncopeRole_id IN
>>>>>>> (SELECT syncopeRole_id
>>>>>>> FROM Membership M2
>>>>>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id
>>>>>>> AND syncopeRole_id NOT IN
>>>>>>> (SELECT id AS syncopeRole_id
>>>>>>> FROM SyncopeRole
>>>>>>> WHERE id=1 OR id=2
>>>>>>> )
>>>>>>> )
>>>>>>>
>>>>>>>
>>>>>>> I don't really understand why two (inter-dependent) subqueries on
>>>>>>> membership are needed.
>>>>>>> I don't see a difference to
>>>>>>>
>>>>>>> WHERE subject_id NOT IN (
>>>>>>> SELECT syncopeUser_id AS subject_id FROM
>>>>>>> Membership M2 WHERE syncopeRole_id NOT IN (
>>>>>>> SELECT id AS
>>>>>>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
>>>>>>> )
>>>>>>> )
>>>>>>>
>>>>>>> ... but probably I'm overlooking something?
>>>>>> Hi Guido,
>>>>>> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
>>>>>> JPASubjectSearchDAO [2] (master) as part of translating the FIQL
>>>>>> query string received via REST into a native SQL query that can work
>>>>>> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
>>>>>> PostgreSQL and H2).
>>>>>>
>>>>>> Naturally, things can always be improved: if you find a way to
>>>>>> simplify the subquery above while keeping everything working an all
>>>>>> supported RDBMS, this is more than welcome!
>>>>>>
>>>>>> You can find information about how to check Syncope against all
>>>>>> supported RDBMS at [3]: if you'd like to propose a change but you
>>>>>> only have chance to verify it on a subset (say H2 and MySQL), please
>>>>>> create a separate feature branch (or a pull request on GitHub) and I
>>>>>> can help you completing all verifications needed.
>>>>>>
>>>>>> Please let me know.
>>>>>> Regards.
>>>>>>
>>>>>> [1] https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
>>>>>> [2] https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
>>>>>> [3] http://syncope.apache.org/building.html#DBMSes
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/
Re: getAdminRolesFilter-query
Posted by Guido Wimmel <gu...@gmx.net>.
Hi Francesco,
finally I had some time to investigate further and create a branch.
See https://issues.apache.org/jira/browse/SYNCOPE-667
Cheers,
Guido
> Gesendet: Freitag, 17. April 2015 um 11:06 Uhr
> Von: "Francesco Chicchiriccò" <il...@apache.org>
> An: dev@syncope.apache.org
> Betreff: Re: getAdminRolesFilter-query
>
> On 17/04/2015 10:41, Guido Wimmel wrote:
> > Hi Francesco,
> >
> > ok, thanks for the clarification. I can see if I can investigate further and provide a patch.
> >
> > I did run the integration tests but I do not think the admin roles filtering is covered too well.
> > It seems that SearchTestITCase performs the rest calls with the global admin user, who owns all entitlements
> > (so nothing is filtered out).
>
> It seems that there is some test coverage, but not in SearchTestITCase:
>
> https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L217
>
> https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L229
>
> Regards.
>
> >> Gesendet: Freitag, 17. April 2015 um 09:05 Uhr
> >> Von: "Francesco Chicchiriccò" <il...@apache.org>
> >> An: dev@syncope.apache.org
> >> Betreff: Re: getAdminRolesFilter-query
> >>
> >> On 16/04/2015 21:20, Guido Wimmel wrote:
> >>> Hi Francesco,
> >>>
> >>> I'm only talking about the part of the query generated by
> >>> SubjectSearchDAOImpl.getAdminRolesFilter().
> >>> The FIQL string is not used for generating this part, only type and
> >>> adminRoles.
> >>> The part I wrote about is only used when type==SubjectType.USER.
> >>>
> >>> I was considering if the original query
> >>>
> >>> SELECT syncopeUser_id AS subject_id
> >>> FROM Membership M1 WHERE syncopeRole_id IN
> >>> (SELECT syncopeRole_id FROM Membership M2
> >>> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
> >>> syncopeRole_id NOT IN
> >>> (SELECT id AS syncopeRole_id FROM
> >>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
> >>> )
> >>>
> >>> can be simplified to
> >>>
> >>> SELECT syncopeUser_id AS subject_id FROM
> >>> Membership M2 WHERE syncopeRole_id NOT IN
> >>> (SELECT id AS syncopeRole_id FROM
> >>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
> >>>
> >>>
> >>> If the original query works with all supported databases, the
> >>> simplified version should work as well.
> >> I might agree with you, but you need to provide a proof - e.g. a patch,
> >> and consider that the same getAdminRolesFilter() method is invoked when
> >> searching for groups (besides users) as well.
> >>
> >>> I changed the code accordingly and ran the normal build with HSQL, and
> >>> there are no errors.
> >>> But it seems that the use cases of getAdminRolesFilter() are not
> >>> covered too well, as the build also seems to run
> >>> successfully if I replace the query by "SELECT syncopeUser_id WHERE
> >>> 0=1" (i.e. just return nothing).
> >> Entitlements and then consequently real admin filters come into the game
> >> only when Spring Security is involved, e.g. not during unit tests, but
> >> only with integration tests.
> >>
> >> If you really want to check your modifications, you'll need to follow
> >> the steps in
> >>
> >> http://syncope.apache.org/building.html
> >>
> >> e.g.
> >>
> >> "mvn -PskipTests" from root then "mvn clean verify" from core/
> >> (this for checking with H2, for other RDBMS see [3] as said below).
> >>
> >> FYI, the test class with particular reference to search features is
> >> SearchTestITCase.
> >>
> >>> I can try and investigate further when I have time.
> >>> I only wanted to know first if somebody maybe still can reproduce why
> >>> the query with M1 and M2 was designed the way it is.
> >> Not really: I can only argue that it reached the current form as
> >> consequence of several incremental changes, with reference to RDBMS
> >> support and different subject (users, roles).
> >>
> >> This is why I believe it is definitely not the best option, but anyway
> >> working: if you are able to improve such query, while keeping all
> >> integration tests happy with all supported RDBMS, that would be great.
> >>
> >> Regards.
> >>
> >>> On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
> >>>> On 16/04/2015 11:00, Guido Wimmel wrote:
> >>>>> Hi,
> >>>>>
> >>>>> minor detail, but I recently stubled upon the query generated by
> >>>>> SubjectSearchDAOImpl.getAdminRolesFilter(),
> >>>>> used by search() and count(). Maybe it could be simplified.
> >>>>>
> >>>>> It seems to determine all users which have roles not in adminRoles
> >>>>> (to be able to filter them out
> >>>>> by WHERE subject_id NOT IN ... ).
> >>>>>
> >>>>> The generated query looks like (for type==USER and adminRoles={1,2}):
> >>>>>
> >>>>> (SELECT syncopeUser_id AS subject_id
> >>>>> FROM Membership M1
> >>>>> WHERE syncopeRole_id IN
> >>>>> (SELECT syncopeRole_id
> >>>>> FROM Membership M2
> >>>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id
> >>>>> AND syncopeRole_id NOT IN
> >>>>> (SELECT id AS syncopeRole_id
> >>>>> FROM SyncopeRole
> >>>>> WHERE id=1 OR id=2
> >>>>> )
> >>>>> )
> >>>>>
> >>>>>
> >>>>> I don't really understand why two (inter-dependent) subqueries on
> >>>>> membership are needed.
> >>>>> I don't see a difference to
> >>>>>
> >>>>> WHERE subject_id NOT IN (
> >>>>> SELECT syncopeUser_id AS subject_id FROM
> >>>>> Membership M2 WHERE syncopeRole_id NOT IN (
> >>>>> SELECT id AS
> >>>>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
> >>>>> )
> >>>>> )
> >>>>>
> >>>>> ... but probably I'm overlooking something?
> >>>> Hi Guido,
> >>>> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
> >>>> JPASubjectSearchDAO [2] (master) as part of translating the FIQL
> >>>> query string received via REST into a native SQL query that can work
> >>>> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
> >>>> PostgreSQL and H2).
> >>>>
> >>>> Naturally, things can always be improved: if you find a way to
> >>>> simplify the subquery above while keeping everything working an all
> >>>> supported RDBMS, this is more than welcome!
> >>>>
> >>>> You can find information about how to check Syncope against all
> >>>> supported RDBMS at [3]: if you'd like to propose a change but you
> >>>> only have chance to verify it on a subset (say H2 and MySQL), please
> >>>> create a separate feature branch (or a pull request on GitHub) and I
> >>>> can help you completing all verifications needed.
> >>>>
> >>>> Please let me know.
> >>>> Regards.
> >>>>
> >>>> [1]
> >>>> https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
> >>>> [2]
> >>>> https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
> >>>> [3] http://syncope.apache.org/building.html#DBMSes
>
> --
> Francesco Chicchiriccò
>
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
>
> Involved at The Apache Software Foundation:
> member, Syncope PMC chair, Cocoon PMC, Olingo PMC
> http://people.apache.org/~ilgrosso/
>
>
Re: getAdminRolesFilter-query
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 17/04/2015 10:41, Guido Wimmel wrote:
> Hi Francesco,
>
> ok, thanks for the clarification. I can see if I can investigate further and provide a patch.
>
> I did run the integration tests but I do not think the admin roles filtering is covered too well.
> It seems that SearchTestITCase performs the rest calls with the global admin user, who owns all entitlements
> (so nothing is filtered out).
It seems that there is some test coverage, but not in SearchTestITCase:
https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L217
https://github.com/apache/syncope/blob/1_2_X/core/src/test/java/org/apache/syncope/core/rest/AuthenticationTestITCase.java#L229
Regards.
>> Gesendet: Freitag, 17. April 2015 um 09:05 Uhr
>> Von: "Francesco Chicchiriccò" <il...@apache.org>
>> An: dev@syncope.apache.org
>> Betreff: Re: getAdminRolesFilter-query
>>
>> On 16/04/2015 21:20, Guido Wimmel wrote:
>>> Hi Francesco,
>>>
>>> I'm only talking about the part of the query generated by
>>> SubjectSearchDAOImpl.getAdminRolesFilter().
>>> The FIQL string is not used for generating this part, only type and
>>> adminRoles.
>>> The part I wrote about is only used when type==SubjectType.USER.
>>>
>>> I was considering if the original query
>>>
>>> SELECT syncopeUser_id AS subject_id
>>> FROM Membership M1 WHERE syncopeRole_id IN
>>> (SELECT syncopeRole_id FROM Membership M2
>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
>>> syncopeRole_id NOT IN
>>> (SELECT id AS syncopeRole_id FROM
>>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
>>> )
>>>
>>> can be simplified to
>>>
>>> SELECT syncopeUser_id AS subject_id FROM
>>> Membership M2 WHERE syncopeRole_id NOT IN
>>> (SELECT id AS syncopeRole_id FROM
>>> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
>>>
>>>
>>> If the original query works with all supported databases, the
>>> simplified version should work as well.
>> I might agree with you, but you need to provide a proof - e.g. a patch,
>> and consider that the same getAdminRolesFilter() method is invoked when
>> searching for groups (besides users) as well.
>>
>>> I changed the code accordingly and ran the normal build with HSQL, and
>>> there are no errors.
>>> But it seems that the use cases of getAdminRolesFilter() are not
>>> covered too well, as the build also seems to run
>>> successfully if I replace the query by "SELECT syncopeUser_id WHERE
>>> 0=1" (i.e. just return nothing).
>> Entitlements and then consequently real admin filters come into the game
>> only when Spring Security is involved, e.g. not during unit tests, but
>> only with integration tests.
>>
>> If you really want to check your modifications, you'll need to follow
>> the steps in
>>
>> http://syncope.apache.org/building.html
>>
>> e.g.
>>
>> "mvn -PskipTests" from root then "mvn clean verify" from core/
>> (this for checking with H2, for other RDBMS see [3] as said below).
>>
>> FYI, the test class with particular reference to search features is
>> SearchTestITCase.
>>
>>> I can try and investigate further when I have time.
>>> I only wanted to know first if somebody maybe still can reproduce why
>>> the query with M1 and M2 was designed the way it is.
>> Not really: I can only argue that it reached the current form as
>> consequence of several incremental changes, with reference to RDBMS
>> support and different subject (users, roles).
>>
>> This is why I believe it is definitely not the best option, but anyway
>> working: if you are able to improve such query, while keeping all
>> integration tests happy with all supported RDBMS, that would be great.
>>
>> Regards.
>>
>>> On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
>>>> On 16/04/2015 11:00, Guido Wimmel wrote:
>>>>> Hi,
>>>>>
>>>>> minor detail, but I recently stubled upon the query generated by
>>>>> SubjectSearchDAOImpl.getAdminRolesFilter(),
>>>>> used by search() and count(). Maybe it could be simplified.
>>>>>
>>>>> It seems to determine all users which have roles not in adminRoles
>>>>> (to be able to filter them out
>>>>> by WHERE subject_id NOT IN ... ).
>>>>>
>>>>> The generated query looks like (for type==USER and adminRoles={1,2}):
>>>>>
>>>>> (SELECT syncopeUser_id AS subject_id
>>>>> FROM Membership M1
>>>>> WHERE syncopeRole_id IN
>>>>> (SELECT syncopeRole_id
>>>>> FROM Membership M2
>>>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id
>>>>> AND syncopeRole_id NOT IN
>>>>> (SELECT id AS syncopeRole_id
>>>>> FROM SyncopeRole
>>>>> WHERE id=1 OR id=2
>>>>> )
>>>>> )
>>>>>
>>>>>
>>>>> I don't really understand why two (inter-dependent) subqueries on
>>>>> membership are needed.
>>>>> I don't see a difference to
>>>>>
>>>>> WHERE subject_id NOT IN (
>>>>> SELECT syncopeUser_id AS subject_id FROM
>>>>> Membership M2 WHERE syncopeRole_id NOT IN (
>>>>> SELECT id AS
>>>>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
>>>>> )
>>>>> )
>>>>>
>>>>> ... but probably I'm overlooking something?
>>>> Hi Guido,
>>>> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
>>>> JPASubjectSearchDAO [2] (master) as part of translating the FIQL
>>>> query string received via REST into a native SQL query that can work
>>>> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
>>>> PostgreSQL and H2).
>>>>
>>>> Naturally, things can always be improved: if you find a way to
>>>> simplify the subquery above while keeping everything working an all
>>>> supported RDBMS, this is more than welcome!
>>>>
>>>> You can find information about how to check Syncope against all
>>>> supported RDBMS at [3]: if you'd like to propose a change but you
>>>> only have chance to verify it on a subset (say H2 and MySQL), please
>>>> create a separate feature branch (or a pull request on GitHub) and I
>>>> can help you completing all verifications needed.
>>>>
>>>> Please let me know.
>>>> Regards.
>>>>
>>>> [1]
>>>> https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
>>>> [2]
>>>> https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
>>>> [3] http://syncope.apache.org/building.html#DBMSes
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/
Re: getAdminRolesFilter-query
Posted by Guido Wimmel <gu...@gmx.net>.
Hi Francesco,
ok, thanks for the clarification. I can see if I can investigate further and provide a patch.
I did run the integration tests but I do not think the admin roles filtering is covered too well.
It seems that SearchTestITCase performs the rest calls with the global admin user, who owns all entitlements
(so nothing is filtered out).
Cheers,
Guido
> Gesendet: Freitag, 17. April 2015 um 09:05 Uhr
> Von: "Francesco Chicchiriccò" <il...@apache.org>
> An: dev@syncope.apache.org
> Betreff: Re: getAdminRolesFilter-query
>
> On 16/04/2015 21:20, Guido Wimmel wrote:
> > Hi Francesco,
> >
> > I'm only talking about the part of the query generated by
> > SubjectSearchDAOImpl.getAdminRolesFilter().
> > The FIQL string is not used for generating this part, only type and
> > adminRoles.
> > The part I wrote about is only used when type==SubjectType.USER.
> >
> > I was considering if the original query
> >
> > SELECT syncopeUser_id AS subject_id
> > FROM Membership M1 WHERE syncopeRole_id IN
> > (SELECT syncopeRole_id FROM Membership M2
> > WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
> > syncopeRole_id NOT IN
> > (SELECT id AS syncopeRole_id FROM
> > SyncopeRole WHERE id=x1 OR id=x2 OR ...)
> > )
> >
> > can be simplified to
> >
> > SELECT syncopeUser_id AS subject_id FROM
> > Membership M2 WHERE syncopeRole_id NOT IN
> > (SELECT id AS syncopeRole_id FROM
> > SyncopeRole WHERE id=x1 OR id=x2 OR ...)
> >
> >
> > If the original query works with all supported databases, the
> > simplified version should work as well.
>
> I might agree with you, but you need to provide a proof - e.g. a patch,
> and consider that the same getAdminRolesFilter() method is invoked when
> searching for groups (besides users) as well.
>
> > I changed the code accordingly and ran the normal build with HSQL, and
> > there are no errors.
> > But it seems that the use cases of getAdminRolesFilter() are not
> > covered too well, as the build also seems to run
> > successfully if I replace the query by "SELECT syncopeUser_id WHERE
> > 0=1" (i.e. just return nothing).
>
> Entitlements and then consequently real admin filters come into the game
> only when Spring Security is involved, e.g. not during unit tests, but
> only with integration tests.
>
> If you really want to check your modifications, you'll need to follow
> the steps in
>
> http://syncope.apache.org/building.html
>
> e.g.
>
> "mvn -PskipTests" from root then "mvn clean verify" from core/
> (this for checking with H2, for other RDBMS see [3] as said below).
>
> FYI, the test class with particular reference to search features is
> SearchTestITCase.
>
> > I can try and investigate further when I have time.
> > I only wanted to know first if somebody maybe still can reproduce why
> > the query with M1 and M2 was designed the way it is.
>
> Not really: I can only argue that it reached the current form as
> consequence of several incremental changes, with reference to RDBMS
> support and different subject (users, roles).
>
> This is why I believe it is definitely not the best option, but anyway
> working: if you are able to improve such query, while keeping all
> integration tests happy with all supported RDBMS, that would be great.
>
> Regards.
>
> > On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
> >> On 16/04/2015 11:00, Guido Wimmel wrote:
> >>> Hi,
> >>>
> >>> minor detail, but I recently stubled upon the query generated by
> >>> SubjectSearchDAOImpl.getAdminRolesFilter(),
> >>> used by search() and count(). Maybe it could be simplified.
> >>>
> >>> It seems to determine all users which have roles not in adminRoles
> >>> (to be able to filter them out
> >>> by WHERE subject_id NOT IN ... ).
> >>>
> >>> The generated query looks like (for type==USER and adminRoles={1,2}):
> >>>
> >>> (SELECT syncopeUser_id AS subject_id
> >>> FROM Membership M1
> >>> WHERE syncopeRole_id IN
> >>> (SELECT syncopeRole_id
> >>> FROM Membership M2
> >>> WHERE M2.syncopeUser_id=M1.syncopeUser_id
> >>> AND syncopeRole_id NOT IN
> >>> (SELECT id AS syncopeRole_id
> >>> FROM SyncopeRole
> >>> WHERE id=1 OR id=2
> >>> )
> >>> )
> >>>
> >>>
> >>> I don't really understand why two (inter-dependent) subqueries on
> >>> membership are needed.
> >>> I don't see a difference to
> >>>
> >>> WHERE subject_id NOT IN (
> >>> SELECT syncopeUser_id AS subject_id FROM
> >>> Membership M2 WHERE syncopeRole_id NOT IN (
> >>> SELECT id AS
> >>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
> >>> )
> >>> )
> >>>
> >>> ... but probably I'm overlooking something?
> >>
> >> Hi Guido,
> >> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
> >> JPASubjectSearchDAO [2] (master) as part of translating the FIQL
> >> query string received via REST into a native SQL query that can work
> >> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
> >> PostgreSQL and H2).
> >>
> >> Naturally, things can always be improved: if you find a way to
> >> simplify the subquery above while keeping everything working an all
> >> supported RDBMS, this is more than welcome!
> >>
> >> You can find information about how to check Syncope against all
> >> supported RDBMS at [3]: if you'd like to propose a change but you
> >> only have chance to verify it on a subset (say H2 and MySQL), please
> >> create a separate feature branch (or a pull request on GitHub) and I
> >> can help you completing all verifications needed.
> >>
> >> Please let me know.
> >> Regards.
> >>
> >> [1]
> >> https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
> >> [2]
> >> https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
> >> [3] http://syncope.apache.org/building.html#DBMSes
>
> --
> Francesco Chicchiriccò
>
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
>
> Involved at The Apache Software Foundation:
> member, Syncope PMC chair, Cocoon PMC, Olingo PMC
> http://people.apache.org/~ilgrosso/
>
>
>
Re: getAdminRolesFilter-query
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 16/04/2015 21:20, Guido Wimmel wrote:
> Hi Francesco,
>
> I'm only talking about the part of the query generated by
> SubjectSearchDAOImpl.getAdminRolesFilter().
> The FIQL string is not used for generating this part, only type and
> adminRoles.
> The part I wrote about is only used when type==SubjectType.USER.
>
> I was considering if the original query
>
> SELECT syncopeUser_id AS subject_id
> FROM Membership M1 WHERE syncopeRole_id IN
> (SELECT syncopeRole_id FROM Membership M2
> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
> syncopeRole_id NOT IN
> (SELECT id AS syncopeRole_id FROM
> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
> )
>
> can be simplified to
>
> SELECT syncopeUser_id AS subject_id FROM
> Membership M2 WHERE syncopeRole_id NOT IN
> (SELECT id AS syncopeRole_id FROM
> SyncopeRole WHERE id=x1 OR id=x2 OR ...)
>
>
> If the original query works with all supported databases, the
> simplified version should work as well.
I might agree with you, but you need to provide a proof - e.g. a patch,
and consider that the same getAdminRolesFilter() method is invoked when
searching for groups (besides users) as well.
> I changed the code accordingly and ran the normal build with HSQL, and
> there are no errors.
> But it seems that the use cases of getAdminRolesFilter() are not
> covered too well, as the build also seems to run
> successfully if I replace the query by "SELECT syncopeUser_id WHERE
> 0=1" (i.e. just return nothing).
Entitlements and then consequently real admin filters come into the game
only when Spring Security is involved, e.g. not during unit tests, but
only with integration tests.
If you really want to check your modifications, you'll need to follow
the steps in
http://syncope.apache.org/building.html
e.g.
"mvn -PskipTests" from root then "mvn clean verify" from core/
(this for checking with H2, for other RDBMS see [3] as said below).
FYI, the test class with particular reference to search features is
SearchTestITCase.
> I can try and investigate further when I have time.
> I only wanted to know first if somebody maybe still can reproduce why
> the query with M1 and M2 was designed the way it is.
Not really: I can only argue that it reached the current form as
consequence of several incremental changes, with reference to RDBMS
support and different subject (users, roles).
This is why I believe it is definitely not the best option, but anyway
working: if you are able to improve such query, while keeping all
integration tests happy with all supported RDBMS, that would be great.
Regards.
> On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
>> On 16/04/2015 11:00, Guido Wimmel wrote:
>>> Hi,
>>>
>>> minor detail, but I recently stubled upon the query generated by
>>> SubjectSearchDAOImpl.getAdminRolesFilter(),
>>> used by search() and count(). Maybe it could be simplified.
>>>
>>> It seems to determine all users which have roles not in adminRoles
>>> (to be able to filter them out
>>> by WHERE subject_id NOT IN ... ).
>>>
>>> The generated query looks like (for type==USER and adminRoles={1,2}):
>>>
>>> (SELECT syncopeUser_id AS subject_id
>>> FROM Membership M1
>>> WHERE syncopeRole_id IN
>>> (SELECT syncopeRole_id
>>> FROM Membership M2
>>> WHERE M2.syncopeUser_id=M1.syncopeUser_id
>>> AND syncopeRole_id NOT IN
>>> (SELECT id AS syncopeRole_id
>>> FROM SyncopeRole
>>> WHERE id=1 OR id=2
>>> )
>>> )
>>>
>>>
>>> I don't really understand why two (inter-dependent) subqueries on
>>> membership are needed.
>>> I don't see a difference to
>>>
>>> WHERE subject_id NOT IN (
>>> SELECT syncopeUser_id AS subject_id FROM
>>> Membership M2 WHERE syncopeRole_id NOT IN (
>>> SELECT id AS
>>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
>>> )
>>> )
>>>
>>> ... but probably I'm overlooking something?
>>
>> Hi Guido,
>> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
>> JPASubjectSearchDAO [2] (master) as part of translating the FIQL
>> query string received via REST into a native SQL query that can work
>> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
>> PostgreSQL and H2).
>>
>> Naturally, things can always be improved: if you find a way to
>> simplify the subquery above while keeping everything working an all
>> supported RDBMS, this is more than welcome!
>>
>> You can find information about how to check Syncope against all
>> supported RDBMS at [3]: if you'd like to propose a change but you
>> only have chance to verify it on a subset (say H2 and MySQL), please
>> create a separate feature branch (or a pull request on GitHub) and I
>> can help you completing all verifications needed.
>>
>> Please let me know.
>> Regards.
>>
>> [1]
>> https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
>> [2]
>> https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
>> [3] http://syncope.apache.org/building.html#DBMSes
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/
Re: getAdminRolesFilter-query
Posted by Guido Wimmel <gu...@gmx.net>.
Hi Francesco,
I'm only talking about the part of the query generated by
SubjectSearchDAOImpl.getAdminRolesFilter().
The FIQL string is not used for generating this part, only type and
adminRoles.
The part I wrote about is only used when type==SubjectType.USER.
I was considering if the original query
SELECT syncopeUser_id AS subject_id
FROM Membership M1 WHERE syncopeRole_id IN
(SELECT syncopeRole_id FROM Membership M2
WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
syncopeRole_id NOT IN
(SELECT id AS syncopeRole_id FROM
SyncopeRole WHERE id=x1 OR id=x2 OR ...)
)
can be simplified to
SELECT syncopeUser_id AS subject_id FROM
Membership M2 WHERE syncopeRole_id NOT IN
(SELECT id AS syncopeRole_id FROM
SyncopeRole WHERE id=x1 OR id=x2 OR ...)
If the original query works with all supported databases, the simplified
version should work as well.
I changed the code accordingly and ran the normal build with HSQL, and
there are no errors.
But it seems that the use cases of getAdminRolesFilter() are not covered
too well, as the build also seems to run
successfully if I replace the query by "SELECT syncopeUser_id WHERE 0=1"
(i.e. just return nothing).
I can try and investigate further when I have time.
I only wanted to know first if somebody maybe still can reproduce why
the query with M1 and M2 was designed the way it is.
Cheers,
Guido
On 16.04.2015 11:52, Francesco Chicchiriccò wrote:
> On 16/04/2015 11:00, Guido Wimmel wrote:
>> Hi,
>>
>> minor detail, but I recently stubled upon the query generated by
>> SubjectSearchDAOImpl.getAdminRolesFilter(),
>> used by search() and count(). Maybe it could be simplified.
>>
>> It seems to determine all users which have roles not in adminRoles
>> (to be able to filter them out
>> by WHERE subject_id NOT IN ... ).
>>
>> The generated query looks like (for type==USER and adminRoles={1,2}):
>>
>> (SELECT syncopeUser_id AS subject_id
>> FROM Membership M1
>> WHERE syncopeRole_id IN
>> (SELECT syncopeRole_id
>> FROM Membership M2
>> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND
>> syncopeRole_id NOT IN
>> (SELECT id AS syncopeRole_id
>> FROM SyncopeRole
>> WHERE id=1 OR id=2
>> )
>> )
>>
>>
>> I don't really understand why two (inter-dependent) subqueries on
>> membership are needed.
>> I don't see a difference to
>>
>> WHERE subject_id NOT IN (
>> SELECT syncopeUser_id AS subject_id FROM
>> Membership M2 WHERE syncopeRole_id NOT IN (
>> SELECT id AS
>> syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
>> )
>> )
>>
>> ... but probably I'm overlooking something?
>
> Hi Guido,
> this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
> JPASubjectSearchDAO [2] (master) as part of translating the FIQL query
> string received via REST into a native SQL query that can work
> reliably on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server,
> PostgreSQL and H2).
>
> Naturally, things can always be improved: if you find a way to
> simplify the subquery above while keeping everything working an all
> supported RDBMS, this is more than welcome!
>
> You can find information about how to check Syncope against all
> supported RDBMS at [3]: if you'd like to propose a change but you only
> have chance to verify it on a subset (say H2 and MySQL), please create
> a separate feature branch (or a pull request on GitHub) and I can help
> you completing all verifications needed.
>
> Please let me know.
> Regards.
>
> [1]
> https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
> [2]
> https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
> [3] http://syncope.apache.org/building.html#DBMSes
>
Re: getAdminRolesFilter-query
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 16/04/2015 11:00, Guido Wimmel wrote:
> Hi,
>
> minor detail, but I recently stubled upon the query generated by SubjectSearchDAOImpl.getAdminRolesFilter(),
> used by search() and count(). Maybe it could be simplified.
>
> It seems to determine all users which have roles not in adminRoles (to be able to filter them out
> by WHERE subject_id NOT IN ... ).
>
> The generated query looks like (for type==USER and adminRoles={1,2}):
>
> (SELECT syncopeUser_id AS subject_id
> FROM Membership M1
> WHERE syncopeRole_id IN
> (SELECT syncopeRole_id
> FROM Membership M2
> WHERE M2.syncopeUser_id=M1.syncopeUser_id AND syncopeRole_id NOT IN
> (SELECT id AS syncopeRole_id
> FROM SyncopeRole
> WHERE id=1 OR id=2
> )
> )
>
>
> I don't really understand why two (inter-dependent) subqueries on membership are needed.
> I don't see a difference to
>
> WHERE subject_id NOT IN (
> SELECT syncopeUser_id AS subject_id FROM Membership M2 WHERE syncopeRole_id NOT IN (
> SELECT id AS syncopeRole_id FROM SyncopeRole WHERE id=1 OR id=2
> )
> )
>
> ... but probably I'm overlooking something?
Hi Guido,
this query is generated by SubjectSearchDAOImpl [1] (1_2_X) /
JPASubjectSearchDAO [2] (master) as part of translating the FIQL query
string received via REST into a native SQL query that can work reliably
on all supported RDBMS (MySQL, MariaDB, Oracle, SQL Server, PostgreSQL
and H2).
Naturally, things can always be improved: if you find a way to simplify
the subquery above while keeping everything working an all supported
RDBMS, this is more than welcome!
You can find information about how to check Syncope against all
supported RDBMS at [3]: if you'd like to propose a change but you only
have chance to verify it on a subset (say H2 and MySQL), please create a
separate feature branch (or a pull request on GitHub) and I can help you
completing all verifications needed.
Please let me know.
Regards.
[1]
https://github.com/apache/syncope/blob/1_2_X/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
[2]
https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPASubjectSearchDAO.java
[3] http://syncope.apache.org/building.html#DBMSes
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/