From 45337003701fbc2fde6c615b98ecfb1d68fe2efe Mon Sep 17 00:00:00 2001 From: Tim F <120514393+tim-aero@users.noreply.github.com> Date: Sun, 19 May 2024 20:37:41 -0600 Subject: [PATCH 1/2] Fixed issue with CREATE_ONLY RecordExistsAction If a policy specified either by defaultPolicy or using the policy hierarchy has a RecordExistsAction of CREATE_ONLY, the save method was overwriting this with REPLACE. Changed this to only replace the policy with REPLACE if the passed RecordExistsAction is UPDATE. --- .../aerospike/mapper/tools/AeroMapper.java | 4 +- .../mapper/tools/ReactiveAeroMapper.java | 4 +- .../aerospike/mapper/InsertOnlyModeTest.java | 71 +++++++++++++++++++ .../reactive/ReactiveInsertOnlyModeTest.java | 65 +++++++++++++++++ 4 files changed, 142 insertions(+), 2 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 18b5bb3..98e8d2e 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/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"); + }); + } +} From 2fed6dbef019e6dee4788c9d4f0dd79e9c592b36 Mon Sep 17 00:00:00 2001 From: Tim F <120514393+tim-aero@users.noreply.github.com> Date: Fri, 14 Jun 2024 10:38:05 -0600 Subject: [PATCH 2/2] NPE using refs with the YAML file Whilst references work correctly with annotations, they would throw a NullPointerException when being used with an external configuration such as a YAML file. The issue was caused by the `BatchLoad` property which was initialized by default to true on the annotation but was left as null if not specified in the file --- .../aerospike/mapper/tools/configuration/ClassConfig.java | 4 ++-- .../mapper/tools/configuration/ReferenceConfig.java | 3 ++- .../java/com/aerospike/mapper/tools/utils/TypeUtils.java | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/aerospike/mapper/tools/configuration/ClassConfig.java b/src/main/java/com/aerospike/mapper/tools/configuration/ClassConfig.java index d5da40e..9b23975 100644 --- a/src/main/java/com/aerospike/mapper/tools/configuration/ClassConfig.java +++ b/src/main/java/com/aerospike/mapper/tools/configuration/ClassConfig.java @@ -279,12 +279,12 @@ public Builder mappingToBin(String name) { } public Builder beingReferencedBy(AerospikeReference.ReferenceType type) { - this.binConfig.setReference(new ReferenceConfig(type, false)); + this.binConfig.setReference(new ReferenceConfig(type, false, true)); return this.end(); } public Builder beingLazilyReferencedBy(AerospikeReference.ReferenceType type) { - this.binConfig.setReference(new ReferenceConfig(type, true)); + this.binConfig.setReference(new ReferenceConfig(type, true, true)); return this.end(); } diff --git a/src/main/java/com/aerospike/mapper/tools/configuration/ReferenceConfig.java b/src/main/java/com/aerospike/mapper/tools/configuration/ReferenceConfig.java index ddf6c03..39cc510 100644 --- a/src/main/java/com/aerospike/mapper/tools/configuration/ReferenceConfig.java +++ b/src/main/java/com/aerospike/mapper/tools/configuration/ReferenceConfig.java @@ -8,9 +8,10 @@ public class ReferenceConfig { private Boolean batchLoad; public ReferenceConfig() {} - public ReferenceConfig(ReferenceType type, boolean lazy) { + public ReferenceConfig(ReferenceType type, boolean lazy, boolean batchLoad) { this.type = type; this.lazy = lazy; + this.batchLoad = batchLoad; } public ReferenceType getType() { diff --git a/src/main/java/com/aerospike/mapper/tools/utils/TypeUtils.java b/src/main/java/com/aerospike/mapper/tools/utils/TypeUtils.java index 4592c32..02d740e 100644 --- a/src/main/java/com/aerospike/mapper/tools/utils/TypeUtils.java +++ b/src/main/java/com/aerospike/mapper/tools/utils/TypeUtils.java @@ -229,7 +229,11 @@ private static TypeMapper getMapper(Class clazz, AnnotatedType type, IBaseAer } else { // Reference ReferenceConfig ref = binConfig.getReference(); - typeMapper = new ObjectReferenceMapper(ClassCache.getInstance().loadClass(clazz, mapper), ref.getLazy(), ref.getBatchLoad(), ref.getType(), mapper); + typeMapper = new ObjectReferenceMapper( + ClassCache.getInstance().loadClass(clazz, mapper), + ref.getLazy() == null? false : ref.getLazy(), + ref.getBatchLoad() == null ? true : ref.getBatchLoad(), + ref.getType(), mapper); addToMap = false; } } else {