Skip to content

DATAES-976 - Implement CrudRepository.delete(Iterable<ID> ids). #554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

DATAES-976 - Implement CrudRepository.delete(Iterable<ID> ids). #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c582d3
DATAES-976 - Prepare branch
schauder Nov 12, 2020
d9cb5a9
DATAES-976 - Implement CrudRepository.delete(Iterable<ID> ids).
schauder Nov 12, 2020
a570b63
DATAES-976 - Undo the import of java.util.Arrays.*.
schauder Nov 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-elasticsearch</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-DATAES-976-SNAPSHOT</version>

<parent>
<groupId>org.springframework.data.build</groupId>
Expand All @@ -22,7 +22,7 @@
<elasticsearch>7.9.3</elasticsearch>
<log4j>2.13.3</log4j>
<netty>4.1.52.Final</netty>
<springdata.commons>2.5.0-SNAPSHOT</springdata.commons>
<springdata.commons>2.4.0-DATACMNS-800-SNAPSHOT</springdata.commons>
<java-module-name>spring.data.elasticsearch</java-module-name>
</properties>

Expand Down
26 changes: 26 additions & 0 deletions .../springframework/data/elasticsearch/repository/support/SimpleElasticsearchRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
* @author Murali Chevuri
* @author Peter-Josef Meisch
* @author Aleksei Arsenev
* @author Jens Schauder
*/
public class SimpleElasticsearchRepository<T, ID> implements ElasticsearchRepository<T, ID> {

Expand Down Expand Up @@ -315,6 +316,31 @@ public void deleteAll(Iterable<? extends T> entities) {
});
}

@Override
public void deleteAllById(Iterable<? extends ID> ids) {

Assert.notNull(ids, "Cannot delete 'null' list.");

IndexCoordinates indexCoordinates = getIndexCoordinates();
IdsQueryBuilder idsQueryBuilder = idsQuery();
for (ID id : ids) {
if (id != null) {
idsQueryBuilder.addIds(stringIdRepresentation(id));
}
}

if (idsQueryBuilder.ids().isEmpty()) {
return;
}

Query query = new NativeSearchQueryBuilder().withQuery(idsQueryBuilder).build();

executeAndRefresh((OperationsCallback<Void>) operations -> {
operations.delete(query, entityClass, indexCoordinates);
return null;
});
}

private void doDelete(@Nullable ID id, @Nullable String routing, IndexCoordinates indexCoordinates) {

if (id != null) {
Expand Down
19 changes: 18 additions & 1 deletion ...ramework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@
import org.springframework.data.elasticsearch.core.query.Query;
import org.springframework.data.elasticsearch.core.query.StringQuery;
import org.springframework.data.elasticsearch.repository.ReactiveElasticsearchRepository;
import org.springframework.data.util.Streamable;
import org.springframework.util.Assert;

/**
* @author Christoph Strobl
* @author Peter-Josef Meisch
* @author Aleksei Arsenev
* @author Jens Schauder
* @since 3.2
*/
public class SimpleReactiveElasticsearchRepository<T, ID> implements ReactiveElasticsearchRepository<T, ID> {
Expand All @@ -52,7 +54,7 @@ public class SimpleReactiveElasticsearchRepository<T, ID> implements ReactiveEla
private final ReactiveIndexOperations indexOperations;

public SimpleReactiveElasticsearchRepository(ElasticsearchEntityInformation<T, ID> entityInformation,
ReactiveElasticsearchOperations operations) {
ReactiveElasticsearchOperations operations) {

Assert.notNull(entityInformation, "EntityInformation must not be null!");
Assert.notNull(operations, "ElasticsearchOperations must not be null!");
Expand Down Expand Up @@ -211,6 +213,21 @@ public Mono<Void> deleteAll(Iterable<? extends T> entities) {
return deleteAll(Flux.fromIterable(entities));
}

@Override
public Mono<Void> deleteAllById(Iterable<? extends ID> ids) {

Assert.notNull(ids, "Ids must not be null!");

return Mono.just(Streamable.of(ids) //
.map(this::convertId).toList() //
).map(objects -> new StringQuery(QueryBuilders.idsQuery() //
.addIds(objects.toArray(new String[0])) //
.toString()) //
).flatMap(
query -> operations.delete(query, entityInformation.getJavaType(), entityInformation.getIndexCoordinates())) //
.then(doRefresh());
}

@Override
public Mono<Void> deleteAll(Publisher<? extends T> entityStream) {

Expand Down
26 changes: 20 additions & 6 deletions ...ork/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.data.elasticsearch.repository.support;

import static java.util.Arrays.*;
import static org.assertj.core.api.Assertions.*;
import static org.springframework.data.elasticsearch.annotations.FieldType.*;
import static org.springframework.data.elasticsearch.core.query.Query.*;
Expand Down Expand Up @@ -64,6 +65,7 @@
/**
* @author Christoph Strobl
* @author Peter-Josef Meisch
* @author Jens Schauder
*/
@SpringIntegrationTest
@ContextConfiguration(classes = { SimpleReactiveElasticsearchRepositoryTests.Config.class })
Expand Down Expand Up @@ -107,7 +109,7 @@ private Mono<Boolean> documentWithIdExistsInIndex(String id) {
public void saveShouldComputeMultipleEntities() {

repository
.saveAll(Arrays.asList(SampleEntity.builder().build(), SampleEntity.builder().build(),
.saveAll(asList(SampleEntity.builder().build(), SampleEntity.builder().build(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We generally don't import Arrays statically to avoid clashes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I only knew the rule that static imports are ok in tests. Will undo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted those changes.

SampleEntity.builder().build())) //
.map(SampleEntity::getId) //
.flatMap(this::documentWithIdExistsInIndex) //
Expand Down Expand Up @@ -167,7 +169,7 @@ public void findAllShouldReturnAllElements() {

@Test // DATAES-519
public void findAllByIdByIdShouldCompleteIfIndexDoesNotExist() {
repository.findAllById(Arrays.asList("id-two", "id-two")).as(StepVerifier::create).verifyComplete();
repository.findAllById(asList("id-two", "id-two")).as(StepVerifier::create).verifyComplete();
}

@Test // DATAES-519
Expand All @@ -178,7 +180,7 @@ public void findAllByIdShouldRetrieveMatchingDocuments() {
SampleEntity.builder().id("id-three").build()) //
.block();

repository.findAllById(Arrays.asList("id-one", "id-two")) //
repository.findAllById(asList("id-one", "id-two")) //
.as(StepVerifier::create)//
.expectNextMatches(entity -> entity.getId().equals("id-one") || entity.getId().equals("id-two")) //
.expectNextMatches(entity -> entity.getId().equals("id-one") || entity.getId().equals("id-two")) //
Expand All @@ -193,7 +195,7 @@ public void findAllByIdShouldCompleteWhenNothingFound() {
SampleEntity.builder().id("id-three").build()) //
.block();

repository.findAllById(Arrays.asList("can't", "touch", "this")) //
repository.findAllById(asList("can't", "touch", "this")) //
.as(StepVerifier::create)//
.verifyComplete();
}
Expand Down Expand Up @@ -380,6 +382,18 @@ public void deleteByIdShouldDeleteEntry() {
assertThat(documentWithIdExistsInIndex(toBeDeleted.getId()).block()).isFalse();
}

@Test // DATAES-976
public void deleteAllByIdShouldDeleteEntry() {

SampleEntity toBeDeleted = SampleEntity.builder().id("id-two").build();
bulkIndex(SampleEntity.builder().id("id-one").build(), toBeDeleted) //
.block();

repository.deleteAllById(asList(toBeDeleted.getId())).as(StepVerifier::create).verifyComplete();

assertThat(documentWithIdExistsInIndex(toBeDeleted.getId()).block()).isFalse();
}

@Test // DATAES-519
public void deleteShouldDeleteEntry() {

Expand All @@ -402,7 +416,7 @@ public void deleteAllShouldDeleteGivenEntries() {
bulkIndex(toBeDeleted, hangInThere, toBeDeleted2) //
.block();

repository.deleteAll(Arrays.asList(toBeDeleted, toBeDeleted2)).as(StepVerifier::create).verifyComplete();
repository.deleteAll(asList(toBeDeleted, toBeDeleted2)).as(StepVerifier::create).verifyComplete();

assertThat(documentWithIdExistsInIndex(toBeDeleted.getId()).block()).isFalse();
assertThat(documentWithIdExistsInIndex(toBeDeleted2.getId()).block()).isFalse();
Expand Down Expand Up @@ -547,7 +561,7 @@ public void derivedDeleteMethodShouldBeExecutedCorrectly() {
}

Mono<Void> bulkIndex(SampleEntity... entities) {
return operations.saveAll(Arrays.asList(entities), IndexCoordinates.of(INDEX)).then();
return operations.saveAll(asList(entities), IndexCoordinates.of(INDEX)).then();
}

interface ReactiveSampleEntityRepository extends ReactiveCrudRepository<SampleEntity, String> {
Expand Down
34 changes: 34 additions & 0 deletions ...lasticsearch/repository/support/simple/SimpleElasticsearchRepositoryIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,40 @@ public void shouldDeleteById() {
assertThat(result).isEqualTo(1L);
}

@Test //DATAES-976
public void shouldDeleteAllById() {

// given
String id1 = nextIdAsString();
SampleEntity sampleEntity1 = new SampleEntity();
sampleEntity1.setId(id1);
sampleEntity1.setMessage("hello world 1");
sampleEntity1.setAvailable(true);
sampleEntity1.setVersion(System.currentTimeMillis());

String id2 = nextIdAsString();
SampleEntity sampleEntity2 = new SampleEntity();
sampleEntity2.setId(id2);
sampleEntity2.setMessage("hello world 2");
sampleEntity2.setAvailable(true);
sampleEntity2.setVersion(System.currentTimeMillis());

String id3 = nextIdAsString();
SampleEntity sampleEntity3 = new SampleEntity();
sampleEntity3.setId(id3);
sampleEntity3.setMessage("hello world 3");
sampleEntity3.setAvailable(false);
sampleEntity3.setVersion(System.currentTimeMillis());

repository.saveAll(Arrays.asList(sampleEntity1, sampleEntity2, sampleEntity3));

// when
repository.deleteAllById(Arrays.asList(id1, id3));

// then
assertThat(repository.findAll()).extracting(SampleEntity::getId).containsExactly(id2);
}

@Test
public void shouldDeleteByMessageAndReturnList() {

Expand Down