diff --git a/plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/SecurityCatalogFilter.java b/plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/SecurityCatalogFilter.java index 3c2ffb1e8e7..f415a5f7e0b 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/SecurityCatalogFilter.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/SecurityCatalogFilter.java @@ -11,6 +11,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.security.authz.AuthorizationService; import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl.IndexAccessControl; +import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; import org.elasticsearch.xpack.sql.analysis.catalog.EsIndex; import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog; import org.elasticsearch.xpack.sql.type.DataType; @@ -42,21 +43,24 @@ public class SecurityCatalogFilter implements FilteredCatalog.Filter { } @Override - public EsIndex filterIndex(EsIndex index) { + public GetIndexResult filterIndex(GetIndexResult delegateResult) { if (false == licenseState.isAuthAllowed()) { /* If security is disabled the index authorization won't be available. * It is technically possible there to be a race between licensing * being enabled and sql requests that might cause us to fail on those * requests but that should be rare. */ - return index; + return delegateResult; } + EsIndex index = delegateResult.get(); IndexAccessControl permissions = getAccessControlResolver() .apply(OPTIONS, new String[] {index.name()}) .getIndexPermissions(index.name()); /* Fetching the permissions always checks if they are granted. If they aren't - * then it throws an exception. */ + * then it throws an exception. This is ok even for list requests because this + * will only ever be called on indices that are authorized because of security's + * request filtering. */ if (false == permissions.getFieldPermissions().hasFieldLevelSecurity()) { - return index; + return delegateResult; } Map filteredMapping = new HashMap<>(index.mapping().size()); for (Map.Entry entry : index.mapping().entrySet()) { @@ -64,7 +68,7 @@ public class SecurityCatalogFilter implements FilteredCatalog.Filter { filteredMapping.put(entry.getKey(), entry.getValue()); } } - return new EsIndex(index.name(), filteredMapping, index.aliases(), index.settings()); + return GetIndexResult.valid(new EsIndex(index.name(), filteredMapping, index.aliases(), index.settings())); } private BiFunction getAccessControlResolver() { diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/sql/RestSqlTestCase.java b/qa/sql/src/main/java/org/elasticsearch/xpack/sql/RestSqlTestCase.java index 59b13d7e74b..c0674cd1327 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/sql/RestSqlTestCase.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/sql/RestSqlTestCase.java @@ -124,7 +124,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase { } public void testMissingIndex() throws IOException { - expectBadRequest(() -> runSql("SELECT foo FROM missing"), containsString("1:17: Cannot resolve index [missing]")); + expectBadRequest(() -> runSql("SELECT foo FROM missing"), containsString("1:17: index [missing] does not exist")); } public void testMissingField() throws IOException { diff --git a/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ErrorsIT.java b/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ErrorsIT.java index 9a10fa51f16..15eb2d32722 100644 --- a/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ErrorsIT.java +++ b/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ErrorsIT.java @@ -17,7 +17,7 @@ public class ErrorsIT extends JdbcIntegrationTestCase { public void testSelectFromMissingTable() throws Exception { try (Connection c = esJdbc()) { SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * from test").executeQuery()); - assertEquals("line 1:15: Cannot resolve index [test]", e.getMessage()); + assertEquals("line 1:15: index [test] does not exist", e.getMessage()); } } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/Catalog.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/Catalog.java index d0225bc9508..95e61d4d065 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/Catalog.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/Catalog.java @@ -9,17 +9,84 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.Nullable; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import java.util.Objects; + /** * Converts from Elasticsearch's internal metadata ({@link ClusterState}) * into a representation that is compatible with SQL (@{link {@link EsIndex}). */ public interface Catalog { /** - * Lookup the information for a table, returning {@code null} if - * the index is not found. - * @throws SqlIllegalArgumentException if the index is in some way invalid - * for use with SQL + * Lookup the information for a table. */ @Nullable - EsIndex getIndex(String index) throws SqlIllegalArgumentException; + GetIndexResult getIndex(String index); + + class GetIndexResult { + public static GetIndexResult valid(EsIndex index) { + Objects.requireNonNull(index, "index must not be null if it was found"); + return new GetIndexResult(index, null); + } + public static GetIndexResult invalid(String invalid) { + Objects.requireNonNull(invalid, "invalid must not be null to signal that the index is invalid"); + return new GetIndexResult(null, invalid); + } + public static GetIndexResult notFound(String name) { + Objects.requireNonNull(name, "name must not be null"); + return invalid("index [" + name + "] does not exist"); + } + + @Nullable + private final EsIndex index; + @Nullable + private final String invalid; + + private GetIndexResult(@Nullable EsIndex index, @Nullable String invalid) { + this.index = index; + this.invalid = invalid; + } + + /** + * Get the {@linkplain EsIndex} built by the {@linkplain Catalog}. + * @throws SqlIllegalArgumentException if the index is invalid for + * use with sql + */ + public EsIndex get() { + if (invalid != null) { + throw new SqlIllegalArgumentException(invalid); + } + return index; + } + + /** + * Is the index valid for use with sql? Returns {@code false} if the + * index wasn't found. + */ + public boolean isValid() { + return invalid == null; + } + + @Override + public boolean equals(Object obj) { + if (obj == null || obj.getClass() != getClass()) { + return false; + } + GetIndexResult other = (GetIndexResult) obj; + return Objects.equals(index, other.index) + && Objects.equals(invalid, other.invalid); + } + + @Override + public int hashCode() { + return Objects.hash(index, invalid); + } + + @Override + public String toString() { + if (invalid != null) { + return "GetIndexResult[" + invalid + "]"; + } + return "GetIndexResult[" + index + "]"; + } + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalog.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalog.java index d53b0ae30f5..a538369ce28 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalog.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalog.java @@ -10,13 +10,15 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; -import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import org.elasticsearch.xpack.sql.type.DataType; +import org.elasticsearch.xpack.sql.type.Types; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; public class EsCatalog implements Catalog { private final ClusterState clusterState; @@ -26,51 +28,47 @@ public class EsCatalog implements Catalog { } @Override - public EsIndex getIndex(String index) throws SqlIllegalArgumentException { + public GetIndexResult getIndex(String index) throws SqlIllegalArgumentException { IndexMetaData idx = clusterState.getMetaData().index(index); if (idx == null) { - return null; + return GetIndexResult.notFound(index); } if (idx.getIndex().getName().startsWith(".")) { /* Indices that start with "." are considered internal and * should not be available to SQL. */ - return null; + return GetIndexResult.invalid( + "[" + idx.getIndex().getName() + "] starts with [.] so it is considered internal and incompatible with sql"); } - MappingMetaData type = singleType(idx.getMappings(), idx.getIndex().getName()); - if (type == null) { - return null; - } - return EsIndex.build(idx, type); - } - /** - * Return the single type in the index, {@code null} if there - * are no types in the index, and throw a {@link SqlIllegalArgumentException} - * if there are multiple types in the index. - */ - @Nullable - public MappingMetaData singleType(ImmutableOpenMap mappings, String name) { - /* We actually ignore the _default_ mapping because it is still - * allowed but deprecated. */ - MappingMetaData result = null; + // Make sure that the index contains only a single type + MappingMetaData singleType = null; List typeNames = null; - for (ObjectObjectCursor type : mappings) { + for (ObjectObjectCursor type : idx.getMappings()) { + /* We actually ignore the _default_ mapping because it is still + * allowed but deprecated. */ if ("_default_".equals(type.key)) { continue; } - if (result != null) { + if (singleType != null) { + // There are more than one types if (typeNames == null) { typeNames = new ArrayList<>(); - typeNames.add(result.type()); + typeNames.add(singleType.type()); } typeNames.add(type.key); } - result = type.value; + singleType = type.value; } - if (typeNames == null) { - return result; + if (singleType == null) { + return GetIndexResult.invalid("[" + idx.getIndex().getName() + "] doesn't have any types so it is incompatible with sql"); } - Collections.sort(typeNames); - throw new SqlIllegalArgumentException("[" + name + "] has more than one type " + typeNames); + if (typeNames != null) { + Collections.sort(typeNames); + return GetIndexResult.invalid( + "[" + idx.getIndex().getName() + "] contains more than one type " + typeNames + " so it is incompatible with sql"); + } + Map mapping = Types.fromEs(singleType.sourceAsMap()); + List aliases = Arrays.asList(idx.getAliases().keys().toArray(String.class)); + return GetIndexResult.valid(new EsIndex(idx.getIndex().getName(), mapping, aliases, idx.getSettings())); } } \ No newline at end of file diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsIndex.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsIndex.java index c15975696e0..7421c90c41d 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsIndex.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsIndex.java @@ -5,15 +5,10 @@ */ package org.elasticsearch.xpack.sql.analysis.catalog; -import org.elasticsearch.cluster.metadata.IndexMetaData; -import org.elasticsearch.cluster.metadata.MappingMetaData; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.sql.type.DataType; -import org.elasticsearch.xpack.sql.type.Types; import org.elasticsearch.xpack.sql.util.StringUtils; -import java.util.Arrays; import java.util.List; import java.util.Map; @@ -53,13 +48,6 @@ public class EsIndex { return settings; } - static EsIndex build(IndexMetaData metadata, @Nullable MappingMetaData type) { - Map mapping = type != null ? Types.fromEs(type.sourceAsMap()) : emptyMap(); - - List aliases = Arrays.asList(metadata.getAliases().keys().toArray(String.class)); - return new EsIndex(metadata.getIndex().getName(), mapping, aliases, metadata.getSettings()); - } - @Override public String toString() { return name; diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java index 16b5f2c254d..84acbdd3cb6 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java @@ -11,11 +11,12 @@ package org.elasticsearch.xpack.sql.analysis.catalog; public class FilteredCatalog implements Catalog { public interface Filter { /** - * Filter an index. Returning {@code null} will act as though - * the index wasn't found. Will never be called with a {@code null} - * parameter. + * Filter an index. Will only be called with valid, + * found indices but gets the entire {@link GetIndexResult} + * from the delegate catalog in case it wants to return + * it unchanged. */ - EsIndex filterIndex(EsIndex index); + GetIndexResult filterIndex(GetIndexResult delegateResult); } private Catalog delegate; @@ -27,10 +28,10 @@ public class FilteredCatalog implements Catalog { } @Override - public EsIndex getIndex(String index) { - EsIndex result = delegate.getIndex(index); - if (result == null) { - return null; + public GetIndexResult getIndex(String index) { + GetIndexResult result = delegate.getIndex(index); + if (false == result.isValid() || result.get() == null) { + return result; } return filter.filterIndex(result); } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java index d91f8b1d720..351fbb09fab 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java @@ -27,8 +27,8 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.analysis.catalog.Catalog; +import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; import org.elasticsearch.xpack.sql.analysis.catalog.EsIndex; import java.io.IOException; @@ -162,7 +162,7 @@ public class SqlGetIndicesAction } public static class TransportAction extends TransportMasterNodeReadAction { - private final Function catalog; + private final Function catalogSupplier; private final SqlLicenseChecker licenseChecker; @Inject @@ -171,7 +171,7 @@ public class SqlGetIndicesAction IndexNameExpressionResolver indexNameExpressionResolver, CatalogHolder catalog, SqlLicenseChecker licenseChecker) { super(settings, NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, Request::new); - this.catalog = catalog.catalog; + this.catalogSupplier = catalog.catalogSupplier; this.licenseChecker = licenseChecker; } @@ -189,7 +189,7 @@ public class SqlGetIndicesAction @Override protected void masterOperation(Request request, ClusterState state, ActionListener listener) { licenseChecker.checkIfSqlAllowed(); - operation(indexNameExpressionResolver, catalog, request, state, listener); + operation(indexNameExpressionResolver, catalogSupplier, request, state, listener); } @Override @@ -202,10 +202,10 @@ public class SqlGetIndicesAction * Class that holds that {@link Catalog} to aid in guice binding. */ public static class CatalogHolder { - final Function catalog; + final Function catalogSupplier; - public CatalogHolder(Function catalog) { - this.catalog = catalog; + public CatalogHolder(Function catalogSupplier) { + this.catalogSupplier = catalogSupplier; } } } @@ -219,20 +219,15 @@ public class SqlGetIndicesAction * what the user has permission to access, and leaves an appropriate * audit trail. */ - public static void operation(IndexNameExpressionResolver indexNameExpressionResolver, Function catalog, + public static void operation(IndexNameExpressionResolver indexNameExpressionResolver, Function catalogSupplier, Request request, ClusterState state, ActionListener listener) { String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request); List results = new ArrayList<>(concreteIndices.length); + Catalog catalog = catalogSupplier.apply(state); for (String index : concreteIndices) { - EsIndex esIndex; - try { - esIndex = catalog.apply(state).getIndex(index); - } catch (SqlIllegalArgumentException e) { - assert e.getMessage().contains("has more than one type"); - esIndex = null; - } - if (esIndex != null) { - results.add(esIndex); + GetIndexResult result = catalog.getIndex(index); + if (result.isValid()) { + results.add(result.get()); } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java index 5f03e016a6d..57bca259f63 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java @@ -95,7 +95,7 @@ public class SqlSession { * Get an index. Prefer not to use this method as it cannot be made to work with cross cluster search. */ public EsIndex getIndexSync(String index) { - return catalog.getIndex(index); + return catalog.getIndex(index).get(); } public Planner planner() { diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalogTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalogTests.java index ac25c5e3ac7..52a59aacdee 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalogTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalogTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; import java.io.IOException; @@ -21,47 +22,87 @@ import static java.util.Collections.emptyMap; public class EsCatalogTests extends ESTestCase { public void testEmpty() { Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT).build()); - assertNull(catalog.getIndex("test")); + assertEquals(GetIndexResult.notFound("test"), catalog.getIndex("test")); } public void testIndexExists() throws IOException { Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) .metaData(MetaData.builder() - .put(index() - .putMapping("test", "{}")) - .build()) + .put(index("test") + .putMapping("test", "{}"))) .build()); - assertEquals(emptyMap(), catalog.getIndex("test").mapping()); + GetIndexResult result = catalog.getIndex("test"); + assertTrue(result.isValid()); + assertEquals(emptyMap(), result.get().mapping()); + } + + public void testIndexAbsent() throws IOException { + Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) + .metaData(MetaData.builder() + .put(index("test") + .putMapping("test", "{}"))) + .build()); + + GetIndexResult result = catalog.getIndex("foo"); + assertFalse(result.isValid()); } public void testIndexWithDefaultType() throws IOException { Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) .metaData(MetaData.builder() - .put(index() + .put(index("test") .putMapping("test", "{}") - .putMapping("_default_", "{}")) - .build()) + .putMapping("_default_", "{}"))) .build()); - assertEquals(emptyMap(), catalog.getIndex("test").mapping()); + GetIndexResult result = catalog.getIndex("test"); + assertTrue(result.isValid()); + assertEquals(emptyMap(), result.get().mapping()); } public void testIndexWithTwoTypes() throws IOException { Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) .metaData(MetaData.builder() - .put(index() + .put(index("test") .putMapping("first_type", "{}") - .putMapping("second_type", "{}")) + .putMapping("second_type", "{}"))) + .build()); + + GetIndexResult result = catalog.getIndex("test"); + assertFalse(result.isValid()); + Exception e = expectThrows(SqlIllegalArgumentException.class, result::get); + assertEquals(e.getMessage(), "[test] contains more than one type [first_type, second_type] so it is incompatible with sql"); + } + + public void testIndexWithNoTypes() throws IOException { + Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) + .metaData(MetaData.builder() + .put(index("test"))) + .build()); + + GetIndexResult result = catalog.getIndex("test"); + assertFalse(result.isValid()); + Exception e = expectThrows(SqlIllegalArgumentException.class, result::get); + assertEquals(e.getMessage(), "[test] doesn't have any types so it is incompatible with sql"); + } + + public void testIndexStartsWithDot() throws IOException { + Catalog catalog = new EsCatalog(ClusterState.builder(ClusterName.DEFAULT) + .metaData(MetaData.builder() + .put(index(".security") + .putMapping("test", "{}")) .build()) .build()); - Exception e = expectThrows(SqlIllegalArgumentException.class, () -> catalog.getIndex("test")); - assertEquals(e.getMessage(), "[test] has more than one type [first_type, second_type]"); + GetIndexResult result = catalog.getIndex(".security"); + assertFalse(result.isValid()); + Exception e = expectThrows(SqlIllegalArgumentException.class, result::get); + assertEquals(e.getMessage(), "[.security] starts with [.] so it is considered internal and incompatible with sql"); } - private IndexMetaData.Builder index() throws IOException { - return IndexMetaData.builder("test") + private IndexMetaData.Builder index(String name) throws IOException { + return IndexMetaData.builder(name) .settings(Settings.builder() .put("index.version.created", Version.CURRENT) .put("index.number_of_shards", 1) diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java index a0c63c3fd7c..45aba6e9766 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java @@ -7,7 +7,7 @@ package org.elasticsearch.xpack.sql.analysis.catalog; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog.Filter; +import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; import java.util.Arrays; import java.util.List; @@ -19,53 +19,47 @@ import static java.util.stream.Collectors.toList; public class FilteredCatalogTests extends ESTestCase { public void testGetTypeNoopCatalog() { Catalog orig = inMemoryCatalog("a", "b", "c"); - Catalog filtered = new FilteredCatalog(orig, new Filter() { - @Override - public EsIndex filterIndex(EsIndex index) { - return index; - } - }); - assertSame(orig.getIndex("a"), filtered.getIndex("a")); - assertSame(orig.getIndex("b"), filtered.getIndex("b")); - assertSame(orig.getIndex("c"), filtered.getIndex("c")); - assertSame(orig.getIndex("missing"), filtered.getIndex("missing")); + Catalog filtered = new FilteredCatalog(orig, delegateResult -> delegateResult); + assertEquals(orig.getIndex("a"), filtered.getIndex("a")); + assertEquals(orig.getIndex("b"), filtered.getIndex("b")); + assertEquals(orig.getIndex("c"), filtered.getIndex("c")); + assertEquals(orig.getIndex("missing"), filtered.getIndex("missing")); } public void testGetTypeFiltering() { Catalog orig = inMemoryCatalog("cat", "dog"); - Catalog filtered = new FilteredCatalog(orig, new Filter() { - @Override - public EsIndex filterIndex(EsIndex index) { - return index.name().equals("cat") ? index : null; + Catalog filtered = new FilteredCatalog(orig, delegateResult -> { + if (delegateResult.get().name().equals("cat")) { + return delegateResult; } + return GetIndexResult.notFound(delegateResult.get().name()); }); - assertSame(orig.getIndex("cat"), filtered.getIndex("cat")); - assertNull(filtered.getIndex("dog")); + assertEquals(orig.getIndex("cat"), filtered.getIndex("cat")); + assertEquals(GetIndexResult.notFound("dog"), filtered.getIndex("dog")); } public void testGetTypeModifying() { Catalog orig = inMemoryCatalog("cat", "dog"); - Catalog filtered = new FilteredCatalog(orig, new Filter() { - @Override - public EsIndex filterIndex(EsIndex index) { + Catalog filtered = new FilteredCatalog(orig, delegateResult -> { + EsIndex index = delegateResult.get(); if (index.name().equals("cat")) { - return index; + return delegateResult; } - return new EsIndex("rat", index.mapping(), index.aliases(), index.settings()); - } - }); - assertSame(orig.getIndex("cat"), filtered.getIndex("cat")); - assertEquals("rat", filtered.getIndex("dog").name()); + return GetIndexResult.valid(new EsIndex("rat", index.mapping(), index.aliases(), index.settings())); + }); + assertEquals(orig.getIndex("cat"), filtered.getIndex("cat")); + assertEquals("rat", filtered.getIndex("dog").get().name()); } - public void testGetTypeFilterIgnoresNull() { - Catalog filtered = new FilteredCatalog(inMemoryCatalog(), new Filter() { - @Override - public EsIndex filterIndex(EsIndex index) { - return index.name().equals("cat") ? index : null; + public void testGetTypeFilterIgnoresMissing() { + Catalog orig = inMemoryCatalog(); + Catalog filtered = new FilteredCatalog(orig, delegateResult -> { + if (delegateResult.get().name().equals("cat")) { + return delegateResult; } + return GetIndexResult.notFound(delegateResult.get().name()); }); - assertNull(filtered.getIndex("missing")); + assertEquals(orig.getIndex("missing"), filtered.getIndex("missing")); } private Catalog inMemoryCatalog(String... indexNames) { diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/InMemoryCatalog.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/InMemoryCatalog.java index 19dbe984f50..0012bf6a2be 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/InMemoryCatalog.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/InMemoryCatalog.java @@ -22,7 +22,8 @@ class InMemoryCatalog implements Catalog { } @Override - public EsIndex getIndex(String index) { - return indices.get(index); + public GetIndexResult getIndex(String index) { + EsIndex result = indices.get(index); + return result == null ? GetIndexResult.notFound(index) : GetIndexResult.valid(result); } } \ No newline at end of file