From cfbf15d40f7e08cf466cdf8c43818da1c2decc2d Mon Sep 17 00:00:00 2001 From: Tim F <120514393+tim-aero@users.noreply.github.com> Date: Sat, 18 May 2024 19:35:29 -0600 Subject: [PATCH] Fixed issues with RecordExistsAction If a policy with a RecordExistsAction is passed into `save(obj)` the save passed UPDATE instead of the actual policy which should be used. Corrected this by checking to see if the REA on the poiicy was UPDATE before overwriting it. See https://discuss.aerospike.com/t/recordexistsaction-create-only-and-sendkey-isnt-working-on-java-object-mapper/11111/7 --- .../aerospike/mapper/tools/AeroMapper.java | 4 +- .../mapper/tools/ReactiveAeroMapper.java | 4 +- .../aerospike/mapper/AeroMapperBaseTest.java | 2 +- .../aerospike/mapper/InsertOnlyModeTest.java | 71 +++++++++++++++++++ .../reactive/ReactiveInsertOnlyModeTest.java | 65 +++++++++++++++++ 5 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/aerospike/mapper/InsertOnlyModeTest.java create mode 100644 src/test/java/com/aerospike/mapper/reactive/ReactiveInsertOnlyModeTest.java diff --git a/src/main/java/com/aerospike/mapper/tools/AeroMapper.java b/src/main/java/com/aerospike/mapper/tools/AeroMapper.java index 6479af3..e47911a 100644 --- a/src/main/java/com/aerospike/mapper/tools/AeroMapper.java +++ b/src/main/java/com/aerospike/mapper/tools/AeroMapper.java @@ -78,7 +78,9 @@ private void save(WritePolicy writePolicy, @NotNull T object, RecordExistsAc ClassCacheEntry entry = MapperUtils.getEntryAndValidateNamespace(clazz, this); if (writePolicy == null) { writePolicy = new WritePolicy(entry.getWritePolicy()); - if (recordExistsAction != null) { + if (recordExistsAction != null && (writePolicy.recordExistsAction == null || writePolicy.recordExistsAction == RecordExistsAction.UPDATE)) { + // Override the default with the passed policy. Only do this if the policy is already at the default. + // Otherwise, "save" with an INSERT_ONLY policy would fail for example. writePolicy.recordExistsAction = recordExistsAction; } diff --git a/src/main/java/com/aerospike/mapper/tools/ReactiveAeroMapper.java b/src/main/java/com/aerospike/mapper/tools/ReactiveAeroMapper.java index 5c54cae..073aaf1 100644 --- a/src/main/java/com/aerospike/mapper/tools/ReactiveAeroMapper.java +++ b/src/main/java/com/aerospike/mapper/tools/ReactiveAeroMapper.java @@ -74,7 +74,9 @@ private Mono save(WritePolicy writePolicy, @NotNull T object, RecordExist ClassCacheEntry entry = MapperUtils.getEntryAndValidateNamespace(clazz, this); if (writePolicy == null) { writePolicy = new WritePolicy(entry.getWritePolicy()); - if (recordExistsAction != null) { + if (recordExistsAction != null && (writePolicy.recordExistsAction == null || writePolicy.recordExistsAction == RecordExistsAction.UPDATE)) { + // Override the default with the passed policy. Only do this if the policy is already at the default. + // Otherwise, "save" with an INSERT_ONLY policy would fail for example. writePolicy.recordExistsAction = recordExistsAction; } diff --git a/src/test/java/com/aerospike/mapper/AeroMapperBaseTest.java b/src/test/java/com/aerospike/mapper/AeroMapperBaseTest.java index f6b8935..0c96f5f 100644 --- a/src/test/java/com/aerospike/mapper/AeroMapperBaseTest.java +++ b/src/test/java/com/aerospike/mapper/AeroMapperBaseTest.java @@ -25,7 +25,7 @@ public static void setupClass() { ClientPolicy policy = new ClientPolicy(); // Set event loops to use in asynchronous commands. policy.eventLoops = new NioEventLoops(1); - client = new AerospikeClient(policy, "localhost", 3000); + client = new AerospikeClient(policy, "localhost", 3100); } @AfterAll diff --git a/src/test/java/com/aerospike/mapper/InsertOnlyModeTest.java b/src/test/java/com/aerospike/mapper/InsertOnlyModeTest.java new file mode 100644 index 0000000..a5f5718 --- /dev/null +++ b/src/test/java/com/aerospike/mapper/InsertOnlyModeTest.java @@ -0,0 +1,71 @@ +package com.aerospike.mapper; + +import static org.junit.jupiter.api.Assertions.fail; + +import org.junit.jupiter.api.Test; + +import com.aerospike.client.AerospikeException; +import com.aerospike.client.policy.ClientPolicy; +import com.aerospike.client.policy.RecordExistsAction; +import com.aerospike.client.policy.WritePolicy; +import com.aerospike.mapper.annotations.AerospikeKey; +import com.aerospike.mapper.annotations.AerospikeRecord; +import com.aerospike.mapper.tools.AeroMapper; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +public class InsertOnlyModeTest extends AeroMapperBaseTest { + @AerospikeRecord(namespace = "test", set = "custs") + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class Customer { + @AerospikeKey + private String name; + private int age; + } + + @Test + public void tetDefaultPolicies() { + WritePolicy writePolicy = new WritePolicy(); + writePolicy.sendKey = true; + writePolicy.recordExistsAction=RecordExistsAction.CREATE_ONLY; + + ClientPolicy policy = new ClientPolicy(); + policy.writePolicyDefault = writePolicy; + + AeroMapper mapper = new AeroMapper.Builder(client).withWritePolicy(writePolicy).forAll().build(); + + Customer customer = new Customer("Tim", 312); + mapper.delete(customer); + // First one should succeed. + mapper.save(customer); + try { + mapper.save(customer); + fail("Expected an exception to be thrown"); + } + catch (AerospikeException e) { + } + } + @Test + public void testExplicitPolicies() { + WritePolicy writePolicy = new WritePolicy(); + writePolicy.sendKey = true; + writePolicy.recordExistsAction=RecordExistsAction.CREATE_ONLY; + + AeroMapper mapper = new AeroMapper.Builder(client).withWritePolicy(writePolicy).forAll().build(); + + Customer customer = new Customer("Tim", 312); + mapper.delete(customer); + // First one should succeed. + mapper.save(customer); + try { + mapper.save(customer); + fail("Expected an exception to be thrown"); + } + catch (AerospikeException e) { + } + } +} diff --git a/src/test/java/com/aerospike/mapper/reactive/ReactiveInsertOnlyModeTest.java b/src/test/java/com/aerospike/mapper/reactive/ReactiveInsertOnlyModeTest.java new file mode 100644 index 0000000..9653f21 --- /dev/null +++ b/src/test/java/com/aerospike/mapper/reactive/ReactiveInsertOnlyModeTest.java @@ -0,0 +1,65 @@ +package com.aerospike.mapper.reactive; + +import static org.junit.jupiter.api.Assertions.fail; + +import org.junit.jupiter.api.Test; + +import com.aerospike.client.AerospikeException; +import com.aerospike.client.policy.ClientPolicy; +import com.aerospike.client.policy.RecordExistsAction; +import com.aerospike.client.policy.WritePolicy; +import com.aerospike.mapper.annotations.AerospikeKey; +import com.aerospike.mapper.annotations.AerospikeRecord; +import com.aerospike.mapper.tools.ReactiveAeroMapper; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +public class ReactiveInsertOnlyModeTest extends ReactiveAeroMapperBaseTest { + @AerospikeRecord(namespace = "test", set = "custs") + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class Customer { + @AerospikeKey + private String name; + private int age; + } + + @Test + public void tetDefaultPolicies() { + WritePolicy writePolicy = new WritePolicy(); + writePolicy.sendKey = true; + writePolicy.recordExistsAction=RecordExistsAction.CREATE_ONLY; + + ClientPolicy policy = new ClientPolicy(); + policy.writePolicyDefault = writePolicy; + + ReactiveAeroMapper mapper = new ReactiveAeroMapper.Builder(reactorClient).withWritePolicy(writePolicy).forAll().build(); + + Customer customer = new Customer("Tim", 312); + mapper.delete(customer); + // First one should succeed. + mapper.save(customer).doOnError(c -> fail("Expected an succcess")); + mapper.save(customer).doOnSuccess(c -> { + fail("Expected an exception to be thrown"); + }); + } + @Test + public void testExplicitPolicies() { + WritePolicy writePolicy = new WritePolicy(); + writePolicy.sendKey = true; + writePolicy.recordExistsAction=RecordExistsAction.CREATE_ONLY; + + ReactiveAeroMapper mapper = new ReactiveAeroMapper.Builder(reactorClient).withWritePolicy(writePolicy).forAll().build(); + + Customer customer = new Customer("Tim", 312); + mapper.delete(customer); + // First one should succeed. + mapper.save(customer).doOnError(c -> fail("Expected an succcess")); + mapper.save(customer).doOnSuccess(c -> { + fail("Expected an exception to be thrown"); + }); + } +}