spring-data-mongodb icon indicating copy to clipboard operation
spring-data-mongodb copied to clipboard

Seperate `QueryMapper.getMappedObject()` recusive logic.

Open RyanGibb opened this issue 2 years ago • 2 comments

Summary

This issue is proposing separating the recursive logic of QueryMappergetMappedObject() from the the public method it is invoked by. This allows to handle the outer Document query and the nested levels of Document query separately.

Detail

We (Cisco Defense Orchestrator) are overriding MongoTemplate to provide some restrictions on database access. Specifically, injecting a Criteria to all queries, setting a field on all objects stored in the database, and preventing that field being updated.

To inject the Criteria we’ve set MongoTemplate.queryMapper() to an overridden implementation of QueryMapper through reflection that modifies the Document query parameter of QueryMapper.getMappedObject().

An issue we’ve encountered is that QueryMapper.getMappedObject() is called recursively inside QueryMapper, so this modification is done at every level.

For example, if we had the query:

{ $text: { $search: "java coffee shop" } }

We would want:

{ $text: { $search: "java coffee shop" }, <new criteria> }

But we would get:

{ $text: { $search: "java coffee shop", <new criteria> }, <new criteria> }

Which fails with UncategorizedMongoDbException: Query failed with error code 2 and error message 'extra fields in $text'.

We solved this by setting adding an isNested parameter:

public class OverriddenQueryMapper extends QueryMapper {

  private ThreadLocal<Boolean> isNested = ThreadLocal.withInitial(() -> Boolean.FALSE);

  @SneakyThrows
  @Override
  public Document getMappedObject(Bson query, MongoPersistentEntity<?> entity) {
    if (isNested.get()) {
      return super.getMappedObject(query, entity);
    }
    <inject criteria here>
    isNested.set(Boolean.TRUE);
    Document mappedQuery = super.getMappedObject(query, entity);
    isNested.set(Boolean.FALSE);
    return mappedQuery;
  }

  @Override
  public Document getMappedFields(Document fieldsObject, MongoPersistentEntity<?> entity) {
    isNested.set(Boolean.TRUE);
    Document mappedFields = super.getMappedFields(fieldsObject, entity);
    isNested.set(Boolean.FALSE);
    return mappedFields;
  }
}

The ThreadLocal is for thread safety.

A nicer solution would be provided by introducing a separate protected recurse method called internally in QueryMapper, so that the public getMappedObject could be overridden without effecting recursive calls. See the proposed change here: https://github.com/RyanGibb/spring-data-mongodb/commit/19a1f759344715d4b0c6abea59b073cc5319f664

Any thoughts or feedback would be much appreciated.

RyanGibb avatar Sep 15 '21 11:09 RyanGibb

Thanks for bringing this up.

christophstrobl avatar Sep 16 '21 05:09 christophstrobl

You're very welcome.

RyanGibb avatar Sep 16 '21 20:09 RyanGibb