Skip to content

Commit

Permalink
Resource Requset
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied committed Dec 19, 2023
1 parent c66425a commit c191cc8
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 36 deletions.
41 changes: 41 additions & 0 deletions server/src/main/java/org/opensearch/action/ResourceRequest.java
Original file line number Diff line number Diff line change
@@ -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<String, String> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, String> 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<String, String> getResourceIds() {
return Map.of(RestViewAction.VIEW_ID, view.name);
}
}
55 changes: 33 additions & 22 deletions server/src/main/java/org/opensearch/index/view/views-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ curl localhost:9200/views/hi/_search

## Appendix

```
VIEW MODEL
{
name: STRING, // [Optional] Friendly name resolves to ID
Expand All @@ -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

Expand Down Expand Up @@ -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: [
{
Expand All @@ -157,5 +169,4 @@ targets: [
],
fields: [ FIELD_NAME,... ]
}


```
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,33 +74,34 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
channel.sendResponse(new BytesRestResponse(RestStatus.NOT_FOUND, ""));
}

final Optional<View> view = Optional.ofNullable(clusterService.state().getMetadata())
final Optional<View> 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));
};
}
}

0 comments on commit c191cc8

Please sign in to comment.