Skip to content

Remove create parameter from put query ruleset API call #96883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Remove create parameter from put query ruleset API call #96883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions rest-api-spec/src/main/resources/rest-api-spec/api/query_ruleset.put.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@
}
]
},
"params": {
"create": {
"type": "boolean",
"description": "If true, requires that a query_ruleset with the specified resource_id does not already exist. (default: false)"
}
},
"body": {
"description": "The query ruleset configuration, including `rules`",
"required": true
Expand Down
6 changes: 1 addition & 5 deletions ...qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/200_query_ruleset_put.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
'Create Query Ruleset - Resource already exists':
- do:
query_ruleset.put:
create: true
ruleset_id: test-query-ruleset-recreating
body:
ruleset_id: 'test-query-ruleset-recreating'
Expand All @@ -55,9 +54,7 @@
- match: { result: 'created' }

- do:
catch: conflict
query_ruleset.put:
create: true
ruleset_id: test-query-ruleset-recreating
body:
ruleset_id: 'test-query-ruleset-recreating'
Expand All @@ -72,7 +69,7 @@
ids:
- 'id2'

- match: { error.type: 'version_conflict_engine_exception' }
- match: { result: 'updated' }

---
'Create Query Ruleset - Insufficient privilege':
Expand All @@ -84,7 +81,6 @@
headers: { Authorization: "Basic ZW50c2VhcmNoLXVzZXI6ZW50c2VhcmNoLXVzZXItcGFzc3dvcmQ=" } # user
query_ruleset.put:
ruleset_id: forbidden-query-ruleset
create: true
body:
ruleset_id: 'forbidden-query-ruleset'
rules:
Expand Down
6 changes: 2 additions & 4 deletions ...earch/src/main/java/org/elasticsearch/xpack/application/rules/QueryRulesIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,13 @@ private List<QueryRuleCriteria> parseCriteria(List<Map<String, Object>> rawCrite
* Creates or updates the {@link QueryRuleset} in the underlying index.
*
* @param queryRuleset The query ruleset object.
* @param create If true, a query ruleset with the specified unique identifier must not already exist
* @param listener The action listener to invoke on response/failure.
*/
public void putQueryRuleset(QueryRuleset queryRuleset, boolean create, ActionListener<IndexResponse> listener) {
public void putQueryRuleset(QueryRuleset queryRuleset, ActionListener<IndexResponse> listener) {
try {
DocWriteRequest.OpType opType = (create ? DocWriteRequest.OpType.CREATE : DocWriteRequest.OpType.INDEX);
final IndexRequest indexRequest = new IndexRequest(QUERY_RULES_ALIAS_NAME).opType(DocWriteRequest.OpType.INDEX)
.id(queryRuleset.id())
.opType(opType)
.opType(DocWriteRequest.OpType.INDEX)
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.source(queryRuleset.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS));
clientWithOrigin.index(indexRequest, listener);
Expand Down
17 changes: 4 additions & 13 deletions ...src/main/java/org/elasticsearch/xpack/application/rules/action/PutQueryRulesetAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,18 @@ public PutQueryRulesetAction() {
public static class Request extends ActionRequest {

private final QueryRuleset queryRuleset;
private final boolean create;

public Request(StreamInput in) throws IOException {
super(in);
this.queryRuleset = new QueryRuleset(in);
this.create = in.readBoolean();
}

public Request(QueryRuleset queryRuleset, boolean create) {
public Request(QueryRuleset queryRuleset) {
this.queryRuleset = queryRuleset;
this.create = create;
}

public Request(String rulesetId, boolean create, BytesReference content, XContentType contentType) {
public Request(String rulesetId, BytesReference content, XContentType contentType) {
this.queryRuleset = QueryRuleset.fromXContentBytes(rulesetId, content, contentType);
this.create = create;
}

@Override
Expand All @@ -79,28 +75,23 @@ public ActionRequestValidationException validate() {
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
queryRuleset.writeTo(out);
out.writeBoolean(create);
}

public QueryRuleset queryRuleset() {
return queryRuleset;
}

public boolean create() {
return create;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Request request = (Request) o;
return create == request.create && Objects.equals(queryRuleset, request.queryRuleset);
return Objects.equals(queryRuleset, request.queryRuleset);
}

@Override
public int hashCode() {
return Objects.hash(queryRuleset, create);
return Objects.hash(queryRuleset);
}
}

Expand Down
1 change: 0 additions & 1 deletion ...main/java/org/elasticsearch/xpack/application/rules/action/RestPutQueryRulesetAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public List<Route> routes() {
protected RestChannelConsumer innerPrepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
PutQueryRulesetAction.Request request = new PutQueryRulesetAction.Request(
restRequest.param("ruleset_id"),
restRequest.paramAsBoolean("create", false),
restRequest.content(),
restRequest.getXContentType()
);
Expand Down
3 changes: 1 addition & 2 deletions ...java/org/elasticsearch/xpack/application/rules/action/TransportPutQueryRulesetAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ public TransportPutQueryRulesetAction(TransportService transportService, ActionF
@Override
protected void doExecute(Task task, PutQueryRulesetAction.Request request, ActionListener<PutQueryRulesetAction.Response> listener) {
QueryRuleset queryRuleset = request.queryRuleset();
boolean create = request.create();
systemIndexService.putQueryRuleset(queryRuleset, create, listener.map(r -> new PutQueryRulesetAction.Response(r.getResult())));
systemIndexService.putQueryRuleset(queryRuleset, listener.map(r -> new PutQueryRulesetAction.Response(r.getResult())));

}
}
38 changes: 6 additions & 32 deletions .../src/test/java/org/elasticsearch/xpack/application/rules/QueryRulesIndexServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.elasticsearch.action.delete.DeleteResponse;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.engine.VersionConflictEngineException;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.SystemIndexPlugin;
Expand Down Expand Up @@ -59,31 +58,6 @@ public void testEmptyState() throws Exception {
assertThat(listResults.totalResults(), equalTo(0L));
}

public void testCreateQueryRuleset() throws Exception {
final QueryRule myQueryRule1 = new QueryRule(
"my_rule1",
QueryRuleType.PINNED,
List.of(new QueryRuleCriteria(CriteriaType.EXACT, "query_string", "foo")),
Map.of("ids", List.of("id1", "id2"))
);
final QueryRule myQueryRule2 = new QueryRule(
"my_rule2",
QueryRuleType.PINNED,
List.of(new QueryRuleCriteria(CriteriaType.EXACT, "query_string", "bar")),
Map.of("ids", List.of("id3", "id4"))
);
final QueryRuleset myQueryRuleset = new QueryRuleset("my_ruleset", List.of(myQueryRule1, myQueryRule2));

IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset, true);
assertThat(resp.status(), equalTo(RestStatus.CREATED));
assertThat(resp.getIndex(), equalTo(QUERY_RULES_CONCRETE_INDEX_NAME));

QueryRuleset getQueryRuleset = awaitGetQueryRuleset(myQueryRuleset.id());
assertThat(getQueryRuleset, equalTo(myQueryRuleset));

expectThrows(VersionConflictEngineException.class, () -> awaitPutQueryRuleset(myQueryRuleset, true));
}

public void testUpdateQueryRuleset() throws Exception {
{
final QueryRule myQueryRule1 = new QueryRule(
Expand All @@ -93,7 +67,7 @@ public void testUpdateQueryRuleset() throws Exception {
Map.of("ids", List.of("id1", "id2"))
);
final QueryRuleset myQueryRuleset = new QueryRuleset("my_ruleset", Collections.singletonList(myQueryRule1));
IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset, false);
IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset);
assertThat(resp.status(), anyOf(equalTo(RestStatus.CREATED), equalTo(RestStatus.OK)));
assertThat(resp.getIndex(), equalTo(QUERY_RULES_CONCRETE_INDEX_NAME));

Expand All @@ -114,7 +88,7 @@ public void testUpdateQueryRuleset() throws Exception {
Map.of("docs", List.of(Map.of("_index", "my_index1", "_id", "id3"), Map.of("_index", "my_index2", "_id", "id4")))
);
final QueryRuleset myQueryRuleset = new QueryRuleset("my_ruleset", List.of(myQueryRule1, myQueryRule2));
IndexResponse newResp = awaitPutQueryRuleset(myQueryRuleset, false);
IndexResponse newResp = awaitPutQueryRuleset(myQueryRuleset);
assertThat(newResp.status(), equalTo(RestStatus.OK));
assertThat(newResp.getIndex(), equalTo(QUERY_RULES_CONCRETE_INDEX_NAME));
QueryRuleset getQueryRuleset = awaitGetQueryRuleset(myQueryRuleset.id());
Expand All @@ -140,7 +114,7 @@ public void testListQueryRulesets() throws Exception {
);
final QueryRuleset myQueryRuleset = new QueryRuleset("my_ruleset_" + i, rules);

IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset, false);
IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset);
assertThat(resp.status(), equalTo(RestStatus.CREATED));
assertThat(resp.getIndex(), equalTo(QUERY_RULES_CONCRETE_INDEX_NAME));
}
Expand Down Expand Up @@ -188,7 +162,7 @@ public void testDeleteQueryRuleset() throws Exception {
Map.of("ids", List.of("id3", "id4"))
);
final QueryRuleset myQueryRuleset = new QueryRuleset("my_ruleset", List.of(myQueryRule1, myQueryRule2));
IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset, false);
IndexResponse resp = awaitPutQueryRuleset(myQueryRuleset);
assertThat(resp.status(), anyOf(equalTo(RestStatus.CREATED), equalTo(RestStatus.OK)));
assertThat(resp.getIndex(), equalTo(QUERY_RULES_CONCRETE_INDEX_NAME));

Expand All @@ -201,11 +175,11 @@ public void testDeleteQueryRuleset() throws Exception {
expectThrows(ResourceNotFoundException.class, () -> awaitGetQueryRuleset("my_ruleset"));
}

private IndexResponse awaitPutQueryRuleset(QueryRuleset queryRuleset, boolean create) throws Exception {
private IndexResponse awaitPutQueryRuleset(QueryRuleset queryRuleset) throws Exception {
CountDownLatch latch = new CountDownLatch(1);
final AtomicReference<IndexResponse> resp = new AtomicReference<>(null);
final AtomicReference<Exception> exc = new AtomicReference<>(null);
queryRulesIndexService.putQueryRuleset(queryRuleset, create, new ActionListener<>() {
queryRulesIndexService.putQueryRuleset(queryRuleset, new ActionListener<>() {
@Override
public void onResponse(IndexResponse indexResponse) {
resp.set(indexResponse);
Expand Down
2 changes: 1 addition & 1 deletion ...icsearch/xpack/application/rules/action/PutQueryRulesetActionRequestSerializingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ protected Writeable.Reader<PutQueryRulesetAction.Request> instanceReader() {

@Override
protected PutQueryRulesetAction.Request createTestInstance() {
return new PutQueryRulesetAction.Request(SearchApplicationTestUtils.randomQueryRuleset(), randomBoolean());
return new PutQueryRulesetAction.Request(SearchApplicationTestUtils.randomQueryRuleset());
}

@Override
Expand Down