Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cosmos spring data - patch support #32630

Merged
merged 29 commits into from
Jan 12, 2023
Merged

cosmos spring data - patch support #32630

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0e6522d
initial patches changes
Dec 9, 2022
492efef
initial patch changes
Dec 9, 2022
5bd32c0
edits
Dec 14, 2022
2d0040b
add unit tests and Trevor multitenant pieces
Dec 15, 2022
83b2170
edits after Trevor review
Dec 16, 2022
60605eb
edits after Trevor review
Dec 16, 2022
7ec75e1
remove uneccessary line
Dec 16, 2022
37dc1c2
re-factor to fit Spring Data contract better
Dec 16, 2022
9fa7ed6
fix javadocs errors
Dec 16, 2022
0bcd7a0
fix javadocs errors
Dec 17, 2022
b1dbccb
amend contracts to be more consistent
Dec 17, 2022
5c985ec
amend unit tests
Dec 17, 2022
2a86579
fix build failures
Dec 17, 2022
86b5b03
Patch improvements with less API and implementation improvements
kushagraThapar Dec 20, 2022
3ea2c15
Merge branch 'main' into cosmos-spring-data-patch
TheovanKraay Dec 21, 2022
57a69bb
fix javadocs format errors
Dec 21, 2022
1a21dca
Adding overrides to revapi.json.
trande4884 Dec 21, 2022
0211b29
Revising overrides to revapi.json.
trande4884 Dec 21, 2022
8965a33
Revising overrides to revapi.json.
trande4884 Dec 22, 2022
0259b1d
Fixing reactive tests.
trande4884 Dec 22, 2022
be371e1
Updating the changelog.
trande4884 Jan 3, 2023
48013a0
address Fabian review + enhance non reactive tests
Jan 6, 2023
9dd8c39
revert design + Annie/Trevor review
Jan 10, 2023
e9e2261
fix build errors
Jan 10, 2023
e40896e
fix unit tests
Jan 10, 2023
b65aaa2
update revapi.json
Jan 10, 2023
4352685
expand options descriptions in templates
Jan 11, 2023
e930de9
add respository test cases
Jan 11, 2023
3a2e90b
Adding reactive tests and changing one assert.
trande4884 Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions eng/code-quality-reports/src/main/resources/revapi/revapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,30 @@
"new": "method .* com\\.azure\\.resourcemanager\\..*",
"justification": "resourcemanager interfaces are allowed to add methods."
},
{
"ignore": true,
"code": "java.method.addedToInterface",
"new": "method <T> T com.azure.spring.data.cosmos.core.CosmosOperations::patch(T, com.azure.cosmos.models.CosmosPatchOperations, com.azure.cosmos.models.CosmosPatchItemRequestOptions)",
"justification": "Spring interfaces are allowed to add methods."
},
{
"ignore": true,
"code": "java.method.addedToInterface",
"new": "method <T> reactor.core.publisher.Mono<T> com.azure.spring.data.cosmos.core.ReactiveCosmosOperations::patch(T, com.azure.cosmos.models.CosmosPatchOperations, com.azure.cosmos.models.CosmosPatchItemRequestOptions)",
"justification": "Spring interfaces are allowed to add methods."
},
{
"ignore": true,
"code": "java.method.addedToInterface",
"new": "method <S extends T> S com.azure.spring.data.cosmos.repository.CosmosRepository<T, ID extends java.io.Serializable>::save(S, com.azure.cosmos.models.CosmosPatchOperations, com.azure.cosmos.models.CosmosPatchItemRequestOptions)",
"justification": "Spring interfaces are allowed to add methods."
},
{
"ignore": true,
"code": "java.method.addedToInterface",
"new": "method <S extends T> reactor.core.publisher.Mono<S> com.azure.spring.data.cosmos.repository.ReactiveCosmosRepository<T, K>::save(S, com.azure.cosmos.models.CosmosPatchOperations, com.azure.cosmos.models.CosmosPatchItemRequestOptions)",
"justification": "Spring interfaces are allowed to add methods."
},
{
"regex": true,
"code": "java\\.class\\.externalClassExposedInAPI",
Expand Down
12 changes: 12 additions & 0 deletions ...ing-data-cosmos-test/src/test/java/com/azure/spring/data/cosmos/common/TestConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import com.azure.cosmos.models.IndexingMode;
import com.azure.spring.data.cosmos.domain.Address;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -18,7 +20,9 @@ public final class TestConstants {
private static final Address ADDRESS_1 = new Address("201107", "Zixing Road", "Shanghai");
private static final Address ADDRESS_2 = new Address("200000", "Xuhui", "Shanghai");
public static final String HOBBY1 = "photography";
public static final String PATCH_HOBBY1 = "shopping";
public static final List<String> HOBBIES = Arrays.asList(HOBBY1, "fishing");
public static final List<String> PATCH_HOBBIES = Arrays.asList(HOBBY1, "fishing", PATCH_HOBBY1);
public static final List<Address> ADDRESSES = Arrays.asList(ADDRESS_1, ADDRESS_2);

public static final String ROLE_COLLECTION_NAME = "RoleCollectionName";
Expand All @@ -45,6 +49,7 @@ public final class TestConstants {

public static final String DB_NAME = "testdb";
public static final String FIRST_NAME = "first_name_li";
public static final String PATCH_FIRST_NAME = "first_name_replace";
public static final String LAST_NAME = "last_name_p";
public static final Integer ZIP_CODE = 12345;
public static final String ID_1 = "id-1";
Expand Down Expand Up @@ -98,12 +103,19 @@ public final class TestConstants {
public static final String DEPARTMENT = "test-department";

public static final Integer AGE = 24;
public static final Integer PATCH_AGE_1 = 25;
public static final Integer PATCH_AGE_INCREMENT = 2;

public static final Map<String, String> PASSPORT_IDS_BY_COUNTRY = new HashMap<String, String>() {{
put("United States of America", "123456789");
put("Côte d'Ivoire", "IC1234567");
}};

public static final Map<String, String> NEW_PASSPORT_IDS_BY_COUNTRY = new HashMap<String, String>() {{
put("United Kingdom", "123456789");
put("Germany", "IC1234567");
}};

private TestConstants() {
}
}
Expand Down
63 changes: 63 additions & 0 deletions ...ng-data-cosmos-test/src/test/java/com/azure/spring/data/cosmos/core/CosmosTemplateIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
import com.azure.cosmos.CosmosClientBuilder;
import com.azure.cosmos.CosmosException;
import com.azure.cosmos.implementation.ConflictException;
import com.azure.cosmos.implementation.PreconditionFailedException;
import com.azure.cosmos.models.CosmosContainerProperties;
import com.azure.cosmos.models.CosmosPatchItemRequestOptions;
import com.azure.cosmos.models.CosmosPatchOperations;
import com.azure.cosmos.models.PartitionKey;
import com.azure.cosmos.models.SqlQuerySpec;
import com.azure.cosmos.models.ThroughputResponse;
Expand Down Expand Up @@ -35,6 +38,9 @@
import com.azure.spring.data.cosmos.repository.TestRepositoryConfig;
import com.azure.spring.data.cosmos.repository.repository.AuditableRepository;
import com.azure.spring.data.cosmos.repository.support.CosmosEntityInformation;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.assertj.core.util.Lists;
import org.junit.Before;
import org.junit.ClassRule;
Expand Down Expand Up @@ -71,11 +77,17 @@
import static com.azure.spring.data.cosmos.common.TestConstants.LAST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.NEW_FIRST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.NEW_LAST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.NEW_PASSPORT_IDS_BY_COUNTRY;
import static com.azure.spring.data.cosmos.common.TestConstants.NOT_EXIST_ID;
import static com.azure.spring.data.cosmos.common.TestConstants.PAGE_SIZE_1;
import static com.azure.spring.data.cosmos.common.TestConstants.PAGE_SIZE_2;
import static com.azure.spring.data.cosmos.common.TestConstants.PAGE_SIZE_3;
import static com.azure.spring.data.cosmos.common.TestConstants.PASSPORT_IDS_BY_COUNTRY;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_AGE_1;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_AGE_INCREMENT;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_FIRST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_HOBBIES;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_HOBBY1;
import static com.azure.spring.data.cosmos.common.TestConstants.UPDATED_FIRST_NAME;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
Expand All @@ -98,6 +110,23 @@ public class CosmosTemplateIT {

private static final String WRONG_ETAG = "WRONG_ETAG";

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final JsonNode NEW_PASSPORT_IDS_BY_COUNTRY_JSON = OBJECT_MAPPER.convertValue(NEW_PASSPORT_IDS_BY_COUNTRY, JsonNode.class);

private static final CosmosPatchOperations operations = CosmosPatchOperations
.create()
.replace("/age", PATCH_AGE_1);

CosmosPatchOperations multiPatchOperations = CosmosPatchOperations
.create()
.replace("/firstName", PATCH_FIRST_NAME)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also test set and remove patch operations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we should, @TheovanKraay please add these tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the two extra operations in multiPatchOperations.

.replace("/passportIdsByCountry", NEW_PASSPORT_IDS_BY_COUNTRY_JSON)
.add("/hobbies/2", PATCH_HOBBY1)
.increment("/age", PATCH_AGE_INCREMENT);

private static final CosmosPatchItemRequestOptions options = new CosmosPatchItemRequestOptions();


@ClassRule
public static final IntegrationTestCollectionManager collectionManager = new IntegrationTestCollectionManager();

Expand All @@ -117,6 +146,9 @@ public class CosmosTemplateIT {
@Autowired
private ResponseDiagnosticsTestUtils responseDiagnosticsTestUtils;

public CosmosTemplateIT() throws JsonProcessingException {
}

@Before
public void setUp() throws ClassNotFoundException {
if (cosmosTemplate == null) {
Expand Down Expand Up @@ -267,6 +299,37 @@ public void testUpdate() {
assertEquals(person, updated);
}

@Test
public void testPatch() {
insertedPerson = cosmosTemplate.patch(insertedPerson, operations, null);
Person patchedPerson = cosmosTemplate.findById(containerName, insertedPerson.getId(), Person.class);
assertEquals(insertedPerson.getAge(), PATCH_AGE_1);
Copy link
Member

@xinlian12 xinlian12 Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to get patchedPerson? it is not used anywhere

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is from the old tests before they were updated. @TheovanKraay please remove.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

@Test
public void testPatchMultiOperations() {
insertedPerson = cosmosTemplate.patch(insertedPerson, multiPatchOperations, null);
Person patchedPerson = cosmosTemplate.findById(containerName, insertedPerson.getId(), Person.class);
assertEquals(insertedPerson.getAge().intValue(), (AGE + PATCH_AGE_INCREMENT));
assertEquals(insertedPerson.getHobbies(), PATCH_HOBBIES);
assertEquals(insertedPerson.getFirstName(), PATCH_FIRST_NAME);
assertEquals(insertedPerson.getPassportIdsByCountry(), NEW_PASSPORT_IDS_BY_COUNTRY);
}

@Test
public void testPatchPreConditionFail() {
try {
options.setFilterPredicate("FROM person p WHERE p.lastName = 'dummy'");
insertedPerson = cosmosTemplate.patch(insertedPerson, operations,options);
Person patchedPerson = cosmosTemplate.findById(containerName, insertedPerson.getId(), Person.class);
assertEquals(insertedPerson.getAge(), patchedPerson.getAge());
fail();
} catch (CosmosAccessException ex) {
assertThat(ex.getCosmosException()).isInstanceOf(PreconditionFailedException.class);
assertThat(responseDiagnosticsTestUtils.getCosmosDiagnostics()).isNotNull();
}
}

@Test
public void testOptimisticLockWhenUpdatingWithWrongEtag() {
final Person updated = new Person(TEST_PERSON.getId(), UPDATED_FIRST_NAME,
Expand Down
56 changes: 56 additions & 0 deletions ...cosmos-test/src/test/java/com/azure/spring/data/cosmos/core/ReactiveCosmosTemplateIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import com.azure.cosmos.CosmosClientBuilder;
import com.azure.cosmos.CosmosException;
import com.azure.cosmos.implementation.ConflictException;
import com.azure.cosmos.implementation.PreconditionFailedException;
import com.azure.cosmos.models.CosmosContainerResponse;
import com.azure.cosmos.models.CosmosPatchItemRequestOptions;
import com.azure.cosmos.models.CosmosPatchOperations;
import com.azure.cosmos.models.PartitionKey;
import com.azure.cosmos.models.SqlQuerySpec;
import com.azure.cosmos.models.ThroughputResponse;
Expand All @@ -33,6 +36,8 @@
import com.azure.spring.data.cosmos.repository.TestRepositoryConfig;
import com.azure.spring.data.cosmos.repository.repository.AuditableRepository;
import com.azure.spring.data.cosmos.repository.support.CosmosEntityInformation;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.assertj.core.api.Assertions;
import org.junit.After;
import org.junit.Assert;
Expand Down Expand Up @@ -65,7 +70,13 @@
import static com.azure.spring.data.cosmos.common.TestConstants.FIRST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.HOBBIES;
import static com.azure.spring.data.cosmos.common.TestConstants.LAST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.NEW_PASSPORT_IDS_BY_COUNTRY;
import static com.azure.spring.data.cosmos.common.TestConstants.PASSPORT_IDS_BY_COUNTRY;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_AGE_1;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_AGE_INCREMENT;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_FIRST_NAME;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_HOBBIES;
import static com.azure.spring.data.cosmos.common.TestConstants.PATCH_HOBBY1;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand All @@ -89,6 +100,22 @@ public class ReactiveCosmosTemplateIT {
private static final String PRECONDITION_IS_NOT_MET = "is not met";
private static final String WRONG_ETAG = "WRONG_ETAG";

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final JsonNode NEW_PASSPORT_IDS_BY_COUNTRY_JSON = OBJECT_MAPPER.convertValue(NEW_PASSPORT_IDS_BY_COUNTRY, JsonNode.class);

private static final CosmosPatchOperations operations = CosmosPatchOperations
.create()
.replace("/age", PATCH_AGE_1);

CosmosPatchOperations multiPatchOperations = CosmosPatchOperations
.create()
.replace("/firstName", PATCH_FIRST_NAME)
.replace("/passportIdsByCountry", NEW_PASSPORT_IDS_BY_COUNTRY_JSON)
.add("/hobbies/2", PATCH_HOBBY1)
.increment("/age", PATCH_AGE_INCREMENT);

private static final CosmosPatchItemRequestOptions options = new CosmosPatchItemRequestOptions();

@ClassRule
public static final ReactiveIntegrationTestCollectionManager collectionManager = new ReactiveIntegrationTestCollectionManager();

Expand Down Expand Up @@ -281,6 +308,35 @@ public void testUpsert() {
assertThat(responseDiagnosticsTestUtils.getCosmosDiagnostics()).isNotNull();
}

@Test
public void testPatch() {
final Mono<Person> patch = cosmosTemplate.patch(insertedPerson, operations, null);
StepVerifier.create(patch).expectNextCount(1).verifyComplete();
Mono<Person> patchedPerson = cosmosTemplate.findById(containerName, insertedPerson.getId(), Person.class);
StepVerifier.create(patchedPerson).expectNextCount(1).verifyComplete();
assertEquals(patchedPerson.block().getAge(), PATCH_AGE_1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder whether the age verification can also be part of StepVerifier flow -> expectNextMatches(person -> person.getAge() == PATCH_AGE_1)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

}

@Test
public void testPatchMultiOperations() {
final Mono<Person> patch = cosmosTemplate.patch(insertedPerson, multiPatchOperations, null);
StepVerifier.create(patch).expectNextCount(1).verifyComplete();
Mono<Person> patchedPerson = cosmosTemplate.findById(containerName, insertedPerson.getId(), Person.class);
StepVerifier.create(patchedPerson).expectNextCount(1).verifyComplete();
assertEquals(patchedPerson.block().getAge().intValue(), (AGE + PATCH_AGE_INCREMENT));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of calling .block() for each asserts, either we can add this validation be part of the StepVerifier flow or just call .block() once (to make it a little bit more efficient), else it will trigger multiple calls

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to single block()

assertEquals(patchedPerson.block().getHobbies(),PATCH_HOBBIES);
assertEquals(patchedPerson.block().getFirstName(), PATCH_FIRST_NAME);
assertEquals(patchedPerson.block().getPassportIdsByCountry(), NEW_PASSPORT_IDS_BY_COUNTRY);
}

@Test
public void testPatchPreConditionFail() {
options.setFilterPredicate("FROM person p WHERE p.lastName = 'dummy'");
Mono<Person> person = cosmosTemplate.patch(insertedPerson, operations, options);
StepVerifier.create(person).expectErrorMatches(ex -> ex instanceof CosmosAccessException &&
((CosmosAccessException) ex).getCosmosException() instanceof PreconditionFailedException).verify();
}

@Test
public void testOptimisticLockWhenUpdatingWithWrongEtag() {
final Person updated = new Person(TEST_PERSON.getId(), TestConstants.UPDATED_FIRST_NAME,
Expand Down
1 change: 1 addition & 0 deletions sdk/cosmos/azure-spring-data-cosmos/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#### Features Added
* Added support for multi-tenancy at the Database level via `CosmosFactory` - See [PR 32516](https://github.com/Azure/azure-sdk-for-java/pull/32516)
* Added support for patch - See [PR 32630](https://github.com/Azure/azure-sdk-for-java/pull/32630)

#### Breaking Changes

Expand Down
13 changes: 13 additions & 0 deletions ...-spring-data-cosmos/src/main/java/com/azure/spring/data/cosmos/core/CosmosOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package com.azure.spring.data.cosmos.core;

import com.azure.cosmos.models.CosmosContainerProperties;
import com.azure.cosmos.models.CosmosPatchItemRequestOptions;
import com.azure.cosmos.models.CosmosPatchOperations;
import com.azure.cosmos.models.PartitionKey;
import com.azure.cosmos.models.SqlQuerySpec;
import com.azure.spring.data.cosmos.core.convert.MappingCosmosConverter;
Expand Down Expand Up @@ -123,6 +125,17 @@ public interface CosmosOperations {
*/
<T> T insert(T objectToSave, PartitionKey partitionKey);

/**
* patches item
* @param entityToPatch must not be {@literal null}, must contain {@link org.springframework.data.annotation.Id}
* and {@link com.azure.spring.data.cosmos.core.mapping.PartitionKey} field
* @param patchOperations must not be {@literal null}, max operations is 10
* @param options Optional CosmosPatchItemRequestOptions, e.g. options.setFilterPredicate("FROM products p WHERE p.used = false");
* @param <T> type class of domain type
* @return the patched item
*/
<T> T patch(T entityToPatch, CosmosPatchOperations patchOperations, CosmosPatchItemRequestOptions options);

/**
* Inserts item
*
Expand Down
Loading