From 440c0b5863a52a1531eb4f9db14fb128c98f5d78 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Thu, 6 Apr 2017 21:16:46 -0500 Subject: [PATCH 1/8] WIP: work on creating initial parsing --- .../search/RemoteClusterConnection.java | 2 +- .../action/search/RemoteClusterService.java | 2 +- .../ClusterNameExpressionResolver.java | 136 ++++++++++++++++++ 3 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java index aea1aab7d3e36..6aca67bdf01c0 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java @@ -79,7 +79,7 @@ * {@link RemoteClusterService#REMOTE_CONNECTIONS_PER_CLUSTER} until either all eligible nodes are exhausted or the maximum number of * connections per cluster has been reached. */ -final class RemoteClusterConnection extends AbstractComponent implements TransportConnectionListener, Closeable { +public final class RemoteClusterConnection extends AbstractComponent implements TransportConnectionListener, Closeable { private final TransportService transportService; private final ConnectionProfile remoteProfile; diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java index 089ce57a1146b..627906d7056e0 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java @@ -221,7 +221,7 @@ Map> groupClusterIndices(String[] requestIndices, Predicate clusterName = remoteClusterName; } } - perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList()).add(indexName); + perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName); } return perClusterIndices; } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java new file mode 100644 index 0000000000000..637e0a57b24db --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java @@ -0,0 +1,136 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.metadata; + +import org.elasticsearch.action.search.RemoteClusterConnection; +import org.elasticsearch.action.search.RemoteClusterService; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.regex.Regex; +import org.elasticsearch.common.settings.Settings; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class ClusterNameExpressionResolver extends AbstractComponent { + + public ClusterNameExpressionResolver(Settings settings) { + super(settings); + new WildcardExpressionResolver(); + } + + private class WildcardExpressionResolver { + + private List resolve(Map remoteClusters, List expressions) { + if (isEmptyOrTrivialWildcard(expressions)) { + return resolveEmptyOrTrivialWildcard(remoteClusters); + } + + List names = innerResolve(remoteClusters, expressions); + + if (names.isEmpty()) { + // TODO: Do I need to handle? + } + + return names; + } + + private List innerResolve(Map remoteClusters, List expressions) { + Set result = new HashSet<>(); + + boolean wildcardSeen = false; + for (int i = 0; i < expressions.size(); i++) { + String expression = expressions.get(i); + if (remoteClusters.containsKey(expression)) { + if (result != null) { + result.add(expression); + } + continue; + } + if (Strings.isEmpty(expression)) { + // In the current system I believe we do nothing if a Cluster is not found +// throw infe(expression); + } + boolean add = true; +// if (expression.charAt(0) == '+') { +// // if its the first, add empty result set +// if (i == 0) { +// result = new HashSet<>(); +// } +// expression = expression.substring(1); +// } else if (expression.charAt(0) == '-') { +// // if there is a negation without a wildcard being previously seen, add it verbatim, +// // otherwise return the expression +// if (wildcardSeen) { +// add = false; +// expression = expression.substring(1); +// } else { +// add = true; +// } +// } + if (result == null) { + // add all the previous ones... + result = new HashSet<>(expressions.subList(0, i)); + } +// if (!Regex.isSimpleMatchPattern(expression)) { +// if (!unavailableIgnoredOrExists(options, metaData, expression)) { +// throw infe(expression); +// } +// if (add) { +// result.add(expression); +// } else { +// result.remove(expression); +// } +// continue; +// } +// +// final IndexMetaData.State excludeState = excludeState(options); +// final Map matches = matches(metaData, expression); +// Set expand = expand(context, excludeState, matches); +// if (add) { +// result.addAll(expand); +// } else { +// result.removeAll(expand); +// } +// +// if (!noIndicesAllowedOrMatches(options, matches)) { +// throw infe(expression); +// } + + if (Regex.isSimpleMatchPattern(expression)) { + wildcardSeen = true; + } + } + + return new ArrayList<>(result); + } + + private boolean isEmptyOrTrivialWildcard(List expressions) { + return expressions.isEmpty() || (expressions.size() == 1 && Regex.isMatchAllPattern(expressions.get(0))); + } + + private List resolveEmptyOrTrivialWildcard(Map remoteClusters) { + return new ArrayList<>(remoteClusters.keySet()); + } + } +} From 85071a2b8700e7b03f733243c12312b365aad772 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 7 Apr 2017 15:49:29 -0500 Subject: [PATCH 2/8] Work on integrating name resolver --- .../action/search/RemoteClusterService.java | 21 ++- .../ClusterNameExpressionResolver.java | 125 ++++++------------ .../ClusterNameExpressionResolverTests.java | 75 +++++++++++ 3 files changed, 129 insertions(+), 92 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java index 627906d7056e0..260dd2735d2c4 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java @@ -25,6 +25,7 @@ import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsGroup; import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsResponse; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.cluster.metadata.ClusterNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.PlainShardIterator; @@ -55,6 +56,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -111,11 +113,13 @@ public final class RemoteClusterService extends AbstractComponent implements Clo private final TransportService transportService; private final int numRemoteConnections; + private final ClusterNameExpressionResolver clusterNameResolver; private volatile Map remoteClusters = Collections.emptyMap(); RemoteClusterService(Settings settings, TransportService transportService) { super(settings); this.transportService = transportService; + this.clusterNameResolver = new ClusterNameExpressionResolver(settings); numRemoteConnections = REMOTE_CONNECTIONS_PER_CLUSTER.get(settings); } @@ -203,25 +207,30 @@ boolean isRemoteNodeConnected(final String remoteCluster, final DiscoveryNode no */ Map> groupClusterIndices(String[] requestIndices, Predicate indexExists) { Map> perClusterIndices = new HashMap<>(); + Set remoteClusterNames = this.remoteClusters.keySet(); for (String index : requestIndices) { int i = index.indexOf(REMOTE_CLUSTER_INDEX_SEPARATOR); - String indexName = index; - String clusterName = LOCAL_CLUSTER_GROUP_KEY; if (i >= 0) { String remoteClusterName = index.substring(0, i); - if (isRemoteClusterRegistered(remoteClusterName)) { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusterNames, remoteClusterName); + if (clusters != null) { if (indexExists.test(index)) { // we use : as a separator for remote clusters. might conflict if there is an index that is actually named // remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias // if that happens throw new IllegalArgumentException("Can not filter indices; index " + index + " exists but there is also a remote cluster named: " + remoteClusterName); + } + String indexName = index.substring(i + 1); + for (String clusterName : clusters) { + perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName); } - indexName = index.substring(i + 1); - clusterName = remoteClusterName; + } else { + perClusterIndices.computeIfAbsent(LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index); } + } else { + perClusterIndices.computeIfAbsent(LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index); } - perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName); } return perClusterIndices; } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java index 637e0a57b24db..904acf31f3e42 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java @@ -19,118 +19,71 @@ package org.elasticsearch.cluster.metadata; -import org.elasticsearch.action.search.RemoteClusterConnection; -import org.elasticsearch.action.search.RemoteClusterService; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; import java.util.ArrayList; -import java.util.HashSet; +import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; public class ClusterNameExpressionResolver extends AbstractComponent { + private final WildcardExpressionResolver wildcardResolver = new WildcardExpressionResolver(); + public ClusterNameExpressionResolver(Settings settings) { super(settings); new WildcardExpressionResolver(); } - private class WildcardExpressionResolver { - - private List resolve(Map remoteClusters, List expressions) { - if (isEmptyOrTrivialWildcard(expressions)) { - return resolveEmptyOrTrivialWildcard(remoteClusters); - } + public List resolveClusterNames(Set remoteClusters, String clusterExpression) { + if (remoteClusters.contains(clusterExpression)) { + return Collections.singletonList(clusterExpression); + } else if (Regex.isSimpleMatchPattern(clusterExpression)) { + return wildcardResolver.resolve(remoteClusters, clusterExpression); + } else { + return null; + } + } - List names = innerResolve(remoteClusters, expressions); + private static class WildcardExpressionResolver { - if (names.isEmpty()) { - // TODO: Do I need to handle? + private List resolve(Set remoteClusters, String clusterExpression) { + if (isTrivialWildcard(clusterExpression)) { + return resolveTrivialWildcard(remoteClusters); } - return names; + Set matches = matches(remoteClusters, clusterExpression); + if (matches.isEmpty()) { + return null; + } else { + return new ArrayList<>(matches); + } } - private List innerResolve(Map remoteClusters, List expressions) { - Set result = new HashSet<>(); - - boolean wildcardSeen = false; - for (int i = 0; i < expressions.size(); i++) { - String expression = expressions.get(i); - if (remoteClusters.containsKey(expression)) { - if (result != null) { - result.add(expression); - } - continue; - } - if (Strings.isEmpty(expression)) { - // In the current system I believe we do nothing if a Cluster is not found -// throw infe(expression); - } - boolean add = true; -// if (expression.charAt(0) == '+') { -// // if its the first, add empty result set -// if (i == 0) { -// result = new HashSet<>(); -// } -// expression = expression.substring(1); -// } else if (expression.charAt(0) == '-') { -// // if there is a negation without a wildcard being previously seen, add it verbatim, -// // otherwise return the expression -// if (wildcardSeen) { -// add = false; -// expression = expression.substring(1); -// } else { -// add = true; -// } -// } - if (result == null) { - // add all the previous ones... - result = new HashSet<>(expressions.subList(0, i)); - } -// if (!Regex.isSimpleMatchPattern(expression)) { -// if (!unavailableIgnoredOrExists(options, metaData, expression)) { -// throw infe(expression); -// } -// if (add) { -// result.add(expression); -// } else { -// result.remove(expression); -// } -// continue; -// } -// -// final IndexMetaData.State excludeState = excludeState(options); -// final Map matches = matches(metaData, expression); -// Set expand = expand(context, excludeState, matches); -// if (add) { -// result.addAll(expand); -// } else { -// result.removeAll(expand); -// } -// -// if (!noIndicesAllowedOrMatches(options, matches)) { -// throw infe(expression); -// } - - if (Regex.isSimpleMatchPattern(expression)) { - wildcardSeen = true; - } - } + private boolean isTrivialWildcard(String clusterExpression) { + return Regex.isMatchAllPattern(clusterExpression); + } - return new ArrayList<>(result); + private List resolveTrivialWildcard(Set remoteClusters) { + return new ArrayList<>(remoteClusters); } - private boolean isEmptyOrTrivialWildcard(List expressions) { - return expressions.isEmpty() || (expressions.size() == 1 && Regex.isMatchAllPattern(expressions.get(0))); + private static Set matches(Set remoteClusters, String expression) { + if (expression.indexOf("*") == expression.length() - 1) { + return otherWildcard(remoteClusters, expression); + } else { + return otherWildcard(remoteClusters, expression); + } } - private List resolveEmptyOrTrivialWildcard(Map remoteClusters) { - return new ArrayList<>(remoteClusters.keySet()); + private static Set otherWildcard(Set remoteClusters, String expression) { + final String pattern = expression; + return remoteClusters.stream() + .filter(n -> Regex.simpleMatch(pattern, n)) + .collect(Collectors.toSet()); } } } diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java new file mode 100644 index 0000000000000..c1ec0b7418db1 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java @@ -0,0 +1,75 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.metadata; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class ClusterNameExpressionResolverTests extends ESTestCase { + + private ClusterNameExpressionResolver clusterNameResolver = new ClusterNameExpressionResolver(Settings.EMPTY); + private static final Set remoteClusters = new HashSet<>(); + + static { + remoteClusters.add("cluster1"); + remoteClusters.add("cluster2"); + remoteClusters.add("totallyDifferent"); + } + + public void testExactMatch() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totallyDifferent"); + assertEquals(new HashSet<>(Arrays.asList("totallyDifferent")), new HashSet<>(clusters)); + } + + public void testNoWildCardNoMatch() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totallyDifferent2"); + assertNull(clusters); + } + + public void testWildCardNoMatch() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totally*2"); + assertNull(clusters); + } + + public void testSimpleWildCard() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "*"); + assertEquals(new HashSet<>(Arrays.asList("cluster1", "cluster2", "totallyDifferent")), new HashSet<>(clusters)); + } + + public void testSuffixWildCard() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "cluster*"); + assertEquals(new HashSet<>(Arrays.asList("cluster1", "cluster2")), new HashSet<>(clusters)); + } + + public void testPrefixWildCard() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "*Different"); + assertEquals(new HashSet<>(Arrays.asList("totallyDifferent")), new HashSet<>(clusters)); + } + + public void testMiddleWildCard() { + List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "clu*1"); + assertEquals(new HashSet<>(Arrays.asList("cluster1")), new HashSet<>(clusters)); + } +} From 2798d89084e0cbfcdbfac8444c1faa7585c1b010 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 7 Apr 2017 17:19:01 -0500 Subject: [PATCH 3/8] Modify remote cluster service tests --- .../action/search/RemoteClusterServiceTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java b/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java index d0f0427e71084..81ee9141e2b59 100644 --- a/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java @@ -143,14 +143,14 @@ public void testGroupClusterIndices() throws IOException { assertTrue(service.isRemoteClusterRegistered("cluster_2")); assertFalse(service.isRemoteClusterRegistered("foo")); Map> perClusterIndices = service.groupClusterIndices(new String[]{"foo:bar", "cluster_1:bar", - "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, i -> false); + "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo", "cluster*:baz", "*:boo", "no*match:boo"}, i -> false); String[] localIndices = perClusterIndices.computeIfAbsent(RemoteClusterService.LOCAL_CLUSTER_GROUP_KEY, k -> Collections.emptyList()).toArray(new String[0]); assertNotNull(perClusterIndices.remove(RemoteClusterService.LOCAL_CLUSTER_GROUP_KEY)); - assertArrayEquals(new String[]{"foo:bar", "foo"}, localIndices); + assertArrayEquals(new String[]{"foo:bar", "foo", "no*match:boo"}, localIndices); assertEquals(2, perClusterIndices.size()); - assertEquals(Arrays.asList("bar", "test"), perClusterIndices.get("cluster_1")); - assertEquals(Arrays.asList("foo:bar", "foo*"), perClusterIndices.get("cluster_2")); + assertEquals(Arrays.asList("bar", "test", "baz", "boo"), perClusterIndices.get("cluster_1")); + assertEquals(Arrays.asList("foo:bar", "foo*", "baz", "boo"), perClusterIndices.get("cluster_2")); IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> service.groupClusterIndices(new String[]{"foo:bar", "cluster_1:bar", From c7a83044a01c1edefe207ac1cfafe70c8167c825 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Mon, 10 Apr 2017 22:31:12 -0500 Subject: [PATCH 4/8] Add basic rest test for wildcard search --- .../test/multi_cluster/10_basic.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yaml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yaml index 28ff1e52b876e..bca0703d457bf 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yaml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/10_basic.yaml @@ -118,6 +118,30 @@ - match: { hits.total: 6 } - match: { hits.hits.0._index: "test_remote_cluster:test_index" } +--- +"Test wildcard search": + - do: + cluster.get_settings: + include_defaults: true + + - set: { defaults.search.remote.my_remote_cluster.seeds.0: remote_ip } + + - do: + cluster.put_settings: + flat_settings: true + body: + transient: + search.remote.test_remote_cluster.seeds: $remote_ip + + - match: {transient: {search.remote.test_remote_cluster.seeds: $remote_ip}} + + - do: + search: + index: "*:test_index" + + - match: { _shards.total: 6 } + - match: { hits.total: 12 } + --- "Search an filtered alias on the remote cluster": From 5a80f24acfc3b0e6be99f648c8bd42422302085b Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 11 Apr 2017 10:16:27 -0500 Subject: [PATCH 5/8] Return empty list opposed to null --- .../action/search/RemoteClusterConnection.java | 2 +- .../elasticsearch/action/search/RemoteClusterService.java | 2 +- .../cluster/metadata/ClusterNameExpressionResolver.java | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java index 6aca67bdf01c0..aea1aab7d3e36 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterConnection.java @@ -79,7 +79,7 @@ * {@link RemoteClusterService#REMOTE_CONNECTIONS_PER_CLUSTER} until either all eligible nodes are exhausted or the maximum number of * connections per cluster has been reached. */ -public final class RemoteClusterConnection extends AbstractComponent implements TransportConnectionListener, Closeable { +final class RemoteClusterConnection extends AbstractComponent implements TransportConnectionListener, Closeable { private final TransportService transportService; private final ConnectionProfile remoteProfile; diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java index 260dd2735d2c4..9ae67ed18a3be 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java @@ -213,7 +213,7 @@ Map> groupClusterIndices(String[] requestIndices, Predicate if (i >= 0) { String remoteClusterName = index.substring(0, i); List clusters = clusterNameResolver.resolveClusterNames(remoteClusterNames, remoteClusterName); - if (clusters != null) { + if (clusters.isEmpty()) { if (indexExists.test(index)) { // we use : as a separator for remote clusters. might conflict if there is an index that is actually named // remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java index 904acf31f3e42..b3ed3917b3501 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java @@ -29,13 +29,12 @@ import java.util.Set; import java.util.stream.Collectors; -public class ClusterNameExpressionResolver extends AbstractComponent { +public final class ClusterNameExpressionResolver extends AbstractComponent { private final WildcardExpressionResolver wildcardResolver = new WildcardExpressionResolver(); public ClusterNameExpressionResolver(Settings settings) { super(settings); - new WildcardExpressionResolver(); } public List resolveClusterNames(Set remoteClusters, String clusterExpression) { @@ -44,7 +43,7 @@ public List resolveClusterNames(Set remoteClusters, String clust } else if (Regex.isSimpleMatchPattern(clusterExpression)) { return wildcardResolver.resolve(remoteClusters, clusterExpression); } else { - return null; + return Collections.emptyList(); } } @@ -57,7 +56,7 @@ private List resolve(Set remoteClusters, String clusterExpressio Set matches = matches(remoteClusters, clusterExpression); if (matches.isEmpty()) { - return null; + return Collections.emptyList(); } else { return new ArrayList<>(matches); } From 56168f92520154c8d408edd6ec4aacc6b5b60a1e Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 11 Apr 2017 10:35:42 -0500 Subject: [PATCH 6/8] Add basic javadoc --- .../metadata/ClusterNameExpressionResolver.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java index b3ed3917b3501..2032c2f4ef3ba 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java @@ -29,6 +29,10 @@ import java.util.Set; import java.util.stream.Collectors; +/** + * Resolves cluster names from an expression. The expression must be the exact match of a cluster + * name or must be a wildcard expression. + */ public final class ClusterNameExpressionResolver extends AbstractComponent { private final WildcardExpressionResolver wildcardResolver = new WildcardExpressionResolver(); @@ -37,6 +41,14 @@ public ClusterNameExpressionResolver(Settings settings) { super(settings); } + /** + * Resolves the provided cluster expression to matching cluster names. This method only + * supports exact or wildcard matches. + * + * @param remoteClusters the aliases for remote clusters + * @param clusterExpression the expressions that can be resolved to cluster names. + * @return the resolved cluster aliases. + */ public List resolveClusterNames(Set remoteClusters, String clusterExpression) { if (remoteClusters.contains(clusterExpression)) { return Collections.singletonList(clusterExpression); From b4545d24a1731c43a2a34fd90f881f7e4f04c388 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 11 Apr 2017 10:47:32 -0500 Subject: [PATCH 7/8] Fix boolean error --- .../org/elasticsearch/action/search/RemoteClusterService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java index 9ae67ed18a3be..f5f7d43cb2c8a 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java @@ -213,7 +213,7 @@ Map> groupClusterIndices(String[] requestIndices, Predicate if (i >= 0) { String remoteClusterName = index.substring(0, i); List clusters = clusterNameResolver.resolveClusterNames(remoteClusterNames, remoteClusterName); - if (clusters.isEmpty()) { + if (clusters.isEmpty() == false) { if (indexExists.test(index)) { // we use : as a separator for remote clusters. might conflict if there is an index that is actually named // remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias From 5db0bd7fa69cf11e24053f9c4bc3773c36c41f86 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 11 Apr 2017 11:36:01 -0500 Subject: [PATCH 8/8] Fix test assertions --- .../cluster/metadata/ClusterNameExpressionResolverTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java index c1ec0b7418db1..d6c8707c1d76e 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java @@ -45,12 +45,12 @@ public void testExactMatch() { public void testNoWildCardNoMatch() { List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totallyDifferent2"); - assertNull(clusters); + assertTrue(clusters.isEmpty()); } public void testWildCardNoMatch() { List clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totally*2"); - assertNull(clusters); + assertTrue(clusters.isEmpty()); } public void testSimpleWildCard() {