Description
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.