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.

Sign up for GitHub

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

Allow RestHighLevelClient to use plugins #25024

Closed
wants to merge 1 commit into from
Closed

Allow RestHighLevelClient to use plugins #25024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
75 changes: 65 additions & 10 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.common.CheckedFunction;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ContextParser;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.join.aggregations.ChildrenAggregationBuilder;
import org.elasticsearch.join.aggregations.ParsedChildren;
import org.elasticsearch.join.ParentJoinPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.plugins.SearchPlugin;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.aggregations.Aggregation;
Expand Down Expand Up @@ -90,8 +93,7 @@
import org.elasticsearch.search.aggregations.bucket.terms.ParsedLongTerms;
import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringTerms;
import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
import org.elasticsearch.search.aggregations.matrix.stats.MatrixStatsAggregationBuilder;
import org.elasticsearch.search.aggregations.matrix.stats.ParsedMatrixStats;
import org.elasticsearch.search.aggregations.matrix.MatrixAggregationPlugin;
import org.elasticsearch.search.aggregations.metrics.avg.AvgAggregationBuilder;
import org.elasticsearch.search.aggregations.metrics.avg.ParsedAvg;
import org.elasticsearch.search.aggregations.metrics.cardinality.CardinalityAggregationBuilder;
Expand Down Expand Up @@ -140,6 +142,9 @@
import org.elasticsearch.search.suggest.term.TermSuggestion;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -160,23 +165,52 @@
* Can be sub-classed to expose additional client methods that make use of endpoints added to Elasticsearch through plugins, or to
* add support for custom response sections, again added to Elasticsearch through plugins.
*/
@SuppressWarnings("varargs")
Copy link
Member

Choose a reason for hiding this comment

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

Nit: curious about what this does, it shows up as "Unsupported @SuppressWarnings" in Eclipse

public class RestHighLevelClient {

static final Collection<Class<? extends Plugin>> PRE_INSTALLED_PLUGINS =
Collections.unmodifiableList(Arrays.asList(ParentJoinPlugin.class, MatrixAggregationPlugin.class));

private final RestClient client;
private final NamedXContentRegistry registry;

/**
* Creates a {@link RestHighLevelClient} given the low level {@link RestClient} that it should use to perform requests.
* Creates a {@link RestHighLevelClient} with a given low level {@link RestClient} and pre installed plugins
*
* @param restClient the low level {@link RestClient} used to perform requests
* @param plugins an optional array of additional plugins to run with the {@link RestHighLevelClient}
*/
@SafeVarargs
public RestHighLevelClient(RestClient restClient, Class<? extends Plugin>... plugins) {
this(restClient, Arrays.asList(plugins));
}

/**
* Creates a {@link RestHighLevelClient} with a given low level {@link RestClient} and pre installed plugins
*
* @param restClient the low level {@link RestClient} used to perform requests
* @param plugins a collection of additional plugins to run with the {@link RestHighLevelClient}
*/
public RestHighLevelClient(RestClient restClient) {
this(restClient, Collections.emptyList());
public RestHighLevelClient(RestClient restClient, Collection<Class<? extends Plugin>> plugins) {
this(restClient, Settings.EMPTY, plugins);
}

/**
* Creates a {@link RestHighLevelClient} with a given low level {@link RestClient} and pre installed plugins
*
* @param restClient the low level {@link RestClient} used to perform requests
* @param settings the settings passed to the {@link RestHighLevelClient}
* @param plugins a collection of additional plugins to run with the {@link RestHighLevelClient}
*/
public RestHighLevelClient(RestClient restClient, Settings settings, Collection<Class<? extends Plugin>> plugins) {
this(restClient, pluginsNamedXContents(settings, PRE_INSTALLED_PLUGINS, plugins));
}

/**
* Creates a {@link RestHighLevelClient} given the low level {@link RestClient} that it should use to perform requests and
* a list of entries that allow to parse custom response sections added to Elasticsearch through plugins.
*/
protected RestHighLevelClient(RestClient restClient, List<NamedXContentRegistry.Entry> namedXContentEntries) {
private RestHighLevelClient(RestClient restClient, List<NamedXContentRegistry.Entry> namedXContentEntries) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Could this be moved into the ctor where it is called now?

this.client = Objects.requireNonNull(restClient);
this.registry = new NamedXContentRegistry(Stream.of(getDefaultNamedXContents().stream(), namedXContentEntries.stream())
.flatMap(Function.identity()).collect(toList()));
Expand Down Expand Up @@ -542,8 +576,6 @@ static List<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
map.put(SignificantLongTerms.NAME, (p, c) -> ParsedSignificantLongTerms.fromXContent(p, (String) c));
map.put(SignificantStringTerms.NAME, (p, c) -> ParsedSignificantStringTerms.fromXContent(p, (String) c));
map.put(ScriptedMetricAggregationBuilder.NAME, (p, c) -> ParsedScriptedMetric.fromXContent(p, (String) c));
map.put(ChildrenAggregationBuilder.NAME, (p, c) -> ParsedChildren.fromXContent(p, (String) c));
map.put(MatrixStatsAggregationBuilder.NAME, (p, c) -> ParsedMatrixStats.fromXContent(p, (String) c));
Copy link
Member

Choose a reason for hiding this comment

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

Why are these removed? Should the user have to specify them as client plugins explicitely, even if they are bundled with core by default?

Copy link
Member

Choose a reason for hiding this comment

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

I think I understand now, is it because they are part of PRE_INSTALLED_PLUGINS?

List<NamedXContentRegistry.Entry> entries = map.entrySet().stream()
.map(entry -> new NamedXContentRegistry.Entry(Aggregation.class, new ParseField(entry.getKey()), entry.getValue()))
.collect(Collectors.toList());
Expand All @@ -555,4 +587,27 @@ static List<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
(parser, context) -> CompletionSuggestion.fromXContent(parser, (String)context)));
return entries;
}

static List<NamedXContentRegistry.Entry> pluginsNamedXContents(Settings settings,
Copy link
Member

Choose a reason for hiding this comment

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

nit: private?

Collection<Class<? extends Plugin>> preInstalledPlugins,
Collection<Class<? extends Plugin>> plugins) {
List<Class<? extends Plugin>> listOfPlugins = new ArrayList<>(preInstalledPlugins);
for (Class<? extends Plugin> plugin : plugins) {
if (listOfPlugins.contains(plugin)) {
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity: plugin equality here is exact object equality, since plugins are singletons, no?

throw new IllegalArgumentException("plugin already exists: " + plugin);
}
listOfPlugins.add(plugin);
}

PluginsService pluginsService = new PluginsService(settings, null, null, listOfPlugins);

List<NamedXContentRegistry.Entry> entries = new ArrayList<>();
pluginsService.filterPlugins(Plugin.class)
.forEach(plugin -> entries.addAll(plugin.getNamedXContent()));
pluginsService.filterPlugins(SearchPlugin.class).stream()
.flatMap(plugin -> plugin.getAggregations().stream())
.flatMap(aggregationSpec -> aggregationSpec.getResultParsers().entrySet().stream())
.forEach(e -> entries.add(new NamedXContentRegistry.Entry(Aggregation.class, new ParseField(e.getKey()), e.getValue())));
return entries;
}
}
13 changes: 7 additions & 6 deletions ...t/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientExtTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

Expand All @@ -36,7 +37,7 @@
import static org.mockito.Mockito.mock;

/**
* This test works against a {@link RestHighLevelClient} subclass that simulats how custom response sections returned by
* This test works against a {@link RestHighLevelClient} subclass that simulates how custom response sections returned by
* Elasticsearch plugins can be parsed using the high level client.
*/
public class RestHighLevelClientExtTests extends ESTestCase {
Expand All @@ -46,7 +47,7 @@ public class RestHighLevelClientExtTests extends ESTestCase {
@Before
public void initClient() throws IOException {
RestClient restClient = mock(RestClient.class);
restHighLevelClient = new RestHighLevelClientExt(restClient);
restHighLevelClient = new RestHighLevelClient(restClient, RestHighLevelClientExtPlugin.class);
}

public void testParseEntityCustomResponseSection() throws IOException {
Expand All @@ -66,13 +67,13 @@ public void testParseEntityCustomResponseSection() throws IOException {
}
}

private static class RestHighLevelClientExt extends RestHighLevelClient {
public static class RestHighLevelClientExtPlugin extends Plugin {

private RestHighLevelClientExt(RestClient restClient) {
super(restClient, getNamedXContentsExt());
public RestHighLevelClientExtPlugin() {
}

private static List<NamedXContentRegistry.Entry> getNamedXContentsExt() {
@Override
public List<NamedXContentRegistry.Entry> getNamedXContent() {
List<NamedXContentRegistry.Entry> entries = new ArrayList<>();
entries.add(new NamedXContentRegistry.Entry(BaseCustomResponseSection.class, new ParseField("custom1"),
CustomResponseSection1::fromXContent));
Expand Down
22 changes: 19 additions & 3 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.http.message.BasicRequestLine;
import org.apache.http.message.BasicStatusLine;
import org.apache.http.nio.entity.NStringEntity;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.Build;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
Expand All @@ -48,12 +49,14 @@
import org.elasticsearch.action.search.ShardSearchFailure;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.CheckedFunction;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.cbor.CborXContent;
import org.elasticsearch.common.xcontent.smile.SmileXContent;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.aggregations.Aggregation;
Expand All @@ -76,6 +79,7 @@

import static org.elasticsearch.client.RestClientTestUtil.randomHeaders;
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.mockito.Matchers.anyMapOf;
import static org.mockito.Matchers.anyObject;
Expand Down Expand Up @@ -598,9 +602,9 @@ public void testWrapResponseListenerOnResponseExceptionWithIgnoresErrorValidBody
assertEquals("Elasticsearch exception [type=exception, reason=test error message]", elasticsearchException.getMessage());
}

public void testNamedXContents() {
public void testDefaultNamedXContents() {
List<NamedXContentRegistry.Entry> namedXContents = RestHighLevelClient.getDefaultNamedXContents();
assertEquals(45, namedXContents.size());
assertEquals(43, namedXContents.size());
Map<Class<?>, Integer> categories = new HashMap<>();
for (NamedXContentRegistry.Entry namedXContent : namedXContents) {
Integer counter = categories.putIfAbsent(namedXContent.categoryClass, 1);
Expand All @@ -609,10 +613,22 @@ public void testNamedXContents() {
}
}
assertEquals(2, categories.size());
assertEquals(Integer.valueOf(42), categories.get(Aggregation.class));
assertEquals(Integer.valueOf(40), categories.get(Aggregation.class));
assertEquals(Integer.valueOf(3), categories.get(Suggest.Suggestion.class));
}

public void testPreInstalledPlugins() {
for (Class<? extends Plugin> pluginClass : RestHighLevelClient.PRE_INSTALLED_PLUGINS) {
LuceneTestCase.ThrowingRunnable runnable = randomFrom(
() -> new RestHighLevelClient(restClient, pluginClass),
() -> new RestHighLevelClient(restClient, Collections.singletonList(pluginClass)),
() -> new RestHighLevelClient(restClient, Settings.EMPTY, Collections.singletonList(pluginClass)));

IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, runnable);
assertThat(exception.getMessage(), containsString("plugin already exists: " + pluginClass));
}
}

private static class TrackingActionListener implements ActionListener<Integer> {
private final AtomicInteger statusCode = new AtomicInteger(-1);
private final AtomicReference<Exception> exception = new AtomicReference<>();
Expand Down
Loading