DATAES-312 - Fix NullHandling.NULLS_LAST in query.sort

Original pull request: #163

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

@@ -66,7 +66,9 @@ import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder;
+import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
+import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.search.suggest.SuggestBuilder;
 import org.slf4j.Logger;
@@ -111,6 +113,7 @@ import org.springframework.util.Assert;
  * @author Ilkang Na
  * @author Alen Turkovic
  * @author Sascha Woo
+ * @author Ted Liang
  */
 public class ElasticsearchTemplate implements ElasticsearchOperations, ApplicationContextAware {
 
@@ -1011,8 +1014,15 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati
 
 		if (query.getSort() != null) {
 			for (Sort.Order order : query.getSort()) {
-				searchRequestBuilder.addSort(order.getProperty(),
+				FieldSortBuilder sort = SortBuilders.fieldSort(order.getProperty())
-						order.getDirection() == Sort.Direction.DESC ? SortOrder.DESC : SortOrder.ASC);
+						.order(order.getDirection().isDescending() ? SortOrder.DESC : SortOrder.ASC);
+				if (order.getNullHandling() == Sort.NullHandling.NULLS_FIRST) {
+					sort.missing("_first");
+				}
+				else if (order.getNullHandling() == Sort.NullHandling.NULLS_LAST) {
+					sort.missing("_last");
+				}
+				searchRequestBuilder.addSort(sort);
 			}
 		}
 

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

@@ -494,6 +494,106 @@ public class ElasticsearchTemplateTests {
 		assertThat(sampleEntities.getContent().get(1).getMessage(), is(sampleEntity1.getMessage()));
 	}
 
+	@Test // DATAES-312
+	public void shouldSortResultsGivenNullFirstSortCriteria() {
+		// given
+		List<IndexQuery> indexQueries;
+
+		// first document
+		String documentId = randomNumeric(5);
+		SampleEntity sampleEntity1 = SampleEntity.builder()
+			.id(documentId)
+			.message("abc")
+			.rate(15)
+			.version(System.currentTimeMillis())
+			.build();
+
+		// second document
+		String documentId2 = randomNumeric(5);
+		SampleEntity sampleEntity2 = SampleEntity.builder()
+			.id(documentId2)
+			.message("xyz")
+			.rate(5)
+			.version(System.currentTimeMillis())
+			.build();
+
+		// third document
+		String documentId3 = randomNumeric(5);
+		SampleEntity sampleEntity3 = SampleEntity.builder()
+			.id(documentId3)
+			.rate(10)
+			.version(System.currentTimeMillis())
+			.build();
+
+		indexQueries = getIndexQueries(Arrays.asList(sampleEntity1, sampleEntity2, sampleEntity3));
+
+		elasticsearchTemplate.bulkIndex(indexQueries);
+		elasticsearchTemplate.refresh(SampleEntity.class);
+
+		SearchQuery searchQuery = new NativeSearchQueryBuilder()
+			.withQuery(matchAllQuery())
+			.withPageable(PageRequest.of(0, 10, Sort.by(Sort.Order.asc("message").nullsFirst())))
+			.build();
+
+		// when
+		Page<SampleEntity> sampleEntities = elasticsearchTemplate.queryForPage(searchQuery, SampleEntity.class);
+
+		// then
+		assertThat(sampleEntities.getTotalElements(), equalTo(3L));
+		assertThat(sampleEntities.getContent().get(0).getRate(), is(sampleEntity3.getRate()));
+		assertThat(sampleEntities.getContent().get(1).getMessage(), is(sampleEntity1.getMessage()));
+	}
+
+	@Test // DATAES-312
+	public void shouldSortResultsGivenNullLastSortCriteria() {
+		// given
+		List<IndexQuery> indexQueries;
+
+		// first document
+		String documentId = randomNumeric(5);
+		SampleEntity sampleEntity1 = SampleEntity.builder()
+			.id(documentId)
+			.message("abc")
+			.rate(15)
+			.version(System.currentTimeMillis())
+			.build();
+
+		// second document
+		String documentId2 = randomNumeric(5);
+		SampleEntity sampleEntity2 = SampleEntity.builder()
+			.id(documentId2)
+			.message("xyz")
+			.rate(5)
+			.version(System.currentTimeMillis())
+			.build();
+
+		// third document
+		String documentId3 = randomNumeric(5);
+		SampleEntity sampleEntity3 = SampleEntity.builder()
+			.id(documentId3)
+			.rate(10)
+			.version(System.currentTimeMillis())
+			.build();
+
+		indexQueries = getIndexQueries(Arrays.asList(sampleEntity1, sampleEntity2, sampleEntity3));
+
+		elasticsearchTemplate.bulkIndex(indexQueries);
+		elasticsearchTemplate.refresh(SampleEntity.class);
+
+		SearchQuery searchQuery = new NativeSearchQueryBuilder()
+			.withQuery(matchAllQuery())
+			.withPageable(PageRequest.of(0, 10, Sort.by(Sort.Order.asc("message").nullsLast())))
+			.build();
+
+		// when
+		Page<SampleEntity> sampleEntities = elasticsearchTemplate.queryForPage(searchQuery, SampleEntity.class);
+
+		// then
+		assertThat(sampleEntities.getTotalElements(), equalTo(3L));
+		assertThat(sampleEntities.getContent().get(0).getRate(), is(sampleEntity1.getRate()));
+		assertThat(sampleEntities.getContent().get(1).getMessage(), is(sampleEntity2.getMessage()));
+	}
+
 	@Test
 	public void shouldExecuteStringQuery() {
 		// given
@@ -2100,4 +2200,4 @@ public class ElasticsearchTemplateTests {
 			this.lastName = lastName;
 		}
 	}
-}
+}