From 6bf2cf67e77deaf3698d46ec530fff5489774a1a Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 8 Dec 2023 21:23:20 +0000 Subject: [PATCH 01/13] Basic view stubs Signed-off-by: Peter Nied --- .../index/views/ViewDefinition.java | 26 ++++++++ .../opensearch/index/views/ViewService.java | 20 ++++++ .../opensearch/index/views/package-info.java | 10 +++ .../action/admin/indices/RestViewAction.java | 63 +++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 server/src/main/java/org/opensearch/index/views/ViewDefinition.java create mode 100644 server/src/main/java/org/opensearch/index/views/ViewService.java create mode 100644 server/src/main/java/org/opensearch/index/views/package-info.java create mode 100644 server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java diff --git a/server/src/main/java/org/opensearch/index/views/ViewDefinition.java b/server/src/main/java/org/opensearch/index/views/ViewDefinition.java new file mode 100644 index 0000000000000..b9fa80204e267 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/views/ViewDefinition.java @@ -0,0 +1,26 @@ +package org.opensearch.index.views; + +import java.util.List; + +import org.joda.time.DateTime; + +public class ViewDefinition /* implements Writeable, ToXContentFragment */ { + + private String identifier; + private String description; + private DateTime createdAt; + private DateTime modifiedAt; + private String query; /** TBD format? */ + private List indexPatterns; + + public static enum Computation { + Materialized, + Projected; + } + + public static class IndexPattern { + private String indexPattern; + + /** Define patterns properties */ + } +} diff --git a/server/src/main/java/org/opensearch/index/views/ViewService.java b/server/src/main/java/org/opensearch/index/views/ViewService.java new file mode 100644 index 0000000000000..30560d67db8ba --- /dev/null +++ b/server/src/main/java/org/opensearch/index/views/ViewService.java @@ -0,0 +1,20 @@ +package org.opensearch.index.views; + +import org.opensearch.common.annotation.ExperimentalApi; + +@ExperimentalApi +public class ViewService { + + public ViewDefinition getViewDefinition(final String id) {} + + public ViewDefinition createOrUpdateViewDefinition(final ViewDefinition view) {} + + public ViewDefinition removeDefinition(final String id) {} + + + public List listViews() {} + + + public + +} diff --git a/server/src/main/java/org/opensearch/index/views/package-info.java b/server/src/main/java/org/opensearch/index/views/package-info.java new file mode 100644 index 0000000000000..2b81ef1b1911a --- /dev/null +++ b/server/src/main/java/org/opensearch/index/views/package-info.java @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** Responsible for handling all projection of data in OpenSearch */ +package org.opensearch.index.views; diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java new file mode 100644 index 0000000000000..3c74bcc62441e --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -0,0 +1,63 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.admin.indices; + +import org.opensearch.rest.NamedRoute; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.Booleans; +import org.opensearch.common.logging.DeprecationLogger; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; + +import java.io.IOException; +import java.util.List; + +import static org.opensearch.rest.RestRequest.Method.DELETE; +import static org.opensearch.rest.RestRequest.Method.GET; +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; + + +public class RestViewAction extends BaseRestHandler { + + @Override + public List routes() { + return List.of( + new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster.views.list").handler(this::handleGet).build(), + new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster.views.create").handler(this::handlePost).build(), + new NamedRoute.Builder().path("/views/{view_id}").method(GET).uniqueName("cluster.views.get").handler(this::handleSingleGet).build(), + new NamedRoute.Builder().path("/views/{view_id}").method(DELETE).uniqueName("cluster.views.delete").handler(this::handleSingleDelete).build(), + new NamedRoute.Builder().path("/views/{view_id}").method(PUT).uniqueName("cluster.views.update").handler(this::handleSinglePut).build() + ); + } + + @Override + public String getName() { + return "refresh_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + return channel -> { + // Consume the rest channel + }; + } + + public RestResponse handleGet(final RestRequest r) { return null; } + public RestResponse handlePost(final RestRequest r) { return null; } + + public RestResponse handleSingleGet(final RestRequest r) { return null; } + public RestResponse handleSinglePut(final RestRequest r) { return null; } + public RestResponse handleSingleDelete(final RestRequest r) { return null; } + +} \ No newline at end of file From 3522cc7b45e02e55d1ed3b7bd368b806318ba902 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 14 Dec 2023 22:07:27 +0000 Subject: [PATCH 02/13] Cluster metadat Signed-off-by: Peter Nied --- .../org/opensearch/cluster/ClusterModule.java | 3 + .../opensearch/cluster/metadata/Metadata.java | 34 ++++ .../org/opensearch/cluster/metadata/View.java | 120 ++++++++++++ .../cluster/metadata/ViewMetadata.java | 185 ++++++++++++++++++ .../index/{views => view}/package-info.java | 4 +- .../index/views/ViewDefinition.java | 26 --- .../opensearch/index/views/ViewService.java | 20 -- .../action/admin/indices/RestViewAction.java | 51 +++-- 8 files changed, 379 insertions(+), 64 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/View.java create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java rename server/src/main/java/org/opensearch/index/{views => view}/package-info.java (66%) delete mode 100644 server/src/main/java/org/opensearch/index/views/ViewDefinition.java delete mode 100644 server/src/main/java/org/opensearch/index/views/ViewService.java diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index bad881f8bda76..d2f4888ae8971 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -49,6 +49,7 @@ import org.opensearch.cluster.metadata.MetadataMappingService; import org.opensearch.cluster.metadata.MetadataUpdateSettingsService; import org.opensearch.cluster.metadata.RepositoriesMetadata; +import org.opensearch.cluster.metadata.ViewMetadata; import org.opensearch.cluster.metadata.WeightedRoutingMetadata; import org.opensearch.cluster.routing.DelayedAllocationService; import org.opensearch.cluster.routing.allocation.AllocationService; @@ -195,6 +196,7 @@ public static List getNamedWriteables() { ComposableIndexTemplateMetadata::readDiffFrom ); registerMetadataCustom(entries, DataStreamMetadata.TYPE, DataStreamMetadata::new, DataStreamMetadata::readDiffFrom); + registerMetadataCustom(entries, ViewMetadata.TYPE, ViewMetadata::new, ViewMetadata::readDiffFrom); registerMetadataCustom(entries, WeightedRoutingMetadata.TYPE, WeightedRoutingMetadata::new, WeightedRoutingMetadata::readDiffFrom); registerMetadataCustom( entries, @@ -292,6 +294,7 @@ public static List getNamedXWriteables() { DataStreamMetadata::fromXContent ) ); + entries.add(new NamedXContentRegistry.Entry(Metadata.Custom.class, new ParseField(ViewMetadata.TYPE), ViewMetadata::fromXContent)); entries.add( new NamedXContentRegistry.Entry( Metadata.Custom.class, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 1871ed24973c2..69e49e7aec6eb 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -831,6 +831,10 @@ public Map dataStreams() { .orElse(Collections.emptyMap()); } + public Map views() { + return Optional.ofNullable((ViewMetadata) this.custom(ViewMetadata.TYPE)).map(ViewMetadata::views).orElse(Collections.emptyMap()); + } + public DecommissionAttributeMetadata decommissionAttributeMetadata() { return custom(DecommissionAttributeMetadata.TYPE); } @@ -1325,6 +1329,36 @@ public Builder removeDataStream(String name) { return this; } + private Map getViews() { + return Optional.ofNullable(customs.get(ViewMetadata.TYPE)) + .map(o -> (ViewMetadata) o) + .map(vmd -> vmd.views()) + .orElse(new HashMap<>()); + } + + public View view(final String viewName) { + return getViews().get(viewName); + } + + public Builder views(final Map views) { + this.customs.put(ViewMetadata.TYPE, new ViewMetadata(views)); + return this; + } + + public Builder put(final View view) { + Objects.requireNonNull(view, "view cannot be null"); + final var replacementViews = new HashMap<>(getViews()); + replacementViews.put(view.name, view); + return views(replacementViews); + } + + public Builder removeView(final String viewName) { + Objects.requireNonNull(viewName, "viewName cannot be null"); + final var replacementViews = new HashMap<>(getViews()); + replacementViews.remove(viewName); + return views(replacementViews); + } + public Custom getCustom(String type) { return customs.get(type); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java new file mode 100644 index 0000000000000..a76c995915029 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -0,0 +1,120 @@ + +package org.opensearch.cluster.metadata; + +import org.opensearch.cluster.AbstractDiffable; +import org.opensearch.cluster.Diff; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.ParseField; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.core.xcontent.ConstructingObjectParser; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.List; + +@ExperimentalApi +public class View extends AbstractDiffable implements ToXContentObject { + + final String name; + final String description; + final long createdAt; + final long modifiedAt; + final List targets; + + public View(final String name, final String description, final long createdAt, final long modifiedAt, final List targets) { + this.name = name; + this.description = description; + this.createdAt = createdAt; + this.modifiedAt = modifiedAt; + this.targets = targets; + } + + public View(final StreamInput in) throws IOException { + this(in.readString(), in.readString(), in.readVLong(), in.readVLong(), in.readList(Target::new)); + } + + public static Diff readDiffFrom(StreamInput in) throws IOException { + return readDiffFrom(View::new, in); + } + + public static class Target implements Writeable, ToXContentObject { + + private String indexPattern; + + private Target(final String indexPattern) { + this.indexPattern = indexPattern; + } + + private Target(final StreamInput in) throws IOException { + this(in.readString()); + } + + private static final ParseField INDEX_PATTERN_FIELD = new ParseField("indexPattern"); + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(); + builder.field(INDEX_PATTERN_FIELD.getPreferredName(), indexPattern); + builder.endObject(); + return builder; + } + + public static ViewMetadata fromXContent(final XContentParser parser) throws IOException { + return null; // TODO + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(indexPattern); + } + } + + private static final ParseField NAME_FIELD = new ParseField("name"); + private static final ParseField DESCRIPTION_FIELD = new ParseField("description"); + private static final ParseField CREATED_AT_FIELD = new ParseField("createdAt"); + private static final ParseField MODIFIED_AT_FIELD = new ParseField("modifiedAt"); + private static final ParseField TARGETS_FIELD = new ParseField("targets"); + + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "view", + args -> new View((String) args[0], (String) args[1], (Long) args[2], (Long) args[3], (List) args[4]) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME_FIELD); + PARSER.declareString(ConstructingObjectParser.constructorArg(), DESCRIPTION_FIELD); + PARSER.declareLong(ConstructingObjectParser.constructorArg(), CREATED_AT_FIELD); + PARSER.declareLong(ConstructingObjectParser.constructorArg(), MODIFIED_AT_FIELD); + PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> Target.fromXContent(p), TARGETS_FIELD); + } + + public static View fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(); + builder.field(NAME_FIELD.getPreferredName(), name); + builder.field(DESCRIPTION_FIELD.getPreferredName(), description); + builder.field(CREATED_AT_FIELD.getPreferredName(), createdAt); + builder.field(MODIFIED_AT_FIELD.getPreferredName(), modifiedAt); + builder.field(TARGETS_FIELD.getPreferredName(), targets); + builder.endObject(); + return builder; + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + out.writeString(name); + out.writeString(description); + out.writeVLong(createdAt); + out.writeVLong(modifiedAt); + out.writeList(targets); + } +} diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java new file mode 100644 index 0000000000000..b2db4c9cb564b --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java @@ -0,0 +1,185 @@ +package org.opensearch.cluster.metadata; + +import org.opensearch.Version; +import org.opensearch.cluster.Diff; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.NamedDiff; +import org.opensearch.core.ParseField; +import org.opensearch.core.common.Strings; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ConstructingObjectParser; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import static org.opensearch.cluster.metadata.ComposableIndexTemplateMetadata.MINIMMAL_SUPPORTED_VERSION; + +/** + * TODO: Tests with failures? `./gradlew :server:test` + + - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsActualDeleteIOException + - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsDeleteDedup + - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsActualDeleteNoSuchFileException + - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsActualDelete + * + */ + +public class ViewMetadata implements Metadata.Custom { + + public static final String TYPE = "view"; + private static final ParseField VIEW_FIELD = new ParseField("view"); + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + TYPE, + false, + a -> new ViewMetadata((Map) a[0]) + ); + + static { + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> { + Map views = new HashMap<>(); + while (p.nextToken() != XContentParser.Token.END_OBJECT) { + views.put(p.currentName(), View.fromXContent(p)); + } + return views; + }, VIEW_FIELD); + } + + private final Map views; + + public ViewMetadata(final Map views) { + this.views = views; + } + + public ViewMetadata(final StreamInput in) throws IOException { + this.views = in.readMap(StreamInput::readString, View::new); + } + + public Map views() { + return this.views; + } + + @Override + public Diff diff(final Metadata.Custom before) { + return new ViewMetadata.ViewMetadataDiff((ViewMetadata) before, this); + } + + public static NamedDiff readDiffFrom(final StreamInput in) throws IOException { + return new ViewMetadata.ViewMetadataDiff(in); + } + + @Override + public EnumSet context() { + return Metadata.ALL_CONTEXTS; + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public Version getMinimalSupportedVersion() { + return MINIMMAL_SUPPORTED_VERSION; + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + out.writeMap(this.views, StreamOutput::writeString, (stream, val) -> val.writeTo(stream)); + } + + public static ViewMetadata fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(VIEW_FIELD.getPreferredName()); + for (Map.Entry entry : views.entrySet()) { + builder.field(entry.getKey(), entry.getValue()); + } + builder.endObject(); + return builder; + } + + public static Builder builder() { + return new Builder(); + } + + @Override + public int hashCode() { + return Objects.hash(this.views); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (obj.getClass() != getClass()) { + return false; + } + ViewMetadata other = (ViewMetadata) obj; + return Objects.equals(this.views, other.views); + } + + @Override + public String toString() { + return Strings.toString(MediaTypeRegistry.JSON, this); + } + + /** + * Builder of view metadata. + */ + public static class Builder { + + private final Map views = new HashMap<>(); + + public Builder putDataStream(final View view) { + views.put(view.name, view); + return this; + } + + public ViewMetadata build() { + return new ViewMetadata(views); + } + } + + /** + * A diff between view metadata. + */ + static class ViewMetadataDiff implements NamedDiff { + + final Diff> dataStreamDiff; + + ViewMetadataDiff(ViewMetadata before, ViewMetadata after) { + this.dataStreamDiff = DiffableUtils.diff(before.views, after.views, DiffableUtils.getStringKeySerializer()); + } + + ViewMetadataDiff(StreamInput in) throws IOException { + this.dataStreamDiff = DiffableUtils.readJdkMapDiff(in, DiffableUtils.getStringKeySerializer(), View::new, View::readDiffFrom); + } + + @Override + public Metadata.Custom apply(Metadata.Custom part) { + return new ViewMetadata(dataStreamDiff.apply(((ViewMetadata) part).views)); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + dataStreamDiff.writeTo(out); + } + + @Override + public String getWriteableName() { + return TYPE; + } + } +} diff --git a/server/src/main/java/org/opensearch/index/views/package-info.java b/server/src/main/java/org/opensearch/index/view/package-info.java similarity index 66% rename from server/src/main/java/org/opensearch/index/views/package-info.java rename to server/src/main/java/org/opensearch/index/view/package-info.java index 2b81ef1b1911a..bb65723bdd5cd 100644 --- a/server/src/main/java/org/opensearch/index/views/package-info.java +++ b/server/src/main/java/org/opensearch/index/view/package-info.java @@ -6,5 +6,5 @@ * compatible open source license. */ -/** Responsible for handling all projection of data in OpenSearch */ -package org.opensearch.index.views; +/** Core classes responsible for handling all view operations */ +package org.opensearch.index.view; diff --git a/server/src/main/java/org/opensearch/index/views/ViewDefinition.java b/server/src/main/java/org/opensearch/index/views/ViewDefinition.java deleted file mode 100644 index b9fa80204e267..0000000000000 --- a/server/src/main/java/org/opensearch/index/views/ViewDefinition.java +++ /dev/null @@ -1,26 +0,0 @@ -package org.opensearch.index.views; - -import java.util.List; - -import org.joda.time.DateTime; - -public class ViewDefinition /* implements Writeable, ToXContentFragment */ { - - private String identifier; - private String description; - private DateTime createdAt; - private DateTime modifiedAt; - private String query; /** TBD format? */ - private List indexPatterns; - - public static enum Computation { - Materialized, - Projected; - } - - public static class IndexPattern { - private String indexPattern; - - /** Define patterns properties */ - } -} diff --git a/server/src/main/java/org/opensearch/index/views/ViewService.java b/server/src/main/java/org/opensearch/index/views/ViewService.java deleted file mode 100644 index 30560d67db8ba..0000000000000 --- a/server/src/main/java/org/opensearch/index/views/ViewService.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.opensearch.index.views; - -import org.opensearch.common.annotation.ExperimentalApi; - -@ExperimentalApi -public class ViewService { - - public ViewDefinition getViewDefinition(final String id) {} - - public ViewDefinition createOrUpdateViewDefinition(final ViewDefinition view) {} - - public ViewDefinition removeDefinition(final String id) {} - - - public List listViews() {} - - - public - -} diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index 3c74bcc62441e..e6be71dd93b19 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -8,14 +8,9 @@ package org.opensearch.rest.action.admin.indices; -import org.opensearch.rest.NamedRoute; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; -import org.opensearch.common.Booleans; -import org.opensearch.common.logging.DeprecationLogger; -import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; @@ -27,7 +22,6 @@ import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; - public class RestViewAction extends BaseRestHandler { @Override @@ -35,9 +29,21 @@ public List routes() { return List.of( new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster.views.list").handler(this::handleGet).build(), new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster.views.create").handler(this::handlePost).build(), - new NamedRoute.Builder().path("/views/{view_id}").method(GET).uniqueName("cluster.views.get").handler(this::handleSingleGet).build(), - new NamedRoute.Builder().path("/views/{view_id}").method(DELETE).uniqueName("cluster.views.delete").handler(this::handleSingleDelete).build(), - new NamedRoute.Builder().path("/views/{view_id}").method(PUT).uniqueName("cluster.views.update").handler(this::handleSinglePut).build() + new NamedRoute.Builder().path("/views/{view_id}") + .method(GET) + .uniqueName("cluster.views.get") + .handler(this::handleSingleGet) + .build(), + new NamedRoute.Builder().path("/views/{view_id}") + .method(DELETE) + .uniqueName("cluster.views.delete") + .handler(this::handleSingleDelete) + .build(), + new NamedRoute.Builder().path("/views/{view_id}") + .method(PUT) + .uniqueName("cluster.views.update") + .handler(this::handleSinglePut) + .build() ); } @@ -53,11 +59,24 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC }; } - public RestResponse handleGet(final RestRequest r) { return null; } - public RestResponse handlePost(final RestRequest r) { return null; } + public RestResponse handleGet(final RestRequest r) { + return null; + } + + public RestResponse handlePost(final RestRequest r) { + return null; + } - public RestResponse handleSingleGet(final RestRequest r) { return null; } - public RestResponse handleSinglePut(final RestRequest r) { return null; } - public RestResponse handleSingleDelete(final RestRequest r) { return null; } + public RestResponse handleSingleGet(final RestRequest r) { + return null; + } + + public RestResponse handleSinglePut(final RestRequest r) { + return null; + } + + public RestResponse handleSingleDelete(final RestRequest r) { + return null; + } -} \ No newline at end of file +} From 4e12632fa439e81dc9de5c789e0ba886dec63551 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 15 Dec 2023 22:41:55 +0000 Subject: [PATCH 03/13] Basic request handling Signed-off-by: Peter Nied --- .../cluster/metadata/ViewMetadata.java | 2 +- .../action/admin/indices/RestViewAction.java | 106 +++++++++++++----- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java index b2db4c9cb564b..fc70f77b18ef9 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java @@ -28,7 +28,7 @@ - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsDeleteDedup - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsActualDeleteNoSuchFileException - org.opensearch.index.store.RemoteSegmentStoreDirectoryTests.testDeleteStaleCommitsActualDelete - * + * */ public class ViewMetadata implements Metadata.Custom { diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index e6be71dd93b19..31d5615e3f1ed 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -8,14 +8,29 @@ package org.opensearch.rest.action.admin.indices; +import joptsimple.internal.Strings; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.ClusterStateUpdateTask; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.metadata.View; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; import java.io.IOException; import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; @@ -24,26 +39,27 @@ public class RestViewAction extends BaseRestHandler { + private final static Logger LOG = LogManager.getLogger(RestViewAction.class); + + private static final String VIEW_ID = "view_id"; + + private final ClusterService clusterService; + + @Inject + public RestViewAction(final ClusterService clusterService) { + this.clusterService = clusterService; + } + @Override public List routes() { + final String viewIdParameter = "{" + VIEW_ID + "}"; + return List.of( - new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster.views.list").handler(this::handleGet).build(), - new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster.views.create").handler(this::handlePost).build(), - new NamedRoute.Builder().path("/views/{view_id}") - .method(GET) - .uniqueName("cluster.views.get") - .handler(this::handleSingleGet) - .build(), - new NamedRoute.Builder().path("/views/{view_id}") - .method(DELETE) - .uniqueName("cluster.views.delete") - .handler(this::handleSingleDelete) - .build(), - new NamedRoute.Builder().path("/views/{view_id}") - .method(PUT) - .uniqueName("cluster.views.update") - .handler(this::handleSinglePut) - .build() + new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster.views.list").build(), + new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster.views.create").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(GET).uniqueName("cluster.views.get").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(DELETE).uniqueName("cluster.views.delete").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(PUT).uniqueName("cluster.views.update").build() ); } @@ -54,21 +70,61 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - return channel -> { - // Consume the rest channel - }; + return channel -> {}; } - public RestResponse handleGet(final RestRequest r) { - return null; + public RestResponse handleGet(final RestRequest r, final XContentBuilder builder) throws IOException { + final List views = Optional.ofNullable(clusterService.state().getMetadata()) + .map(m -> m.views()) + .map(v -> v.values()) + .map(v -> v.stream().collect(Collectors.toList())) + .orElse(List.of()); + + return new BytesRestResponse(RestStatus.OK, builder.startObject().array("views", views).endObject()); } - public RestResponse handlePost(final RestRequest r) { + public RestResponse handlePost(final RestRequest r, final XContentBuilder builder) throws IOException { + final View view; + try (final XContentParser parser = r.contentParser()) { + view = View.fromXContent(parser); + } + + var task = new ClusterStateUpdateTask() { + @Override + public ClusterState execute(final ClusterState currentState) throws Exception { + return new ClusterState.Builder(clusterService.state()).metadata(Metadata.builder(currentState.metadata()).put(view)) + .build(); + } + + @Override + public void onFailure(final String source, final Exception e) { + LOG.error("Unable to create view, due to {}", source, e); + } + }; + + clusterService.submitStateUpdateTask("create_view_task", task); + + // TODO: How to unasync? + // TODO: Handle CREATED vs UPDATED return null; } - public RestResponse handleSingleGet(final RestRequest r) { - return null; + public RestResponse handleSingleGet(final RestRequest r, final XContentBuilder builder) throws IOException { + final String viewId = r.param(VIEW_ID); + + if (Strings.isNullOrEmpty(viewId)) { + return new BytesRestResponse(RestStatus.NOT_FOUND, ""); + } + + final Optional view = Optional.ofNullable(clusterService.state().getMetadata()) + .map(m -> m.views()) + .map(views -> views.get(viewId)); + + if (view.isEmpty()) { + return new BytesRestResponse(RestStatus.NOT_FOUND, ""); + } + + return new BytesRestResponse(RestStatus.OK, builder.startObject().value(view).endObject()); } public RestResponse handleSinglePut(final RestRequest r) { From 3a158633eac21a51d163134e2a0c9c941860bd8b Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 15 Dec 2023 23:58:02 +0000 Subject: [PATCH 04/13] Accepting requests Signed-off-by: Peter Nied --- .../org/opensearch/action/ActionModule.java | 1 + .../org/opensearch/cluster/metadata/View.java | 32 ++++++---- .../main/java/org/opensearch/node/Node.java | 3 + .../java/org/opensearch/rest/NamedRoute.java | 2 +- .../action/admin/indices/RestViewAction.java | 60 ++++++++++++++----- 5 files changed, 69 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 46775466aa615..1512eeb1ff44a 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -409,6 +409,7 @@ import org.opensearch.rest.action.admin.indices.RestUpgradeAction; import org.opensearch.rest.action.admin.indices.RestUpgradeStatusAction; import org.opensearch.rest.action.admin.indices.RestValidateQueryAction; +import org.opensearch.rest.action.admin.indices.RestViewAction; import org.opensearch.rest.action.cat.AbstractCatAction; import org.opensearch.rest.action.cat.RestAliasAction; import org.opensearch.rest.action.cat.RestAllocationAction; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index a76c995915029..184a17a0df23f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -19,11 +19,11 @@ @ExperimentalApi public class View extends AbstractDiffable implements ToXContentObject { - final String name; - final String description; - final long createdAt; - final long modifiedAt; - final List targets; + public final String name; + public final String description; + public final long createdAt; + public final long modifiedAt; + public final List targets; public View(final String name, final String description, final long createdAt, final long modifiedAt, final List targets) { this.name = name; @@ -34,7 +34,7 @@ public View(final String name, final String description, final long createdAt, f } public View(final StreamInput in) throws IOException { - this(in.readString(), in.readString(), in.readVLong(), in.readVLong(), in.readList(Target::new)); + this(in.readString(), in.readOptionalString(), in.readVLong(), in.readVLong(), in.readList(Target::new)); } public static Diff readDiffFrom(StreamInput in) throws IOException { @@ -62,9 +62,17 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa builder.endObject(); return builder; } + + private static final ConstructingObjectParser T_PARSER = new ConstructingObjectParser<>( + "target", + args -> new Target((String) args[0]) + ); + static { + T_PARSER.declareString(ConstructingObjectParser.constructorArg(), INDEX_PATTERN_FIELD); + } - public static ViewMetadata fromXContent(final XContentParser parser) throws IOException { - return null; // TODO + public static Target fromXContent(final XContentParser parser) throws IOException { + return T_PARSER.parse(parser, null); } @Override @@ -87,9 +95,9 @@ public void writeTo(StreamOutput out) throws IOException { static { PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME_FIELD); - PARSER.declareString(ConstructingObjectParser.constructorArg(), DESCRIPTION_FIELD); - PARSER.declareLong(ConstructingObjectParser.constructorArg(), CREATED_AT_FIELD); - PARSER.declareLong(ConstructingObjectParser.constructorArg(), MODIFIED_AT_FIELD); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), DESCRIPTION_FIELD); + PARSER.declareLongOrNull(ConstructingObjectParser.optionalConstructorArg(), -1L, CREATED_AT_FIELD); + PARSER.declareLongOrNull(ConstructingObjectParser.optionalConstructorArg(), -1L, MODIFIED_AT_FIELD); PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> Target.fromXContent(p), TARGETS_FIELD); } @@ -112,7 +120,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(name); - out.writeString(description); + out.writeOptionalString(description); out.writeVLong(createdAt); out.writeVLong(modifiedAt); out.writeList(targets); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 4cbf8dc191a9d..8115d583543f8 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -203,6 +203,7 @@ import org.opensearch.repositories.RepositoriesModule; import org.opensearch.repositories.RepositoriesService; import org.opensearch.rest.RestController; +import org.opensearch.rest.action.admin.indices.RestViewAction; import org.opensearch.script.ScriptContext; import org.opensearch.script.ScriptEngine; import org.opensearch.script.ScriptModule; @@ -896,6 +897,8 @@ protected Node( ); modules.add(actionModule); + actionModule.getRestController().registerHandler(new RestViewAction(clusterService)); + final RestController restController = actionModule.getRestController(); final NodeResourceUsageTracker nodeResourceUsageTracker = new NodeResourceUsageTracker( diff --git a/server/src/main/java/org/opensearch/rest/NamedRoute.java b/server/src/main/java/org/opensearch/rest/NamedRoute.java index 109f688a4924e..768d290f0fec9 100644 --- a/server/src/main/java/org/opensearch/rest/NamedRoute.java +++ b/server/src/main/java/org/opensearch/rest/NamedRoute.java @@ -144,7 +144,7 @@ private NamedRoute(Builder builder) { "Invalid route name specified. The route name may include the following characters" + " 'a-z', 'A-Z', '0-9', ':', '/', '*', '_' and be less than " + MAX_LENGTH_OF_ACTION_NAME - + " characters" + + " characters, " + builder.uniqueName ); } this.uniqueName = builder.uniqueName; diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index 31d5615e3f1ed..3800339ec222a 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -24,6 +24,7 @@ import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.NamedRoute; +import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; @@ -55,22 +56,39 @@ public List routes() { final String viewIdParameter = "{" + VIEW_ID + "}"; return List.of( - new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster.views.list").build(), - new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster.views.create").build(), - new NamedRoute.Builder().path("/views/" + viewIdParameter).method(GET).uniqueName("cluster.views.get").build(), - new NamedRoute.Builder().path("/views/" + viewIdParameter).method(DELETE).uniqueName("cluster.views.delete").build(), - new NamedRoute.Builder().path("/views/" + viewIdParameter).method(PUT).uniqueName("cluster.views.update").build() + new NamedRoute.Builder().path("/views").method(GET).uniqueName("cluster:views:list").build(), + new NamedRoute.Builder().path("/views").method(POST).uniqueName("cluster:views:create").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(GET).uniqueName("cluster:views:get").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(DELETE).uniqueName("cluster:views:delete").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter).method(PUT).uniqueName("cluster:views:update").build() ); } @Override public String getName() { - return "refresh_action"; + return "view_actions"; } @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - return channel -> {}; + if (!request.hasParam(VIEW_ID)) { + if (request.method() == RestRequest.Method.GET) { + return channel -> channel.sendResponse(handleGet(request, channel.newBuilder())); + } + + if (request.method() == RestRequest.Method.POST) { + return channel -> handlePost(request, channel); + } + + } else if (request.hasParam(VIEW_ID)) { + if (request.method() == RestRequest.Method.GET && request.hasParam(VIEW_ID)) { + return channel -> channel.sendResponse(handleSingleGet(request, channel.newBuilder())); + } + + } + + // Unhandled + return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, "Unable to process this request")); } public RestResponse handleGet(final RestRequest r, final XContentBuilder builder) throws IOException { @@ -80,16 +98,19 @@ public RestResponse handleGet(final RestRequest r, final XContentBuilder builder .map(v -> v.stream().collect(Collectors.toList())) .orElse(List.of()); - return new BytesRestResponse(RestStatus.OK, builder.startObject().array("views", views).endObject()); + return new BytesRestResponse(RestStatus.OK, builder.startObject().field("views", views).endObject()); } - public RestResponse handlePost(final RestRequest r, final XContentBuilder builder) throws IOException { - final View view; + public RestResponse handlePost(final RestRequest r, final RestChannel channel) throws IOException { + final View inputView; try (final XContentParser parser = r.contentParser()) { - view = View.fromXContent(parser); + inputView = View.fromXContent(parser); } - var task = new ClusterStateUpdateTask() { + final long currentTime = System.currentTimeMillis(); + final View view = new View(inputView.name, inputView.description, currentTime, currentTime, inputView.targets); + + clusterService.submitStateUpdateTask("create_view_task", new ClusterStateUpdateTask() { @Override public ClusterState execute(final ClusterState currentState) throws Exception { return new ClusterState.Builder(clusterService.state()).metadata(Metadata.builder(currentState.metadata()).put(view)) @@ -99,12 +120,19 @@ public ClusterState execute(final ClusterState currentState) throws Exception { @Override public void onFailure(final String source, final Exception e) { LOG.error("Unable to create view, due to {}", source, e); + channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, "Unknown error occurred, see the log for details.")); } - }; - clusterService.submitStateUpdateTask("create_view_task", task); - - // TODO: How to unasync? + @Override + public void clusterStateProcessed(final String source, final ClusterState oldState, final ClusterState newState) { + try { + channel.sendResponse(new BytesRestResponse(RestStatus.CREATED, channel.newBuilder().startObject().field(view.name, view).endObject())); + } catch (final IOException e) { + // TODO? + LOG.error(e); + } + } + }); // TODO: Handle CREATED vs UPDATED return null; } From 23632b6fb6443c3446177d5d70e376731ec58aa7 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Mon, 18 Dec 2023 23:35:08 +0000 Subject: [PATCH 05/13] Search rewriting and design dco Signed-off-by: Peter Nied --- .../org/opensearch/action/ActionModule.java | 1 - .../org/opensearch/cluster/metadata/View.java | 4 +- .../org/opensearch/index/view/views-design.md | 32 ++++++ .../java/org/opensearch/rest/NamedRoute.java | 3 +- .../action/admin/indices/RestViewAction.java | 28 +++-- .../admin/indices/RestViewSearchAction.java | 104 ++++++++++++++++++ 6 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/view/views-design.md create mode 100644 server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 1512eeb1ff44a..46775466aa615 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -409,7 +409,6 @@ import org.opensearch.rest.action.admin.indices.RestUpgradeAction; import org.opensearch.rest.action.admin.indices.RestUpgradeStatusAction; import org.opensearch.rest.action.admin.indices.RestValidateQueryAction; -import org.opensearch.rest.action.admin.indices.RestViewAction; import org.opensearch.rest.action.cat.AbstractCatAction; import org.opensearch.rest.action.cat.RestAliasAction; import org.opensearch.rest.action.cat.RestAllocationAction; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index 184a17a0df23f..3546613d9c4dd 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -43,7 +43,7 @@ public static Diff readDiffFrom(StreamInput in) throws IOException { public static class Target implements Writeable, ToXContentObject { - private String indexPattern; + public final String indexPattern; private Target(final String indexPattern) { this.indexPattern = indexPattern; @@ -62,7 +62,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa builder.endObject(); return builder; } - + private static final ConstructingObjectParser T_PARSER = new ConstructingObjectParser<>( "target", args -> new Target((String) args[0]) diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md new file mode 100644 index 0000000000000..c2a859c956adf --- /dev/null +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -0,0 +1,32 @@ +# Views + +Views define how searches are performed against indices on a cluster, uniform data access that is configured separately from the queries. + + +## Design + +```mermaid +sequenceDiagram + participant Client + participant HTTP_Request as ActionHandler + participant Cluster_Metadata as Cluster Metadata Store + participant Data_Store as Indices + + Client->>HTTP_Request: View List/Get/Update/Create/Delete
/views or /views/{view_id} + HTTP_Request->>Cluster_Metadata: Query Views + alt Update/Create/Delete + Cluster_Metadata->>Cluster_Metadata: Refresh Cluster + end + Cluster_Metadata-->>HTTP_Request: Return + HTTP_Request-->>Client: Return + + Client->>HTTP_Request: Search View
/views/{view_id}/search + HTTP_Request->>Cluster_Metadata: Query Views + Cluster_Metadata-->>HTTP_Request: Return + HTTP_Request->>HTTP_Request: Rewrite Search Request + HTTP_Request->>HTTP_Request: Validate Search Request + HTTP_Request->>Data_Store: Search indices + Data_Store-->>HTTP_Request: Return + HTTP_Request-->>Client: Return +``` + diff --git a/server/src/main/java/org/opensearch/rest/NamedRoute.java b/server/src/main/java/org/opensearch/rest/NamedRoute.java index 768d290f0fec9..b477cd8571416 100644 --- a/server/src/main/java/org/opensearch/rest/NamedRoute.java +++ b/server/src/main/java/org/opensearch/rest/NamedRoute.java @@ -144,7 +144,8 @@ private NamedRoute(Builder builder) { "Invalid route name specified. The route name may include the following characters" + " 'a-z', 'A-Z', '0-9', ':', '/', '*', '_' and be less than " + MAX_LENGTH_OF_ACTION_NAME - + " characters, " + builder.uniqueName + + " characters, " + + builder.uniqueName ); } this.uniqueName = builder.uniqueName; diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index 3800339ec222a..02c6e2adbcafe 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -42,7 +42,7 @@ public class RestViewAction extends BaseRestHandler { private final static Logger LOG = LogManager.getLogger(RestViewAction.class); - private static final String VIEW_ID = "view_id"; + static final String VIEW_ID = "view_id"; private final ClusterService clusterService; @@ -81,14 +81,22 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC } } else if (request.hasParam(VIEW_ID)) { - if (request.method() == RestRequest.Method.GET && request.hasParam(VIEW_ID)) { + if (request.method() == RestRequest.Method.GET) { return channel -> channel.sendResponse(handleSingleGet(request, channel.newBuilder())); } + if (request.method() == RestRequest.Method.PUT) { + return channel -> handleSinglePut(request); + } + + if (request.method() == RestRequest.Method.DELETE) { + return channel -> handleSingleDelete(request); + } } - // Unhandled - return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.BAD_REQUEST, "Unable to process this request")); + return channel -> channel.sendResponse( + new BytesRestResponse(RestStatus.BAD_REQUEST, "Unable to process " + request.method() + " on this endpoint " + request.path()) + ); } public RestResponse handleGet(final RestRequest r, final XContentBuilder builder) throws IOException { @@ -120,13 +128,17 @@ public ClusterState execute(final ClusterState currentState) throws Exception { @Override public void onFailure(final String source, final Exception e) { LOG.error("Unable to create view, due to {}", source, e); - channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, "Unknown error occurred, see the log for details.")); + channel.sendResponse( + new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, "Unknown error occurred, see the log for details.") + ); } @Override public void clusterStateProcessed(final String source, final ClusterState oldState, final ClusterState newState) { try { - channel.sendResponse(new BytesRestResponse(RestStatus.CREATED, channel.newBuilder().startObject().field(view.name, view).endObject())); + channel.sendResponse( + new BytesRestResponse(RestStatus.CREATED, channel.newBuilder().startObject().field(view.name, view).endObject()) + ); } catch (final IOException e) { // TODO? LOG.error(e); @@ -156,11 +168,11 @@ public RestResponse handleSingleGet(final RestRequest r, final XContentBuilder b } public RestResponse handleSinglePut(final RestRequest r) { - return null; + return new BytesRestResponse(RestStatus.NOT_IMPLEMENTED, ""); } public RestResponse handleSingleDelete(final RestRequest r) { - return null; + return new BytesRestResponse(RestStatus.NOT_IMPLEMENTED, ""); } } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java new file mode 100644 index 0000000000000..cd3b0fc06e508 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.admin.indices; + +import joptsimple.internal.Strings; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.search.SearchAction; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.metadata.View; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.NamedRoute; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.RestCancellableNodeClient; +import org.opensearch.rest.action.RestStatusToXContentListener; +import org.opensearch.rest.action.search.RestSearchAction; + +import java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.function.IntConsumer; +import java.util.stream.Collectors; + +import static org.opensearch.rest.RestRequest.Method.GET; +import static org.opensearch.rest.RestRequest.Method.POST; + +public class RestViewSearchAction extends BaseRestHandler { + + private final static Logger LOG = LogManager.getLogger(RestViewSearchAction.class); + + private static final String VIEW_ID = "view_id"; + + private final ClusterService clusterService; + + @Inject + public RestViewSearchAction(final ClusterService clusterService) { + this.clusterService = clusterService; + } + + @Override + public List routes() { + final String viewIdParameter = "{" + VIEW_ID + "}"; + + return List.of( + new NamedRoute.Builder().path("/views/" + viewIdParameter + "/_search").method(GET).uniqueName("cluster:views:search").build(), + new NamedRoute.Builder().path("/views/" + viewIdParameter + "/_search").method(POST).uniqueName("cluster:views:search").build() + ); + } + + @Override + public String getName() { + return "view_search_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + return channel -> { + final String viewId = request.param(VIEW_ID); + + if (Strings.isNullOrEmpty(viewId)) { + channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, "")); + } + + final Optional view = Optional.ofNullable(clusterService.state().getMetadata()) + .map(m -> m.views()) + .map(views -> views.get(viewId)); + + if (view.isEmpty()) { + channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, "")); + } + + final SearchRequest searchRequest = new SearchRequest(); + final IntConsumer setSize = size -> searchRequest.source().size(size); + + request.withContentOrSourceParamParserOrNull( + parser -> RestSearchAction.parseSearchRequest(searchRequest, request, parser, client.getNamedWriteableRegistry(), setSize) + ); + + // TODO: Only allow specific operations that are supported + + final String[] indices = view.get().targets.stream() + .map(target -> target.indexPattern) + .collect(Collectors.toList()) + .toArray(new String[0]); + searchRequest.indices(indices); + + // Resource leak on cancelClient??? Note; is already leaking in + // server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java + final RestCancellableNodeClient cancelClient = new RestCancellableNodeClient(client, request.getHttpChannel()); + cancelClient.execute(SearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel)); + }; + } +} From 603134775c06274d76778ff6f90d79c7a94f255c Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 19 Dec 2023 12:59:37 +0000 Subject: [PATCH 06/13] Functional views Signed-off-by: Peter Nied --- .../org/opensearch/index/view/views-design.md | 129 ++++++++++++++++++ .../main/java/org/opensearch/node/Node.java | 2 + .../admin/indices/RestViewSearchAction.java | 6 +- 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md index c2a859c956adf..ee6d10e7af3ef 100644 --- a/server/src/main/java/org/opensearch/index/view/views-design.md +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -30,3 +30,132 @@ sequenceDiagram HTTP_Request-->>Client: Return ``` + +### Local Testing + +``` +curl localhost:9200/abc/_doc \ + -XPOST \ + --header "Content-Type: application/json" \ + --data '{"foo":"bar"}' \ + +curl localhost:9200/views \ + -XPOST \ + --header "Content-Type: application/json" \ + --data '{"name":"hi", "createdAt": -1, "modifiedAt": -1, "targets":[]}' \ + -v + +curl localhost:9200/views \ + -XPOST \ + --header "Content-Type: application/json" \ + --data '{"name":"hi", "createdAt": -1, "modifiedAt": -1, "targets":[{"indexPattern":"abc"}]}' \ + -v + + +curl localhost:9200/views/hi/_search +``` + +### Appendix + +VIEW MODEL +{ + name: STRING, // [Optional] Friendly name resolves to ID + id: STRING, // Non-mutatable identifier + description: STRING, // [Optional] Description of the view + created: DATE, // Creation time of the view + modified: DATE // Last modified time of the view + query: QUERY, // enforced query + filter: QUERY, // P2 enforced query after transformations + targets: [ + { + indexPattern: STRING, // No wildcard/aliases! + // P2 Allow wildcard/aliases query parameter + query: QUERY, // enforced query specific for this target + filter: QUERY, // P2 enforced query specific after transformations + documentTransformer: SCRIPT // P2 Convert the results in some way + } + ], + documentTransformer: SCRIPT // P2 Convert the results in some way +} + + +View Operations +POST /views +GET /views/{view_id} +PUT /views/{view_id} +PATCH /views/{view_id} +DELETE /views/{view_id} + +Enumerate Views +GET /views + +Search Views // P2? +GET /views/_search +POST /views/_search + +Mapping // P2? Need to understand the utility / impact of not having this +GET /views/{view_id}/_mappings +PUT /views/{view_id}/_mappings +PATCH /views/{view_id}/_mappings + +Search Views +GET /views/{view_id}/_search +POST /views/{view_id}/_search + +// Results do not include any fields '_', how to protect leaking data? + + + +### Response on Create/Enumerate/Search + +views: [ + { + name: STRING, + id: STRING, + description: STRING, + created: DATE, + modified: DATE + } +] + + +## Permissions + +Views can be permissioned directly by name or id. User must also have READ permission for all indices targeted by the view, authorized before any of these operations. + +index:views.view.read[:{view_id}] +index:views.view.modify[:{view_id}] +index:views.view.delete[:{view_id}] + +Searching with a view. Read permission of the view is *not* required. + +index:views.view.search[:{view_id}] + +Creation / Enumeration / Searching views is supported. Only the view name and view id are returned. Create view call requires READ permission for all indices targeted by the view. + +index:views.enumerate +index:views.search +index:views.create + +### Internal functionality + +Search does not expose information about the targets or associated mappings. Internally used to resolve all indices and check permissions. + +GET /views/{view_id}/_resolveTargets + +{ +targets: [ + { + index: STRING, + fields: [ + { + fieldName: STRING + } + ], + _view: VIEW_MIN_OBJECT + }, ... +], +fields: [ FIELD_NAME,... ] +} + + diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 8115d583543f8..d9c139be9f915 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -204,6 +204,7 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.rest.RestController; import org.opensearch.rest.action.admin.indices.RestViewAction; +import org.opensearch.rest.action.admin.indices.RestViewSearchAction; import org.opensearch.script.ScriptContext; import org.opensearch.script.ScriptEngine; import org.opensearch.script.ScriptModule; @@ -898,6 +899,7 @@ protected Node( modules.add(actionModule); actionModule.getRestController().registerHandler(new RestViewAction(clusterService)); + actionModule.getRestController().registerHandler(new RestViewSearchAction(clusterService)); final RestController restController = actionModule.getRestController(); diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java index cd3b0fc06e508..4fe645dc20053 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java @@ -65,8 +65,8 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + final String viewId = request.param(VIEW_ID); return channel -> { - final String viewId = request.param(VIEW_ID); if (Strings.isNullOrEmpty(viewId)) { channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, "")); @@ -87,7 +87,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC parser -> RestSearchAction.parseSearchRequest(searchRequest, request, parser, client.getNamedWriteableRegistry(), setSize) ); - // TODO: Only allow specific operations that are supported + // TODO: Only allow operations that are supported final String[] indices = view.get().targets.stream() .map(target -> target.indexPattern) @@ -95,7 +95,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC .toArray(new String[0]); searchRequest.indices(indices); - // Resource leak on cancelClient??? Note; is already leaking in + // TODO: Look into resource leak on cancelClient? Note; is already leaking in // server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java final RestCancellableNodeClient cancelClient = new RestCancellableNodeClient(client, request.getHttpChannel()); cancelClient.execute(SearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel)); From c66425adb835337899fd8207561f0a4219d02bda Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 19 Dec 2023 15:02:27 +0000 Subject: [PATCH 07/13] Minor changes Signed-off-by: Peter Nied --- .../java/org/opensearch/cluster/metadata/View.java | 12 +++++++----- .../java/org/opensearch/index/view/views-design.md | 2 +- .../rest/action/admin/indices/RestViewAction.java | 1 + .../action/admin/indices/RestViewSearchAction.java | 1 + 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index 3546613d9c4dd..ee777ff0c9115 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.List; +/** TODO */ @ExperimentalApi public class View extends AbstractDiffable implements ToXContentObject { @@ -25,11 +26,11 @@ public class View extends AbstractDiffable implements ToXContentObject { public final long modifiedAt; public final List targets; - public View(final String name, final String description, final long createdAt, final long modifiedAt, final List targets) { + public View(final String name, final String description, final Long createdAt, final Long modifiedAt, final List targets) { this.name = name; this.description = description; - this.createdAt = createdAt; - this.modifiedAt = modifiedAt; + this.createdAt = createdAt != null ? createdAt : -1; + this.modifiedAt = modifiedAt != null ? modifiedAt : -1; this.targets = targets; } @@ -37,10 +38,11 @@ public View(final StreamInput in) throws IOException { this(in.readString(), in.readOptionalString(), in.readVLong(), in.readVLong(), in.readList(Target::new)); } - public static Diff readDiffFrom(StreamInput in) throws IOException { + public static Diff readDiffFrom(final StreamInput in) throws IOException { return readDiffFrom(View::new, in); } + /** TODO */ public static class Target implements Writeable, ToXContentObject { public final String indexPattern; @@ -76,7 +78,7 @@ public static Target fromXContent(final XContentParser parser) throws IOExceptio } @Override - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(final StreamOutput out) throws IOException { out.writeString(indexPattern); } } diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md index ee6d10e7af3ef..b297941083ddc 100644 --- a/server/src/main/java/org/opensearch/index/view/views-design.md +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -55,7 +55,7 @@ curl localhost:9200/views \ curl localhost:9200/views/hi/_search ``` -### Appendix +## Appendix VIEW MODEL { diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index 02c6e2adbcafe..c12f12eec4a29 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -38,6 +38,7 @@ import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; +/** TODO */ public class RestViewAction extends BaseRestHandler { private final static Logger LOG = LogManager.getLogger(RestViewAction.class); diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java index 4fe645dc20053..5a92caff6deab 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java @@ -35,6 +35,7 @@ import static org.opensearch.rest.RestRequest.Method.GET; import static org.opensearch.rest.RestRequest.Method.POST; +/** TODO */ public class RestViewSearchAction extends BaseRestHandler { private final static Logger LOG = LogManager.getLogger(RestViewSearchAction.class); From c191cc86f54237ecdc4abdc427960210004b6c62 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 19 Dec 2023 18:59:58 +0000 Subject: [PATCH 08/13] Resource Requset Signed-off-by: Peter Nied --- .../opensearch/action/ResourceRequest.java | 41 +++++++ .../action/search/SearchRequest.java | 10 +- .../action/search/ViewSearchRequest.java | 113 ++++++++++++++++++ .../org/opensearch/index/view/views-design.md | 55 +++++---- .../action/admin/indices/RestViewAction.java | 2 +- .../admin/indices/RestViewSearchAction.java | 18 +-- 6 files changed, 203 insertions(+), 36 deletions(-) create mode 100644 server/src/main/java/org/opensearch/action/ResourceRequest.java create mode 100644 server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java diff --git a/server/src/main/java/org/opensearch/action/ResourceRequest.java b/server/src/main/java/org/opensearch/action/ResourceRequest.java new file mode 100644 index 0000000000000..64fdc8d647649 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/ResourceRequest.java @@ -0,0 +1,41 @@ +package org.opensearch.action; + +import static org.opensearch.action.ValidateActions.addValidationError; + +import java.util.Map; +import java.util.regex.Pattern; + +import org.opensearch.common.ValidationException; +import org.opensearch.common.annotation.ExperimentalApi; + +/** + * TODO: This validation should be associated with REST requests to ensure the parameter is from the URL + * + * Context: If all of the resource information is in the URL, caching which can include responses or authorization are trivial + */ +@ExperimentalApi +public interface ResourceRequest { + + static final Pattern ALLOWED_KEY_PATTERN = Pattern.compile("[a-zA-Z_-]+"); + static final Pattern ALLOWED_VALUE_PATTERN = Pattern.compile("[a-zA-Z_-]+"); + + /** + * Extracts resource key value pairs from the request parameters + * Note; these resource must be part of the + */ + Map getResourceIds(); + + public static ActionRequestValidationException validResourceIds(final ResourceRequest resourceRequest, final ActionRequestValidationException validationException) { + resourceRequest.getResourceIds().entrySet().forEach(entry -> { + if (!ALLOWED_KEY_PATTERN.matcher(entry.getKey()).matches()) { + addValidationError("Unsupported resource key is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_VALUE_PATTERN.pattern(), validationException); + } + + if (!ALLOWED_VALUE_PATTERN.matcher(entry.getKey()).matches()) { + addValidationError("Unsupported resource value is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_VALUE_PATTERN.pattern(), validationException); + } + }); + + return validationException; + } +} diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 96cea17ff4972..67b78d84ce35d 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -82,13 +82,13 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla private static final long DEFAULT_ABSOLUTE_START_MILLIS = -1; - private final String localClusterAlias; - private final long absoluteStartMillis; - private final boolean finalReduce; + protected final String localClusterAlias; + protected final long absoluteStartMillis; + protected final boolean finalReduce; private SearchType searchType = SearchType.DEFAULT; - private String[] indices = Strings.EMPTY_ARRAY; + protected String[] indices = Strings.EMPTY_ARRAY; @Nullable private String routing; @@ -189,7 +189,7 @@ static SearchRequest subSearchRequest( return new SearchRequest(originalSearchRequest, indices, clusterAlias, absoluteStartMillis, finalReduce); } - private SearchRequest( + protected SearchRequest( SearchRequest searchRequest, String[] indices, String localClusterAlias, diff --git a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java new file mode 100644 index 0000000000000..36c9f3342c2b8 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java @@ -0,0 +1,113 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * 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. + */ + +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.action.search; + +import static org.opensearch.action.ValidateActions.addValidationError; + +import java.io.IOException; +import java.util.Map; +import java.util.Objects; +import java.util.function.Function; +import java.util.regex.Pattern; + +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ResourceRequest; +import org.opensearch.cluster.metadata.View; +import org.opensearch.common.ValidationException; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.rest.action.admin.indices.RestViewAction; + +/** Wraps the functionality of search requests and tailors for what is available when searching through views + */ +@ExperimentalApi +public class ViewSearchRequest extends SearchRequest implements ResourceRequest { + + public final View view; + + public ViewSearchRequest(final View view) { + super(); + this.view = view; + } + + public ViewSearchRequest(final StreamInput in) throws IOException { + super(in); + view = new View(in); + } + + + @Override + public ActionRequestValidationException validate() { + final Function unsupported = (String x) -> x + " is not supported when searching views"; + ActionRequestValidationException validationException = super.validate(); + + if (scroll() != null) { + validationException = addValidationError(unsupported.apply("Scroll"), validationException); + } + + // TODO: Filter out anything additional search features that are not supported + + validationException = ResourceRequest.validResourceIds(this, validationException); + + return validationException; + } + + + @Override + public void writeTo(final StreamOutput out) throws IOException { + super.writeTo(out); + view.writeTo(out); + } + + @Override + public boolean equals(final Object o) { + // TODO: Maybe this isn't standard practice + return this.hashCode() == o.hashCode(); + } + + @Override + public int hashCode() { + return Objects.hash(view, super.hashCode()); + } + + @Override + public String toString() { + return super.toString().replace("SearchRequest{", "ViewSearchRequest{view=" + view + ","); + } + + @Override + public Map getResourceIds() { + return Map.of(RestViewAction.VIEW_ID, view.name); + } +} diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md index b297941083ddc..c2b9d0882a1dd 100644 --- a/server/src/main/java/org/opensearch/index/view/views-design.md +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -57,6 +57,7 @@ curl localhost:9200/views/hi/_search ## Appendix +``` VIEW MODEL { name: STRING, // [Optional] Friendly name resolves to ID @@ -77,34 +78,45 @@ VIEW MODEL ], documentTransformer: SCRIPT // P2 Convert the results in some way } +``` +### View Operations -View Operations -POST /views -GET /views/{view_id} -PUT /views/{view_id} -PATCH /views/{view_id} -DELETE /views/{view_id} +| Method | Path | +| - | - | +| POST | /views | +| GET | /views/{view_id} | +| PUT | /views/{view_id} | +| PATCH | /views/{view_id} | +| DELETE | /views/{view_id} | -Enumerate Views -GET /views +### Enumerate Views -Search Views // P2? -GET /views/_search -POST /views/_search +| Method | Path | +| - | - | +| GET | /views | -Mapping // P2? Need to understand the utility / impact of not having this -GET /views/{view_id}/_mappings -PUT /views/{view_id}/_mappings -PATCH /views/{view_id}/_mappings +### Perform a Search on a view +| Method | Path | +| - | - | +| GET | /views/{view_id}/_search | +| POST | /views/{view_id}/_search | -Search Views -GET /views/{view_id}/_search -POST /views/{view_id}/_search +### Search Views // P2? +| Method | Path | +| - | - | +| GET | /views/_search | +| POST | /views/_search | -// Results do not include any fields '_', how to protect leaking data? +### Mapping // P2? Need to understand the utility / impact of not having this +| Method | Path | +| - | - | +| GET | /views/{view_id}/_mappings | +| PUT | /views/{view_id}/_mappings | +| PATCH | /views/{view_id}/_mappings | +*Results do not include any fields '_', how to protect leaking data?* ### Response on Create/Enumerate/Search @@ -141,8 +153,8 @@ index:views.create Search does not expose information about the targets or associated mappings. Internally used to resolve all indices and check permissions. +``` GET /views/{view_id}/_resolveTargets - { targets: [ { @@ -157,5 +169,4 @@ targets: [ ], fields: [ FIELD_NAME,... ] } - - +``` diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java index c12f12eec4a29..44ace1fcb1f6f 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewAction.java @@ -43,7 +43,7 @@ public class RestViewAction extends BaseRestHandler { private final static Logger LOG = LogManager.getLogger(RestViewAction.class); - static final String VIEW_ID = "view_id"; + public static final String VIEW_ID = "view_id"; private final ClusterService clusterService; diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java index 5a92caff6deab..ab9e57dfc6bd0 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.search.SearchAction; import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.ViewSearchRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.metadata.View; import org.opensearch.cluster.service.ClusterService; @@ -73,33 +74,34 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, "")); } - final Optional view = Optional.ofNullable(clusterService.state().getMetadata()) + final Optional optView = Optional.ofNullable(clusterService.state().getMetadata()) .map(m -> m.views()) .map(views -> views.get(viewId)); - if (view.isEmpty()) { + if (optView.isEmpty()) { channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, "")); } + final View view = optView.get(); - final SearchRequest searchRequest = new SearchRequest(); - final IntConsumer setSize = size -> searchRequest.source().size(size); + final ViewSearchRequest viewSearchRequest = new ViewSearchRequest(view); + final IntConsumer setSize = size -> viewSearchRequest.source().size(size); request.withContentOrSourceParamParserOrNull( - parser -> RestSearchAction.parseSearchRequest(searchRequest, request, parser, client.getNamedWriteableRegistry(), setSize) + parser -> RestSearchAction.parseSearchRequest(viewSearchRequest, request, parser, client.getNamedWriteableRegistry(), setSize) ); // TODO: Only allow operations that are supported - final String[] indices = view.get().targets.stream() + final String[] indices = view.targets.stream() .map(target -> target.indexPattern) .collect(Collectors.toList()) .toArray(new String[0]); - searchRequest.indices(indices); + viewSearchRequest.indices(indices); // TODO: Look into resource leak on cancelClient? Note; is already leaking in // server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java final RestCancellableNodeClient cancelClient = new RestCancellableNodeClient(client, request.getHttpChannel()); - cancelClient.execute(SearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel)); + cancelClient.execute(SearchAction.INSTANCE, viewSearchRequest, new RestStatusToXContentListener<>(channel)); }; } } From 4abefd0a214ae9afb0bdf201e40c8535dea66c5e Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 19 Dec 2023 21:47:36 +0000 Subject: [PATCH 09/13] Add FAQ sectino Signed-off-by: Peter Nied --- .../opensearch/action/ResourceRequest.java | 24 ++++++++++--------- .../action/search/ViewSearchRequest.java | 2 +- .../org/opensearch/index/view/views-design.md | 5 ++++ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/ResourceRequest.java b/server/src/main/java/org/opensearch/action/ResourceRequest.java index 64fdc8d647649..5606a13dc985c 100644 --- a/server/src/main/java/org/opensearch/action/ResourceRequest.java +++ b/server/src/main/java/org/opensearch/action/ResourceRequest.java @@ -5,34 +5,36 @@ import java.util.Map; import java.util.regex.Pattern; -import org.opensearch.common.ValidationException; import org.opensearch.common.annotation.ExperimentalApi; /** * TODO: This validation should be associated with REST requests to ensure the parameter is from the URL + * Note; this should work well in RestHandlers * * Context: If all of the resource information is in the URL, caching which can include responses or authorization are trivial */ @ExperimentalApi public interface ResourceRequest { - static final Pattern ALLOWED_KEY_PATTERN = Pattern.compile("[a-zA-Z_-]+"); - static final Pattern ALLOWED_VALUE_PATTERN = Pattern.compile("[a-zA-Z_-]+"); + static final Pattern ALLOWED_RESOURCE_NAME_PATTERN = Pattern.compile("[a-zA-Z_-]+"); + /** + * Don't allow wildcard patterns + * this has large impact to perf and cachability */ + static final Pattern ALLOWED_RESOURCE_ID_PATTERN = Pattern.compile("[a-zA-Z_-]+"); /** - * Extracts resource key value pairs from the request parameters - * Note; these resource must be part of the + * Validates the resource type and id pairs are in an allowed format */ - Map getResourceIds(); + Map getResourceTypeAndIds(); public static ActionRequestValidationException validResourceIds(final ResourceRequest resourceRequest, final ActionRequestValidationException validationException) { - resourceRequest.getResourceIds().entrySet().forEach(entry -> { - if (!ALLOWED_KEY_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError("Unsupported resource key is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_VALUE_PATTERN.pattern(), validationException); + resourceRequest.getResourceTypeAndIds().entrySet().forEach(entry -> { + if (!ALLOWED_RESOURCE_NAME_PATTERN.matcher(entry.getKey()).matches()) { + addValidationError("Unsupported resource key is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_NAME_PATTERN.pattern(), validationException); } - if (!ALLOWED_VALUE_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError("Unsupported resource value is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_VALUE_PATTERN.pattern(), validationException); + if (!ALLOWED_RESOURCE_ID_PATTERN.matcher(entry.getKey()).matches()) { + addValidationError("Unsupported resource value is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_ID_PATTERN.pattern(), validationException); } }); diff --git a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java index 36c9f3342c2b8..00c1c3c86a04f 100644 --- a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java @@ -107,7 +107,7 @@ public String toString() { } @Override - public Map getResourceIds() { + public Map getResourceTypeAndIds() { return Map.of(RestViewAction.VIEW_ID, view.name); } } diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md index c2b9d0882a1dd..4563cb114d2f8 100644 --- a/server/src/main/java/org/opensearch/index/view/views-design.md +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -30,6 +30,11 @@ sequenceDiagram HTTP_Request-->>Client: Return ``` +### Frequently Asked Questions + +#### How do views work with fine grain access control of index data? + +#### What happens with existing DLS and FLS rules and searches on views? ### Local Testing From dace87ac950f99e366b8de8ac589678b13b0cf09 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 22 Dec 2023 01:25:24 +0000 Subject: [PATCH 10/13] Tidying up Signed-off-by: Peter Nied --- .../opensearch/action/ResourceRequest.java | 46 ++++-- .../action/search/ViewSearchRequest.java | 46 +----- .../org/opensearch/cluster/metadata/View.java | 7 + .../cluster/metadata/ViewMetadata.java | 8 + .../org/opensearch/index/view/views-design.md | 153 ++++++++++++------ .../admin/indices/RestViewSearchAction.java | 9 +- 6 files changed, 167 insertions(+), 102 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/ResourceRequest.java b/server/src/main/java/org/opensearch/action/ResourceRequest.java index 5606a13dc985c..84766aaf810f4 100644 --- a/server/src/main/java/org/opensearch/action/ResourceRequest.java +++ b/server/src/main/java/org/opensearch/action/ResourceRequest.java @@ -1,40 +1,68 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.action; -import static org.opensearch.action.ValidateActions.addValidationError; +import org.opensearch.common.annotation.ExperimentalApi; import java.util.Map; import java.util.regex.Pattern; -import org.opensearch.common.annotation.ExperimentalApi; +import static org.opensearch.action.ValidateActions.addValidationError; /** * TODO: This validation should be associated with REST requests to ensure the parameter is from the URL * Note; this should work well in RestHandlers - * + * * Context: If all of the resource information is in the URL, caching which can include responses or authorization are trivial */ @ExperimentalApi public interface ResourceRequest { static final Pattern ALLOWED_RESOURCE_NAME_PATTERN = Pattern.compile("[a-zA-Z_-]+"); - /** + /** * Don't allow wildcard patterns * this has large impact to perf and cachability */ static final Pattern ALLOWED_RESOURCE_ID_PATTERN = Pattern.compile("[a-zA-Z_-]+"); + /** Returns the resource types and ids associated with this request */ + Map getResourceTypeAndIds(); + /** * Validates the resource type and id pairs are in an allowed format */ - Map getResourceTypeAndIds(); - - public static ActionRequestValidationException validResourceIds(final ResourceRequest resourceRequest, final ActionRequestValidationException validationException) { + public static ActionRequestValidationException validResourceIds( + final ResourceRequest resourceRequest, + final ActionRequestValidationException validationException + ) { resourceRequest.getResourceTypeAndIds().entrySet().forEach(entry -> { if (!ALLOWED_RESOURCE_NAME_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError("Unsupported resource key is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_NAME_PATTERN.pattern(), validationException); + addValidationError( + "Unsupported resource key is not supported; key: " + + entry.getKey() + + " value: " + + entry.getValue() + + " allowed pattern " + + ALLOWED_RESOURCE_NAME_PATTERN.pattern(), + validationException + ); } if (!ALLOWED_RESOURCE_ID_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError("Unsupported resource value is not supported; key: " + entry.getKey() + " value: " + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_ID_PATTERN.pattern(), validationException); + addValidationError( + "Unsupported resource value is not supported; key: " + + entry.getKey() + + " value: " + + entry.getValue() + + " allowed pattern " + + ALLOWED_RESOURCE_ID_PATTERN.pattern(), + validationException + ); } }); diff --git a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java index 00c1c3c86a04f..88d610fe29480 100644 --- a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java @@ -6,50 +6,20 @@ * compatible open source license. */ -/* - * 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. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - package org.opensearch.action.search; -import static org.opensearch.action.ValidateActions.addValidationError; - -import java.io.IOException; -import java.util.Map; -import java.util.Objects; -import java.util.function.Function; -import java.util.regex.Pattern; - import org.opensearch.action.ActionRequestValidationException; import org.opensearch.action.ResourceRequest; import org.opensearch.cluster.metadata.View; -import org.opensearch.common.ValidationException; -import org.opensearch.common.annotation.ExperimentalApi; -import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.common.at org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.rest.action.admin.indices.RestViewAction; +import org.opensearch.rest.action.admin.indicport java.util.Map; +import java.util.Objects; +import java.util.function.Function; + +import static org.opensearch.action.ValidateActions.addValidationError; -/** Wraps the functionality of search requests and tailors for what is available when searching through views +/** Wraps the functionality of search requests and tailors for what is available when searching through views */ @ExperimentalApi public class ViewSearchRequest extends SearchRequest implements ResourceRequest { @@ -66,7 +36,6 @@ public ViewSearchRequest(final StreamInput in) throws IOException { view = new View(in); } - @Override public ActionRequestValidationException validate() { final Function unsupported = (String x) -> x + " is not supported when searching views"; @@ -83,7 +52,6 @@ public ActionRequestValidationException validate() { return validationException; } - @Override public void writeTo(final StreamOutput out) throws IOException { super.writeTo(out); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index ee777ff0c9115..c32bd2e53842f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -1,3 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ package org.opensearch.cluster.metadata; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java index fc70f77b18ef9..f308cfdfb55bf 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java @@ -1,3 +1,11 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.cluster.metadata; import org.opensearch.Version; diff --git a/server/src/main/java/org/opensearch/index/view/views-design.md b/server/src/main/java/org/opensearch/index/view/views-design.md index 4563cb114d2f8..773b359cc1953 100644 --- a/server/src/main/java/org/opensearch/index/view/views-design.md +++ b/server/src/main/java/org/opensearch/index/view/views-design.md @@ -5,6 +5,44 @@ Views define how searches are performed against indices on a cluster, uniform da ## Design +### View data + +Views create a mapping to the resources that hold information to be searched over in a consistent manner. This abstraction allows for indirection with the backing indices, so they might be changed without callers being impacted. This can also be used to simplify the security model - searches over views do not require permissions to the backing indices only permissions to the view itself. + +```mermaid +classDiagram + class View { + +String name + +String description + +long createdAt + +long modifiedAt + +List targets + +toXContent(XContentBuilder, Params) XContentBuilder + +writeTo(StreamOutput) void + } + class Target { + +String indexPattern + +toXContent(XContentBuilder, Params) XContentBuilder + +writeTo(StreamOutput) void + } + class StreamOutput + class XContentBuilder + + View -- Target : contains + View -- StreamOutput : writes to + View -- XContentBuilder : outputs to + Target -- StreamOutput : writes to + Target -- XContentBuilder : outputs to +``` + +### View persistence + +Views are long lived objects in OpenSearch, all operations on them should be fully committed before responding to the caller. Views are intentionally created for user scenarios following a similar creation cadence to indices. + +Committed implies that the updates are synchronized across all nodes in a cluster. The Cluster Metadata Store is already available and allows for acknowledging that changes have been applied to all nodes. While this data could be stored in a new purpose built index, index data replication has delays and ensuring synchronization is non-trivial to implement as is seen in the Security plugins [1]. + +- [1] https://github.com/opensearch-project/security/issues/3275 + ```mermaid sequenceDiagram participant Client @@ -30,11 +68,63 @@ sequenceDiagram HTTP_Request-->>Client: Return ``` -### Frequently Asked Questions +### Resource Request + +In order to permissions views OpenSearch needs a way to consistently refer to them, this is a generic problem and views will be a first use case. Resource requests require a map of types to identifiers for the request, multiple resources could be part of a single request, but only one of each type. + +Considering the request to search a view, `POST /view/{view_id}/_search`, the path parameter 'view_id' is the type and the value from the request would be the identifier. + +```java +public interface ResourceRequest { + /** Returns the resource types and ids associated with this request */ + Map getResourceTypeAndIds(); + + /** Validates the resource type and id pairs are in an allowed format */ + public static ActionRequestValidationException validResourceIds( + final ResourceRequest resourceRequest, + final ActionRequestValidationException validationException + ) {;} +} +``` + +### Resource Permission Grants +With requests include resource type and identifiers the security plugin will need to allow for grants to these new types. Modify the security role to include this information so it can be checked and then the request can be permitted. + +```yaml +all_access: + reserved: true + hidden: false + static: true + description: "Allow full access to all indices and all cluster APIs" + cluster_permissions: + - "*" + index_permissions: + - index_patterns: + - "*" + allowed_actions: + - "*" + tenant_permissions: + - tenant_patterns: + - "*" + allowed_actions: + - "kibana_all_write" + resource_permissions: + - resource_type: "view" + resource_ids: ["songs", "albums"] +``` + +## Frequently Asked Questions + +### How do views work with fine grain access control of index data? +*To be determined...* -#### How do views work with fine grain access control of index data? +### What happens with existing DLS and FLS rules and searches on views? +*To be determined...* -#### What happens with existing DLS and FLS rules and searches on views? +### Additional Question(s) +*To be determined...* + +## Appendix ### Local Testing @@ -60,7 +150,7 @@ curl localhost:9200/views \ curl localhost:9200/views/hi/_search ``` -## Appendix +### v0 View Data Model ``` VIEW MODEL @@ -85,7 +175,7 @@ VIEW MODEL } ``` -### View Operations +#### View Operations | Method | Path | | - | - | @@ -95,25 +185,25 @@ VIEW MODEL | PATCH | /views/{view_id} | | DELETE | /views/{view_id} | -### Enumerate Views +#### Enumerate Views | Method | Path | | - | - | | GET | /views | -### Perform a Search on a view +#### Perform a Search on a view | Method | Path | | - | - | | GET | /views/{view_id}/_search | | POST | /views/{view_id}/_search | -### Search Views // P2? +#### Search Views // P2? | Method | Path | | - | - | | GET | /views/_search | | POST | /views/_search | -### Mapping // P2? Need to understand the utility / impact of not having this +#### Mapping // P2? Need to understand the utility / impact of not having this | Method | Path | | - | - | | GET | /views/{view_id}/_mappings | @@ -123,7 +213,7 @@ VIEW MODEL *Results do not include any fields '_', how to protect leaking data?* -### Response on Create/Enumerate/Search +#### Response on Create/Enumerate/Search views: [ { @@ -133,45 +223,4 @@ views: [ created: DATE, modified: DATE } -] - - -## Permissions - -Views can be permissioned directly by name or id. User must also have READ permission for all indices targeted by the view, authorized before any of these operations. - -index:views.view.read[:{view_id}] -index:views.view.modify[:{view_id}] -index:views.view.delete[:{view_id}] - -Searching with a view. Read permission of the view is *not* required. - -index:views.view.search[:{view_id}] - -Creation / Enumeration / Searching views is supported. Only the view name and view id are returned. Create view call requires READ permission for all indices targeted by the view. - -index:views.enumerate -index:views.search -index:views.create - -### Internal functionality - -Search does not expose information about the targets or associated mappings. Internally used to resolve all indices and check permissions. - -``` -GET /views/{view_id}/_resolveTargets -{ -targets: [ - { - index: STRING, - fields: [ - { - fieldName: STRING - } - ], - _view: VIEW_MIN_OBJECT - }, ... -], -fields: [ FIELD_NAME,... ] -} -``` +] \ No newline at end of file diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java index ab9e57dfc6bd0..bdda2971ac38a 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestViewSearchAction.java @@ -12,7 +12,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.action.search.SearchAction; -import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.ViewSearchRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.metadata.View; @@ -87,7 +86,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final IntConsumer setSize = size -> viewSearchRequest.source().size(size); request.withContentOrSourceParamParserOrNull( - parser -> RestSearchAction.parseSearchRequest(viewSearchRequest, request, parser, client.getNamedWriteableRegistry(), setSize) + parser -> RestSearchAction.parseSearchRequest( + viewSearchRequest, + request, + parser, + client.getNamedWriteableRegistry(), + setSize + ) ); // TODO: Only allow operations that are supported From ac25c960045ab6b7ac3ceea77a0ad7a380aa028e Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Mon, 29 Jan 2024 22:25:41 +0000 Subject: [PATCH 11/13] Fix build issues Signed-off-by: Peter Nied --- .../opensearch/action/ResourceRequest.java | 71 ------------------- .../action/search/ViewSearchRequest.java | 15 ++-- .../org/opensearch/cluster/metadata/View.java | 3 +- 3 files changed, 6 insertions(+), 83 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/action/ResourceRequest.java diff --git a/server/src/main/java/org/opensearch/action/ResourceRequest.java b/server/src/main/java/org/opensearch/action/ResourceRequest.java deleted file mode 100644 index 84766aaf810f4..0000000000000 --- a/server/src/main/java/org/opensearch/action/ResourceRequest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.action; - -import org.opensearch.common.annotation.ExperimentalApi; - -import java.util.Map; -import java.util.regex.Pattern; - -import static org.opensearch.action.ValidateActions.addValidationError; - -/** - * TODO: This validation should be associated with REST requests to ensure the parameter is from the URL - * Note; this should work well in RestHandlers - * - * Context: If all of the resource information is in the URL, caching which can include responses or authorization are trivial - */ -@ExperimentalApi -public interface ResourceRequest { - - static final Pattern ALLOWED_RESOURCE_NAME_PATTERN = Pattern.compile("[a-zA-Z_-]+"); - /** - * Don't allow wildcard patterns - * this has large impact to perf and cachability */ - static final Pattern ALLOWED_RESOURCE_ID_PATTERN = Pattern.compile("[a-zA-Z_-]+"); - - /** Returns the resource types and ids associated with this request */ - Map getResourceTypeAndIds(); - - /** - * Validates the resource type and id pairs are in an allowed format - */ - public static ActionRequestValidationException validResourceIds( - final ResourceRequest resourceRequest, - final ActionRequestValidationException validationException - ) { - resourceRequest.getResourceTypeAndIds().entrySet().forEach(entry -> { - if (!ALLOWED_RESOURCE_NAME_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError( - "Unsupported resource key is not supported; key: " - + entry.getKey() - + " value: " - + entry.getValue() - + " allowed pattern " - + ALLOWED_RESOURCE_NAME_PATTERN.pattern(), - validationException - ); - } - - if (!ALLOWED_RESOURCE_ID_PATTERN.matcher(entry.getKey()).matches()) { - addValidationError( - "Unsupported resource value is not supported; key: " - + entry.getKey() - + " value: " - + entry.getValue() - + " allowed pattern " - + ALLOWED_RESOURCE_ID_PATTERN.pattern(), - validationException - ); - } - }); - - return validationException; - } -} diff --git a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java index 88d610fe29480..0c3215e8e647a 100644 --- a/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java @@ -9,11 +9,11 @@ package org.opensearch.action.search; import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.action.ResourceRequest; import org.opensearch.cluster.metadata.View; -import org.opensearch.common.at org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.rest.action.admin.indicport java.util.Map; +import java.io.IOException; import java.util.Objects; import java.util.function.Function; @@ -22,7 +22,7 @@ /** Wraps the functionality of search requests and tailors for what is available when searching through views */ @ExperimentalApi -public class ViewSearchRequest extends SearchRequest implements ResourceRequest { +public class ViewSearchRequest extends SearchRequest { public final View view; @@ -47,8 +47,6 @@ public ActionRequestValidationException validate() { // TODO: Filter out anything additional search features that are not supported - validationException = ResourceRequest.validResourceIds(this, validationException); - return validationException; } @@ -73,9 +71,4 @@ public int hashCode() { public String toString() { return super.toString().replace("SearchRequest{", "ViewSearchRequest{view=" + view + ","); } - - @Override - public Map getResourceTypeAndIds() { - return Map.of(RestViewAction.VIEW_ID, view.name); - } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index c32bd2e53842f..26678b9eb123c 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -50,11 +50,12 @@ public static Diff readDiffFrom(final StreamInput in) throws IOException { } /** TODO */ + @ExperimentalApi public static class Target implements Writeable, ToXContentObject { public final String indexPattern; - private Target(final String indexPattern) { + Target(final String indexPattern) { this.indexPattern = indexPattern; } From 8e41fbeeb6ac54aeea57600da03860fb4b6d1881 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Mon, 29 Jan 2024 22:49:43 +0000 Subject: [PATCH 12/13] Unit tests for view Signed-off-by: Peter Nied --- .../cluster/metadata/ViewTests.java | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java new file mode 100644 index 0000000000000..4eab59d32ca0c --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java @@ -0,0 +1,97 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.cluster.metadata; + +import org.opensearch.cluster.metadata.View.Target; +import org.opensearch.common.UUIDs; +import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.core.index.Index; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.test.AbstractSerializingTestCase; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +import static org.opensearch.cluster.DataStreamTestHelper.createTimestampField; +import static org.opensearch.cluster.metadata.DataStream.getDefaultBackingIndexName; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; + +public class ViewTests extends AbstractSerializingTestCase { + + private static List randomTargets() { + int numTargets = randomIntBetween(0, 128); + List targets = new ArrayList<>(numTargets); + for (int i = 0; i < numTargets; i++) { + targets.add(new Target(randomAlphaOfLength(10).toLowerCase(Locale.ROOT)); + } + return targets; + } + + private static View randomInstance() { + final List targets = randomTargets(); + final String viewName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + final String description = randomAlphaOfLength(100).toLowerCase(Locale.ROOT); + return new View(viewName, description, Math.abs(randomLong()), Math.abs(randomLong()), targets); + } + + @Override + protected View doParseInstance(XContentParser parser) throws IOException { + return View.fromXContent(parser); + } + + @Override + protected Writeable.Reader instanceReader() { + return View::new; + } + + @Override + protected View createTestInstance() { + return randomInstance(); + } + + public void testNullName() { + final NullPointerException npe = assertThrows(NullPointerException.class, () -> new View(null, null, null, null, null)); + + assertThat(npe.getMessage(), equalTo("Name must be provided")); + } + + public void testNullTargets() { + final NullPointerException npe = assertThrows(NullPointerException.class, () -> new View("name", null, null, null, null)); + + assertThat(npe.getMessage(), equalTo("Targets are required on a view")); + } + + public void testNullTargetIndexPattern() { + final NullPointerException npe = assertThrows(NullPointerException.class, () -> new View.Target(null)); + + assertThat(npe.getMessage(), equalTo("IndexPattern is required")); + } + + + public void testDefaultValues() { + final View view = new View("myName", null, null, null, List.of()); + + assertThat(view.name, equalTo("myName")); + assertThat(view.description, equalTo(null)); + assertThat(view.createdAt, equalTo(-1L)); + assertThat(view.modifiedAt, equalTo(-1L)); + assertThat(view.targets, empty()); + } + + +} From cf22a8d7f7875e1ac30e176a67cd7a754bc2728d Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 1 Feb 2024 20:39:38 +0000 Subject: [PATCH 13/13] Basic pattern for decoupled views in metadata vs transport requests Signed-off-by: Peter Nied --- .../org/opensearch/action/ActionModule.java | 4 + .../admin/indices/view/CreateViewAction.java | 193 ++++++++++++++++++ .../admin/indices/view/package-info.java | 10 + .../org/opensearch/cluster/metadata/View.java | 11 +- .../cluster/metadata/ViewService.java | 54 +++++ .../main/java/org/opensearch/node/Node.java | 9 +- .../cluster/metadata/ViewTests.java | 3 - 7 files changed, 273 insertions(+), 11 deletions(-) create mode 100644 server/src/main/java/org/opensearch/action/admin/indices/view/CreateViewAction.java create mode 100644 server/src/main/java/org/opensearch/action/admin/indices/view/package-info.java create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/ViewService.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 46775466aa615..e2a738ac959a3 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -224,6 +224,7 @@ import org.opensearch.action.admin.indices.upgrade.post.UpgradeSettingsAction; import org.opensearch.action.admin.indices.validate.query.TransportValidateQueryAction; import org.opensearch.action.admin.indices.validate.query.ValidateQueryAction; +import org.opensearch.action.admin.indices.view.CreateViewAction; import org.opensearch.action.bulk.BulkAction; import org.opensearch.action.bulk.TransportBulkAction; import org.opensearch.action.bulk.TransportShardBulkAction; @@ -721,6 +722,9 @@ public void reg actions.register(ResolveIndexAction.INSTANCE, ResolveIndexAction.TransportAction.class); actions.register(DataStreamsStatsAction.INSTANCE, DataStreamsStatsAction.TransportAction.class); + // Views: + actions.register(CreateViewAction.INSTANCE, CreateViewAction.TransportAction.class); + // Persistent tasks: actions.register(StartPersistentTaskAction.INSTANCE, StartPersistentTaskAction.TransportAction.class); actions.register(UpdatePersistentTaskStatusAction.INSTANCE, UpdatePersistentTaskStatusAction.TransportAction.class); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/view/CreateViewAction.java b/server/src/main/java/org/opensearch/action/admin/indices/view/CreateViewAction.java new file mode 100644 index 0000000000000..10d84bd832229 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/indices/view/CreateViewAction.java @@ -0,0 +1,193 @@ +package org.opensearch.action.admin.indices.view; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ActionType; +import org.opensearch.action.ValidateActions; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.clustermanager.ClusterManagerNodeRequest; +import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.metadata.ViewService; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.Strings; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; + +/** Action to create a view */ +public class CreateViewAction extends ActionType { + + public static final CreateViewAction INSTANCE = new CreateViewAction(); + public static final String NAME = "cluster:views:create"; + + private CreateViewAction() { + super(NAME, CreateViewAction.Response::new); + } + + + /** View target representation for create requests */ + public static class ViewTarget implements Writeable { + public final String indexPattern; + + public ViewTarget(final String indexPattern) { + this.indexPattern = indexPattern; + } + + public ViewTarget(final StreamInput in) throws IOException { + this.indexPattern = in.readString(); + } + + public String getIndexPattern() { + return indexPattern; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(indexPattern); + } + + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + + if (Strings.isNullOrEmpty(indexPattern)) { + validationException = ValidateActions.addValidationError("index pattern cannot be empty or null", validationException); + } + + return validationException; + } + + } + + /** + * Request for Creating View + */ + public static class Request extends ClusterManagerNodeRequest { + private final String name; + private final String description; + private final List targets; + + public Request(final String name, final String description, final List targets) { + this.name = name; + this.description = description; + this.targets = targets; + } + + public String getName() { + return name; + } + + public String getDescription() { + return description; + } + + public List getTargets() { + return new ArrayList<>(targets); + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (Strings.isNullOrEmpty(name)) { + validationException = ValidateActions.addValidationError("Name is cannot be empty or null", validationException); + } + if (targets.isEmpty()) { + validationException = ValidateActions.addValidationError("targets cannot be empty", validationException); + } + + for (final ViewTarget target : targets) { + validationException = target.validate(); + } + + return validationException; + } + + public Request(final StreamInput in) throws IOException { + super(in); + this.name = in.readString(); + this.description = in.readString(); + this.targets = in.readList(ViewTarget::new); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(name); + out.writeString(description); + out.writeList(targets); + } + } + + /** Response after view is created */ + public static class Response extends ActionResponse { + + private final org.opensearch.cluster.metadata.View createdView; + + public Response(final org.opensearch.cluster.metadata.View createdView) { + this.createdView = createdView; + } + + public Response(final StreamInput in) throws IOException { + super(in); + this.createdView = new org.opensearch.cluster.metadata.View(in); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + this.createdView.writeTo(out); + } + } + + /** + * Transport Action for creating a View + */ + public static class TransportAction extends TransportClusterManagerNodeAction { + + private final ViewService viewService; + + @Inject + public TransportAction( + final TransportService transportService, + final ClusterService clusterService, + final ThreadPool threadPool, + final ActionFilters actionFilters, + final IndexNameExpressionResolver indexNameExpressionResolver, + final ViewService viewService + ) { + super(NAME, transportService, clusterService, threadPool, actionFilters, Request::new, indexNameExpressionResolver); + this.viewService = viewService; + } + + @Override + protected String executor() { + return ThreadPool.Names.SAME; + } + + @Override + protected Response read(StreamInput in) throws IOException { + return new Response(in); + } + + @Override + protected void clusterManagerOperation(Request request, ClusterState state, ActionListener listener) + throws Exception { + viewService.createView(request, listener); + } + + @Override + protected ClusterBlockException checkBlock(Request request, ClusterState state) { + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); + } + } +} diff --git a/server/src/main/java/org/opensearch/action/admin/indices/view/package-info.java b/server/src/main/java/org/opensearch/action/admin/indices/view/package-info.java new file mode 100644 index 0000000000000..db0556b1bf334 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/indices/view/package-info.java @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** View transport handlers. */ +package org.opensearch.action.admin.indices.view; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/View.java b/server/src/main/java/org/opensearch/cluster/metadata/View.java index 26678b9eb123c..8db65c6afaebe 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/View.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/View.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; +import java.util.Objects; /** TODO */ @ExperimentalApi @@ -34,11 +35,11 @@ public class View extends AbstractDiffable implements ToXContentObject { public final List targets; public View(final String name, final String description, final Long createdAt, final Long modifiedAt, final List targets) { - this.name = name; + this.name = Objects.requireNonNull(name, "Name must be provided"); this.description = description; this.createdAt = createdAt != null ? createdAt : -1; this.modifiedAt = modifiedAt != null ? modifiedAt : -1; - this.targets = targets; + this.targets = Objects.requireNonNull(targets, "Targets are required on a view"); } public View(final StreamInput in) throws IOException { @@ -55,11 +56,11 @@ public static class Target implements Writeable, ToXContentObject { public final String indexPattern; - Target(final String indexPattern) { - this.indexPattern = indexPattern; + public Target(final String indexPattern) { + this.indexPattern = Objects.requireNonNull(indexPattern, "IndexPattern is required"); } - private Target(final StreamInput in) throws IOException { + public Target(final StreamInput in) throws IOException { this(in.readString()); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ViewService.java b/server/src/main/java/org/opensearch/cluster/metadata/ViewService.java new file mode 100644 index 0000000000000..4056e477f2226 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/ViewService.java @@ -0,0 +1,54 @@ +package org.opensearch.cluster.metadata; + +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.admin.indices.view.CreateViewAction; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.ClusterStateUpdateTask; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.core.action.ActionListener; + +/** Service to interact with views, create, retrieve, update, and delete */ +public class ViewService { + + private final static Logger LOG = LogManager.getLogger(ViewService.class); + private final ClusterService clusterService; + + public ViewService(final ClusterService clusterService) { + this.clusterService = clusterService; + } + + public void createView(final CreateViewAction.Request request, final ActionListener listener) { + final long currentTime = System.currentTimeMillis(); + + final List targets = request.getTargets() + .stream() + .map(target -> new View.Target(target.getIndexPattern())) + .collect(Collectors.toList()); + final View view = new View(request.getName(), request.getDescription(), currentTime, currentTime, targets); + + clusterService.submitStateUpdateTask("create_view_task", new ClusterStateUpdateTask() { + @Override + public ClusterState execute(final ClusterState currentState) throws Exception { + return new ClusterState.Builder(clusterService.state()).metadata(Metadata.builder(currentState.metadata()).put(view)) + .build(); + } + + @Override + public void onFailure(final String source, final Exception e) { + LOG.error("Unable to create view, due to {}", source, e); + listener.onFailure(e); + } + + @Override + public void clusterStateProcessed(final String source, final ClusterState oldState, final ClusterState newState) { + final View createdView = newState.getMetadata().views().get(request.getName()); + final CreateViewAction.Response response = new CreateViewAction.Response(createdView); + listener.onResponse(response); + } + }); + } +} diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index d9c139be9f915..d26a7deae9e77 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -72,6 +72,7 @@ import org.opensearch.cluster.metadata.MetadataIndexUpgradeService; import org.opensearch.cluster.metadata.SystemIndexMetadataUpgradeService; import org.opensearch.cluster.metadata.TemplateUpgradeService; +import org.opensearch.cluster.metadata.ViewService; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.routing.BatchedRerouteService; @@ -862,6 +863,10 @@ protected Node( metadataCreateIndexService ); + final ViewService viewService = new ViewService( + clusterService + ); + Collection pluginComponents = pluginsService.filterPlugins(Plugin.class) .stream() .flatMap( @@ -898,9 +903,6 @@ protected Node( ); modules.add(actionModule); - actionModule.getRestController().registerHandler(new RestViewAction(clusterService)); - actionModule.getRestController().registerHandler(new RestViewSearchAction(clusterService)); - final RestController restController = actionModule.getRestController(); final NodeResourceUsageTracker nodeResourceUsageTracker = new NodeResourceUsageTracker( @@ -1222,6 +1224,7 @@ protected Node( b.bind(MetadataCreateIndexService.class).toInstance(metadataCreateIndexService); b.bind(AwarenessReplicaBalance.class).toInstance(awarenessReplicaBalance); b.bind(MetadataCreateDataStreamService.class).toInstance(metadataCreateDataStreamService); + b.bind(ViewService.class).toInstance(viewService); b.bind(SearchService.class).toInstance(searchService); b.bind(SearchTransportService.class).toInstance(searchTransportService); b.bind(SearchPhaseController.class) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java index 4eab59d32ca0c..c1184ddeca915 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ViewTests.java @@ -82,7 +82,6 @@ public void testNullTargetIndexPattern() { assertThat(npe.getMessage(), equalTo("IndexPattern is required")); } - public void testDefaultValues() { final View view = new View("myName", null, null, null, List.of()); @@ -92,6 +91,4 @@ public void testDefaultValues() { assertThat(view.modifiedAt, equalTo(-1L)); assertThat(view.targets, empty()); } - - }