The semantics of SimpleCouchbaseRepository.save have changed between 3.x and 4.x

[aaronjwhiteside]

Saving some entity with the same document id/key used to raise a DuplicateKeyException, now it just replaces the document silently.

I couldn't find anything about this change in the migration guide, so my guess is it's unintentional.

https://docs.spring.io/spring-data/couchbase/docs/current/reference/html/#couchbase.migrating

https://github.com/spring-projects/spring-data-couchbase/blob/main/src/main/java/org/springframework/data/couchbase/repository/support/SimpleCouchbaseRepository.java#L70-L82

[mikereiche]

Please refer to #934.

btw - for every change to spring data projects, there will be a an issue - https://github.com/spring-projects/spring-data-couchbase/issues - which should shed some light on the change.

[aaronjwhiteside]

@mikereiche

So looking at the recent changes made (0ce0f36) and the previous code (https://github.com/spring-projects/spring-data-couchbase/blob/3.2.x/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java#L617-L627)

This looks like a regression to me.

I believe we can bring back the original semantics of the save() method.

1. If a @Version field is not present, call upsertById.
2. If a @Version field exists, and is non-zero, call replaceById.
3. If a @Version field exists, and is null or zero, call insertById.

What do you think?

[aaronjwhiteside]

Something like this:

    @Override
    @SuppressWarnings("unchecked")
    public <S extends T> S save(final S entity) {
        Assert.notNull(entity, "Entity must not be null!");
        final CouchbasePersistentEntity<?> mapperEntity = couchbaseOperations.getConverter().getMappingContext().getPersistentEntity(entity.getClass());
        final CouchbasePersistentProperty versionProperty = mapperEntity.getVersionProperty();
        final boolean versionPresent = versionProperty != null;
        final Long version = versionProperty == null || versionProperty.getField() == null
                ? null
                : (Long) ReflectionUtils.getField(versionProperty.getField(), entity);
        final boolean existingDocument = version != null && version > 0;

        if (!versionPresent) {
            //No version field - no cas
            return (S) couchbaseOperations.upsertById(getEntityInformation().getJavaType()).one(entity);
        } else if (existingDocument) {
            //Updating existing document with cas
            return (S) couchbaseOperations.replaceById(getEntityInformation().getJavaType()).one(entity);
        } else {
            //Creating new document
            return (S) couchbaseOperations.insertById(getEntityInformation().getJavaType()).one(entity);
        }
    }

[jorgerod]

Hi @mikereiche

I agree with @aaronjwhiteside .
This change would also be very useful for me since I had to make a custom implementation to do the behavior proposed by @aaronjwhiteside.