Skip to content

Commit

Permalink
FMWK-315 Add query methods validation, refactor creating combined rep…
Browse files Browse the repository at this point in the history
…ository queries (#701)

- Add validation for standard repository query methods
- API change: add a special QueryParam for unambiguous transferring of arguments when using combined queries
- API change: combined queries now require explicitly given arguments (e.g., "findBy...Or..." requires arguments for both parts of the query)
  • Loading branch information
agrgr authored Feb 20, 2024
1 parent b7ba6da commit bf374b5
Show file tree
Hide file tree
Showing 17 changed files with 1,582 additions and 210 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
*/
public interface AerospikeConverter extends AerospikeReader<Object>, AerospikeWriter<Object> {

/**
* Key that identifies POJO's class.
*/
public static final String CLASS_KEY = "@_class";

/**
* Access Aerospike-specific conversion service.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@

import java.util.Map;

import static org.springframework.data.aerospike.convert.AerospikeConverter.CLASS_KEY;

public class AerospikeTypeAliasAccessor implements TypeAliasAccessor<Map<String, Object>> {

private static final String TYPE_KEY = "@_class";
private final String typeKey;

public AerospikeTypeAliasAccessor(String typeKey) {
this.typeKey = typeKey;
}

public AerospikeTypeAliasAccessor() {
this.typeKey = TYPE_KEY;
this.typeKey = CLASS_KEY;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.springframework.data.aerospike.query;

/**
* This class stores arguments passed to each part of a combined repository query, e.g.,
* <pre>repository.findByNameAndEmail(QueryParam.of("John"), QueryParam.of("[email protected]"))</pre>
*/
public record QueryParam(Object[] arguments) {

/**
* This method is required to pass arguments to each part of a combined query
*
* @param arguments necessary objects
* @return instance of {@link QueryParam}
*/
public static QueryParam of(Object... arguments) {
return new QueryParam(arguments);
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ public interface CriteriaDefinition {
*/
String getKey();

enum AerospikeMapCriteria {
enum AerospikeQueryCriteria {
KEY, VALUE, VALUE_CONTAINING
}

enum AerospikeNullQueryCriteria {
NULL
}

enum AerospikeMetadata {
SINCE_UPDATE_TIME, LAST_UPDATE_TIME, VOID_TIME, TTL, RECORD_SIZE_ON_DISK, RECORD_SIZE_IN_MEMORY
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,26 @@
import org.springframework.dao.InvalidDataAccessResourceUsageException;
import org.springframework.util.StringUtils;

import java.io.File;
import java.net.InetAddress;
import java.net.URI;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.time.ZoneId;
import java.time.temporal.Temporal;
import java.util.Arrays;
import java.util.Currency;
import java.util.Date;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.TimeZone;
import java.util.UUID;
import java.util.concurrent.ThreadLocalRandom;
import java.util.regex.Pattern;

import static org.springframework.util.ClassUtils.isPrimitiveOrWrapper;

/**
* Utility class containing useful methods for interacting with Aerospike across the entire implementation
Expand Down Expand Up @@ -104,4 +120,35 @@ public static Optional<Integer> getIntegerProperty(String property) {
public static boolean allArrayElementsAreNull(Object[] array) {
return Arrays.stream(array).allMatch(Objects::isNull);
}

/**
* Checking that at least one of the arguments is of the following type: a primitive or primitive wrapper, an Enum,
* a String or other CharSequence, a Number, a Date, a Temporal, a UUID, a URI, a URL, a Locale, or a Class.
*/
public static boolean isSimpleValueType(Class<?> type) {
return (Void.class != type && void.class != type &&
(isPrimitiveOrWrapper(type) ||
Enum.class.isAssignableFrom(type) ||
CharSequence.class.isAssignableFrom(type) ||
Number.class.isAssignableFrom(type) ||
Date.class.isAssignableFrom(type) ||
Temporal.class.isAssignableFrom(type) ||
ZoneId.class.isAssignableFrom(type) ||
TimeZone.class.isAssignableFrom(type) ||
File.class.isAssignableFrom(type) ||
Path.class.isAssignableFrom(type) ||
Charset.class.isAssignableFrom(type) ||
Currency.class.isAssignableFrom(type) ||
InetAddress.class.isAssignableFrom(type) ||
URI.class == type ||
URL.class == type ||
UUID.class == type ||
Locale.class == type ||
Pattern.class == type ||
Class.class == type));
}

public static boolean isBoolean(Class<?> type) {
return Boolean.TYPE.equals(type) || Boolean.class.equals(type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ public void findByListValueInRange() {
@Test
public void findByMapKeysContaining() {
Query query = QueryUtils.createQueryForMethodWithArgs("findByStringMapContaining", "key1",
CriteriaDefinition.AerospikeMapCriteria.KEY);
CriteriaDefinition.AerospikeQueryCriteria.KEY);
Stream<Person> result = template.find(query, Person.class);

assertThat(result)
Expand All @@ -376,7 +376,7 @@ public void findByMapKeysContaining() {
@Test
public void findByMapValuesContaining() {
Query query = QueryUtils.createQueryForMethodWithArgs("findByStringMapContaining", "val2",
CriteriaDefinition.AerospikeMapCriteria.VALUE);
CriteriaDefinition.AerospikeQueryCriteria.VALUE);
Stream<Person> result = template.find(query, Person.class);

assertThat(result)
Expand Down Expand Up @@ -436,7 +436,7 @@ public void findByMapKeyValueStartsWith() {
@Test
public void findByMapKeyValueContains() {
Query query = QueryUtils.createQueryForMethodWithArgs("findByStringMapContaining", "key4", "al",
CriteriaDefinition.AerospikeMapCriteria.VALUE_CONTAINING);
CriteriaDefinition.AerospikeQueryCriteria.VALUE_CONTAINING);
Stream<Person> result = template.find(query, Person.class);

assertThat(result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class ReactiveAerospikeTemplateFindByQueryTests extends BaseReactiveInteg
@BeforeAll
public void beforeAllSetUp() {
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class);
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class, OVERRIDE_SET_NAME);
additionalAerospikeTestOperations.createIndex(Person.class, "person_age_index",
"age", IndexType.NUMERIC);
additionalAerospikeTestOperations.createIndex(Person.class, "person_last_name_index",
Expand All @@ -46,6 +47,7 @@ public void beforeAllSetUp() {
@BeforeEach
public void setUp() {
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class);
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class, OVERRIDE_SET_NAME);
super.setUp();
}

Expand All @@ -54,6 +56,8 @@ public void afterAll() {
additionalAerospikeTestOperations.dropIndex(Person.class, "person_age_index");
additionalAerospikeTestOperations.dropIndex(Person.class, "person_last_name_index");
additionalAerospikeTestOperations.dropIndex(Person.class, "person_first_name_index");
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class);
additionalAerospikeTestOperations.deleteAllAndVerify(Person.class, OVERRIDE_SET_NAME);
}

@Test
Expand Down Expand Up @@ -438,7 +442,7 @@ public void findByMapKeysContaining() {
reactiveTemplate.insertAll(persons).blockLast();

Query query = QueryUtils.createQueryForMethodWithArgs("findByStringMapContaining", "key1",
CriteriaDefinition.AerospikeMapCriteria.KEY);
CriteriaDefinition.AerospikeQueryCriteria.KEY);

List<Person> result = reactiveTemplate.find(query, Person.class)
.subscribeOn(Schedulers.parallel())
Expand All @@ -459,7 +463,7 @@ public void findByMapValuesContaining() {
reactiveTemplate.insertAll(persons).blockLast();

Query query = QueryUtils.createQueryForMethodWithArgs("findByStringMapContaining", "val1",
CriteriaDefinition.AerospikeMapCriteria.VALUE);
CriteriaDefinition.AerospikeQueryCriteria.VALUE);

List<Person> result = reactiveTemplate.find(query, Person.class)
.subscribeOn(Schedulers.parallel())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.aerospike.AsCollections;
import org.springframework.data.aerospike.BaseReactiveIntegrationTests;
import org.springframework.data.aerospike.ReactiveBlockingAerospikeTestOperations;
import org.springframework.data.aerospike.query.FilterOperation;
import org.springframework.data.aerospike.query.Qualifier;
import org.springframework.data.aerospike.query.QueryParam;
import org.springframework.data.aerospike.repository.query.CriteriaDefinition;
import org.springframework.data.aerospike.repository.query.Query;
import org.springframework.data.aerospike.sample.Address;
Expand All @@ -30,9 +32,8 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.springframework.data.aerospike.AsCollections.of;
import static org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeMapCriteria.VALUE;
import static org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeMetadata.SINCE_UPDATE_TIME;
import static org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeQueryCriteria.VALUE;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public class ReactiveIndexedPersonRepositoryQueryTests extends BaseReactiveIntegrationTests {
Expand All @@ -41,19 +42,19 @@ public class ReactiveIndexedPersonRepositoryQueryTests extends BaseReactiveInteg
.age(42).strings(Arrays.asList("str1", "str2"))
.address(new Address("Foo Street 1", 1, "C0123", "Bar")).build();
static final IndexedPerson luc = IndexedPerson.builder().id(nextId()).firstName("Luc").lastName("Besson").age(39)
.stringMap(of("key1", "val1", "key2", "val2")).address(new Address(null, null, null, null))
.stringMap(AsCollections.of("key1", "val1", "key2", "val2")).address(new Address(null, null, null, null))
.build();
static final IndexedPerson lilly = IndexedPerson.builder().id(nextId()).firstName("Lilly").lastName("Bertineau")
.age(28).intMap(of("key1", 1, "key2", 2))
.age(28).intMap(AsCollections.of("key1", 1, "key2", 2))
.address(new Address("Foo Street 2", 2, "C0124", "C0123")).build();
static final IndexedPerson daniel = IndexedPerson.builder().id(nextId()).firstName("Daniel").lastName("Morales")
.age(29).ints(Arrays.asList(500, 550, 990)).build();
static final IndexedPerson petra = IndexedPerson.builder().id(nextId()).firstName("Petra")
.lastName("Coutant-Kerbalec")
.age(34).stringMap(of("key1", "val1", "key2", "val2", "key3", "val3")).build();
.age(34).stringMap(AsCollections.of("key1", "val1", "key2", "val2", "key3", "val3")).build();
static final IndexedPerson emilien = IndexedPerson.builder().id(nextId()).firstName("Emilien")
.lastName("Coutant-Kerbalec").age(30)
.intMap(of("key1", 0, "key2", 1)).ints(Arrays.asList(450, 550, 990)).build();
.intMap(AsCollections.of("key1", 0, "key2", 1)).ints(Arrays.asList(450, 550, 990)).build();
public static final List<IndexedPerson> allIndexedPersons = Arrays.asList(alain, luc, lilly, daniel, petra,
emilien);
@Autowired
Expand Down Expand Up @@ -230,7 +231,9 @@ void findDistinctByFriendLastNameStartingWith() {

@Test
public void findPersonsByFirstnameAndByAge() {
List<IndexedPerson> results = reactiveRepository.findByFirstNameAndAge("Lilly", 28)
QueryParam firstName = QueryParam.of("Lilly");
QueryParam age = QueryParam.of(28);
List<IndexedPerson> results = reactiveRepository.findByFirstNameAndAge(firstName, age)
.subscribeOn(Schedulers.parallel()).collectList().block();

assertThat(results).containsOnly(lilly);
Expand All @@ -247,7 +250,7 @@ public void findPersonInAgeRangeCorrectly() {
@Test
public void findByMapKeysContaining() {
List<IndexedPerson> results = reactiveRepository.findByStringMapContaining("key1",
CriteriaDefinition.AerospikeMapCriteria.KEY).subscribeOn(Schedulers.parallel()).collectList().block();
CriteriaDefinition.AerospikeQueryCriteria.KEY).subscribeOn(Schedulers.parallel()).collectList().block();

assertThat(results).contains(luc, petra);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.junit.jupiter.api.TestInstance;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.aerospike.BaseBlockingIntegrationTests;
import org.springframework.data.aerospike.query.QueryParam;
import org.springframework.data.aerospike.query.model.Index;
import org.springframework.data.aerospike.repository.query.CriteriaDefinition;
import org.springframework.data.aerospike.sample.Address;
Expand All @@ -34,7 +35,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.springframework.data.aerospike.AsCollections.of;
import static org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeMapCriteria.VALUE;
import static org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeQueryCriteria.VALUE;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public class IndexedPersonRepositoryQueryTests extends BaseBlockingIntegrationTests {
Expand Down Expand Up @@ -233,7 +234,9 @@ public void findsPersonsByFirstName() {
@Test
public void findsPersonsByActiveAndFirstName() {
assertThat(tricia.isActive()).isFalse();
List<IndexedPerson> result = repository.findByIsActiveAndFirstName(false, "Tricia");
QueryParam isActive = QueryParam.of(false);
QueryParam firstNames = QueryParam.of("Tricia");
List<IndexedPerson> result = repository.findByIsActiveAndFirstName(isActive, firstNames);

assertThat(result)
.hasSize(1)
Expand Down Expand Up @@ -310,10 +313,14 @@ public void findByAgeGreaterThan_forEmptyResult() {

@Test
public void findsPersonsByFirstNameAndByAge() {
List<IndexedPerson> result = repository.findByFirstNameAndAge("Billy", 25);
QueryParam firstNames = QueryParam.of("Billy");
QueryParam ages = QueryParam.of(25);
List<IndexedPerson> result = repository.findByFirstNameAndAge(firstNames, ages);
assertThat(result).containsOnly(billy);

result = repository.findByFirstNameAndAge("Peter", 41);
firstNames = QueryParam.of("Peter");
ages = QueryParam.of(41);
result = repository.findByFirstNameAndAge(firstNames, ages);
assertThat(result).containsOnly(peter);
}

Expand All @@ -331,19 +338,27 @@ public void findsPersonInAgeRangeCorrectlyOrderByLastName() {

@Test
public void findsPersonInAgeRangeAndNameCorrectly() {
Iterable<IndexedPerson> it = repository.findByAgeBetweenAndLastName(40, 45, "Matthews");
QueryParam ageBetween = QueryParam.of(40, 45);
QueryParam lastNames = QueryParam.of("Matthews");
Iterable<IndexedPerson> it = repository.findByAgeBetweenAndLastName(ageBetween, lastNames);
assertThat(it).hasSize(0);

Iterable<IndexedPerson> result = repository.findByAgeBetweenAndLastName(20, 26, "Smith");
ageBetween = QueryParam.of(20, 26);
lastNames = QueryParam.of("Smith");
Iterable<IndexedPerson> result = repository.findByAgeBetweenAndLastName(ageBetween, lastNames);
assertThat(result).hasSize(1).contains(billy);
}

@Test
public void findsPersonInAgeRangeOrNameCorrectly() {
Iterable<IndexedPerson> it = repository.findByAgeBetweenOrLastName(40, 45, "James");
QueryParam ageBetween = QueryParam.of(40, 45);
QueryParam lastNames = QueryParam.of("James");
Iterable<IndexedPerson> it = repository.findByAgeBetweenOrLastName(ageBetween, lastNames);
assertThat(it).containsExactlyInAnyOrder(john, peter, tricia);

Iterable<IndexedPerson> result = repository.findByAgeBetweenOrLastName(20, 26, "Macintosh");
ageBetween = QueryParam.of(20, 26);
lastNames = QueryParam.of("Macintosh");
Iterable<IndexedPerson> result = repository.findByAgeBetweenOrLastName(ageBetween, lastNames);
assertThat(result).containsExactlyInAnyOrder(billy, peter);
}

Expand All @@ -352,7 +367,7 @@ void findByMapKeysContaining() {
assertThat(billy.getStringMap()).containsKey("key1");

List<IndexedPerson> persons = repository.findByStringMapContaining("key1",
CriteriaDefinition.AerospikeMapCriteria.KEY);
CriteriaDefinition.AerospikeQueryCriteria.KEY);
assertThat(persons).contains(billy);
}

Expand All @@ -361,7 +376,7 @@ void findByMapKeysNotContaining() {
assertThat(billy.getStringMap()).containsKey("key1");

List<IndexedPerson> persons = repository.findByStringMapNotContaining("key3",
CriteriaDefinition.AerospikeMapCriteria.KEY);
CriteriaDefinition.AerospikeQueryCriteria.KEY);
assertThat(persons).contains(billy);
}

Expand Down
Loading

0 comments on commit bf374b5

Please sign in to comment.