DATAES-972 - BeforeConvertCallback should be called before index query is built.

Originap PR: #555
This commit is contained in:
Peter-Josef Meisch 2020-11-16 12:58:00 +01:00 committed by GitHub
parent 6debccffbd
commit 98043348f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 310 additions and 160 deletions

View File

@ -147,13 +147,14 @@ public abstract class AbstractElasticsearchTemplate implements ElasticsearchOper
Assert.notNull(entity, "entity must not be null");
Assert.notNull(index, "index must not be null");
IndexQuery query = getIndexQuery(entity);
index(query, index);
T entityAfterBeforeConvert = maybeCallbackBeforeConvert(entity, index);
// suppressing because it's either entity itself or something of a correct type returned by an entity callback
@SuppressWarnings("unchecked")
T castResult = (T) query.getObject();
return castResult;
IndexQuery query = getIndexQuery(entityAfterBeforeConvert);
doIndex(query, index);
T entityAfterAfterSave = maybeCallbackAfterSave(entityAfterBeforeConvert, index);
return entityAfterAfterSave;
}
@Override
@ -192,6 +193,20 @@ public abstract class AbstractElasticsearchTemplate implements ElasticsearchOper
return save(Arrays.asList(entities));
}
@Override
public String index(IndexQuery query, IndexCoordinates index) {
maybeCallbackBeforeConvertWithQuery(query, index);
String documentId = doIndex(query, index);
maybeCallbackAfterSaveWithQuery(query, index);
return documentId;
}
public abstract String doIndex(IndexQuery query, IndexCoordinates indexCoordinates);
@Override
@Nullable
public <T> T get(String id, Class<T> clazz) {
@ -261,11 +276,38 @@ public abstract class AbstractElasticsearchTemplate implements ElasticsearchOper
return bulkIndex(queries, bulkOptions, getIndexCoordinatesFor(clazz));
}
@Override
public final List<IndexedObjectInformation> bulkIndex(List<IndexQuery> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
Assert.notNull(queries, "List of IndexQuery must not be null");
Assert.notNull(bulkOptions, "BulkOptions must not be null");
return bulkOperation(queries, bulkOptions, index);
}
@Override
public void bulkUpdate(List<UpdateQuery> queries, Class<?> clazz) {
bulkUpdate(queries, getIndexCoordinatesFor(clazz));
}
public List<IndexedObjectInformation> bulkOperation(List<?> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
Assert.notNull(queries, "List of IndexQuery must not be null");
Assert.notNull(bulkOptions, "BulkOptions must not be null");
maybeCallbackBeforeConvertWithQueries(queries, index);
List<IndexedObjectInformation> indexedObjectInformations = doBulkOperation(queries, bulkOptions, index);
maybeCallbackAfterSaveWithQueries(queries, index);
return indexedObjectInformations;
}
public abstract List<IndexedObjectInformation> doBulkOperation(List<?> queries, BulkOptions bulkOptions,
IndexCoordinates index);
// endregion
// region SearchOperations
@ -620,6 +662,20 @@ public abstract class AbstractElasticsearchTemplate implements ElasticsearchOper
if (queryObject != null) {
queryObject = maybeCallbackBeforeConvert(queryObject, index);
indexQuery.setObject(queryObject);
// the callback might have set som values relevant for the IndexQuery
IndexQuery newQuery = getIndexQuery(queryObject);
if (indexQuery.getRouting() == null && newQuery.getRouting() != null) {
indexQuery.setRouting(newQuery.getRouting());
}
if (indexQuery.getSeqNo() == null && newQuery.getSeqNo() != null) {
indexQuery.setSeqNo(newQuery.getSeqNo());
}
if (indexQuery.getPrimaryTerm() == null && newQuery.getPrimaryTerm() != null) {
indexQuery.setPrimaryTerm(newQuery.getPrimaryTerm());
}
}
}
}

View File

@ -137,10 +137,7 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
// endregion
// region DocumentOperations
@Override
public String index(IndexQuery query, IndexCoordinates index) {
maybeCallbackBeforeConvertWithQuery(query, index);
public String doIndex(IndexQuery query, IndexCoordinates index) {
IndexRequest request = requestFactory.indexRequest(query, index);
IndexResponse indexResponse = execute(client -> client.index(request, RequestOptions.DEFAULT));
@ -152,8 +149,6 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
indexResponse.getPrimaryTerm(), indexResponse.getVersion()));
}
maybeCallbackAfterSaveWithQuery(query, index);
return indexResponse.getId();
}
@ -187,16 +182,6 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
return execute(client -> client.get(request, RequestOptions.DEFAULT).isExists());
}
@Override
public List<IndexedObjectInformation> bulkIndex(List<IndexQuery> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
Assert.notNull(queries, "List of IndexQuery must not be null");
Assert.notNull(bulkOptions, "BulkOptions must not be null");
return doBulkOperation(queries, bulkOptions, index);
}
@Override
public void bulkUpdate(List<UpdateQuery> queries, BulkOptions bulkOptions, IndexCoordinates index) {
@ -237,14 +222,12 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
return new UpdateResponse(result);
}
private List<IndexedObjectInformation> doBulkOperation(List<?> queries, BulkOptions bulkOptions,
public List<IndexedObjectInformation> doBulkOperation(List<?> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
maybeCallbackBeforeConvertWithQueries(queries, index);
BulkRequest bulkRequest = requestFactory.bulkRequest(queries, bulkOptions, index);
List<IndexedObjectInformation> indexedObjectInformationList = checkForBulkOperationFailure(
execute(client -> client.bulk(bulkRequest, RequestOptions.DEFAULT)));
updateIndexedObjectsWithQueries(queries, indexedObjectInformationList);
maybeCallbackAfterSaveWithQueries(queries, index);
return indexedObjectInformationList;
}
// endregion

View File

@ -144,10 +144,7 @@ public class ElasticsearchTemplate extends AbstractElasticsearchTemplate {
// endregion
// region DocumentOperations
@Override
public String index(IndexQuery query, IndexCoordinates index) {
maybeCallbackBeforeConvertWithQuery(query, index);
public String doIndex(IndexQuery query, IndexCoordinates index) {
IndexRequestBuilder indexRequestBuilder = requestFactory.indexRequestBuilder(client, query, index);
ActionFuture<IndexResponse> future = indexRequestBuilder.execute();
@ -166,8 +163,6 @@ public class ElasticsearchTemplate extends AbstractElasticsearchTemplate {
response.getPrimaryTerm(), response.getVersion()));
}
maybeCallbackAfterSaveWithQuery(query, index);
return documentId;
}
@ -201,22 +196,6 @@ public class ElasticsearchTemplate extends AbstractElasticsearchTemplate {
return getRequestBuilder.execute().actionGet().isExists();
}
@Override
public List<IndexedObjectInformation> bulkIndex(List<IndexQuery> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
Assert.notNull(queries, "List of IndexQuery must not be null");
Assert.notNull(bulkOptions, "BulkOptions must not be null");
List<IndexedObjectInformation> indexedObjectInformations = doBulkOperation(queries, bulkOptions, index);
updateIndexedObjectsWithQueries(queries, indexedObjectInformations);
maybeCallbackAfterSaveWithQueries(queries, index);
return indexedObjectInformations;
}
@Override
public void bulkUpdate(List<UpdateQuery> queries, BulkOptions bulkOptions, IndexCoordinates index) {
@ -261,11 +240,13 @@ public class ElasticsearchTemplate extends AbstractElasticsearchTemplate {
return new UpdateResponse(result);
}
private List<IndexedObjectInformation> doBulkOperation(List<?> queries, BulkOptions bulkOptions,
public List<IndexedObjectInformation> doBulkOperation(List<?> queries, BulkOptions bulkOptions,
IndexCoordinates index) {
maybeCallbackBeforeConvertWithQueries(queries, index);
BulkRequestBuilder bulkRequest = requestFactory.bulkRequestBuilder(client, queries, bulkOptions, index);
return checkForBulkOperationFailure(bulkRequest.execute().actionGet());
final List<IndexedObjectInformation> indexedObjectInformations = checkForBulkOperationFailure(
bulkRequest.execute().actionGet());
updateIndexedObjectsWithQueries(queries, indexedObjectInformations);
return indexedObjectInformations;
}
// endregion

View File

@ -0,0 +1,235 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.elasticsearch.core.event;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import lombok.Data;
import java.util.Collections;
import java.util.List;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.annotation.Id;
import org.springframework.data.elasticsearch.annotations.Document;
import org.springframework.data.elasticsearch.annotations.JoinTypeRelation;
import org.springframework.data.elasticsearch.annotations.JoinTypeRelations;
import org.springframework.data.elasticsearch.core.AbstractElasticsearchTemplate;
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
import org.springframework.data.elasticsearch.core.IndexOperations;
import org.springframework.data.elasticsearch.core.join.JoinField;
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.elasticsearch.core.query.IndexQuery;
import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm;
import org.springframework.data.elasticsearch.junit.jupiter.SpringIntegrationTest;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Component;
/**
* @author Peter-Josef Meisch
* @author Roman Puchkovskiy
*/
@SpringIntegrationTest
abstract class ElasticsearchOperationsCallbackIntegrationTest {
private static final String INDEX = "test-operations-callback";
@Autowired private ElasticsearchOperations originalOperations;
// need a spy here on the abstract implementation class
private AbstractElasticsearchTemplate operations;
@Nullable private static SeqNoPrimaryTerm seqNoPrimaryTerm = null;
@Configuration
static class Config {
@Component
static class SampleEntityBeforeConvertCallback implements BeforeConvertCallback<SampleEntity> {
@Override
public SampleEntity onBeforeConvert(SampleEntity entity, IndexCoordinates index) {
entity.setText("converted");
JoinField<String> joinField = new JoinField<>("answer", "42");
entity.setJoinField(joinField);
if (seqNoPrimaryTerm != null) {
entity.setSeqNoPrimaryTerm(seqNoPrimaryTerm);
}
return entity;
}
}
}
@BeforeEach
void setUp() {
seqNoPrimaryTerm = null;
operations = (AbstractElasticsearchTemplate) spy(originalOperations);
IndexOperations indexOps = operations.indexOps(SampleEntity.class);
indexOps.delete();
indexOps.create();
indexOps.putMapping(SampleEntity.class);
// store one entity to have a seq_no and primary_term
final SampleEntity initial = new SampleEntity("1", "initial");
final SampleEntity saved = operations.save(initial);
seqNoPrimaryTerm = saved.getSeqNoPrimaryTerm();
}
@AfterEach
void tearDown() {
IndexOperations indexOps = operations.indexOps(SampleEntity.class);
indexOps.delete();
}
@Test // DATAES-68
void shouldCallBeforeConvertCallback() {
SampleEntity entity = new SampleEntity("1", "test");
SampleEntity saved = operations.save(entity);
assertThat(saved.getText()).isEqualTo("converted");
}
@Test // DATAES-972
@DisplayName("should apply conversion result to IndexQuery on save")
void shouldApplyConversionResultToIndexQueryOnSave() {
SampleEntity entity = new SampleEntity("1", "test");
operations.save(entity);
ArgumentCaptor<IndexQuery> indexQueryCaptor = ArgumentCaptor.forClass(IndexQuery.class);
verify(operations, times(2)).doIndex(indexQueryCaptor.capture(), any());
final IndexQuery capturedIndexQuery = indexQueryCaptor.getValue();
SampleEntity convertedEntity = (SampleEntity) capturedIndexQuery.getObject();
final JoinField<String> joinField = convertedEntity.getJoinField();
assertThat(joinField.getName()).isEqualTo("answer");
assertThat(joinField.getParent()).isEqualTo("42");
assertThat(capturedIndexQuery.getRouting()).isEqualTo("42");
assertThat(capturedIndexQuery.getSeqNo()).isEqualTo(seqNoPrimaryTerm.getSequenceNumber());
assertThat(capturedIndexQuery.getPrimaryTerm()).isEqualTo(seqNoPrimaryTerm.getPrimaryTerm());
}
@Test // DATAES-972
@DisplayName("should apply conversion result to IndexQuery when not set ")
void shouldApplyConversionResultToIndexQueryWhenNotSet() {
SampleEntity entity = new SampleEntity("1", "test");
final IndexQuery indexQuery = new IndexQuery();
indexQuery.setId(entity.getId());
indexQuery.setObject(entity);
operations.index(indexQuery, IndexCoordinates.of(INDEX));
ArgumentCaptor<IndexQuery> indexQueryCaptor = ArgumentCaptor.forClass(IndexQuery.class);
verify(operations, times(2)).doIndex(indexQueryCaptor.capture(), any());
final IndexQuery capturedIndexQuery = indexQueryCaptor.getValue();
SampleEntity convertedEntity = (SampleEntity) capturedIndexQuery.getObject();
final JoinField<String> joinField = convertedEntity.getJoinField();
assertThat(joinField.getName()).isEqualTo("answer");
assertThat(joinField.getParent()).isEqualTo("42");
assertThat(capturedIndexQuery.getRouting()).isEqualTo("42");
assertThat(capturedIndexQuery.getSeqNo()).isEqualTo(seqNoPrimaryTerm.getSequenceNumber());
assertThat(capturedIndexQuery.getPrimaryTerm()).isEqualTo(seqNoPrimaryTerm.getPrimaryTerm());
}
@Test // DATAES-972
@DisplayName("should not apply conversion result to IndexQuery when already set ")
void shouldNotApplyConversionResultToIndexQueryWhenAlreadySet() {
SeqNoPrimaryTerm seqNoPrimaryTermOriginal = seqNoPrimaryTerm;
seqNoPrimaryTerm = new SeqNoPrimaryTerm(7, 8);
SampleEntity entity = new SampleEntity("1", "test");
final IndexQuery indexQuery = new IndexQuery();
indexQuery.setId(entity.getId());
indexQuery.setObject(entity);
indexQuery.setRouting("12");
indexQuery.setSeqNo(seqNoPrimaryTermOriginal.getSequenceNumber());
indexQuery.setPrimaryTerm(seqNoPrimaryTermOriginal.getPrimaryTerm());
operations.index(indexQuery, IndexCoordinates.of(INDEX));
ArgumentCaptor<IndexQuery> indexQueryCaptor = ArgumentCaptor.forClass(IndexQuery.class);
verify(operations, times(2)).doIndex(indexQueryCaptor.capture(), any());
final IndexQuery capturedIndexQuery = indexQueryCaptor.getValue();
SampleEntity convertedEntity = (SampleEntity) capturedIndexQuery.getObject();
final JoinField<String> joinField = convertedEntity.getJoinField();
assertThat(joinField.getName()).isEqualTo("answer");
assertThat(joinField.getParent()).isEqualTo("42");
assertThat(capturedIndexQuery.getRouting()).isEqualTo("12");
assertThat(capturedIndexQuery.getSeqNo()).isEqualTo(seqNoPrimaryTermOriginal.getSequenceNumber());
assertThat(capturedIndexQuery.getPrimaryTerm()).isEqualTo(seqNoPrimaryTermOriginal.getPrimaryTerm());
}
@Test // DATAES-972
@DisplayName("should apply conversion result to IndexQuery in bulkIndex")
void shouldApplyConversionResultToIndexQueryInBulkIndex() {
SampleEntity entity = new SampleEntity("1", "test");
final IndexQuery indexQuery = new IndexQuery();
indexQuery.setId(entity.getId());
indexQuery.setObject(entity);
operations.bulkIndex(Collections.singletonList(indexQuery), SampleEntity.class);
ArgumentCaptor<List<IndexQuery>> indexQueryListCaptor = ArgumentCaptor.forClass(List.class);
verify(operations, times(1)).bulkOperation(indexQueryListCaptor.capture(), any(), any());
final List<IndexQuery> capturedIndexQueries = indexQueryListCaptor.getValue();
assertThat(capturedIndexQueries).hasSize(1);
final IndexQuery capturedIndexQuery = capturedIndexQueries.get(0);
SampleEntity convertedEntity = (SampleEntity) capturedIndexQuery.getObject();
final JoinField<String> joinField = convertedEntity.getJoinField();
assertThat(joinField.getName()).isEqualTo("answer");
assertThat(joinField.getParent()).isEqualTo("42");
assertThat(capturedIndexQuery.getRouting()).isEqualTo("42");
assertThat(capturedIndexQuery.getSeqNo()).isEqualTo(seqNoPrimaryTerm.getSequenceNumber());
assertThat(capturedIndexQuery.getPrimaryTerm()).isEqualTo(seqNoPrimaryTerm.getPrimaryTerm());
}
@Data
@Document(indexName = INDEX)
static class SampleEntity {
@Id private String id;
private String text;
@JoinTypeRelations(relations = { @JoinTypeRelation(parent = "question",
children = { "answer" }) }) @Nullable private JoinField<String> joinField;
private SeqNoPrimaryTerm seqNoPrimaryTerm;
public SampleEntity(String id, String text) {
this.id = id;
this.text = text;
}
}
}

View File

@ -1,102 +0,0 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.elasticsearch.core.event;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.annotation.Id;
import org.springframework.data.elasticsearch.annotations.Document;
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
import org.springframework.data.elasticsearch.core.IndexOperations;
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.stereotype.Component;
/**
* @author Peter-Josef Meisch
* @author Roman Puchkovskiy
*/
abstract class ElasticsearchOperationsCallbackTest {
@Autowired private ElasticsearchOperations operations;
@Configuration
static class Config {
@Component
static class SampleEntityBeforeConvertCallback implements BeforeConvertCallback<SampleEntity> {
@Override
public SampleEntity onBeforeConvert(SampleEntity entity, IndexCoordinates index) {
entity.setText("converted");
return entity;
}
}
}
@BeforeEach
void setUp() {
IndexOperations indexOps = operations.indexOps(SampleEntity.class);
indexOps.delete();
indexOps.create();
indexOps.putMapping(SampleEntity.class);
}
@AfterEach
void tearDown() {
IndexOperations indexOps = operations.indexOps(SampleEntity.class);
indexOps.delete();
}
@Test
void shouldCallBeforeConvertCallback() {
SampleEntity entity = new SampleEntity("1", "test");
SampleEntity saved = operations.save(entity);
assertThat(saved.getText()).isEqualTo("converted");
}
@Document(indexName = "test-operations-callback")
static class SampleEntity {
@Id private String id;
private String text;
public SampleEntity(String id, String text) {
this.id = id;
this.text = text;
}
public String getId() {
return id;
}
public void setId(String id) {
this.id = id;
}
public String getText() {
return text;
}
public void setText(String text) {
this.text = text;
}
}
}

View File

@ -16,12 +16,11 @@
package org.springframework.data.elasticsearch.core.event;
import org.springframework.data.elasticsearch.junit.jupiter.ElasticsearchRestTemplateConfiguration;
import org.springframework.data.elasticsearch.junit.jupiter.SpringIntegrationTest;
import org.springframework.test.context.ContextConfiguration;
/**
* @author Peter-Josef Meisch
*/
@SpringIntegrationTest
@ContextConfiguration(classes = { ElasticsearchRestTemplateConfiguration.class, ElasticsearchOperationsCallbackTest.Config.class })
class ElasticsearchRestOperationsCallbackTest extends ElasticsearchOperationsCallbackTest {}
@ContextConfiguration(classes = { ElasticsearchRestTemplateConfiguration.class,
ElasticsearchOperationsCallbackIntegrationTest.Config.class })
class ElasticsearchRestOperationsCallbackIntegrationTest extends ElasticsearchOperationsCallbackIntegrationTest {}

View File

@ -16,12 +16,10 @@
package org.springframework.data.elasticsearch.core.event;
import org.springframework.data.elasticsearch.junit.jupiter.ElasticsearchTemplateConfiguration;
import org.springframework.data.elasticsearch.junit.jupiter.SpringIntegrationTest;
import org.springframework.test.context.ContextConfiguration;
/**
* @author Peter-Josef Meisch
*/
@SpringIntegrationTest
@ContextConfiguration(classes = { ElasticsearchTemplateConfiguration.class, ElasticsearchOperationsCallbackTest.Config.class })
class ElasticsearchTransportOperationsCallbackTest extends ElasticsearchOperationsCallbackTest {}
@ContextConfiguration(classes = { ElasticsearchTemplateConfiguration.class, ElasticsearchOperationsCallbackIntegrationTest.Config.class })
class ElasticsearchTransportOperationsCallbackIntegrationTest extends ElasticsearchOperationsCallbackIntegrationTest {}