Skip to content
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

Merged
8 changes: 8 additions & 0 deletions .github/workflows/test-integration-unreleased.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,11 @@ jobs:
run: |
cd opensearch-java
./gradlew clean integrationTest -Dhttps=false

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

- name: Upload Reports
if: failure()
uses: actions/upload-artifact@v3
with:
name: test-reports
path: opensearch-java/java-client/build/reports/
retention-days: 30
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably 30 days is a lot, 7 days? (a week)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value is 90 ->
updated to 7.

Should be enough to read the report :D

8 changes: 8 additions & 0 deletions .github/workflows/test-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ jobs:
- name: Run Integration Test
run: ./gradlew clean integrationTest

- name: Upload Reports
if: failure()
uses: actions/upload-artifact@v3
with:
name: test-reports
path: java-client/build/reports/
retention-days: 30

- name: Stop Docker
run: |
docker-compose --project-directory .ci/opensearch down
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not very straightforward: definitely 👍 to move off the Date, but using string to pass the dates through is not a best option I think:

  • should we use LocateDateTime / Instant instead` ?
  • should be use the timestamp value (long)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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:

@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ssZ", timezone = "UTC")
private Instant expDate;

build.gradle.kts:
testImplementation("com.fasterxml.jackson.datatype", "jackson-datatype-jsr310", jacksonVersion)

JacksonJsonpMapper -> findAndRegisterModules method used:

new ObjectMapper()
                .findAndRegisterModules()
                .configure(SerializationFeature.INDENT_OUTPUT, false)
                .setSerializationInclusion(JsonInclude.Include.NON_NULL)

private Long expDate; does not require anything more.
Seems it works but the minus is that the Document is created with numeric values:

  "_index": "test-date-range-aggregation",
  "_type": "_doc",
  "_id": "1",
  "_score": 1,
  "_source": {
    "name": "egg",
    "cost": 2,
    "expDate": 1676937600000
  }
}

So I'm not sure what is better than simple String ;P

Copy link
Collaborator

@reta reta Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String allows to insert date in ISO format without any transformations.

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).

private Long expDate; does not require anything more.

That's fine, the OS is able to deal with date in different formats.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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{
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,166 +8,18 @@

package org.opensearch.client.opensearch.integTest.restclient;

import com.fasterxml.jackson.annotation.JsonFormat;
import org.apache.hc.core5.http.HttpHost;
import org.junit.Test;
import org.opensearch.client.json.jackson.JacksonJsonpMapper;
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.core.SearchResponse;
import org.opensearch.client.opensearch.integTest.AbstractRequestIT;
import org.opensearch.client.opensearch.integTest.AbstractAggregationRequestIT;
import org.opensearch.client.transport.OpenSearchTransport;
import org.opensearch.client.transport.rest_client.RestClientTransport;
import org.opensearch.common.settings.Settings;

import java.io.IOException;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.Date;
import java.util.List;

public class AggregationRequestIT extends AbstractRequestIT {

public class AggregationRequestIT extends AbstractAggregationRequestIT {
@Override
public OpenSearchTransport buildTransport(Settings settings, HttpHost[] hosts) throws IOException {
return new RestClientTransport(buildClient(settings, hosts), new JacksonJsonpMapper());
}

@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).getTime()))
.to(FieldDateMath.of(builder -> builder.value((double) getDatePlusDays(3).getTime() - 1000)))
.key("from-1-to-2-days")
.build(),
new DateRangeExpression.Builder()
.from(builder -> builder.value((double) getDatePlusDays(3).getTime()))
.to(FieldDateMath.of(builder -> builder.value((double) getDatePlusDays(5).getTime() - 1000)))
.key("from-3-to-4-days")
.build(),
new DateRangeExpression.Builder()
.from(builder -> builder.value((double) getDatePlusDays(5).getTime()))
.to(FieldDateMath.of(builder -> builder.value((double) getDatePlusDays(7).getTime() - 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 {
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 ProductDetails createProduct(String name, int cost, int plusDays) {
return new ProductDetails(name, cost, getDatePlusDays(plusDays));
}

private Date getDatePlusDays(int plusDays) {
return java.sql.Date.from(LocalDateTime.of(2023, 2, 20, 0, 0, 0).plusDays(plusDays).toInstant(ZoneOffset.UTC));
}

public static class ProductDetails {
private String name;
private int cost;
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss.SSS")
private Date expDate;

public ProductDetails() {
}

public ProductDetails(String name, int cost, Date expDate) {
this.name = name;
this.cost = cost;
this.expDate = expDate;
}

public String getName() {
return name;
}

public int getCost() {
return cost;
}

public Date getExpDate() {
return expDate;
}
}
}