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/
