Uploaded image for project: 'OpenIDM'
  1. OpenIDM
  2. OPENIDM-13657

Implement Filter to map all queryId-based QueryRequests to queryFilter-based QueryRequests



    • Target Version/s:
    • Verified Version/s:
    • Story Points:
    • 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.




          Issue Links



              dhogan Dirk Hogan
              dhogan Dirk Hogan
              QA Assignee:
              Son Nguyen Son Nguyen
              0 Vote for this issue
              5 Start watching this issue