Skip to content

Commit 9bf6b6a

Browse files
mp911demp911de
committed
DATAES-976 - Polishing.
Use deleteAllById(…) from deleteAll(…). Simplify implementation in SimpleReactiveElasticsearchRepository. Original pull request: #554.
1 parent 749270b commit 9bf6b6a

File tree

5 files changed

+119
-134
lines changed

5 files changed

+119
-134
lines changed

‎src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public <T> T get(String id, Class<T> clazz, IndexCoordinates index) {
166166
public <T> List<T> multiGet(Query query, Class<T> clazz, IndexCoordinates index) {
167167

168168
Assert.notNull(index, "index must not be null");
169-
Assert.notEmpty(query.getIds(), "No Id define for Query");
169+
Assert.notEmpty(query.getIds(), "No Id defined for Query");
170170

171171
MultiGetRequest request = requestFactory.multiGetRequest(query, clazz, index);
172172
MultiGetResponse result = execute(client -> client.mget(request, RequestOptions.DEFAULT));

‎src/main/java/org/springframework/data/elasticsearch/repository/support/SimpleElasticsearchRepository.java

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.index.query.QueryBuilder;
2828
import org.slf4j.Logger;
2929
import org.slf4j.LoggerFactory;
30+
3031
import org.springframework.data.domain.Page;
3132
import org.springframework.data.domain.PageImpl;
3233
import org.springframework.data.domain.PageRequest;
@@ -37,7 +38,6 @@
3738
import org.springframework.data.elasticsearch.core.SearchHit;
3839
import org.springframework.data.elasticsearch.core.SearchHitSupport;
3940
import org.springframework.data.elasticsearch.core.SearchHits;
40-
import org.springframework.data.elasticsearch.core.SearchPage;
4141
import org.springframework.data.elasticsearch.core.aggregation.AggregatedPage;
4242
import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentEntity;
4343
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
@@ -50,6 +50,7 @@
5050
import org.springframework.data.util.Streamable;
5151
import org.springframework.lang.Nullable;
5252
import org.springframework.util.Assert;
53+
import org.springframework.util.CollectionUtils;
5354

5455
/**
5556
* Elasticsearch specific repository implementation. Likely to be used as target within
@@ -151,14 +152,12 @@ public Iterable<T> findAllById(Iterable<ID> ids) {
151152
Assert.notNull(ids, "ids can't be null.");
152153

153154
List<T> result = new ArrayList<>();
154-
List<String> stringIds = stringIdsRepresentation(ids);
155-
156-
if (stringIds.isEmpty()) {
155+
Query idQuery = getIdQuery(ids);
156+
if (CollectionUtils.isEmpty(idQuery.getIds())) {
157157
return result;
158158
}
159159

160-
NativeSearchQuery query = new NativeSearchQueryBuilder().withIds(stringIds).build();
161-
List<T> multiGetEntities = execute(operations -> operations.multiGet(query, entityClass, getIndexCoordinates()));
160+
List<T> multiGetEntities = execute(operations -> operations.multiGet(idQuery, entityClass, getIndexCoordinates()));
162161

163162
if (multiGetEntities != null) {
164163
multiGetEntities.forEach(entity -> {
@@ -291,54 +290,41 @@ public void delete(T entity) {
291290
}
292291

293292
@Override
294-
public void deleteAll(Iterable<? extends T> entities) {
293+
public void deleteAllById(Iterable<? extends ID> ids) {
295294

296-
Assert.notNull(entities, "Cannot delete 'null' list.");
295+
Assert.notNull(ids, "Cannot delete 'null' list.");
297296

298297
IndexCoordinates indexCoordinates = getIndexCoordinates();
299298
IdsQueryBuilder idsQueryBuilder = idsQuery();
300-
for (T entity : entities) {
301-
ID id = extractIdFromBean(entity);
302-
if (id != null) {
303-
idsQueryBuilder.addIds(stringIdRepresentation(id));
304-
}
299+
for (ID id : ids) {
300+
idsQueryBuilder.addIds(stringIdRepresentation(id));
305301
}
306302

307303
if (idsQueryBuilder.ids().isEmpty()) {
308304
return;
309305
}
310306

311-
Query query = new NativeSearchQueryBuilder().withQuery(idsQueryBuilder).build();
312-
313307
executeAndRefresh((OperationsCallback<Void>) operations -> {
314-
operations.delete(query, entityClass, indexCoordinates);
308+
operations.delete(new NativeSearchQueryBuilder().withQuery(idsQueryBuilder).build(), entityClass,
309+
indexCoordinates);
315310
return null;
316311
});
317312
}
318313

319314
@Override
320-
public void deleteAllById(Iterable<? extends ID> ids) {
315+
public void deleteAll(Iterable<? extends T> entities) {
321316

322-
Assert.notNull(ids, "Cannot delete 'null' list.");
317+
Assert.notNull(entities, "Cannot delete 'null' list.");
323318

324-
IndexCoordinates indexCoordinates = getIndexCoordinates();
325-
IdsQueryBuilder idsQueryBuilder = idsQuery();
326-
for (ID id : ids) {
319+
List<ID> ids = new ArrayList<>();
320+
for (T entity : entities) {
321+
ID id = extractIdFromBean(entity);
327322
if (id != null) {
328-
idsQueryBuilder.addIds(stringIdRepresentation(id));
323+
ids.add(id);
329324
}
330325
}
331326

332-
if (idsQueryBuilder.ids().isEmpty()) {
333-
return;
334-
}
335-
336-
Query query = new NativeSearchQueryBuilder().withQuery(idsQueryBuilder).build();
337-
338-
executeAndRefresh((OperationsCallback<Void>) operations -> {
339-
operations.delete(query, entityClass, indexCoordinates);
340-
return null;
341-
});
327+
deleteAllById(ids);
342328
}
343329

344330
private void doDelete(@Nullable ID id, @Nullable String routing, IndexCoordinates indexCoordinates) {
@@ -369,7 +355,7 @@ protected ID extractIdFromBean(T entity) {
369355
return entityInformation.getId(entity);
370356
}
371357

372-
private List<String> stringIdsRepresentation(Iterable<ID> ids) {
358+
private List<String> stringIdsRepresentation(Iterable<? extends ID> ids) {
373359

374360
Assert.notNull(ids, "ids can't be null.");
375361

@@ -385,6 +371,12 @@ private IndexCoordinates getIndexCoordinates() {
385371
return operations.getIndexCoordinatesFor(entityClass);
386372
}
387373

374+
private Query getIdQuery(Iterable<? extends ID> ids) {
375+
List<String> stringIds = stringIdsRepresentation(ids);
376+
377+
return new NativeSearchQueryBuilder().withIds(stringIds).build();
378+
}
379+
388380
// region operations callback
389381
@FunctionalInterface
390382
public interface OperationsCallback<R> {

‎src/main/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepository.java

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@
1515
*/
1616
package org.springframework.data.elasticsearch.repository.support;
1717

18+
import org.elasticsearch.index.query.IdsQueryBuilder;
1819
import reactor.core.publisher.Flux;
1920
import reactor.core.publisher.Mono;
2021

2122
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
22-
import org.elasticsearch.index.query.QueryBuilders;
2323
import org.reactivestreams.Publisher;
2424
import org.slf4j.Logger;
2525
import org.slf4j.LoggerFactory;
26+
2627
import org.springframework.data.domain.Pageable;
2728
import org.springframework.data.domain.Sort;
2829
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
@@ -33,9 +34,7 @@
3334
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
3435
import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder;
3536
import org.springframework.data.elasticsearch.core.query.Query;
36-
import org.springframework.data.elasticsearch.core.query.StringQuery;
3737
import org.springframework.data.elasticsearch.repository.ReactiveElasticsearchRepository;
38-
import org.springframework.data.util.Streamable;
3938
import org.springframework.util.Assert;
4039

4140
/**
@@ -167,7 +166,7 @@ public Flux<T> findAllById(Publisher<ID> idStream) {
167166

168167
Assert.notNull(idStream, "IdStream must not be null!");
169168
return Flux.from(idStream) //
170-
.map(ID::toString) //
169+
.map(this::convertId) //
171170
.collectList() //
172171
.map(ids -> new NativeSearchQueryBuilder().withIds(ids).build()) //
173172
.flatMapMany(query -> {
@@ -206,43 +205,37 @@ public Mono<Void> delete(T entity) {
206205
.then(doRefresh());
207206
}
208207

209-
@Override
210-
public Mono<Void> deleteAll(Iterable<? extends T> entities) {
211-
212-
Assert.notNull(entities, "Entities must not be null!");
213-
return deleteAll(Flux.fromIterable(entities));
214-
}
215-
216208
@Override
217209
public Mono<Void> deleteAllById(Iterable<? extends ID> ids) {
218210

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

221-
return Mono.just(Streamable.of(ids) //
222-
.map(this::convertId).toList() //
223-
).map(objects -> new StringQuery(QueryBuilders.idsQuery() //
224-
.addIds(objects.toArray(new String[0])) //
225-
.toString()) //
226-
).flatMap(
227-
query -> operations.delete(query, entityInformation.getJavaType(), entityInformation.getIndexCoordinates())) //
213+
return Flux.fromIterable(ids) //
214+
.map(this::convertId) //
215+
.collectList() //
216+
.map(it -> new NativeSearchQueryBuilder().withQuery(new IdsQueryBuilder().addIds(it.toArray(new String[0])))
217+
.build())
218+
.flatMap(it -> operations.delete(it, entityInformation.getJavaType(), entityInformation.getIndexCoordinates())) //
228219
.then(doRefresh());
229220
}
230221

222+
@Override
223+
public Mono<Void> deleteAll(Iterable<? extends T> entities) {
224+
225+
Assert.notNull(entities, "Entities must not be null!");
226+
return deleteAll(Flux.fromIterable(entities));
227+
}
228+
231229
@Override
232230
public Mono<Void> deleteAll(Publisher<? extends T> entityStream) {
233231

234232
Assert.notNull(entityStream, "EntityStream must not be null!");
235233
return Flux.from(entityStream) //
236-
.map(entity -> {
237-
238-
ID id = entityInformation.getId(entity);
239-
if (id == null) {
240-
throw new IllegalStateException("Entity id must not be null!");
241-
}
242-
return convertId(id);
243-
}).collectList().map(objects -> new StringQuery(QueryBuilders.idsQuery() //
244-
.addIds(objects.toArray(new String[0])) //
245-
.toString())) //
234+
.map(entityInformation::getRequiredId) //
235+
.map(this::convertId) //
236+
.collectList() //
237+
.map(it -> new NativeSearchQueryBuilder().withQuery(new IdsQueryBuilder().addIds(it.toArray(new String[0])))
238+
.build())
246239
.flatMap(
247240
query -> operations.delete(query, entityInformation.getJavaType(), entityInformation.getIndexCoordinates())) //
248241
.then(doRefresh());

0 commit comments

Comments
 (0)