Skip to content

Commit

Permalink
Remove types from Graph API (#46935)
Browse files Browse the repository at this point in the history 
The actual usage of types within the graph code was removed by #42112,
so this commit just removes them from the Rest API and high-level client
  • Loading branch information
@romseygeek
romseygeek authored Sep 23, 2019
1 parent b733f9e commit 8927735
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 174 deletions.
2 changes: 1 addition & 1 deletion client/rest-high-level/src/main/java/org/elasticsearch/client/GraphRequestConverters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class GraphRequestConverters {
private GraphRequestConverters() {}

static Request explore(GraphExploreRequest exploreRequest) throws IOException {
String endpoint = RequestConverters.endpoint(exploreRequest.indices(), exploreRequest.types(), "_graph/explore");
String endpoint = RequestConverters.endpoint(exploreRequest.indices(), "_graph/explore");
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
request.setEntity(RequestConverters.createEntity(exploreRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
return request;
Expand Down
40 changes: 7 additions & 33 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/graph/GraphExploreRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public class GraphExploreRequest implements IndicesRequest.Replaceable, ToXConte
public static final String NO_VERTICES_ERROR_MESSAGE = "Graph explore hop must have at least one VertexRequest";
private String[] indices = Strings.EMPTY_ARRAY;
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, false, true, false);
private String[] types = Strings.EMPTY_ARRAY;
private String routing;
private TimeValue timeout;

Expand Down Expand Up @@ -106,31 +105,6 @@ public GraphExploreRequest indicesOptions(IndicesOptions indicesOptions) {
return this;
}

/**
* The document types to execute the explore against. Defaults to be executed against
* all types.
*
* @deprecated Types are in the process of being removed. Instead of using a type, prefer to
* filter on a field on the document.
*/
@Deprecated
public String[] types() {
return this.types;
}

/**
* The document types to execute the explore request against. Defaults to be executed against
* all types.
*
* @deprecated Types are in the process of being removed. Instead of using a type, prefer to
* filter on a field on the document.
*/
@Deprecated
public GraphExploreRequest types(String... types) {
this.types = types;
return this;
}

public String routing() {
return this.routing;
}
Expand All @@ -154,7 +128,7 @@ public TimeValue timeout() {
* operations involved in each hop are limited to the remaining time
* available but can still overrun due to the nature of their "best efforts"
* timeout support. When a timeout occurs partial results are returned.
*
*
* @param timeout
* a {@link TimeValue} object which determines the maximum length
* of time to spend exploring
Expand All @@ -174,7 +148,7 @@ public GraphExploreRequest timeout(String timeout) {

@Override
public String toString() {
return "graph explore [" + Arrays.toString(indices) + "][" + Arrays.toString(types) + "]";
return "graph explore [" + Arrays.toString(indices) + "]";
}

/**
Expand All @@ -190,7 +164,7 @@ public String toString() {
* better with smaller samples as there are less look-ups required for
* background frequencies of terms found in the documents
* </p>
*
*
* @param maxNumberOfDocsPerHop
* shard-level sample size in documents
*/
Expand Down Expand Up @@ -231,7 +205,7 @@ public int maxDocsPerDiversityValue() {
* default value is true which means terms are selected based on
* significance (see the {@link SignificantTerms} aggregation) rather than
* popularity (using the {@link TermsAggregator}).
*
*
* @param value
* true if the significant_terms algorithm should be used.
*/
Expand All @@ -246,7 +220,7 @@ public boolean useSignificance() {
/**
* Return detailed information about vertex frequencies as part of JSON
* results - defaults to false
*
*
* @param value
* true if detailed information is required in JSON responses
*/
Expand All @@ -262,7 +236,7 @@ public boolean returnDetailedInfo() {
* Add a stage in the graph exploration. Each hop represents a stage of
* querying elasticsearch to identify terms which can then be connnected to
* other terms in a subsequent hop.
*
*
* @param guidingQuery
* optional choice of query which influences which documents are
* considered in this stage
Expand Down Expand Up @@ -316,7 +290,7 @@ public float getBoost() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();

builder.startObject("controls");
{
if (sampleSize != SamplerAggregationBuilder.DEFAULT_SHARD_SAMPLE_SIZE) {
Expand Down
5 changes: 2 additions & 3 deletions ...ch/client/GrapRequestConvertersTests.java → ...h/client/GraphRequestConvertersTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@

import static org.hamcrest.Matchers.is;

public class GrapRequestConvertersTests extends ESTestCase {
public class GraphRequestConvertersTests extends ESTestCase {

public void testGraphExplore() throws Exception {
Map<String, String> expectedParams = new HashMap<>();

GraphExploreRequest graphExploreRequest = new GraphExploreRequest();
graphExploreRequest.sampleDiversityField("diversity");
graphExploreRequest.indices("index1", "index2");
graphExploreRequest.types("type1", "type2");
int timeout = randomIntBetween(10000, 20000);
graphExploreRequest.timeout(TimeValue.timeValueMillis(timeout));
graphExploreRequest.useSignificance(randomBoolean());
Expand All @@ -58,7 +57,7 @@ public void testGraphExplore() throws Exception {
}
Request request = GraphRequestConverters.explore(graphExploreRequest);
assertEquals(HttpGet.METHOD_NAME, request.getMethod());
assertEquals("/index1,index2/type1,type2/_graph/explore", request.getEndpoint());
assertEquals("/index1,index2/_graph/explore", request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertThat(request.getEntity().getContentType().getValue(), is(XContentType.JSON.mediaTypeWithoutParameters()));
RequestConvertersTests.assertToXContentBody(graphExploreRequest, request.getEntity());
Expand Down
54 changes: 16 additions & 38 deletions ...plugin/core/src/main/java/org/elasticsearch/protocol/xpack/graph/GraphExploreRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.protocol.xpack.graph;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.IndicesRequest;
Expand All @@ -24,7 +25,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;

/**
Expand All @@ -37,7 +37,6 @@ public class GraphExploreRequest extends ActionRequest implements IndicesRequest
public static final String NO_VERTICES_ERROR_MESSAGE = "Graph explore hop must have at least one VertexRequest";
private String[] indices = Strings.EMPTY_ARRAY;
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, false, true, false);
private String[] types = Strings.EMPTY_ARRAY;
private String routing;
private TimeValue timeout;

Expand Down Expand Up @@ -96,37 +95,15 @@ public GraphExploreRequest indicesOptions(IndicesOptions indicesOptions) {
return this;
}

/**
* The document types to execute the explore against. Defaults to be executed against
* all types.
*
* @deprecated Types are in the process of being removed. Instead of using a type, prefer to
* filter on a field on the document.
*/
@Deprecated
public String[] types() {
return this.types;
}

/**
* The document types to execute the explore request against. Defaults to be executed against
* all types.
*
* @deprecated Types are in the process of being removed. Instead of using a type, prefer to
* filter on a field on the document.
*/
@Deprecated
public GraphExploreRequest types(String... types) {
this.types = types;
return this;
}

public GraphExploreRequest(StreamInput in) throws IOException {
super(in);

indices = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
types = in.readStringArray();
if (in.getVersion().before(Version.V_8_0_0)) {
String[] types = in.readStringArray();
assert types.length == 0;
}
routing = in.readOptionalString();
timeout = in.readOptionalTimeValue();
sampleSize = in.readInt();
Expand Down Expand Up @@ -169,7 +146,7 @@ public TimeValue timeout() {
* operations involved in each hop are limited to the remaining time
* available but can still overrun due to the nature of their "best efforts"
* timeout support. When a timeout occurs partial results are returned.
*
*
* @param timeout
* a {@link TimeValue} object which determines the maximum length
* of time to spend exploring
Expand All @@ -192,7 +169,9 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeStringArray(indices);
indicesOptions.writeIndicesOptions(out);
out.writeStringArray(types);
if (out.getVersion().before(Version.V_8_0_0)) {
out.writeStringArray(Strings.EMPTY_ARRAY);
}
out.writeOptionalString(routing);
out.writeOptionalTimeValue(timeout);

Expand All @@ -203,15 +182,14 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(useSignificance);
out.writeBoolean(returnDetailedInfo);
out.writeInt(hops.size());
for (Iterator<Hop> iterator = hops.iterator(); iterator.hasNext();) {
Hop hop = iterator.next();
for (Hop hop : hops) {
hop.writeTo(out);
}
}

@Override
public String toString() {
return "graph explore [" + Arrays.toString(indices) + "][" + Arrays.toString(types) + "]";
return "graph explore [" + Arrays.toString(indices) + "]";
}

/**
Expand All @@ -227,7 +205,7 @@ public String toString() {
* better with smaller samples as there are less look-ups required for
* background frequencies of terms found in the documents
* </p>
*
*
* @param maxNumberOfDocsPerHop
* shard-level sample size in documents
*/
Expand Down Expand Up @@ -268,7 +246,7 @@ public int maxDocsPerDiversityValue() {
* default value is true which means terms are selected based on
* significance (see the {@link SignificantTerms} aggregation) rather than
* popularity (using the {@link TermsAggregator}).
*
*
* @param value
* true if the significant_terms algorithm should be used.
*/
Expand All @@ -283,7 +261,7 @@ public boolean useSignificance() {
/**
* Return detailed information about vertex frequencies as part of JSON
* results - defaults to false
*
*
* @param value
* true if detailed information is required in JSON responses
*/
Expand All @@ -299,7 +277,7 @@ public boolean returnDetailedInfo() {
* Add a stage in the graph exploration. Each hop represents a stage of
* querying elasticsearch to identify terms which can then be connnected to
* other terms in a subsequent hop.
*
*
* @param guidingQuery
* optional choice of query which influences which documents are
* considered in this stage
Expand Down Expand Up @@ -364,7 +342,7 @@ void writeTo(StreamOutput out) throws IOException {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();

builder.startObject("controls");
{
if (sampleSize != SamplerAggregationBuilder.DEFAULT_SHARD_SAMPLE_SIZE) {
Expand Down
29 changes: 10 additions & 19 deletions ...e/src/main/java/org/elasticsearch/xpack/core/graph/action/GraphExploreRequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

/**
* Creates a new {@link GraphExploreRequestBuilder}
*
*
* @see GraphExploreRequest
*/
public class GraphExploreRequestBuilder extends ActionRequestBuilder<GraphExploreRequest, GraphExploreResponse> {
Expand Down Expand Up @@ -106,16 +106,7 @@ public GraphExploreRequestBuilder setTimeout(String timeout) {
}

/**
* The types of documents the graph exploration will run against. Defaults
* to all types.
*/
public GraphExploreRequestBuilder setTypes(String... types) {
request.types(types);
return this;
}

/**
* Add a stage in the graph exploration. Each hop represents a stage of
* Add a stage in the graph exploration. Each hop represents a stage of
* querying elasticsearch to identify terms which can then be connnected
* to other terms in a subsequent hop.
* @param guidingQuery optional choice of query which influences which documents
Expand All @@ -129,27 +120,27 @@ public Hop createNextHop(@Nullable QueryBuilder guidingQuery) {
/**
* Controls the choice of algorithm used to select interesting terms. The default
* value is true which means terms are selected based on significance (see the {@link SignificantTerms}
* aggregation) rather than popularity (using the {@link TermsAggregator}).
* aggregation) rather than popularity (using the {@link TermsAggregator}).
* @param value true if the significant_terms algorithm should be used.
*/
public GraphExploreRequestBuilder useSignificance(boolean value) {
request.useSignificance(value);
return this;
}


/**
* The number of top-matching documents that are considered during each hop (default is
* The number of top-matching documents that are considered during each hop (default is
* {@link SamplerAggregationBuilder#DEFAULT_SHARD_SAMPLE_SIZE}
* Very small values (less than 50) may not provide sufficient weight-of-evidence to identify
* significant connections between terms.
* <p> Very large values (many thousands) are not recommended with loosely defined queries (fuzzy queries or
* significant connections between terms.
* <p> Very large values (many thousands) are not recommended with loosely defined queries (fuzzy queries or
* those with many OR clauses).
* This is because any useful signals in the best documents are diluted with irrelevant noise from low-quality matches.
* Performance is also typically better with smaller samples as there are less look-ups required for background frequencies
* of terms found in the documents
* Performance is also typically better with smaller samples as there are less look-ups required for background frequencies
* of terms found in the documents
* </p>
*
*
* @param maxNumberOfDocsPerHop the shard-level sample size in documents
*/
public GraphExploreRequestBuilder sampleSize(int maxNumberOfDocsPerHop) {
Expand Down
22 changes: 4 additions & 18 deletions ...plugin/graph/src/main/java/org/elasticsearch/xpack/graph/rest/action/RestGraphAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
public class RestGraphAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGraphAction.class));
public static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
" Specifying types in graph requests is deprecated.";

public static final ParseField TIMEOUT_FIELD = new ParseField("timeout");
public static final ParseField SIGNIFICANCE_FIELD = new ParseField("use_significance");
Expand All @@ -65,20 +63,12 @@ public class RestGraphAction extends BaseRestHandler {
public RestGraphAction(RestController controller) {
// TODO: remove deprecated endpoint in 8.0.0
controller.registerWithDeprecatedHandler(
GET, "/{index}/_graph/explore", this,
GET, "/{index}/_xpack/graph/_explore", deprecationLogger);
GET, "/{index}/_graph/explore", this,
GET, "/{index}/_xpack/graph/_explore", deprecationLogger);
// TODO: remove deprecated endpoint in 8.0.0
controller.registerWithDeprecatedHandler(
POST, "/{index}/_graph/explore", this,
POST, "/{index}/_xpack/graph/_explore", deprecationLogger);
// TODO: remove deprecated endpoint in 8.0.0
controller.registerWithDeprecatedHandler(
GET, "/{index}/{type}/_graph/explore", this,
GET, "/{index}/{type}/_xpack/graph/_explore", deprecationLogger);
// TODO: remove deprecated endpoint in 8.0.0
controller.registerWithDeprecatedHandler(
POST, "/{index}/{type}/_graph/explore", this,
POST, "/{index}/{type}/_xpack/graph/_explore", deprecationLogger);
POST, "/{index}/_graph/explore", this,
POST, "/{index}/_xpack/graph/_explore", deprecationLogger);
}

@Override
Expand Down Expand Up @@ -111,10 +101,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
parseHop(parser, currentHop, graphRequest);
}

if (request.hasParam("type")) {
deprecationLogger.deprecatedAndMaybeLog("graph_with_types", TYPES_DEPRECATION_MESSAGE);
graphRequest.types(Strings.splitStringByCommaToArray(request.param("type")));
}
return channel -> client.execute(INSTANCE, graphRequest, new RestToXContentListener<>(channel));
}

Expand Down
Loading

0 comments on commit 8927735

Please sign in to comment.