spring-projects / spring-projects/spring-data-mongodb

Seperate `QueryMapper.getMappedObject()` recusive logic.

Open
#3,824 2 comments 8 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: mapping status: pending-design-work theme: 4.2
Dominant language
Java
Stars
1.7k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with QueryMapper.getMappedObject and the recursive calls described in the issue; compare them with the proposed change in the linked commit. Separate outer-document handling from recursive mapping so an override affects only the outer query, then verify nested operators such as $text no longer receive the injected criteria.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, mongodb, spring
Domain
backend, database
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.