Skip to content

[ML-DataFrame] migrate to PageParams for get and stats, move PageParams into core #41851

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

Merged
merged 3 commits into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.dataframe.DeleteDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformStatsRequest;
Expand Down Expand Up @@ -57,11 +58,11 @@ static Request getDataFrameTransform(GetDataFrameTransformRequest getRequest) {
.addPathPart(Strings.collectionToCommaDelimitedString(getRequest.getId()))
.build();
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
if (getRequest.getFrom() != null) {
request.addParameter("from", getRequest.getFrom().toString());
if (getRequest.getPageParams() != null && getRequest.getPageParams().getFrom() != null) {
request.addParameter(PageParams.FROM.getPreferredName(), getRequest.getPageParams().getFrom().toString());
}
if (getRequest.getSize() != null) {
request.addParameter("size", getRequest.getSize().toString());
if (getRequest.getPageParams() != null && getRequest.getPageParams().getSize() != null) {
request.addParameter(PageParams.SIZE.getPreferredName(), getRequest.getPageParams().getSize().toString());
}
return request;
}
Expand Down Expand Up @@ -120,6 +121,13 @@ static Request getDataFrameTransformStats(GetDataFrameTransformStatsRequest stat
.addPathPart(statsRequest.getId())
.addPathPartAsIs("_stats")
.build();
return new Request(HttpGet.METHOD_NAME, endpoint);
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
if (statsRequest.getPageParams() != null && statsRequest.getPageParams().getFrom() != null) {
request.addParameter(PageParams.FROM.getPreferredName(), statsRequest.getPageParams().getFrom().toString());
}
if (statsRequest.getPageParams() != null && statsRequest.getPageParams().getSize() != null) {
request.addParameter(PageParams.SIZE.getPreferredName(), statsRequest.getPageParams().getSize().toString());
}
return request;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.http.nio.entity.NByteArrayEntity;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.client.RequestConverters.EndpointBuilder;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.CloseJobRequest;
import org.elasticsearch.client.ml.DeleteCalendarEventRequest;
import org.elasticsearch.client.ml.DeleteCalendarJobRequest;
Expand Down Expand Up @@ -71,7 +72,6 @@
import org.elasticsearch.client.ml.UpdateFilterRequest;
import org.elasticsearch.client.ml.UpdateJobRequest;
import org.elasticsearch.client.ml.UpdateModelSnapshotRequest;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.XContentType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.client.ml.job.util;
package org.elasticsearch.client.core;

import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
Expand Down Expand Up @@ -57,11 +57,11 @@ public PageParams(@Nullable Integer from, @Nullable Integer size) {
this.size = size;
}

public int getFrom() {
public Integer getFrom() {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, I did the same thing in feature branch:
https://github.com/elastic/elasticsearch/pull/41530/files

return from;
}

public int getSize() {
public Integer getSize() {
return size;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.client.Validatable;
import org.elasticsearch.client.ValidationException;
import org.elasticsearch.client.core.PageParams;

import java.util.Arrays;
import java.util.List;
Expand All @@ -29,10 +30,6 @@

public class GetDataFrameTransformRequest implements Validatable {

private final List<String> ids;
private Integer from;
private Integer size;

/**
* Helper method to create a request that will get ALL Data Frame Transforms
* @return new {@link GetDataFrameTransformRequest} object for the id "_all"
Expand All @@ -41,6 +38,9 @@ public static GetDataFrameTransformRequest getAllDataFrameTransformsRequest() {
return new GetDataFrameTransformRequest("_all");
}

private final List<String> ids;
private PageParams pageParams;

public GetDataFrameTransformRequest(String... ids) {
this.ids = Arrays.asList(ids);
}
Expand All @@ -49,20 +49,12 @@ public List<String> getId() {
return ids;
}

public Integer getFrom() {
return from;
}

public void setFrom(Integer from) {
this.from = from;
}

public Integer getSize() {
return size;
public PageParams getPageParams() {
return pageParams;
}

public void setSize(Integer size) {
this.size = size;
public void setPageParams(PageParams pageParams) {
this.pageParams = pageParams;
}

@Override
Expand All @@ -78,7 +70,7 @@ public Optional<ValidationException> validate() {

@Override
public int hashCode() {
return Objects.hash(ids);
return Objects.hash(ids, pageParams);
}

@Override
Expand All @@ -91,6 +83,6 @@ public boolean equals(Object obj) {
return false;
}
GetDataFrameTransformRequest other = (GetDataFrameTransformRequest) obj;
return Objects.equals(ids, other.ids);
return Objects.equals(ids, other.ids) && Objects.equals(pageParams, other.pageParams);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@

import org.elasticsearch.client.Validatable;
import org.elasticsearch.client.ValidationException;
import org.elasticsearch.client.core.PageParams;

import java.util.Objects;
import java.util.Optional;

public class GetDataFrameTransformStatsRequest implements Validatable {
private final String id;
private PageParams pageParams;

public GetDataFrameTransformStatsRequest(String id) {
this.id = id;
Expand All @@ -36,6 +38,14 @@ public String getId() {
return id;
}

public PageParams getPageParams() {
return pageParams;
}

public void setPageParams(PageParams pageParams) {
this.pageParams = pageParams;
}

@Override
public Optional<ValidationException> validate() {
if (id == null) {
Expand All @@ -49,7 +59,7 @@ public Optional<ValidationException> validate() {

@Override
public int hashCode() {
return Objects.hash(id);
return Objects.hash(id, pageParams);
}

@Override
Expand All @@ -62,6 +72,6 @@ public boolean equals(Object obj) {
return false;
}
GetDataFrameTransformStatsRequest other = (GetDataFrameTransformStatsRequest) obj;
return Objects.equals(id, other.id);
return Objects.equals(id, other.id) && Objects.equals(pageParams, other.pageParams);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.results.Result;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.calendars.Calendar;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.calendars.Calendar;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;

import org.elasticsearch.client.ml.job.util.PageParams;

import java.io.IOException;
import java.util.Objects;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.MlFilter;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
package org.elasticsearch.client.ml;

import org.elasticsearch.client.Validatable;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.job.config.Job;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.dataframe.DeleteDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformStatsRequest;
Expand All @@ -43,7 +44,9 @@
import java.io.IOException;
import java.util.Collections;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;

public class DataFrameRequestConvertersTests extends ESTestCase {

Expand Down Expand Up @@ -147,6 +150,23 @@ public void testGetDataFrameTransformStats() {

assertEquals(HttpGet.METHOD_NAME, request.getMethod());
assertThat(request.getEndpoint(), equalTo("/_data_frame/transforms/foo/_stats"));

assertFalse(request.getParameters().containsKey("from"));
assertFalse(request.getParameters().containsKey("size"));

getStatsRequest.setPageParams(new PageParams(0, null));
request = DataFrameRequestConverters.getDataFrameTransformStats(getStatsRequest);
assertThat(request.getParameters(), hasEntry("from", "0"));
assertEquals(null, request.getParameters().get("size"));

getStatsRequest.setPageParams(new PageParams(null, 50));
request = DataFrameRequestConverters.getDataFrameTransformStats(getStatsRequest);
assertEquals(null, request.getParameters().get("from"));
assertThat(request.getParameters(), hasEntry("size", "50"));

getStatsRequest.setPageParams(new PageParams(0, 10));
request = DataFrameRequestConverters.getDataFrameTransformStats(getStatsRequest);
assertThat(request.getParameters(), allOf(hasEntry("from", "0"), hasEntry("size", "10")));
}

public void testGetDataFrameTransform() {
Expand All @@ -159,11 +179,19 @@ public void testGetDataFrameTransform() {
assertFalse(request.getParameters().containsKey("from"));
assertFalse(request.getParameters().containsKey("size"));

getRequest.setFrom(0);
getRequest.setSize(10);
getRequest.setPageParams(new PageParams(0, null));
request = DataFrameRequestConverters.getDataFrameTransform(getRequest);
assertThat(request.getParameters(), hasEntry("from", "0"));
assertEquals(null, request.getParameters().get("size"));

getRequest.setPageParams(new PageParams(null, 50));
request = DataFrameRequestConverters.getDataFrameTransform(getRequest);
assertEquals(null, request.getParameters().get("from"));
assertThat(request.getParameters(), hasEntry("size", "50"));

getRequest.setPageParams(new PageParams(0, 10));
request = DataFrameRequestConverters.getDataFrameTransform(getRequest);
assertEquals("0", request.getParameters().get("from"));
assertEquals("10", request.getParameters().get("size"));
assertThat(request.getParameters(), allOf(hasEntry("from", "0"), hasEntry("size", "10")));
}

public void testGetDataFrameTransform_givenMulitpleIds() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.client.core.AcknowledgedResponse;
import org.elasticsearch.client.core.IndexerState;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.dataframe.DeleteDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformRequest;
import org.elasticsearch.client.dataframe.GetDataFrameTransformResponse;
Expand Down Expand Up @@ -217,8 +218,7 @@ public void testGetAllAndPageTransforms() throws IOException {
assertThat(getResponse.getTransformConfigurations(), hasSize(2));
assertEquals(transform, getResponse.getTransformConfigurations().get(1));

getRequest.setFrom(0);
getRequest.setSize(1);
getRequest.setPageParams(new PageParams(0,1));
getResponse = execute(getRequest, client::getDataFrameTransform,
client::getDataFrameTransformAsync);
assertNull(getResponse.getInvalidTransforms());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.CloseJobRequest;
import org.elasticsearch.client.ml.DeleteCalendarEventRequest;
import org.elasticsearch.client.ml.DeleteCalendarJobRequest;
Expand Down Expand Up @@ -82,7 +83,6 @@
import org.elasticsearch.client.ml.job.config.JobUpdateTests;
import org.elasticsearch.client.ml.job.config.MlFilter;
import org.elasticsearch.client.ml.job.config.MlFilterTests;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.client.core.PageParams;
import org.elasticsearch.client.ml.GetBucketsRequest;
import org.elasticsearch.client.ml.GetBucketsResponse;
import org.elasticsearch.client.ml.GetCategoriesRequest;
Expand All @@ -43,7 +44,6 @@
import org.elasticsearch.client.ml.job.results.Bucket;
import org.elasticsearch.client.ml.job.results.Influencer;
import org.elasticsearch.client.ml.job.results.OverallBucket;
import org.elasticsearch.client.ml.job.util.PageParams;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentType;
import org.junit.After;
Expand Down
Loading