"update one" aggregation update with optimistic locking throws exception

[tiboun]

Hi,

I was wondering if I used the aggregation update badly.

I'm currently using spring-data-mongodb 3.3.4.

In fact, update first throws an exception while update all doesn't because of the optimistic lock checking in the method doUpdate of ReactiveMongoTemplate.

In order to reproduce the exception, we need to do an aggregation update and specify a matching query that doesn't match any document. I currently have two stages in my aggregation pipeline.

The exception occurs while trying to get MappedUpdate from the entity before checking if it contains the version property in order to throw an OptimisticLockingFailureException. A "'name' must not be null" is thrown.

Above this method we can see that we make a difference between aggregation update and basic update. My suggestion would be to change the code as follows but didn't check it yet :

Change from :

if (entity != null && entity.hasVersionProperty() && !multi) {
                if (updateResult.wasAcknowledged() && updateResult.getMatchedCount() == 0) {

                    Document updateObj = updateContext.getMappedUpdate(entity);
                    if (containsVersionProperty(queryObj, entity))
                        throw new OptimisticLockingFailureException("Optimistic lock exception on saving entity: "
                                + updateObj.toString() + " to collection " + collectionName);
                }
            }

to:

if (entity != null && entity.hasVersionProperty() && !multi) {
                if (updateResult.wasAcknowledged() && updateResult.getMatchedCount() == 0) {
                    if (containsVersionProperty(queryObj, entity)){
                        List<Document> updateObjs = updateContext.isAggregationUpdate() ? updateContext.getUpdatePipeline(entityClass): List.of(updateContext.getMappedUpdate(entity));
                                            String updateObjStr = updateObjs.stream().map(String::valueOf).collect(Collectors.joining("[", ",\n", "]"));
                        throw new OptimisticLockingFailureException("Optimistic lock exception on saving entity: "
                                + updateObjStr + " to collection " + collectionName);
                                        }
                }
            }

Moving the build of updateObj to the if block avoid building it when we know that query object doesn't contain the version property save some computation.

We may, as well, move it to the previous if condition if getting version property don't do any computation as follow :

if (!multi && containsVersionProperty(queryObj, entity)) {
                if (updateResult.wasAcknowledged() && updateResult.getMatchedCount() == 0) {
                        List<Document> updateObjs = updateContext.isAggregationUpdate() ? updateContext.getUpdatePipeline(entityClass): List.of(updateContext.getMappedUpdate(entity));
                                            String updateObjStr = updateObjs.stream().map(String::valueOf).collect(Collectors.joining("[", ",\n", "]"));
                        throw new OptimisticLockingFailureException("Optimistic lock exception on saving entity: "
                                + updateObjStr + " to collection " + collectionName);
                }
            }

As a workaround, since the match query target a document id, I used all instead of first and all works as expected since I don't use the optimistic locking in my case.

[christophstrobl]

It would be great to have a minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

[tiboun]

I've modified one of the ReactiveMongoTemplateUpdateTest in order to reproduce the problem.

You can reproduce so with the following unit test:

@Test
    public void versionedAggregateUpdateWithSet() {

        Versioned source = new Versioned("id-1", "value-0");
        template.insert(Versioned.class).one(source).then().as(StepVerifier::create).verifyComplete();

        AggregationUpdate update = AggregationUpdate.update().set("value").toValue("changed");
        template.update(Versioned.class).matching(Query.query(Criteria.where("id").is(source.id + " no match"))).apply(update).first()
                .then().as(StepVerifier::create).verifyComplete();

        Flux.from(collection(Versioned.class).find(new org.bson.Document("_id", source.id)).limit(1)).collectList()
                .as(StepVerifier::create).consumeNextWith(it -> {
                    assertThat(it).containsExactly(
                            new org.bson.Document("_id", source.id).append("version", 1L).append("value", "changed").append("_class",
                                    "org.springframework.data.mongodb.core.ReactiveMongoTemplateUpdateTests$Versioned"));
                }).verifyComplete();
    }

It will throw the exception. The unit test above is not a valid one. Purpose of this test was just to reproduce the behavior that I wasn't expected.

The failure result is the following :

ReactiveMongoTemplateUpdateTests.versionedAggregateUpdateWithSet expectation "expectComplete" failed (expected: onComplete(); actual: onError(java.lang.IllegalArgumentException: Name must not be null))

Best regards,

[osadchuk-roman]

Hey @mp911de @christophstrobl I have faced with same issue. Here is a simple POC with reproduce in tests: https://github.com/osadchuk-roman/mongo-optimistic-locking-with-aggregation-update-issue?tab=readme-ov-file. And it seems this issue exists in latest version of spring data mongodb. My setup: Spring boot version: 3.3.2 Spring data mongodb version: 3.4.2

More details:

When I'm trying to executed aggregation update with optimistic locking and there were no matched documents in UpdateResult, then java.lang.IllegalArgumentException: Name must not be null is thrown during attempt to create a MappedUpdate from AggregationUpdate.

java.lang.IllegalArgumentException: Name must not be null
    at org.springframework.util.Assert.hasText(Assert.java:240)
    at org.springframework.data.mongodb.core.convert.QueryMapper$Field.<init>(QueryMapper.java:1024)
    at org.springframework.data.mongodb.core.convert.QueryMapper$MetadataBackedField.<init>(QueryMapper.java:1169)
    at org.springframework.data.mongodb.core.convert.QueryMapper$MetadataBackedField.<init>(QueryMapper.java:1153)
    at org.springframework.data.mongodb.core.convert.UpdateMapper$MetadataBackedUpdateField.<init>(UpdateMapper.java:294)
    at org.springframework.data.mongodb.core.convert.UpdateMapper.createPropertyField(UpdateMapper.java:254)
    at org.springframework.data.mongodb.core.convert.QueryMapper.getMappedObject(QueryMapper.java:168)
    at org.springframework.data.mongodb.core.convert.UpdateMapper.getMappedObject(UpdateMapper.java:66)
    at org.springframework.data.mongodb.core.QueryOperations$UpdateContext.getMappedUpdate(QueryOperations.java:905)
    at org.springframework.data.mongodb.core.ReactiveMongoTemplate.lambda$doUpdate$75(ReactiveMongoTemplate.java:1816)