[OPENIDM-13657] Implement Filter to map all queryId-based QueryRequests to queryFilter-based QueryRequests Created: 08/Aug/19  Updated: 06/Nov/20  Resolved: 07/Nov/19

Status: Closed
Project: OpenIDM
Component/s: Module - Repository framework
Affects Version/s: 7.0.0
Fix Version/s: 7.0.0

Type: Story Priority: Major
Reporter: Dirk Hogan Assignee: Dirk Hogan
Resolution: Fixed Votes: 0
Labels: CLARK
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
is required by OPENIDM-12683 Embedded DJ explicit user object has ... Closed
is documented by OPENIDM-13898 Verify queryId and queryFilter discus... Closed
caused OPENIDM-15780 IDM should not transform queryId to q... Resolved
relates to OPENIDM-12814 Setting returnByDefault for a relatio... Closed
relates to OPENIDM-12827 Setting returnByDefault to true on re... Closed
relates to OPENIDM-13375 REST2LDAP: Null source on query-all-ids Closed
relates to OPENDJ-6503 returnNullForMissingProperties=true s... Done
relates to OPENIDM-8356 Separate system configuration from us... Closed
OPENIDM-13915 Push repo config changes to CDM Sub-task Closed Dirk Hogan  
Target Version/s:
Verified Version/s:
QA Assignee: Son Nguyen
Story Points: 8
Sprint: 2019.12 - IDM, 2019.13 - IDM, 2019.14 - IDM, 2019.15 - IDM


When query-all-ids is dispatched, returnByDefault relationship fields will be returned from the DS repo. This occurs because the PopulateMissingEdgesFilter processes responses from the DS repo layer, adding any fields specified in the RelationshipSchemaContext. The fields in the RelationshipSchemaContext are added by the RelationshipSchemaContextDecoration, invoked by Internal/ManagedObjectSet#queryCollection, because an empty fields specification means that returnByDefault relationships should be returned. 

This can cause major problems for recons making use of the default query-all-ids query, if this default relationship field is not mapped(OPENIDM-12814), or if more than 3 such relationship fields are specified as returnByDefault, as this will cause the recon query logic to not query for the full object, as the recon query will believe that it has already obtained the full object(OPENIDM-12827). (See ReconTypeBase#hasFullObject for details.)

Discussions concluded that the specification of fields for queryId QueryRequests should not be supported, as invocation-specific field amendment in a user-defined query does not make sense. However, exactly such a field amendment is made by the authZ context while invoking the for-userName queryId, a query defined in authZ config, which cannot be modified without incurring a migration headache. The presence of this query prevents a scheme which circumvents the addition of returnByDefault relationship fields by restricting RelationshipSchemaContext construction to only queryFilter-based QueryRequests - such a scheme broke all the functional tests due to a systemic authZ failure.

Thus the proposed scheme is to define a Filter on the root resource which will map all queryId-based QueryRequests to queryFilter-based QueryRequests. This mapping will take advantage of the mapping already present in the DS repo config. This Filter will map any queries it has knowledge of, and let others pass through, as we still have to support custom queries in jdbc. The DS repo layer will reject any queryId-based QueryRequests. The JDBC repo layer will continue to leverage the existing queryId lookup, to support customer custom queries.(Rationale for not supporting custom queryIds in DS: queryIds were a means to write custom sql. In DS, they are simply translated to a queryFilter, and thus function as a queryFilter alias. In DS, just dispatch the queryRequest with a queryFilter).

This approach will solve the problem because query-all-ids will be translated into a queryFilter: true with fields of _id and _rev. The RelationshipsSchemaContextDecoration class only adds returnByDefault relationship fields if the _fields param is either empty, or ‘*’. If the _id and _rev fields are explicitly asked-for, then the returnByDefault relationship fields will not be added.

Finally, why does this problem occur with DS, and not with jdbc? Why does the returnByDefault relationship field, added to the query-all-ids request by RelationshipSchemaContextDecoration, not result in this relationship field being returned from jdbc? The answer has to do with https://bugster.forgerock.org/jira/browse/OPENIDM-10480, which documented the failure of a jdbc query-all-ids request, when the managed schema defined a returnByDefault relationship field. The solution was to define the validInRelationshipQuery property, which contains the queryIds which will trigger mega-query execution in jdbc. The query-all-ids query is not specified in this property. DS does not have this property, and the PopulateMissingEdgesFilter simply queries the fields in the RelationshipSchemaContext. Because query-all-ids does not specify any fields, any returnByDefault relationship fields are added to the RelationshipSchemaContext, and thus queried by the PopulateMissingEdgesFilter, again because the DS layer does not have the equivalent to a validInRelationshipQuery property which would prompt the PopulateMissingEdgesFilter to ignore fields in the RelationshipSchemaContext.  

This issue is likely related to https://bugster.forgerock.org/jira/browse/OPENIDM-13375 and https://bugster.forgerock.org/jira/browse/OPENDJ-6503 as here, the hypothesis is that the ResourceFiltering in InternalConnection#queryAsync is processing the field-less query-all-ids QueryRequest, and thus does not filter out null values. Rest2ldap, consumed from the DS-repo layer, is a client-side CREST-to-ldap translation, and as such, does not do any Resource filtering - the DS server only sees an ldap request. Thus the InternalConnection used to dispatch the request which ultimately hits rest2ldap is responsible for the filtering.

This impacts the type of Filter used to implement this logic: Servlet, CHF, or CREST, as we have to be sure that the queryFilter-with-fields is specified prior to traversal of the InternalConnection which will do the field-filtering. It is believed that IDM requests only traverse the org.forgerock.json.resource.InternalConnection class only once, as the request enters CREST, and thus the filter must perform the transform at the CHF level. This will allow all RequestHandler instances, many of which support query-all-ids, to benefit from this transform. Care must be taken to ensure that the queryFilter replacement for query-all-ids will support the current semantics.

Finally, these changes will be beneficial for CDM/FRAAS, as it will dramatically reduce the configuration surface because all queryId definitions will be removed from repo.ds.json and repo.jdbc.json. The translations from queryId to queryFilter, like those currently defined for the DS repo layer, will be defined in a resource bundled with the repo module.



Comment by Dirk Hogan [ 21/Aug/19 ]

Lana Frost Nabil Maynard as part of this change, only user-defined custom queryIds will be supported. The issue summary states that they will only be supported for JDBC - I don't understand why they could not also be supported for DS, but I will clarify and update. This means that e.g. setting the sourceQuery in recon to query-all-ids, for example, will result in an error. A filter at the outside boundary of IDM will ensure that client requests will be appropriately translated, but requests internal to IDM, like a sourceQuery of query-all-ids against managed/user, will result in an error. The same query, directed against system source/targets will continue to work. Clear as mud. Consider this a notification signaling the start of discussions to hash out the complicated doc implications of this change.


Added the following to issue summary: (Rationale for not supporting custom queryIds in DS: queryIds were a means to write custom sql. In DS, they are simply translated to a queryFilter, and thus function as a queryFilter alias. In DS, just dispatch the queryRequest with a queryFilter).

Comment by Son Nguyen [ 15/Jun/20 ]

Verified successfully with OpenIDM: 7.0.0-SNAPSHOT ca359bb:

  • automated tests updated
  • no regressions found
  • no new issues found
Generated at Sun Jan 24 17:50:15 UTC 2021 using Jira 7.13.12#713012-sha1:6e07c38070d5191bbf7353952ed38f111754533a.