Azure / azure-sdk-for-java

This repository is for active development of the Azure SDK for Java. For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/java/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-java.
MIT License
2.35k stars 1.99k forks source link

[BUG] Recent versions no longer set audit data when saving multiple entities with partition keys #39226

Closed Blackbaud-JasonBodnar closed 6 months ago

Blackbaud-JasonBodnar commented 7 months ago

Describe the bug There seems to be a regression between 3.18 and 3.42. I believe it also exists in the latest 5.x version.

In 3.18, SimpleCosmosRepository.saveAll():

public <S extends T> Iterable<S> saveAll(Iterable<S> entities) {
        Assert.notNull(entities, "Iterable entities should not be null");

        final List<S> savedEntities = new ArrayList<>();
        entities.forEach(entity -> {
            final S savedEntity = this.save(entity);
            savedEntities.add(savedEntity);
        });

        return savedEntities;
    }

By calling save() on each entity of the Iterable passed, any auditing information was filled in (ie, @CreatedBy, @LastModifiedBy, etc).

But in 3.48:

public <S extends T> Iterable<S> saveAll(Iterable<S> entities) {
        Assert.notNull(entities, "Iterable entities should not be null");

        if (information.getPartitionKeyFieldName() != null) {
            return operation.insertAll(this.information, entities);
        } else {
            final List<S> savedEntities = new ArrayList<>();
            entities.forEach(entity -> {
                final S savedEntity = this.save(entity);
                savedEntities.add(savedEntity);
            });

            return savedEntities;
        }
    }

If there's a partition key, operation.insertAll(this.information, entities) does not add auditing information.

To Reproduce See https://github.com/Blackbaud-JasonBodnar/cosmos-sql-bug/blob/v5.x/src/test/groovy/com/example/cosmossqlbug/TestEntityRepositorySpec.groovy#L28

Code Snippet

public <S extends T> Iterable<S> saveAll(Iterable<S> entities) {
        Assert.notNull(entities, "Iterable entities should not be null");

        if (information.getPartitionKeyFieldName() != null) {
            return operation.insertAll(this.information, entities);
        } else {
            final List<S> savedEntities = new ArrayList<>();
            entities.forEach(entity -> {
                final S savedEntity = this.save(entity);
                savedEntities.add(savedEntity);
            });

            return savedEntities;
        }
    }

Expected behavior Any annotated fields will be populated with audit data.

Setup (please complete the following information):

Additional context

Originally reported when trying to find a solution for https://github.com/Azure/azure-sdk-for-java/issues/38691 but it should be its own bug.

Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

github-actions[bot] commented 7 months ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @kushagraThapar @pjohari-ms @TheovanKraay.

kushagraThapar commented 7 months ago

thanks @Blackbaud-JasonBodnar for creating this issue. @trande4884 can you please work on it? Seems like related to our bulk implementation for saveAll(). Similar issue might be happening for deleteAll() and findAll as well if we have bulk implementations for those as well.

Blackbaud-JasonBodnar commented 7 months ago

@kushagraThapar , @trande4884 : Any updates on this bug?

trande4884 commented 7 months ago

@Blackbaud-JasonBodnar I am beginning investigation of this today and plan to have it in PR by end of next week by the latest.