Skip to content

Commit 795e1ef

Browse files
StexxenStexxen
authored
Refactor routing and null handling with a new getRouting methods.
1 parent a08ffc8 commit 795e1ef

2 files changed

Lines changed: 91 additions & 82 deletions

File tree

‎src/main/java/org/springframework/data/elasticsearch/client/elc/RequestConverter.java‎

Lines changed: 73 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import java.util.function.Function;
7575
import java.util.stream.Collectors;
7676
import java.util.stream.Stream;
77+
import java.util.Optional;
7778

7879
import org.apache.commons.logging.Log;
7980
import org.apache.commons.logging.LogFactory;
@@ -113,6 +114,7 @@
113114
* @author cdalxndr
114115
* @author scoobyzhang
115116
* @author Haibo Liu
117+
* @author Steven Pearce
116118
* @since 4.4
117119
*/
118120
class RequestConverter extends AbstractQueryProcessor {
@@ -173,17 +175,9 @@ public co.elastic.clients.elasticsearch.cluster.PutComponentTemplateRequest clus
173175
private co.elastic.clients.elasticsearch.indices.Alias.Builder buildAlias(AliasActionParameters parameters,
174176
co.elastic.clients.elasticsearch.indices.Alias.Builder aliasBuilder) {
175177

176-
if (parameters.getRouting() != null) {
177-
aliasBuilder.routing(parameters.getRouting());
178-
}
179-
180-
if (parameters.getIndexRouting() != null) {
181-
aliasBuilder.indexRouting(parameters.getIndexRouting());
182-
}
183-
184-
if (parameters.getSearchRouting() != null) {
185-
aliasBuilder.searchRouting(parameters.getSearchRouting());
186-
}
178+
getRouting(parameters.getRouting()).ifPresent(aliasBuilder::routing);
179+
getRouting(parameters.getIndexRouting()).ifPresent(aliasBuilder::indexRouting);
180+
getRouting(parameters.getSearchRouting()).ifPresent(aliasBuilder::searchRouting);
187181

188182
if (parameters.getHidden() != null) {
189183
aliasBuilder.isHidden(parameters.getHidden());
@@ -239,12 +233,16 @@ public CreateIndexRequest indicesCreateRequest(CreateIndexSettings indexSettings
239233
Map<String, co.elastic.clients.elasticsearch.indices.Alias> aliases = new HashMap<>();
240234
for (Alias alias : indexSettings.getAliases()) {
241235
co.elastic.clients.elasticsearch.indices.Alias esAlias = co.elastic.clients.elasticsearch.indices.Alias
242-
.of(ab -> ab.filter(getQuery(alias.getFilter(), null))
243-
.routing(alias.getRouting())
244-
.indexRouting(alias.getIndexRouting())
245-
.searchRouting(alias.getSearchRouting())
246-
.isHidden(alias.getHidden())
247-
.isWriteIndex(alias.getWriteIndex()));
236+
.of(ab -> {
237+
co.elastic.clients.elasticsearch.indices.Alias.Builder aliasBuilder = ab.filter(getQuery(alias.getFilter(), null))
238+
.isHidden(alias.getHidden())
239+
.isWriteIndex(alias.getWriteIndex());
240+
getRouting(alias.getRouting()).ifPresent(aliasBuilder::routing);
241+
getRouting(alias.getIndexRouting()).ifPresent(aliasBuilder::indexRouting);
242+
getRouting(alias.getSearchRouting()).ifPresent(aliasBuilder::searchRouting);
243+
244+
return aliasBuilder;
245+
});
248246
aliases.put(alias.getAlias(), esAlias);
249247
}
250248

@@ -318,10 +316,11 @@ private Action.Builder getBuilder(AliasAction aliasAction) {
318316
addActionBuilder //
319317
.indices(Arrays.asList(parameters.getIndices())) //
320318
.isHidden(parameters.getHidden()) //
321-
.isWriteIndex(parameters.getWriteIndex()) //
322-
.routing(parameters.getRouting()) //
323-
.indexRouting(parameters.getIndexRouting()) //
324-
.searchRouting(parameters.getSearchRouting()); //
319+
.isWriteIndex(parameters.getWriteIndex()); //
320+
321+
getRouting(parameters.getRouting()).ifPresent(addActionBuilder::routing);
322+
getRouting(parameters.getIndexRouting()).ifPresent(addActionBuilder::indexRouting);
323+
getRouting(parameters.getSearchRouting()).ifPresent(addActionBuilder::searchRouting);
325324

326325
if (parameters.getAliases() != null) {
327326
addActionBuilder.aliases(Arrays.asList(parameters.getAliases()));
@@ -593,9 +592,7 @@ public IndexRequest<?> documentIndexRequest(IndexQuery query, IndexCoordinates i
593592
.ifSeqNo(query.getSeqNo())
594593
.ifPrimaryTerm(query.getPrimaryTerm());
595594

596-
if (query.getRouting() != null) {
597-
builder.routing(query.getRouting()); //
598-
}
595+
getRouting(query.getRouting()).ifPresent(builder::routing);
599596

600597
if (query.getOpType() != null) {
601598
switch (query.getOpType()) {
@@ -645,8 +642,9 @@ private IndexOperation<?> bulkIndexOperation(IndexQuery query, IndexCoordinates
645642

646643
builder //
647644
.ifSeqNo(query.getSeqNo()) //
648-
.ifPrimaryTerm(query.getPrimaryTerm()) //
649-
.routing(query.getRouting()); //
645+
.ifPrimaryTerm(query.getPrimaryTerm()); //
646+
647+
getRouting(query.getRouting()).ifPresent(builder::routing);
650648

651649
return builder.build();
652650
}
@@ -687,8 +685,9 @@ private CreateOperation<?> bulkCreateOperation(IndexQuery query, IndexCoordinate
687685

688686
builder //
689687
.ifSeqNo(query.getSeqNo()) //
690-
.ifPrimaryTerm(query.getPrimaryTerm()) //
691-
.routing(query.getRouting()); //
688+
.ifPrimaryTerm(query.getPrimaryTerm()); //
689+
690+
getRouting(query.getRouting()).ifPresent(builder::routing);
692691

693692
return builder.build();
694693
}
@@ -725,11 +724,12 @@ private CreateOperation<?> bulkCreateOperation(IndexQuery query, IndexCoordinate
725724
});
726725

727726
uob
728-
.routing(query.getRouting())
729727
.ifSeqNo(query.getIfSeqNo())
730728
.ifPrimaryTerm(query.getIfPrimaryTerm())
731729
.retryOnConflict(query.getRetryOnConflict());
732730

731+
getRouting(query.getRouting()).ifPresent(uob::routing);
732+
733733
// no refresh, timeout, waitForActiveShards on UpdateOperation or UpdateAction
734734

735735
return uob.build();
@@ -779,9 +779,7 @@ public BulkRequest documentBulkRequest(List<?> queries, BulkOptions bulkOptions,
779779
builder.pipeline(bulkOptions.getPipeline());
780780
}
781781

782-
if (bulkOptions.getRoutingId() != null) {
783-
builder.routing(bulkOptions.getRoutingId());
784-
}
782+
getRouting(bulkOptions.getRoutingId()).ifPresent(builder::routing);
785783

786784
List<BulkOperation> operations = queries.stream().map(query -> {
787785
BulkOperation.Builder ob = new BulkOperation.Builder();
@@ -808,10 +806,13 @@ public GetRequest documentGetRequest(String id, @Nullable String routing, IndexC
808806
Assert.notNull(id, "id must not be null");
809807
Assert.notNull(indexCoordinates, "indexCoordinates must not be null");
810808

811-
return GetRequest.of(grb -> grb //
812-
.index(indexCoordinates.getIndexName()) //
813-
.id(id) //
814-
.routing(routing));
809+
return GetRequest.of(grb -> {
810+
GetRequest.Builder builder = grb //
811+
.index(indexCoordinates.getIndexName()) //
812+
.id(id); //
813+
getRouting(routing).ifPresent(builder::routing);
814+
return builder;
815+
});
815816
}
816817

817818
public co.elastic.clients.elasticsearch.core.ExistsRequest documentExistsRequest(String id, @Nullable String routing,
@@ -820,10 +821,13 @@ public co.elastic.clients.elasticsearch.core.ExistsRequest documentExistsRequest
820821
Assert.notNull(id, "id must not be null");
821822
Assert.notNull(indexCoordinates, "indexCoordinates must not be null");
822823

823-
return co.elastic.clients.elasticsearch.core.ExistsRequest.of(erb -> erb
824-
.index(indexCoordinates.getIndexName())
825-
.id(id)
826-
.routing(routing));
824+
return co.elastic.clients.elasticsearch.core.ExistsRequest.of(erb -> {
825+
co.elastic.clients.elasticsearch.core.ExistsRequest.Builder builder = erb
826+
.index(indexCoordinates.getIndexName())
827+
.id(id);
828+
getRouting(routing).ifPresent(builder::routing);
829+
return builder;
830+
});
827831
}
828832

829833
public <T> MgetRequest documentMgetRequest(Query query, Class<T> clazz, IndexCoordinates index) {
@@ -841,11 +845,14 @@ public <T> MgetRequest documentMgetRequest(Query query, Class<T> clazz, IndexCoo
841845
SourceConfig sourceConfig = getSourceConfig(query);
842846

843847
List<MultiGetOperation> multiGetOperations = query.getIdsWithRouting().stream()
844-
.map(idWithRouting -> MultiGetOperation.of(mgo -> mgo //
845-
.index(index.getIndexName()) //
846-
.id(idWithRouting.id()) //
847-
.routing(idWithRouting.routing()) //
848-
.source(sourceConfig)))
848+
.map(idWithRouting -> MultiGetOperation.of(mgo -> {
849+
MultiGetOperation.Builder builder = mgo //
850+
.index(index.getIndexName()) //
851+
.id(idWithRouting.id()) //
852+
.source(sourceConfig);
853+
getRouting(idWithRouting.routing()).ifPresent(builder::routing);
854+
return builder;
855+
}))
849856
.collect(Collectors.toList());
850857

851858
return MgetRequest.of(mg -> mg//
@@ -967,10 +974,7 @@ public DeleteRequest documentDeleteRequest(String id, @Nullable String routing,
967974

968975
return DeleteRequest.of(r -> {
969976
r.id(id).index(index.getIndexName());
970-
971-
if (routing != null) {
972-
r.routing(routing);
973-
}
977+
getRouting(routing).ifPresent(r::routing);
974978
r.refresh(refresh(refreshPolicy));
975979
return r;
976980
});
@@ -994,11 +998,7 @@ public DeleteByQueryRequest documentDeleteByQueryRequest(Query query, @Nullable
994998

995999
b.scroll(time(query.getScrollTime()));
9961000

997-
if (query.getRoute() != null) {
998-
b.routing(query.getRoute());
999-
} else if (StringUtils.hasText(routing)) {
1000-
b.routing(routing);
1001-
}
1001+
getRouting(query.getRoute(), routing).ifPresent(b::routing);
10021002

10031003
return b;
10041004
});
@@ -1018,11 +1018,7 @@ public DeleteByQueryRequest documentDeleteByQueryRequest(DeleteQuery query, @Nul
10181018
.scroll(time(query.getScroll()))
10191019
.scrollSize(query.getScrollSize());
10201020

1021-
if (query.getRouting() != null) {
1022-
dqb.routing(query.getRouting());
1023-
} else if (StringUtils.hasText(routing)) {
1024-
dqb.routing(routing);
1025-
}
1021+
getRouting(query.getRouting(), routing).ifPresent(dqb::routing);
10261022

10271023
if (query.getQ() != null) {
10281024
dqb.q(query.getQ())
@@ -1102,14 +1098,15 @@ public DeleteByQueryRequest documentDeleteByQueryRequest(DeleteQuery query, @Nul
11021098
uqb
11031099
.doc(query.getDocument())
11041100
.upsert(query.getUpsert())
1105-
.routing(query.getRouting() != null ? query.getRouting() : routing)
11061101
.scriptedUpsert(query.getScriptedUpsert())
11071102
.docAsUpsert(query.getDocAsUpsert())
11081103
.ifSeqNo(query.getIfSeqNo())
11091104
.ifPrimaryTerm(query.getIfPrimaryTerm())
11101105
.refresh(query.getRefreshPolicy() != null ? refresh(query.getRefreshPolicy()) : refresh(refreshPolicy))
11111106
.retryOnConflict(query.getRetryOnConflict());
11121107

1108+
getRouting(query.getRouting(), routing).ifPresent(uqb::routing);
1109+
11131110
if (query.getFetchSource() != null) {
11141111
uqb.source(sc -> sc.fetch(query.getFetchSource()));
11151112
}
@@ -1152,13 +1149,14 @@ public UpdateByQueryRequest documentUpdateByQueryRequest(UpdateQuery updateQuery
11521149
ub //
11531150
.index(Arrays.asList(index.getIndexNames())) //
11541151
.refresh(refreshPolicy == RefreshPolicy.IMMEDIATE) //
1155-
.routing(updateQuery.getRouting()) //
11561152
.script(getScript(updateQuery.getScriptData())) //
11571153
.maxDocs(updateQuery.getMaxDocs() != null ? Long.valueOf(updateQuery.getMaxDocs()) : null) //
11581154
.pipeline(updateQuery.getPipeline()) //
11591155
.requestsPerSecond(updateQuery.getRequestsPerSecond()) //
11601156
.slices(slices(updateQuery.getSlices() != null ? Long.valueOf(updateQuery.getSlices()) : null));
11611157

1158+
getRouting(updateQuery.getRouting()).ifPresent(ub::routing);
1159+
11621160
if (updateQuery.getAbortOnVersionConflict() != null) {
11631161
ub.conflicts(updateQuery.getAbortOnVersionConflict() ? Conflicts.Abort : Conflicts.Proceed);
11641162
}
@@ -1232,12 +1230,7 @@ public <T> SearchRequest searchRequest(Query query, @Nullable String routing, @N
12321230

12331231
builder.query(getQuery(query, clazz));
12341232

1235-
if (StringUtils.hasText(query.getRoute())) {
1236-
builder.routing(query.getRoute());
1237-
}
1238-
if (StringUtils.hasText(routing)) {
1239-
builder.routing(routing);
1240-
}
1233+
getRouting(query.getRoute(), routing).ifPresent(builder::routing);
12411234

12421235
addPostFilter(query, builder);
12431236

@@ -1404,11 +1397,7 @@ private Function<MultisearchHeader.Builder, ObjectBuilder<MultisearchHeader>> ms
14041397
.requestCache(query.getRequestCache()) //
14051398
;
14061399

1407-
if (StringUtils.hasText(query.getRoute())) {
1408-
h.routing(query.getRoute());
1409-
} else if (StringUtils.hasText(routing)) {
1410-
h.routing(routing);
1411-
}
1400+
getRouting(query.getRoute(), routing).ifPresent(h::routing);
14121401

14131402
if (query.getPreference() != null) {
14141403
h.preference(query.getPreference());
@@ -1454,11 +1443,7 @@ private <T> void prepareSearchRequest(Query query, @Nullable String routing, @Nu
14541443
builder.expandWildcards(expandWildcards(expandWildcards));
14551444
}
14561445

1457-
if (query.getRoute() != null) {
1458-
builder.routing(query.getRoute());
1459-
} else if (StringUtils.hasText(routing)) {
1460-
builder.routing(routing);
1461-
}
1446+
getRouting(query.getRoute(), routing).ifPresent(builder::routing);
14621447

14631448
if (query.getPreference() != null) {
14641449
builder.preference(query.getPreference());
@@ -1889,11 +1874,7 @@ public SearchTemplateRequest searchTemplate(SearchTemplateQuery query, @Nullable
18891874
if (query.getSource() != null) {
18901875
builder.source(so -> so.scriptString(query.getSource()));
18911876
}
1892-
if (query.getRoute() != null) {
1893-
builder.routing(query.getRoute());
1894-
} else if (StringUtils.hasText(routing)) {
1895-
builder.routing(routing);
1896-
}
1877+
getRouting(query.getRoute(), routing).ifPresent(builder::routing);
18971878

18981879
var expandWildcards = query.getExpandWildcards();
18991880
if (expandWildcards != null && !expandWildcards.isEmpty()) {
@@ -1993,6 +1974,16 @@ private String getPersistentEntityId(Object entity) {
19931974
return null;
19941975
}
19951976

1977+
Optional<String> getRouting(@Nullable String routing) {
1978+
if (StringUtils.hasText(routing)) {
1979+
return Optional.of(routing);
1980+
}
1981+
return Optional.empty();
1982+
}
1983+
Optional<String> getRouting(@Nullable String routing1, @Nullable String routing2) {
1984+
return getRouting(routing1).or(() -> getRouting(routing2));
1985+
}
1986+
19961987
private VersionType retrieveVersionTypeFromPersistentEntity(@Nullable Class<?> clazz) {
19971988

19981989
ElasticsearchPersistentEntity<?> persistentEntity = getPersistentEntity(clazz);

‎src/test/java/org/springframework/data/elasticsearch/client/elc/RequestConverterTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package org.springframework.data.elasticsearch.client.elc;
1717

1818
import static org.assertj.core.api.Assertions.*;
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
1921

2022
import co.elastic.clients.elasticsearch.core.UpdateRequest;
2123
import co.elastic.clients.json.jackson.JacksonJsonpMapper;
@@ -42,6 +44,7 @@
4244
/**
4345
* @author Peter-Josef Meisch
4446
* @author Han Seungwoo
47+
* @author Steven Pearce
4548
*/
4649
class RequestConverterTest {
4750

@@ -131,4 +134,19 @@ void shouldUseScriptNameAsUpdateQueryId() {
131134
assertThat(updateRequest.script().source().scriptString()).isEqualTo("script");
132135
assertThat(updateRequest.script().id()).isEqualTo("scriptName");
133136
}
137+
138+
@Test
139+
void getRouting() {
140+
141+
assertTrue(requestConverter.getRouting(null).isEmpty());
142+
143+
assertTrue(requestConverter.getRouting(null, null).isEmpty());
144+
assertTrue(requestConverter.getRouting("", null).isEmpty());
145+
146+
assertEquals("1", requestConverter.getRouting("1", null).get());
147+
148+
assertEquals("5", requestConverter.getRouting(null, "5").get());
149+
150+
}
151+
134152
}

0 commit comments

Comments
 (0)