-
Notifications
You must be signed in to change notification settings - Fork 194
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.
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
refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421
Changes from 10 commits
65ed8c3
d938d19
bf9b658
51e0821
11210c4
e168d8d
5ee1c48
986d6dc
71944df
c8f6e5e
26753a9
bc4eaec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,3 +55,11 @@ jobs: | |
run: | | ||
cd opensearch-java | ||
./gradlew clean integrationTest -Dhttps=false | ||
|
||
- name: Upload Reports | ||
if: failure() | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: test-reports | ||
path: opensearch-java/java-client/build/reports/ | ||
retention-days: 30 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably 30 days is a lot, 7 days? (a week) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default value is 90 -> Should be enough to read the report :D |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
*/ | ||
|
||
package org.opensearch.client.opensearch.integTest; | ||
|
||
import org.junit.Test; | ||
import org.opensearch.client.opensearch._types.Refresh; | ||
import org.opensearch.client.opensearch._types.aggregations.Aggregation; | ||
import org.opensearch.client.opensearch._types.aggregations.AggregationRange; | ||
import org.opensearch.client.opensearch._types.aggregations.DateRangeAggregation; | ||
import org.opensearch.client.opensearch._types.aggregations.DateRangeExpression; | ||
import org.opensearch.client.opensearch._types.aggregations.FieldDateMath; | ||
import org.opensearch.client.opensearch._types.aggregations.RangeAggregation; | ||
import org.opensearch.client.opensearch._types.mapping.Property; | ||
import org.opensearch.client.opensearch.core.SearchResponse; | ||
|
||
import java.io.IOException; | ||
import java.time.Instant; | ||
import java.time.LocalDateTime; | ||
import java.time.ZoneOffset; | ||
import java.util.List; | ||
|
||
public abstract class AbstractAggregationRequestIT extends OpenSearchJavaClientTestCase { | ||
|
||
@Test | ||
public void testValueRangeAggregation() throws Exception { | ||
var index = "test-value-range-aggregation"; | ||
createDateRangeDocuments(index); | ||
var searchResponse = sendAggregateRequest(index, "cost_ranges", getCostValueRangeAggregation()); | ||
var costRangesAggregations = searchResponse.aggregations().get("cost_ranges"); | ||
var buckets = costRangesAggregations._get() | ||
._toAggregate() | ||
.range() | ||
.buckets() | ||
.array(); | ||
|
||
assertEquals(3, buckets.size()); | ||
assertEquals(2, buckets.get(0).docCount()); | ||
assertEquals(2, buckets.get(1).docCount()); | ||
assertEquals(2, buckets.get(2).docCount()); | ||
} | ||
|
||
@Test | ||
public void testDateRangeAggregation() throws Exception { | ||
var index = "test-date-range-aggregation"; | ||
createDateRangeDocuments(index); | ||
var searchResponse = sendAggregateRequest(index, "expiry_ranges", getExpiryDateRangeAggregation()); | ||
var expiryRangesAggregations = searchResponse.aggregations().get("expiry_ranges"); | ||
var buckets = expiryRangesAggregations._get() | ||
._toAggregate() | ||
.dateRange() | ||
.buckets() | ||
.array(); | ||
|
||
assertEquals(3, buckets.size()); | ||
assertEquals(2, buckets.get(0).docCount()); | ||
assertEquals(2, buckets.get(1).docCount()); | ||
assertEquals(2, buckets.get(2).docCount()); | ||
} | ||
|
||
private Aggregation getExpiryDateRangeAggregation() { | ||
DateRangeAggregation expiryDateRangeAggregation = new DateRangeAggregation.Builder() | ||
.field("expDate") | ||
.ranges(getDateAggregationRanges()) | ||
.build(); | ||
return new Aggregation.Builder().dateRange(expiryDateRangeAggregation).build(); | ||
} | ||
|
||
private Aggregation getCostValueRangeAggregation() { | ||
RangeAggregation costValueRangeAggregation = new RangeAggregation.Builder() | ||
.field("cost") | ||
.ranges(getValueAggregationRanges()) | ||
.build(); | ||
return new Aggregation.Builder().range(costValueRangeAggregation).build(); | ||
} | ||
|
||
private SearchResponse<Void> sendAggregateRequest(String index, String key, Aggregation value) throws IOException { | ||
return javaClient().search( | ||
request -> request.index(index) | ||
.size(0) | ||
.aggregations(key, value), | ||
Void.class); | ||
} | ||
|
||
private List<DateRangeExpression> getDateAggregationRanges() { | ||
return List.of( | ||
new DateRangeExpression.Builder() | ||
.from(builder -> builder.value((double) getDatePlusDays(1).toEpochMilli())) | ||
.to(FieldDateMath.of( | ||
builder -> builder.value((double) getDatePlusDays(3).toEpochMilli() - 1000)) | ||
) | ||
.key("from-1-to-2-days") | ||
.build(), | ||
new DateRangeExpression.Builder() | ||
.from(builder -> builder.value((double) getDatePlusDays(3).toEpochMilli())) | ||
.to(FieldDateMath.of( | ||
builder -> builder.value((double) getDatePlusDays(5).toEpochMilli() - 1000)) | ||
) | ||
.key("from-3-to-4-days") | ||
.build(), | ||
new DateRangeExpression.Builder() | ||
.from(builder -> builder.value((double) getDatePlusDays(5).toEpochMilli())) | ||
.to(FieldDateMath.of( | ||
builder -> builder.value((double) getDatePlusDays(7).toEpochMilli() - 1000)) | ||
) | ||
.key("from-5-to-6-days") | ||
.build() | ||
); | ||
} | ||
|
||
private List<AggregationRange> getValueAggregationRanges() { | ||
return List.of( | ||
new AggregationRange.Builder().to("10").build(), | ||
new AggregationRange.Builder().from("10").to("30").build(), | ||
new AggregationRange.Builder().from("30").build() | ||
); | ||
} | ||
|
||
private void createDateRangeDocuments(String index) throws IOException { | ||
createIndex(index); | ||
javaClient().create(_1 -> _1.index(index).id("1").document(createProduct("egg", 2, 1)).refresh(Refresh.True)); | ||
javaClient().create(_1 -> _1.index(index).id("2").document(createProduct("meat", 15, 2)).refresh(Refresh.True)); | ||
javaClient().create(_1 -> _1.index(index).id("3").document(createProduct("ham", 30, 3)).refresh(Refresh.True)); | ||
javaClient().create(_1 -> _1.index(index).id("4").document(createProduct("cheese", 25, 4)).refresh(Refresh.True)); | ||
javaClient().create(_1 -> _1.index(index).id("5").document(createProduct("pasta", 8, 5)).refresh(Refresh.True)); | ||
javaClient().create(_1 -> _1.index(index).id("6").document(createProduct("oil", 50, 6)).refresh(Refresh.True)); | ||
} | ||
|
||
private void createIndex(String index) throws IOException { | ||
Property nameValueProp = new Property.Builder() | ||
.text(v -> v) | ||
.build(); | ||
Property costValueProp = new Property.Builder() | ||
.integer(v -> v) | ||
.build(); | ||
Property expDateKeywordProp = new Property.Builder() | ||
.date(v -> v) | ||
.build(); | ||
javaClient().indices().create(c -> c.index(index) | ||
.mappings(m -> m.properties("name", nameValueProp) | ||
.properties("cost", costValueProp) | ||
.properties("expDate", expDateKeywordProp) | ||
) | ||
); | ||
} | ||
|
||
private ProductDetails createProduct(String name, int cost, int plusDays) { | ||
return new ProductDetails(name, cost, getDatePlusDays(plusDays).toString()); | ||
} | ||
|
||
private Instant getDatePlusDays(int plusDays) { | ||
return LocalDateTime.of(2023, 2, 20, 0, 0, 0, 0).plusDays(plusDays).toInstant(ZoneOffset.UTC); | ||
} | ||
|
||
public static class ProductDetails { | ||
private String name; | ||
private int cost; | ||
private String expDate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is not very straightforward: definitely 👍 to move off the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. String allows to insert date in ISO format without any transformations. LocateDateTime and Instant are requiring:
build.gradle.kts: JacksonJsonpMapper -> findAndRegisterModules method used:
private Long expDate; does not require anything more.
So I'm not sure what is better than simple String ;P There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is what `@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = ISO) is for, piratically this is how the feature should be used (yes, using string does simplify tests).
That's fine, the OS is able to deal with date in different formats. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, let's stick with Long expDate. |
||
|
||
public ProductDetails() { | ||
} | ||
|
||
public ProductDetails(String name, int cost, String expDate) { | ||
this.name = name; | ||
this.cost = cost; | ||
this.expDate = expDate; | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
||
public int getCost() { | ||
return cost; | ||
} | ||
|
||
public String getExpDate() { | ||
return expDate; | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
*/ | ||
|
||
package org.opensearch.client.opensearch.integTest.httpclient5; | ||
|
||
import org.opensearch.client.opensearch.integTest.AbstractAggregationRequestIT; | ||
|
||
public class AggregationRequestIT extends AbstractAggregationRequestIT implements HttpClient5TransportSupport{ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍