Skip to content

Commit 8bb4ac3

Browse files
acroacmmohsinh
authored andcommitted
DATAES-238 - Fix ElasticsearchTemplate.prepareUpdate(): preserve routing
* need to specify Locale.ENGLISH in String.format() to make tests succeed on non-english locales * that's because the String.format involves floating point numbers which are formatted differently in some locales * ElasticsearchTemplate.prepareUpdate() properly copies routing from given UpdateRequest * add unit tests to ElasticsearchTemplateParentChildTests for update of a child document
1 parent 907571e commit 8bb4ac3

File tree

4 files changed

+72
-4
lines changed

4 files changed

+72
-4
lines changed

src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,7 @@ private UpdateRequestBuilder prepareUpdate(UpdateQuery query) {
538538
Assert.notNull(query.getId(), "No Id define for Query");
539539
Assert.notNull(query.getUpdateRequest(), "No IndexRequest define for Query");
540540
UpdateRequestBuilder updateRequestBuilder = client.prepareUpdate(indexName, type, query.getId());
541+
updateRequestBuilder.setRouting(query.getUpdateRequest().routing());
541542

542543
if (query.getUpdateRequest().script() == null) {
543544
// doc

src/test/java/org/springframework/data/elasticsearch/core/DefaultEntityMapperTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.junit.Assert.*;
2020

2121
import java.io.IOException;
22+
import java.util.Locale;
2223

2324
import org.junit.Before;
2425
import org.junit.Test;
@@ -82,11 +83,11 @@ public void shouldMapGeoPointElasticsearchNames() throws IOException {
8283
//then
8384
assertThat(jsonResult, containsString(pointTemplate("pointA", point)));
8485
assertThat(jsonResult, containsString(pointTemplate("pointB", point)));
85-
assertThat(jsonResult, containsString(String.format("\"%s\":\"%s\"", "pointC", pointAsString)));
86-
assertThat(jsonResult, containsString(String.format("\"%s\":[%.1f,%.1f]", "pointD", pointAsArray[0], pointAsArray[1])));
86+
assertThat(jsonResult, containsString(String.format(Locale.ENGLISH, "\"%s\":\"%s\"", "pointC", pointAsString)));
87+
assertThat(jsonResult, containsString(String.format(Locale.ENGLISH, "\"%s\":[%.1f,%.1f]", "pointD", pointAsArray[0], pointAsArray[1])));
8788
}
8889

8990
private String pointTemplate(String name, Point point) {
90-
return String.format("\"%s\":{\"lat\":%.1f,\"lon\":%.1f}", name, point.getX(), point.getY());
91+
return String.format(Locale.ENGLISH, "\"%s\":{\"lat\":%.1f,\"lon\":%.1f}", name, point.getX(), point.getY());
9192
}
9293
}

src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateParentChildTests.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515
*/
1616
package org.springframework.data.elasticsearch.core;
1717

18+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
1819
import static org.elasticsearch.index.query.QueryBuilders.*;
1920
import static org.hamcrest.Matchers.*;
2021
import static org.junit.Assert.*;
2122

2223
import java.util.List;
2324

25+
import org.elasticsearch.action.RoutingMissingException;
26+
import org.elasticsearch.action.update.UpdateRequest;
27+
import org.elasticsearch.action.update.UpdateResponse;
28+
import org.elasticsearch.common.xcontent.XContentBuilder;
2429
import org.elasticsearch.index.query.QueryBuilder;
2530
import org.elasticsearch.index.query.QueryBuilders;
2631
import org.junit.After;
@@ -30,6 +35,7 @@
3035
import org.springframework.beans.factory.annotation.Autowired;
3136
import org.springframework.data.elasticsearch.core.query.IndexQuery;
3237
import org.springframework.data.elasticsearch.core.query.NativeSearchQuery;
38+
import org.springframework.data.elasticsearch.core.query.UpdateQuery;
3339
import org.springframework.data.elasticsearch.entities.ParentEntity;
3440
import org.springframework.data.elasticsearch.entities.ParentEntity.ChildEntity;
3541
import org.springframework.test.context.ContextConfiguration;
@@ -81,6 +87,55 @@ public void shouldIndexParentChildEntity() {
8187
assertThat("parents", parents, contains(hasProperty("id", is(parent1.getId()))));
8288
}
8389

90+
@Test
91+
public void shouldUpdateChild() throws Exception {
92+
// index parent and child
93+
ParentEntity parent = index("parent", "Parent");
94+
ChildEntity child = index("child", parent.getId(), "Child");
95+
String newChildName = "New Child Name";
96+
97+
// update the child, not forgetting to set the parent id as routing parameter
98+
UpdateRequest updateRequest = new UpdateRequest(ParentEntity.INDEX, ParentEntity.CHILD_TYPE, child.getId());
99+
updateRequest.routing(parent.getId());
100+
XContentBuilder builder;
101+
builder = jsonBuilder().startObject().field("name", newChildName).endObject();
102+
updateRequest.doc(builder);
103+
final UpdateResponse response = update(updateRequest);
104+
105+
assertThat(response.getShardInfo().getSuccessful(), is(1));
106+
}
107+
108+
@Test(expected = RoutingMissingException.class)
109+
public void shouldFailWithRoutingMissingExceptionOnUpdateChildIfNotRoutingSetOnUpdateRequest() throws Exception {
110+
// index parent and child
111+
ParentEntity parent = index("parent", "Parent");
112+
ChildEntity child = index("child", parent.getId(), "Child");
113+
String newChildName = "New Child Name";
114+
115+
// update the child, forget routing parameter
116+
UpdateRequest updateRequest = new UpdateRequest(ParentEntity.INDEX, ParentEntity.CHILD_TYPE, child.getId());
117+
XContentBuilder builder;
118+
builder = jsonBuilder().startObject().field("name", newChildName).endObject();
119+
updateRequest.doc(builder);
120+
update(updateRequest);
121+
}
122+
123+
@Test(expected = RoutingMissingException.class)
124+
public void shouldFailWithRoutingMissingExceptionOnUpdateChildIfRoutingOnlySetOnRequestDoc() throws Exception {
125+
// index parent and child
126+
ParentEntity parent = index("parent", "Parent");
127+
ChildEntity child = index("child", parent.getId(), "Child");
128+
String newChildName = "New Child Name";
129+
130+
// update the child
131+
UpdateRequest updateRequest = new UpdateRequest(ParentEntity.INDEX, ParentEntity.CHILD_TYPE, child.getId());
132+
XContentBuilder builder;
133+
builder = jsonBuilder().startObject().field("name", newChildName).endObject();
134+
updateRequest.doc(builder);
135+
updateRequest.doc().routing(parent.getId());
136+
update(updateRequest);
137+
}
138+
84139
private ParentEntity index(String parentId, String name) {
85140
ParentEntity parent = new ParentEntity(parentId, name);
86141
IndexQuery index = new IndexQuery();
@@ -101,4 +156,13 @@ private ChildEntity index(String childId, String parentId, String name) {
101156

102157
return child;
103158
}
159+
160+
private UpdateResponse update(UpdateRequest updateRequest) {
161+
final UpdateQuery update = new UpdateQuery();
162+
update.setId(updateRequest.id());
163+
update.setType(updateRequest.type());
164+
update.setIndexName(updateRequest.index());
165+
update.setUpdateRequest(updateRequest);
166+
return elasticsearchTemplate.update(update);
167+
}
104168
}

src/test/java/org/springframework/data/elasticsearch/repositories/geo/SpringDataGeoRepositoryTests.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.springframework.test.context.ContextConfiguration;
3030
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
3131

32+
import java.util.Locale;
33+
3234

3335
@RunWith(SpringJUnit4ClassRunner.class)
3436
@ContextConfiguration("classpath:/repository-spring-data-geo-support.xml")
@@ -72,7 +74,7 @@ public void shouldSaveAndLoadGeoPoints() {
7274
}
7375

7476
private String toGeoString(Point point) {
75-
return String.format("%.1f,%.1f", point.getX(), point.getY());
77+
return String.format(Locale.ENGLISH, "%.1f,%.1f", point.getX(), point.getY());
7678
}
7779

7880
private double[] toGeoArray(Point point) {

0 commit comments

Comments
 (0)