Skip to content

Commit

Permalink
disable scans by default (aerospike#91)
Browse files Browse the repository at this point in the history
  • Loading branch information
Aloren committed Jul 16, 2020
1 parent baf9ae5 commit 10f002a
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ public AerospikeTemplate aerospikeTemplate(AerospikeClient aerospikeClient,
@Bean(name = "aerospikeQueryEngine")
public QueryEngine queryEngine(AerospikeClient aerospikeClient,
StatementBuilder statementBuilder) {
return new QueryEngine(aerospikeClient, statementBuilder, aerospikeClient.getQueryPolicyDefault());
QueryEngine queryEngine = new QueryEngine(aerospikeClient, statementBuilder, aerospikeClient.getQueryPolicyDefault());
queryEngine.setScansEnabled(aerospikeDataSettings().isScansEnabled());
return queryEngine;
}

@Bean(name = "aerospikeStatementBuilder")
Expand Down Expand Up @@ -142,6 +144,16 @@ protected FieldNamingStrategy fieldNamingStrategy() {

protected abstract String nameSpace();

protected AerospikeDataSettings aerospikeDataSettings() {
AerospikeDataSettings.AerospikeDataSettingsBuilder builder = AerospikeDataSettings.builder();
configureDataSettings(builder);
return builder.build();
}

protected void configureDataSettings(AerospikeDataSettings.AerospikeDataSettingsBuilder builder) {
builder.scansEnabled(false);
}

protected ClientPolicy getClientPolicy() {
ClientPolicy clientPolicy = new ClientPolicy();
clientPolicy.failIfNotConnected = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ public ReactiveAerospikeTemplate reactiveAerospikeTemplate(MappingAerospikeConve
@Bean(name = "reactiveAerospikeQueryEngine")
public ReactorQueryEngine reactorQueryEngine(AerospikeReactorClient aerospikeReactorClient,
StatementBuilder statementBuilder) {
return new ReactorQueryEngine(aerospikeReactorClient, statementBuilder, aerospikeReactorClient.getQueryPolicyDefault());
ReactorQueryEngine queryEngine = new ReactorQueryEngine(aerospikeReactorClient, statementBuilder, aerospikeReactorClient.getQueryPolicyDefault());
queryEngine.setScansEnabled(aerospikeDataSettings().isScansEnabled());
return queryEngine;
}

@Bean(name = "reactiveAerospikeIndexRefresher")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.springframework.data.aerospike.config;

import lombok.Builder;
import lombok.Value;

@Builder
@Value
public class AerospikeDataSettings {

@Builder.Default
boolean scansEnabled = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@
*/
public class QueryEngine {

public static final String SCANS_DISABLED_MESSAGE =
"Query without a filter will initiate a scan. Since scans are potentially dangerous operations, they are disabled by default in spring-data-aerospike. " +
"If you still need to use them, enable them via `scansEnabled` property in `org.springframework.data.aerospike.config.AerospikeDataSettings`.";

/**
* Scans can potentially slow down Aerospike server, so we are disabling them by default.
* If you still need to use scans, set this property to true.
*/
private boolean scansEnabled = false;

private final AerospikeClient client;
private final StatementBuilder statementBuilder;
private final QueryPolicy queryPolicy;
Expand Down Expand Up @@ -96,8 +106,14 @@ public KeyRecordIterator select(String namespace, String set, Filter filter, Qua
* query with filters
*/
Statement statement = statementBuilder.build(namespace, set, filter, qualifiers);
if(!scansEnabled && statement.getFilter() == null) {
throw new IllegalStateException(SCANS_DISABLED_MESSAGE);
}
RecordSet rs = client.query(queryPolicy, statement);
return new KeyRecordIterator(namespace, rs);
}

public void setScansEnabled(boolean scansEnabled) {
this.scansEnabled = scansEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@
*/
public class ReactorQueryEngine {

private final IAerospikeReactorClient client;
/**
* Scans can potentially slow down Aerospike server, so we are disabling them by default.
* If you still need to use scans, set this property to true.
*/
private boolean scansEnabled = false;

private final IAerospikeReactorClient client;
private final StatementBuilder statementBuilder;

private final QueryPolicy queryPolicy;

public ReactorQueryEngine(IAerospikeReactorClient client, StatementBuilder statementBuilder,
Expand Down Expand Up @@ -72,7 +76,14 @@ public Flux<KeyRecord> select(String namespace, String set, Filter filter, Quali
* query with filters
*/
Statement statement = statementBuilder.build(namespace, set, filter, qualifiers);
if(!scansEnabled && statement.getFilter() == null) {
return Flux.error(new IllegalStateException(QueryEngine.SCANS_DISABLED_MESSAGE));
}
return client.query(queryPolicy, statement);
}

public void setScansEnabled(boolean scansEnabled) {
this.scansEnabled = scansEnabled;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,11 @@ private void updateStatement(Statement stmt, Qualifier[] qualifiers) {
}
}

try {
PredExp[] predexps;
predexps = buildPredExp(qualifiers).toArray(new PredExp[0]);
if (predexps.length > 0) {
stmt.setPredExp(predexps);
return;
} else {
throw new QualifierException("Failed to build Query");
}
} catch (PredExpException e) {
throw new QualifierException(e.getMessage());
PredExp[] predexps = buildPredExp(qualifiers).toArray(new PredExp[0]);
if (predexps.length > 0) {
stmt.setPredExp(predexps);
} else {
throw new QualifierException("Failed to build Query");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,9 @@ BlockingAerospikeTestOperations blockingAerospikeTestOperations(AerospikeTemplat
public QueryEngineTestDataPopulator queryEngineTestDataPopulator(AerospikeClient client) {
return new QueryEngineTestDataPopulator(nameSpace(), client);
}

@Override
protected void configureDataSettings(AerospikeDataSettings.AerospikeDataSettingsBuilder builder) {
builder.scansEnabled(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.springframework.data.aerospike.CollectionUtils.countingInt;
import static org.springframework.data.aerospike.query.Qualifier.FilterOperation.LT;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.AGES;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.AGE_COUNTS;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.BLUE;
Expand Down Expand Up @@ -62,6 +64,19 @@ public void dropIndexes() {
tryDropIndex(namespace, SET_NAME, "color_index");
}

@Test
void throwsExceptionWhenScansDisabled() {
queryEngine.setScansEnabled(false);
try {
Qualifier qualifier = new Qualifier("age", LT, Value.get(26));
assertThatThrownBy(() -> queryEngine.select(namespace, SET_NAME, null, qualifier))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("disabled by default");
} finally {
queryEngine.setScansEnabled(true);
}
}

@Test
public void selectOneWitKey() {
KeyQualifier kq = new KeyQualifier(Value.get("selector-test:3"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
/*
* These tests generate qualifiers on indexed bins.
*/
public class IndexedQualifierTests extends BaseReactiveQueryEngineTests {
public class ReactiveIndexedQualifierTests extends BaseReactiveQueryEngineTests {

@AfterEach
public void assertNoScans() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
/*
* Tests to ensure that Qualifiers are built successfully for non indexed bins.
*/
public class QualifierTests extends BaseReactiveQueryEngineTests {
public class ReactiveQualifierTests extends BaseReactiveQueryEngineTests {

/*
* These bins should not be indexed.
Expand All @@ -77,6 +77,21 @@ public void dropIndexes() {
super.tryDropIndex(namespace, SET_NAME, "color_index");
}

@Test
void throwsExceptionWhenScansDisabled() {
queryEngine.setScansEnabled(false);
try {
Qualifier qualifier = new Qualifier("age", LT, Value.get(26));
StepVerifier.create(queryEngine.select(namespace, SET_NAME, null, qualifier))
.expectErrorSatisfies(e -> assertThat(e)
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("disabled by default"))
.verify();
} finally {
queryEngine.setScansEnabled(true);
}
}

@Test
public void testLTQualifier() {
// Ages range from 25 -> 29. We expected to only get back values with age < 26
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,26 @@

import com.aerospike.client.Value;
import com.aerospike.client.query.KeyRecord;
import org.junit.Assert;
import org.junit.jupiter.api.Test;
import org.springframework.data.aerospike.query.KeyQualifier;
import org.springframework.data.aerospike.query.Qualifier;
import reactor.core.publisher.Flux;
import reactor.test.StepVerifier;

import java.util.Arrays;

import static junit.framework.TestCase.assertTrue;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.data.aerospike.query.Qualifier.FilterOperation.ENDS_WITH;
import static org.springframework.data.aerospike.query.Qualifier.FilterOperation.EQ;
import static org.springframework.data.aerospike.query.Qualifier.FilterOperation.GEO_WITHIN;
import static org.springframework.data.aerospike.query.Qualifier.FilterOperation.START_WITH;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.COLOURS;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.BLUE;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.COLOUR_COUNTS;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.GEO_BIN_NAME;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.GEO_SET;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.ORANGE;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.RECORD_COUNT;
import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.SET_NAME;

public class SelectorTests extends BaseReactiveQueryEngineTests {
public class ReactiveSelectorTests extends BaseReactiveQueryEngineTests {

@Test
public void selectOneWithKey() {
Expand Down Expand Up @@ -57,19 +54,13 @@ public void selectAll() {

@Test
public void selectEndssWith() {
// Number of records containing a color ending with "e"
long expectedEndsWithECount = Arrays.stream(COLOURS)
.filter(c -> c.endsWith("e"))
.mapToLong(c -> COLOUR_COUNTS.get(c))
.sum();

Qualifier qual1 = new Qualifier("color", ENDS_WITH, Value.get("e"));
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, qual1);
StepVerifier.create(flux.collectList())
.expectNextMatches(results -> {
results.forEach(keyRecord ->
Assert.assertTrue(keyRecord.record.getString("color").endsWith("e")));
assertThat((long) results.size()).isEqualTo(expectedEndsWithECount);
assertThat(results)
.allSatisfy(rec -> assertThat(rec.record.getString("color")).endsWith("e"))
.hasSize(COLOUR_COUNTS.get(ORANGE) + COLOUR_COUNTS.get(BLUE));
return true;
})
.verifyComplete();
Expand All @@ -81,9 +72,9 @@ public void selectStartsWith() {
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, startsWithQual);
StepVerifier.create(flux.collectList())
.expectNextMatches(results -> {
results.forEach(keyRecord ->
Assert.assertTrue(keyRecord.record.getString("color").startsWith("bl")));
assertThat(results.size()).isEqualTo(COLOUR_COUNTS.get("blue").intValue());
assertThat(results)
.allSatisfy(rec -> assertThat(rec.record.getString("color")).startsWith("bl"))
.hasSize(COLOUR_COUNTS.get(BLUE));
return true;
})
.verifyComplete();
Expand All @@ -107,6 +98,7 @@ public void equalIgnoreCaseReturnsNoItemsIfNoneMatched() {
Qualifier qual1 = new Qualifier("color", EQ, ignoreCase, Value.get("BLUE"));
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, qual1);
StepVerifier.create(flux)
.expectNextCount(0)
.verifyComplete();
}

Expand All @@ -116,22 +108,22 @@ public void startWithIgnoreCaseReturnsNoItemsIfNoneMatched() {
Qualifier qual1 = new Qualifier("name", START_WITH, ignoreCase, Value.get("NA"));
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, qual1);
StepVerifier.create(flux)
.expectNextCount(0)
.verifyComplete();
}

@Test
public void stringEqualIgnoreCaseWorksOnUnindexedBin() {
boolean ignoreCase = true;
String expectedColor = "blue";
long blueRecordCount = 0;

Qualifier caseInsensitiveQual = new Qualifier("color", EQ, ignoreCase, Value.get("BlUe"));
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, caseInsensitiveQual);
StepVerifier.create(flux.collectList())
.expectNextMatches(results -> {
results.forEach(keyRecord ->
assertThat(keyRecord.record.getString("color")).isEqualTo(expectedColor));
assertThat(results.size()).isEqualTo(COLOUR_COUNTS.get("blue").intValue());
assertThat(results)
.allSatisfy(rec -> assertThat(rec.record.getString("color")).isEqualTo(expectedColor))
.hasSize(COLOUR_COUNTS.get(BLUE));
return true;
})
.verifyComplete();
Expand All @@ -142,9 +134,10 @@ public void stringEqualIgnoreCaseWorksRequiresFullMatch() {
boolean ignoreCase = true;
Qualifier caseInsensitiveQual = new Qualifier("color", EQ, ignoreCase, Value.get("lue"));
Flux<KeyRecord> flux = queryEngine.select(namespace, SET_NAME, null, caseInsensitiveQual);

StepVerifier.create(flux)
.expectNextCount(0)
.verifyComplete();

}

@Test
Expand All @@ -159,8 +152,9 @@ public void selectWithGeoWithin() {
Flux<KeyRecord> flux = queryEngine.select(namespace, GEO_SET, null, qual1);
StepVerifier.create(flux.collectList())
.expectNextMatches(results -> {
results.forEach(keyRecord ->
assertTrue(keyRecord.record.generation >= 1));
assertThat(results)
.allSatisfy(rec -> assertThat(rec.record.generation).isGreaterThanOrEqualTo(1))
.isNotEmpty();
return true;
})
.verifyComplete();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import static org.springframework.data.aerospike.query.QueryEngineTestDataPopulator.USERS_SET;

public class UsersTests extends BaseReactiveQueryEngineTests {
public class ReactiveUsersTests extends BaseReactiveQueryEngineTests {

@Test
public void usersInNorthRegion() {
Expand Down

0 comments on commit 10f002a

Please sign in to comment.