diff --git a/CHANGELOG.md b/CHANGELOG.md index 273c4c3164b71..6b472e38ac6e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Implement parallel shard refresh behind cluster settings ([#17782](https://github.com/opensearch-project/OpenSearch/pull/17782)) - Bump OpenSearch Core main branch to 3.0.0 ([#18039](https://github.com/opensearch-project/OpenSearch/pull/18039)) - [Rule based Auto-tagging] Add wlm `ActionFilter` ([#17791](https://github.com/opensearch-project/OpenSearch/pull/17791)) +- [Rule based auto-tagging] Add update rule API ([#17797](https://github.com/opensearch-project/OpenSearch/pull/17797)) - Update API of Message in index to add the timestamp for lag calculation in ingestion polling ([#17977](https://github.com/opensearch-project/OpenSearch/pull/17977/)) - Add Warm Disk Threshold Allocation Decider for Warm shards ([#18082](https://github.com/opensearch-project/OpenSearch/pull/18082)) - Add composite directory factory ([#17988](https://github.com/opensearch-project/OpenSearch/pull/17988)) diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RulePersistenceService.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RulePersistenceService.java index b29323da421e7..da0396fc57d27 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RulePersistenceService.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RulePersistenceService.java @@ -10,6 +10,13 @@ import org.opensearch.action.support.clustermanager.AcknowledgedResponse; import org.opensearch.core.action.ActionListener; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; +import org.opensearch.rule.action.DeleteRuleRequest; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; /** * Interface for a service that handles rule persistence CRUD operations. @@ -37,4 +44,11 @@ public interface RulePersistenceService { * @param listener The listener that will handle the response or failure. */ void deleteRule(DeleteRuleRequest request, ActionListener listener); + + /** + * Update rule based on the provided request. + * @param request The request containing the details for updating the rule. + * @param listener The listener that will handle the response or failure. + */ + void updateRule(UpdateRuleRequest request, ActionListener listener); } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleQueryMapper.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleQueryMapper.java index 62dce30d04109..f0ee052790b4b 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleQueryMapper.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleQueryMapper.java @@ -9,6 +9,7 @@ package org.opensearch.rule; import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.rule.action.GetRuleRequest; /** * This interface is responsible for creating query objects which storage layer can use diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleRoutingService.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleRoutingService.java index e0d08f371a2aa..f3e0e6febf428 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleRoutingService.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleRoutingService.java @@ -9,6 +9,10 @@ package org.opensearch.rule; import org.opensearch.core.action.ActionListener; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; /** * Interface that handles rule routing logic @@ -22,4 +26,11 @@ public interface RuleRoutingService { * @param listener listener to handle the final response */ void handleCreateRuleRequest(CreateRuleRequest request, ActionListener listener); + + /** + * Handles a update rule request by routing it to the appropriate node. + * @param request the update rule request + * @param listener listener to handle the final response + */ + void handleUpdateRuleRequest(UpdateRuleRequest request, ActionListener listener); } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleUtils.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleUtils.java index 7c66eac988f9b..cbf7eb8ce6ec7 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleUtils.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/RuleUtils.java @@ -9,13 +9,19 @@ package org.opensearch.rule; import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.rule.action.UpdateRuleRequest; import org.opensearch.rule.autotagging.Attribute; +import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule; +import java.nio.charset.StandardCharsets; +import java.time.Instant; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; /** * Utility class for operations related to {@link Rule} objects. @@ -29,6 +35,24 @@ public class RuleUtils { */ public RuleUtils() {} + /** + * Computes a UUID-based hash string for a rule based on its key attributes. + * @param description the rule's description + * @param featureType the rule's feature type + * @param attributeMap the rule's attribute map (will use its toString representation) + * @param featureValue the rule's feature value + */ + public static String computeRuleHash( + String description, + FeatureType featureType, + Map> attributeMap, + String featureValue + ) { + String combined = description + "|" + featureType.getName() + "|" + attributeMap.toString() + "|" + featureValue; + UUID uuid = UUID.nameUUIDFromBytes(combined.getBytes(StandardCharsets.UTF_8)); + return uuid.toString(); + } + /** * Checks if a duplicate rule exists and returns its id. * Two rules are considered to be duplicate when meeting all the criteria below @@ -38,12 +62,11 @@ public RuleUtils() {} * between the current rule and the one being checked. * * @param rule The rule to be validated against ruleMap. - * @param ruleMap This map contains existing rules to be checked + * @param ruleList This list contains existing rules to be checked */ - public static Optional getDuplicateRuleId(Rule rule, Map ruleMap) { + public static Optional getDuplicateRuleId(Rule rule, List ruleList) { Map> targetAttributeMap = rule.getAttributeMap(); - for (Map.Entry entry : ruleMap.entrySet()) { - Rule currRule = entry.getValue(); + for (Rule currRule : ruleList) { Map> existingAttributeMap = currRule.getAttributeMap(); if (rule.getFeatureType() != currRule.getFeatureType() || targetAttributeMap.size() != existingAttributeMap.size()) { @@ -59,9 +82,30 @@ public static Optional getDuplicateRuleId(Rule rule, Map r } } if (allAttributesIntersect) { - return Optional.of(entry.getKey()); + return Optional.of(currRule.getId()); } } return Optional.empty(); } + + /** + * Creates an updated {@link Rule} object by applying non-null fields from the given {@link UpdateRuleRequest} + * to the original rule. Fields not provided in the request will retain their values from the original rule. + * @param originalRule the original rule to update + * @param request the request containing the new values for the rule + * @param featureType the feature type to assign to the updated rule + */ + public static Rule composeUpdatedRule(Rule originalRule, UpdateRuleRequest request, FeatureType featureType) { + String requestDescription = request.getDescription(); + Map> requestMap = request.getAttributeMap(); + String requestLabel = request.getFeatureValue(); + return new Rule( + originalRule.getId(), + requestDescription == null ? originalRule.getDescription() : requestDescription, + requestMap == null || requestMap.isEmpty() ? originalRule.getAttributeMap() : requestMap, + featureType, + requestLabel == null ? originalRule.getFeatureValue() : requestLabel, + Instant.now().toString() + ); + } } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleRequest.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleRequest.java similarity index 98% rename from modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleRequest.java rename to modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleRequest.java index 7963357b893d9..aaf05e698a506 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleRequest.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleRequest.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rule; +package org.opensearch.rule.action; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleResponse.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleResponse.java similarity index 79% rename from modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleResponse.java rename to modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleResponse.java index f040372b69335..60001038bdadc 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/CreateRuleResponse.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/CreateRuleResponse.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rule; +package org.opensearch.rule.action; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.StreamInput; @@ -17,15 +17,12 @@ import org.opensearch.rule.autotagging.Rule; import java.io.IOException; -import java.util.Map; - -import static org.opensearch.rule.autotagging.Rule._ID_STRING; /** * Response for the create API for Rule * Example response: * { - * "_id":"wi6VApYBoX5wstmtU_8l", + * "id":"wi6VApYBoX5wstmtU_8l", * "description":"description1", * "index_pattern":["log*", "uvent*"], * "workload_group":"poOiU851RwyLYvV5lbvv5w", @@ -34,16 +31,13 @@ * @opensearch.experimental */ public class CreateRuleResponse extends ActionResponse implements ToXContent, ToXContentObject { - private final String _id; private final Rule rule; /** * contructor for CreateRuleResponse - * @param id - the id for the rule created * @param rule - the rule created */ - public CreateRuleResponse(String id, final Rule rule) { - this._id = id; + public CreateRuleResponse(final Rule rule) { this.rule = rule; } @@ -52,19 +46,17 @@ public CreateRuleResponse(String id, final Rule rule) { * @param in - The {@link StreamInput} instance to read from. */ public CreateRuleResponse(StreamInput in) throws IOException { - _id = in.readString(); rule = new Rule(in); } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeString(_id); rule.writeTo(out); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return rule.toXContent(builder, new MapParams(Map.of(_ID_STRING, _id))); + return rule.toXContent(builder, params); } /** diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/DeleteRuleRequest.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/DeleteRuleRequest.java similarity index 97% rename from modules/autotagging-commons/common/src/main/java/org/opensearch/rule/DeleteRuleRequest.java rename to modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/DeleteRuleRequest.java index b60755f051be0..de36e3646f5cd 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/DeleteRuleRequest.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/DeleteRuleRequest.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rule; +package org.opensearch.rule.action; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; @@ -80,7 +80,7 @@ public String getRuleId() { * * @return The feature type. */ - public FeatureType getFeatureType() { + FeatureType getFeatureType() { return featureType; } } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleRequest.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java similarity index 96% rename from modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleRequest.java rename to modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java index 630a329688b2b..e6da349b046c7 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleRequest.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rule; +package org.opensearch.rule.action; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; @@ -67,7 +67,7 @@ public GetRuleRequest(StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { if (RuleValidator.isEmpty(id)) { - throw new IllegalArgumentException(Rule._ID_STRING + " cannot be empty."); + throw new IllegalArgumentException(Rule.ID_STRING + " cannot be empty."); } if (RuleValidator.isEmpty(searchAfter)) { throw new IllegalArgumentException("search_after cannot be empty."); diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleResponse.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleResponse.java similarity index 77% rename from modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleResponse.java rename to modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleResponse.java index 2ce79850084db..8928c0fa0d082 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/GetRuleResponse.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleResponse.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rule; +package org.opensearch.rule.action; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.action.ActionResponse; @@ -18,9 +18,7 @@ import org.opensearch.rule.autotagging.Rule; import java.io.IOException; -import java.util.Map; - -import static org.opensearch.rule.autotagging.Rule._ID_STRING; +import java.util.List; /** * Response for the get API for Rule. @@ -28,7 +26,7 @@ * { * "rules": [ * { - * "_id": "z1MJApUB0zgMcDmz-UQq", + * "id": "z1MJApUB0zgMcDmz-UQq", * "description": "Rule for tagging workload_group_id to index123" * "index_pattern": ["index123"], * "workload_group": "workload_group_id", @@ -42,7 +40,7 @@ */ @ExperimentalApi public class GetRuleResponse extends ActionResponse implements ToXContent, ToXContentObject { - private final Map rules; + private final List rules; private final String searchAfter; /** @@ -50,7 +48,7 @@ public class GetRuleResponse extends ActionResponse implements ToXContent, ToXCo * @param rules - Rules get from the request * @param searchAfter - The sort value used for pagination. */ - public GetRuleResponse(final Map rules, String searchAfter) { + public GetRuleResponse(final List rules, String searchAfter) { this.rules = rules; this.searchAfter = searchAfter; } @@ -60,12 +58,12 @@ public GetRuleResponse(final Map rules, String searchAfter) { * @param in - The {@link StreamInput} instance to read from. */ public GetRuleResponse(StreamInput in) throws IOException { - this(in.readMap(StreamInput::readString, Rule::new), in.readOptionalString()); + this(in.readList(Rule::new), in.readOptionalString()); } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeMap(rules, StreamOutput::writeString, (outStream, rule) -> rule.writeTo(outStream)); + out.writeList(rules); out.writeOptionalString(searchAfter); } @@ -73,8 +71,8 @@ public void writeTo(StreamOutput out) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.startArray("rules"); - for (Map.Entry entry : rules.entrySet()) { - entry.getValue().toXContent(builder, new MapParams(Map.of(_ID_STRING, entry.getKey()))); + for (Rule rule : rules) { + rule.toXContent(builder, params); } builder.endArray(); if (searchAfter != null && !searchAfter.isEmpty()) { @@ -87,7 +85,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws /** * rules getter */ - public Map getRules() { + public List getRules() { return rules; } } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleRequest.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleRequest.java new file mode 100644 index 0000000000000..824102c40da6a --- /dev/null +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleRequest.java @@ -0,0 +1,127 @@ +/* + * 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.rule.action; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +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.rule.autotagging.Attribute; +import org.opensearch.rule.autotagging.FeatureType; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * A request for update Rule + * Example request: + * curl -XPUT "localhost:9200/_rules/{featureType}/{_id}" -H 'Content-Type: application/json' -d ' + * { + * "description": "description", + * "index_pattern": ["log*", "event*"], + * "workload_group": "dev_workload_group_id_2" + * }' + * @opensearch.experimental + */ +@ExperimentalApi +public class UpdateRuleRequest extends ActionRequest { + private final String id; + private final String description; + private final Map> attributeMap; + private final String featureValue; + private final FeatureType featureType; + + /** + * constructor for UpdateRuleRequest + * @param id - the rule id to update + * @param description - the description to update + * @param attributeMap - the attribute values to update + * @param featureValue - the feature value to update + * @param featureType - the feature type for the rule + */ + public UpdateRuleRequest( + String id, + String description, + Map> attributeMap, + String featureValue, + FeatureType featureType + ) { + this.id = id; + this.description = description; + this.attributeMap = attributeMap; + this.featureValue = featureValue; + this.featureType = featureType; + } + + /** + * Constructs a UpdateRuleRequest from a StreamInput for deserialization + * @param in - The {@link StreamInput} instance to read from. + */ + public UpdateRuleRequest(StreamInput in) throws IOException { + super(in); + id = in.readString(); + description = in.readOptionalString(); + featureType = FeatureType.from(in); + attributeMap = in.readMap(i -> Attribute.from(i, featureType), i -> new HashSet<>(i.readStringList())); + featureValue = in.readOptionalString(); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(id); + out.writeOptionalString(description); + featureType.writeTo(out); + out.writeMap(attributeMap, (o, a) -> a.writeTo(o), StreamOutput::writeStringCollection); + out.writeOptionalString(featureValue); + } + + /** + * id getter + */ + public String getId() { + return id; + } + + /** + * description getter + */ + public String getDescription() { + return description; + } + + /** + * attributeMap getter + */ + public Map> getAttributeMap() { + return attributeMap; + } + + /** + * featureType getter + */ + public FeatureType getFeatureType() { + return featureType; + } + + /** + * featureValue getter + */ + public String getFeatureValue() { + return featureValue; + } +} diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleResponse.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleResponse.java new file mode 100644 index 0000000000000..8ba8109db7546 --- /dev/null +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/UpdateRuleResponse.java @@ -0,0 +1,70 @@ +/* + * 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.rule.action; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.rule.autotagging.Rule; + +import java.io.IOException; + +/** + * Response for the update API for Rule + * Example response: + * { + * id": "z1MJApUB0zgMcDmz-UQq", + * "description": "Rule for tagging workload_group_id to index123" + * "index_pattern": ["index123"], + * "workload_group": "workload_group_id", + * "updated_at": "2025-02-14T01:19:22.589Z" + * } + * @opensearch.experimental + */ +@ExperimentalApi +public class UpdateRuleResponse extends ActionResponse implements ToXContent, ToXContentObject { + private final Rule rule; + + /** + * constructor for UpdateRuleResponse + * @param rule - the updated rule + */ + public UpdateRuleResponse(final Rule rule) { + this.rule = rule; + } + + /** + * Constructs a UpdateRuleResponse from a StreamInput for deserialization + * @param in - The {@link StreamInput} instance to read from. + */ + public UpdateRuleResponse(StreamInput in) throws IOException { + this(new Rule(in)); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + rule.writeTo(out); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return rule.toXContent(builder, params); + } + + /** + * rule getter + */ + Rule getRule() { + return rule; + } +} diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/package-info.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/package-info.java new file mode 100644 index 0000000000000..5da0779401d77 --- /dev/null +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/package-info.java @@ -0,0 +1,12 @@ +/* + * 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. + */ + +/** + * This package contains action classes for rule lifecycle + */ +package org.opensearch.rule.action; diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/Rule.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/Rule.java index 8a1905fcff23a..ae1cebdda99d5 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/Rule.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/Rule.java @@ -15,6 +15,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParseException; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.rule.RuleUtils; import java.io.IOException; import java.util.HashMap; @@ -29,7 +30,7 @@ * tags to queries based on matching attribute patterns. This class provides an in-memory representation * of a rule. The indexed view may differ in representation. * { - * "_id": "fwehf8302582mglfio349==", + * "id": "fwehf8302582mglfio349==", * "description": "Assign Query Group for Index Logs123" * "index_pattern": ["logs123"], * "workload_group": "dev_workload_group_id", @@ -48,7 +49,7 @@ public class Rule implements Writeable, ToXContentObject { /** * id field */ - public static final String _ID_STRING = "_id"; + public static final String ID_STRING = "id"; /** * description field */ @@ -81,7 +82,7 @@ public Rule( this.attributeMap = attributeMap; this.featureValue = featureValue; this.updatedAt = updatedAt; - this.ruleValidator = new RuleValidator(description, attributeMap, featureValue, updatedAt, featureType); + this.ruleValidator = new RuleValidator(id, description, attributeMap, featureValue, updatedAt, featureType); this.ruleValidator.validate(); } @@ -97,7 +98,7 @@ public Rule(StreamInput in) throws IOException { attributeMap = in.readMap(i -> Attribute.from(i, featureType), i -> new HashSet<>(i.readStringList())); featureValue = in.readString(); updatedAt = in.readString(); - this.ruleValidator = new RuleValidator(description, attributeMap, featureValue, updatedAt, featureType); + this.ruleValidator = new RuleValidator(id, description, attributeMap, featureValue, updatedAt, featureType); this.ruleValidator.validate(); } @@ -173,7 +174,7 @@ public Map> getAttributeMap() { @Override public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { builder.startObject(); - builder.field(_ID_STRING, id); + builder.field(ID_STRING, id); builder.field(DESCRIPTION_STRING, description); for (Map.Entry> entry : attributeMap.entrySet()) { builder.array(entry.getKey().getName(), entry.getValue().toArray(new String[0])); @@ -247,7 +248,7 @@ public static Builder fromXContent(XContentParser parser, FeatureType featureTyp if (token == XContentParser.Token.FIELD_NAME) { fieldName = parser.currentName(); } else if (token.isValue()) { - if (fieldName.equals(_ID_STRING)) { + if (fieldName.equals(ID_STRING)) { builder.id(parser.text()); } else if (fieldName.equals(DESCRIPTION_STRING)) { builder.description(parser.text()); @@ -295,6 +296,18 @@ public Builder id(String id) { return this; } + /** + * sets the id based on description, featureType, attributeMap, and featureValue + * @return + */ + public Builder id() { + if (description == null || featureType == null || attributeMap == null || featureValue == null) { + throw new IllegalStateException("Cannot compute ID: required fields are missing."); + } + this.id = RuleUtils.computeRuleHash(description, featureType, attributeMap, featureValue); + return this; + } + /** * sets the description * @param description @@ -368,5 +381,13 @@ public String getFeatureValue() { public Map> getAttributeMap() { return attributeMap; } + + /** + * Returns description + * @return + */ + public String getDescription() { + return description; + } } } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/RuleValidator.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/RuleValidator.java index 3314598e8211a..dd4eafaceab35 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/RuleValidator.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/autotagging/RuleValidator.java @@ -26,6 +26,7 @@ * @opensearch.experimental */ public class RuleValidator { + private final String id; private final String description; private final Map> attributeMap; private final String featureValue; @@ -38,6 +39,7 @@ public class RuleValidator { /** * deafult constructor + * @param id * @param description * @param attributeMap * @param featureValue @@ -45,12 +47,14 @@ public class RuleValidator { * @param featureType */ public RuleValidator( + String id, String description, Map> attributeMap, String featureValue, String updatedAt, FeatureType featureType ) { + this.id = id; this.description = description; this.attributeMap = attributeMap; this.featureValue = featureValue; @@ -76,6 +80,9 @@ public void validate() { private List validateStringFields() { List errors = new ArrayList<>(); + if (isNullOrEmpty(id)) { + errors.add("Rule id can't be null or empty"); + } if (isNullOrEmpty(description)) { errors.add("Rule description can't be null or empty"); } else if (description.length() > MAX_DESCRIPTION_LENGTH) { @@ -129,14 +136,14 @@ private List validateAttributeMap() { Set attributeValues = entry.getValue(); errors.addAll(validateAttributeExistence(attribute)); errors.addAll(validateMaxAttributeValues(attribute, attributeValues)); - errors.addAll(validateAttributeValuesLength(attributeValues)); + errors.addAll(validateAttributeValuesList(attributeValues)); } } return errors; } private List validateAttributeExistence(Attribute attribute) { - if (featureType.getAttributeFromName(attribute.getName()) == null) { + if (!featureType.isValidAttribute(attribute)) { return List.of(attribute.getName() + " is not a valid attribute within the " + featureType.getName() + " feature."); } return new ArrayList<>(); @@ -164,14 +171,19 @@ private List validateMaxAttributeValues(Attribute attribute, Set return errors; } - private List validateAttributeValuesLength(Set attributeValues) { + private List validateAttributeValuesList(Set attributeValues) { int maxValueLength = featureType.getMaxCharLengthPerAttributeValue(); + List errors = new ArrayList<>(); for (String attributeValue : attributeValues) { if (attributeValue.isEmpty() || attributeValue.length() > maxValueLength) { - return List.of("Attribute value [" + attributeValue + "] is invalid (empty or exceeds " + maxValueLength + " characters)"); + errors.add("Attribute value [" + attributeValue + "] is invalid (empty or exceeds " + maxValueLength + " characters)"); + } + int asteriskCount = (int) attributeValue.chars().filter(c -> c == '*').count(); + if (asteriskCount > 1 || (asteriskCount == 1 && !attributeValue.endsWith("*"))) { + errors.add("Attribute value [" + attributeValue + "] is invalid (only one '*' is allowed and it must appear at the end)"); } } - return new ArrayList<>(); + return errors; } @Override diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/service/IndexStoredRulePersistenceService.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/service/IndexStoredRulePersistenceService.java index b0d31a829b2b6..d7a6f396bed74 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/service/IndexStoredRulePersistenceService.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/service/IndexStoredRulePersistenceService.java @@ -14,10 +14,10 @@ import org.opensearch.action.DocWriteResponse; import org.opensearch.action.delete.DeleteRequest; import org.opensearch.action.index.IndexRequest; -import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.SearchRequestBuilder; import org.opensearch.action.search.SearchResponse; import org.opensearch.action.support.clustermanager.AcknowledgedResponse; +import org.opensearch.action.update.UpdateRequest; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.XContentFactory; @@ -25,27 +25,27 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.query.QueryBuilder; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; -import org.opensearch.rule.DeleteRuleRequest; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.RuleEntityParser; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RuleQueryMapper; import org.opensearch.rule.RuleUtils; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; +import org.opensearch.rule.action.DeleteRuleRequest; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; +import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule; import org.opensearch.search.SearchHit; import org.opensearch.search.sort.SortOrder; import org.opensearch.transport.client.Client; import java.util.Arrays; +import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; - -import static org.opensearch.rule.autotagging.Rule._ID_STRING; /** * This class encapsulates the logic to manage the lifecycle of rules at index level @@ -139,11 +139,10 @@ public void onFailure(Exception e) { */ private void persistRule(Rule rule, ActionListener listener) { try { - IndexRequest indexRequest = new IndexRequest(indexName).source( - rule.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS) - ); - IndexResponse indexResponse = client.index(indexRequest).get(); - listener.onResponse(new CreateRuleResponse(indexResponse.getId(), rule)); + IndexRequest indexRequest = new IndexRequest(indexName).id(rule.getId()) + .source(rule.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)); + client.index(indexRequest).get(); + listener.onResponse(new CreateRuleResponse(rule)); } catch (Exception e) { logger.error("Error saving rule to index: {}", indexName); listener.onFailure(new RuntimeException("Failed to save rule to index.")); @@ -173,7 +172,7 @@ private void getRuleFromIndex(String id, QueryBuilder queryBuilder, String searc try { SearchRequestBuilder searchRequest = client.prepareSearch(indexName).setQuery(queryBuilder).setSize(maxRulesPerPage); if (searchAfter != null) { - searchRequest.addSort(_ID_STRING, SortOrder.ASC).searchAfter(new Object[] { searchAfter }); + searchRequest.addSort("_id", SortOrder.ASC).searchAfter(new Object[] { searchAfter }); } SearchResponse searchResponse = searchRequest.get(); @@ -201,9 +200,9 @@ private static boolean hasNoResults(String id, ActionListener l * @param listener - ActionListener for GetRuleResponse */ void handleGetRuleResponse(List hits, ActionListener listener) { - Map ruleMap = hits.stream().collect(Collectors.toMap(SearchHit::getId, hit -> parser.parse(hit.getSourceAsString()))); - String nextSearchAfter = hits.isEmpty() ? null : hits.get(hits.size() - 1).getId(); - listener.onResponse(new GetRuleResponse(ruleMap, nextSearchAfter)); + List ruleList = hits.stream().map(hit -> parser.parse(hit.getSourceAsString())).toList(); + String nextSearchAfter = hits.isEmpty() || hits.size() < maxRulesPerPage ? null : hits.get(hits.size() - 1).getId(); + listener.onResponse(new GetRuleResponse(ruleList, nextSearchAfter)); } @Override @@ -229,10 +228,56 @@ public void deleteRule(DeleteRuleRequest request, ActionListener listener) { + String ruleId = request.getId(); + FeatureType featureType = request.getFeatureType(); + try (ThreadContext.StoredContext context = stashContext()) { + QueryBuilder query = queryBuilder.from(new GetRuleRequest(ruleId, new HashMap<>(), null, featureType)); + getRuleFromIndex(ruleId, query, null, new ActionListener<>() { + @Override + public void onResponse(GetRuleResponse getRuleResponse) { + if (getRuleResponse == null || getRuleResponse.getRules().isEmpty()) { + listener.onFailure(new ResourceNotFoundException("Rule with ID " + ruleId + " not found.")); + return; + } + List ruleList = getRuleResponse.getRules(); + assert ruleList.size() == 1; + Rule updatedRule = RuleUtils.composeUpdatedRule(ruleList.get(0), request, featureType); + validateNoDuplicateRule( + updatedRule, + ActionListener.wrap(unused -> persistUpdatedRule(ruleId, updatedRule, listener), listener::onFailure) + ); + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + }); + } + } + + /** + * Persist the updated rule in index + * @param ruleId - the rule id to update + * @param updatedRule - the rule we update to + * @param listener - ActionListener for UpdateRuleResponse */ - public String getIndexName() { - return indexName; + private void persistUpdatedRule(String ruleId, Rule updatedRule, ActionListener listener) { + try { + UpdateRequest updateRequest = new UpdateRequest(indexName, ruleId).doc( + updatedRule.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS) + ); + client.update(updateRequest).get(); + listener.onResponse(new UpdateRuleResponse(updatedRule)); + } catch (Exception e) { + logger.error("Error updating rule in index: {}", indexName); + listener.onFailure(new RuntimeException("Failed to update rule to index.")); + } } private ThreadContext.StoredContext stashContext() { diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/storage/IndexBasedRuleQueryMapper.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/storage/IndexBasedRuleQueryMapper.java index 96b77e0b4b643..11b0ad5e564fb 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/storage/IndexBasedRuleQueryMapper.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/storage/IndexBasedRuleQueryMapper.java @@ -12,15 +12,13 @@ import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryBuilders; -import org.opensearch.rule.GetRuleRequest; import org.opensearch.rule.RuleQueryMapper; +import org.opensearch.rule.action.GetRuleRequest; import org.opensearch.rule.autotagging.Attribute; import java.util.Map; import java.util.Set; -import static org.opensearch.rule.autotagging.Rule._ID_STRING; - /** * This class is used to build opensearch index based query object */ @@ -40,7 +38,7 @@ public QueryBuilder from(GetRuleRequest request) { boolQuery.filter(QueryBuilders.existsQuery(request.getFeatureType().getName())); if (id != null) { - return boolQuery.must(QueryBuilders.termQuery(_ID_STRING, id)); + return boolQuery.must(QueryBuilders.termQuery("_id", id)); } for (Map.Entry> entry : attributeFilters.entrySet()) { Attribute attribute = entry.getKey(); diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/IndexStoredRuleUtilsTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/IndexStoredRuleUtilsTests.java index 46aaf6770b17c..555ab59b71250 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/IndexStoredRuleUtilsTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/IndexStoredRuleUtilsTests.java @@ -10,6 +10,7 @@ import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.QueryBuilder; +import org.opensearch.rule.action.GetRuleRequest; import org.opensearch.rule.storage.IndexBasedRuleQueryMapper; import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.test.OpenSearchTestCase; @@ -41,7 +42,7 @@ public void testBuildGetRuleQuery_WithAttributes() { ); assertNotNull(queryBuilder); BoolQueryBuilder query = (BoolQueryBuilder) queryBuilder; - assertTrue(query.must().size() == 1); + assertEquals(1, query.must().size()); assertTrue(query.toString().contains(RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE.getName())); assertTrue(query.toString().contains(RuleTestUtils.ATTRIBUTE_VALUE_ONE)); } diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/RuleUtilsTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/RuleUtilsTests.java index 2780f329925c9..5c330be57f008 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/RuleUtilsTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/RuleUtilsTests.java @@ -8,40 +8,45 @@ package org.opensearch.rule; +import org.opensearch.rule.action.UpdateRuleRequest; import org.opensearch.rule.autotagging.Rule; import org.opensearch.rule.autotagging.RuleTests; import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.test.OpenSearchTestCase; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import static org.opensearch.rule.action.GetRuleResponseTests.ruleOne; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_VALUE_ONE; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_VALUE_TWO; import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_ONE; +import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_TWO; import static org.opensearch.rule.utils.RuleTestUtils.FEATURE_VALUE_ONE; +import static org.opensearch.rule.utils.RuleTestUtils.FEATURE_VALUE_TWO; +import static org.opensearch.rule.utils.RuleTestUtils.MockRuleAttributes; import static org.opensearch.rule.utils.RuleTestUtils.TIMESTAMP_ONE; import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; -import static org.opensearch.rule.utils.RuleTestUtils._ID_TWO; +import static org.opensearch.rule.utils.RuleTestUtils.ruleOne; import static org.opensearch.rule.utils.RuleTestUtils.ruleTwo; public class RuleUtilsTests extends OpenSearchTestCase { public void testDuplicateRuleFound() { - Optional result = RuleUtils.getDuplicateRuleId(ruleOne, Map.of(_ID_ONE, ruleOne, _ID_TWO, ruleTwo)); + Optional result = RuleUtils.getDuplicateRuleId(ruleOne, List.of(ruleOne, ruleTwo)); assertTrue(result.isPresent()); assertEquals(_ID_ONE, result.get()); } public void testNoAttributeIntersection() { - Optional result = RuleUtils.getDuplicateRuleId(ruleOne, Map.of(_ID_TWO, ruleTwo)); + Optional result = RuleUtils.getDuplicateRuleId(ruleOne, List.of(ruleTwo)); assertTrue(result.isEmpty()); } public void testAttributeSizeMismatch() { Rule testRule = Rule.builder() + .id(_ID_ONE) .description(DESCRIPTION_ONE) .featureType(RuleTestUtils.MockRuleFeatureType.INSTANCE) .featureValue(FEATURE_VALUE_ONE) @@ -55,12 +60,13 @@ public void testAttributeSizeMismatch() { ) .updatedAt(TIMESTAMP_ONE) .build(); - Optional result = RuleUtils.getDuplicateRuleId(ruleOne, Map.of(_ID_TWO, testRule)); + Optional result = RuleUtils.getDuplicateRuleId(ruleOne, List.of(testRule)); assertTrue(result.isEmpty()); } public void testPartialAttributeValueIntersection() { Rule ruleWithPartialOverlap = Rule.builder() + .id(_ID_ONE) .description(DESCRIPTION_ONE) .featureType(RuleTestUtils.MockRuleFeatureType.INSTANCE) .featureValue(FEATURE_VALUE_ONE) @@ -68,13 +74,14 @@ public void testPartialAttributeValueIntersection() { .updatedAt(TIMESTAMP_ONE) .build(); - Optional result = RuleUtils.getDuplicateRuleId(ruleWithPartialOverlap, Map.of(_ID_ONE, ruleOne)); + Optional result = RuleUtils.getDuplicateRuleId(ruleWithPartialOverlap, List.of(ruleOne)); assertTrue(result.isPresent()); assertEquals(_ID_ONE, result.get()); } public void testDifferentFeatureTypes() { Rule differentFeatureTypeRule = Rule.builder() + .id(_ID_ONE) .description(DESCRIPTION_ONE) .featureType(RuleTests.TestFeatureType.INSTANCE) .featureValue(FEATURE_VALUE_ONE) @@ -82,7 +89,24 @@ public void testDifferentFeatureTypes() { .updatedAt(TIMESTAMP_ONE) .build(); - Optional result = RuleUtils.getDuplicateRuleId(differentFeatureTypeRule, Map.of(_ID_ONE, ruleOne)); + Optional result = RuleUtils.getDuplicateRuleId(differentFeatureTypeRule, List.of(ruleOne)); assertTrue(result.isEmpty()); } + + public void testComposeUpdateAllFields() { + UpdateRuleRequest request = new UpdateRuleRequest( + _ID_ONE, + DESCRIPTION_TWO, + Map.of(MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE, Set.of(ATTRIBUTE_VALUE_TWO)), + FEATURE_VALUE_TWO, + RuleTestUtils.MockRuleFeatureType.INSTANCE + ); + + Rule updatedRule = RuleUtils.composeUpdatedRule(ruleOne, request, RuleTestUtils.MockRuleFeatureType.INSTANCE); + + assertEquals(_ID_ONE, updatedRule.getId()); + assertEquals(DESCRIPTION_TWO, updatedRule.getDescription()); + assertEquals(FEATURE_VALUE_TWO, updatedRule.getFeatureValue()); + assertEquals(RuleTestUtils.MockRuleFeatureType.INSTANCE, updatedRule.getFeatureType()); + } } diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/XContentRuleParserTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/XContentRuleParserTests.java index 67b7cb8f54c53..b4bb1c5232927 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/XContentRuleParserTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/XContentRuleParserTests.java @@ -17,15 +17,18 @@ import java.time.Instant; import java.util.Locale; +import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; + public class XContentRuleParserTests extends OpenSearchTestCase { public static final String VALID_JSON = String.format(Locale.ROOT, """ { + "id": "%s", "description": "%s", "mock_feature_type": "feature value", "mock_attribute_one": ["attribute_value_one", "attribute_value_two"], "updated_at": "%s" } - """, RuleTestUtils.DESCRIPTION_ONE, Instant.now().toString()); + """, _ID_ONE, RuleTestUtils.DESCRIPTION_ONE, Instant.now().toString()); private static final String INVALID_JSON = """ { diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleRequestTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleRequestTests.java index 7804714c37dcc..4ebdf296cf1c0 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleRequestTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleRequestTests.java @@ -10,7 +10,6 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.rule.CreateRuleRequest; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleResponseTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleResponseTests.java index dc445dad2e82c..eb813c9b12ca3 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleResponseTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/CreateRuleResponseTests.java @@ -13,16 +13,13 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.rule.CreateRuleResponse; import org.opensearch.rule.autotagging.Rule; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.util.Map; -import static org.opensearch.rule.action.GetRuleResponseTests.ruleOne; -import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; -import static org.opensearch.rule.utils.RuleTestUtils.assertEqualRules; +import static org.opensearch.rule.utils.RuleTestUtils.assertEqualRule; +import static org.opensearch.rule.utils.RuleTestUtils.ruleOne; import static org.mockito.Mockito.mock; public class CreateRuleResponseTests extends OpenSearchTestCase { @@ -31,14 +28,14 @@ public class CreateRuleResponseTests extends OpenSearchTestCase { * Test case to verify serialization and deserialization of CreateRuleResponse */ public void testSerialization() throws IOException { - CreateRuleResponse response = new CreateRuleResponse(_ID_ONE, ruleOne); + CreateRuleResponse response = new CreateRuleResponse(ruleOne); BytesStreamOutput out = new BytesStreamOutput(); response.writeTo(out); StreamInput streamInput = out.bytes().streamInput(); CreateRuleResponse otherResponse = new CreateRuleResponse(streamInput); Rule responseRule = response.getRule(); Rule otherResponseRule = otherResponse.getRule(); - assertEqualRules(Map.of(_ID_ONE, responseRule), Map.of(_ID_ONE, otherResponseRule), false); + assertEqualRule(responseRule, otherResponseRule, false); } /** @@ -46,10 +43,10 @@ public void testSerialization() throws IOException { */ public void testToXContentCreateRule() throws IOException { XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); - CreateRuleResponse response = new CreateRuleResponse(_ID_ONE, ruleOne); + CreateRuleResponse response = new CreateRuleResponse(ruleOne); String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); String expected = "{\n" - + " \"_id\" : \"AgfUO5Ja9yfvhdONlYi3TQ==\",\n" + + " \"id\" : \"e9f35a73-ece2-3fa7-857e-7c1af877fc75\",\n" + " \"description\" : \"description_1\",\n" + " \"mock_attribute_one\" : [\n" + " \"mock_attribute_one\"\n" diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/DeleteRuleRequestTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/DeleteRuleRequestTests.java index 55213a245b5ad..bddaaad1f0927 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/DeleteRuleRequestTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/DeleteRuleRequestTests.java @@ -10,7 +10,6 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.rule.DeleteRuleRequest; import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.test.OpenSearchTestCase; diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleRequestTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleRequestTests.java index f8321a38765fb..7a17c26f4818f 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleRequestTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleRequestTests.java @@ -10,16 +10,15 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.autotagging.Attribute; -import org.opensearch.rule.autotagging.Rule; import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; import java.util.HashMap; -import java.util.Map; -import java.util.Set; + +import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_MAP; +import static org.opensearch.rule.utils.RuleTestUtils.SEARCH_AFTER; +import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; public class GetRuleRequestTests extends OpenSearchTestCase { /** @@ -64,65 +63,4 @@ public void testValidate() { request = new GetRuleRequest(_ID_ONE, ATTRIBUTE_MAP, "", RuleTestUtils.MockRuleFeatureType.INSTANCE); assertThrows(IllegalArgumentException.class, request::validate); } - - public static final String _ID_ONE = "id_1"; - public static final String SEARCH_AFTER = "search_after"; - public static final String _ID_TWO = "G5iIq84j7eK1qIAAAAIH53=1"; - public static final String FEATURE_VALUE_ONE = "feature_value_one"; - public static final String FEATURE_VALUE_TWO = "feature_value_two"; - public static final String ATTRIBUTE_VALUE_ONE = "mock_attribute_one"; - public static final String ATTRIBUTE_VALUE_TWO = "mock_attribute_two"; - public static final String DESCRIPTION_ONE = "description_1"; - public static final String DESCRIPTION_TWO = "description_2"; - public static final String TIMESTAMP_ONE = "2024-01-26T08:58:57.558Z"; - public static final String TIMESTAMP_TWO = "2023-01-26T08:58:57.558Z"; - public static final Map> ATTRIBUTE_MAP = Map.of( - RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE, - Set.of(ATTRIBUTE_VALUE_ONE) - ); - - public static final Rule ruleOne = Rule.builder() - .id(_ID_ONE) - .description(DESCRIPTION_ONE) - .featureType(RuleTestUtils.MockRuleFeatureType.INSTANCE) - .featureValue(FEATURE_VALUE_ONE) - .attributeMap(ATTRIBUTE_MAP) - .updatedAt(TIMESTAMP_ONE) - .build(); - - public static final Rule ruleTwo = Rule.builder() - .id(_ID_TWO) - .description(DESCRIPTION_TWO) - .featureType(RuleTestUtils.MockRuleFeatureType.INSTANCE) - .featureValue(FEATURE_VALUE_TWO) - .attributeMap(Map.of(RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_TWO, Set.of(ATTRIBUTE_VALUE_TWO))) - .updatedAt(TIMESTAMP_TWO) - .build(); - - public static Map ruleMap() { - return Map.of(_ID_ONE, ruleOne, _ID_TWO, ruleTwo); - } - - public static void assertEqualRules(Map mapOne, Map mapTwo, boolean ruleUpdated) { - assertEquals(mapOne.size(), mapTwo.size()); - for (Map.Entry entry : mapOne.entrySet()) { - String id = entry.getKey(); - assertTrue(mapTwo.containsKey(id)); - Rule one = mapOne.get(id); - Rule two = mapTwo.get(id); - assertEqualRule(one, two, ruleUpdated); - } - } - - public static void assertEqualRule(Rule one, Rule two, boolean ruleUpdated) { - if (ruleUpdated) { - assertEquals(one.getDescription(), two.getDescription()); - assertEquals(one.getFeatureType(), two.getFeatureType()); - assertEquals(one.getFeatureValue(), two.getFeatureValue()); - assertEquals(one.getAttributeMap(), two.getAttributeMap()); - assertEquals(one.getAttributeMap(), two.getAttributeMap()); - } else { - assertEquals(one, two); - } - } } diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleResponseTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleResponseTests.java index 5515d12e4701f..10817d8b9e899 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleResponseTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/GetRuleResponseTests.java @@ -13,50 +13,28 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.rule.GetRuleResponse; -import org.opensearch.rule.autotagging.Attribute; import org.opensearch.rule.autotagging.Rule; -import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.ArrayList; +import java.util.List; import static org.opensearch.rule.utils.RuleTestUtils.SEARCH_AFTER; -import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; import static org.opensearch.rule.utils.RuleTestUtils.assertEqualRules; -import static org.opensearch.rule.utils.RuleTestUtils.ruleMap; +import static org.opensearch.rule.utils.RuleTestUtils.ruleOne; +import static org.opensearch.rule.utils.RuleTestUtils.ruleTwo; import static org.mockito.Mockito.mock; public class GetRuleResponseTests extends OpenSearchTestCase { - public static final String FEATURE_VALUE_ONE = "feature_value_one"; - public static final String ATTRIBUTE_VALUE_ONE = "mock_attribute_one"; - public static final String DESCRIPTION_ONE = "description_1"; - public static final String TIMESTAMP_ONE = "2024-01-26T08:58:57.558Z"; - static final Map> ATTRIBUTE_MAP = Map.of( - RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE, - Set.of(ATTRIBUTE_VALUE_ONE) - ); - - public static final Rule ruleOne = Rule.builder() - .id(_ID_ONE) - .description(DESCRIPTION_ONE) - .featureType(RuleTestUtils.MockRuleFeatureType.INSTANCE) - .featureValue(FEATURE_VALUE_ONE) - .attributeMap(ATTRIBUTE_MAP) - .updatedAt(TIMESTAMP_ONE) - .build(); - /** * Test case to verify the serialization and deserialization of GetRuleResponse */ public void testSerializationSingleRule() throws IOException { - Map map = new HashMap<>(); - map.put(_ID_ONE, ruleOne); - GetRuleResponse response = new GetRuleResponse(Map.of(_ID_ONE, ruleOne), null); - assertEquals(response.getRules(), map); + List list = new ArrayList<>(); + list.add(ruleOne); + GetRuleResponse response = new GetRuleResponse(list, null); + assertEquals(response.getRules(), list); BytesStreamOutput out = new BytesStreamOutput(); response.writeTo(out); @@ -70,8 +48,10 @@ public void testSerializationSingleRule() throws IOException { * Test case to verify the serialization and deserialization of GetRuleResponse when the result contains multiple rules */ public void testSerializationMultipleRule() throws IOException { - GetRuleResponse response = new GetRuleResponse(ruleMap(), SEARCH_AFTER); - assertEquals(response.getRules(), ruleMap()); + List list = new ArrayList<>(); + list.add(ruleOne); + list.add(ruleTwo); + GetRuleResponse response = new GetRuleResponse(list, SEARCH_AFTER); BytesStreamOutput out = new BytesStreamOutput(); response.writeTo(out); @@ -86,9 +66,9 @@ public void testSerializationMultipleRule() throws IOException { * Test case to verify the serialization and deserialization of GetRuleResponse when the result is empty */ public void testSerializationNull() throws IOException { - Map map = new HashMap<>(); - GetRuleResponse response = new GetRuleResponse(map, SEARCH_AFTER); - assertEquals(response.getRules(), map); + List list = new ArrayList<>(); + GetRuleResponse response = new GetRuleResponse(list, SEARCH_AFTER); + assertEquals(response.getRules(), list); BytesStreamOutput out = new BytesStreamOutput(); response.writeTo(out); @@ -102,15 +82,13 @@ public void testSerializationNull() throws IOException { * Test case to verify the toXContent of GetRuleResponse */ public void testToXContentGetSingleRule() throws IOException { - Map map = new HashMap<>(); - map.put(_ID_ONE, ruleOne); - GetRuleResponse response = new GetRuleResponse(Map.of(_ID_ONE, ruleOne), SEARCH_AFTER); + GetRuleResponse response = new GetRuleResponse(List.of(ruleOne), SEARCH_AFTER); XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); String expected = "{\n" + " \"rules\" : [\n" + " {\n" - + " \"_id\" : \"AgfUO5Ja9yfvhdONlYi3TQ==\",\n" + + " \"id\" : \"e9f35a73-ece2-3fa7-857e-7c1af877fc75\",\n" + " \"description\" : \"description_1\",\n" + " \"mock_attribute_one\" : [\n" + " \"mock_attribute_one\"\n" @@ -131,7 +109,7 @@ public void testToXContentGetSingleRule() throws IOException { */ public void testToXContentGetZeroRule() throws IOException { XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); - GetRuleResponse otherResponse = new GetRuleResponse(new HashMap<>(), null); + GetRuleResponse otherResponse = new GetRuleResponse(new ArrayList<>(), null); String actual = otherResponse.toXContent(builder, mock(ToXContent.Params.class)).toString(); String expected = "{\n" + " \"rules\" : [ ]\n" + "}"; assertEquals(expected, actual); diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleRequestTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleRequestTests.java new file mode 100644 index 0000000000000..8cb851fd308d7 --- /dev/null +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleRequestTests.java @@ -0,0 +1,78 @@ +/* + * 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.rule.action; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.rule.utils.RuleTestUtils; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; + +import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_MAP; +import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_ONE; +import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_TWO; +import static org.opensearch.rule.utils.RuleTestUtils.FEATURE_VALUE_ONE; +import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; + +public class UpdateRuleRequestTests extends OpenSearchTestCase { + /** + * Test case to verify the serialization and deserialization of UpdateRuleRequest + */ + public void testSerialization() throws IOException { + UpdateRuleRequest request = new UpdateRuleRequest( + _ID_ONE, + DESCRIPTION_TWO, + ATTRIBUTE_MAP, + FEATURE_VALUE_ONE, + RuleTestUtils.MockRuleFeatureType.INSTANCE + ); + assertEquals(_ID_ONE, request.getId()); + assertNull(request.validate()); + assertEquals(RuleTestUtils.MockRuleFeatureType.INSTANCE, request.getFeatureType()); + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + UpdateRuleRequest otherRequest = new UpdateRuleRequest(streamInput); + assertEquals(request.getId(), otherRequest.getId()); + assertEquals(request.getAttributeMap(), otherRequest.getAttributeMap()); + assertEquals(request.getDescription(), otherRequest.getDescription()); + assertEquals(request.getFeatureValue(), otherRequest.getFeatureValue()); + } + + /** + * Test case to verify the serialization and deserialization of UpdateRuleRequest when some fields are null + */ + public void testSerializationWithNull() throws IOException { + UpdateRuleRequest request = new UpdateRuleRequest(_ID_ONE, null, ATTRIBUTE_MAP, null, RuleTestUtils.MockRuleFeatureType.INSTANCE); + assertNull(request.getDescription()); + assertNull(request.getFeatureValue()); + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + UpdateRuleRequest otherRequest = new UpdateRuleRequest(streamInput); + assertEquals(request.getId(), otherRequest.getId()); + assertEquals(request.getAttributeMap(), otherRequest.getAttributeMap()); + assertEquals(request.getDescription(), otherRequest.getDescription()); + assertEquals(request.getFeatureValue(), otherRequest.getFeatureValue()); + } + + public void testValidate() { + UpdateRuleRequest request = new UpdateRuleRequest( + _ID_ONE, + "", + ATTRIBUTE_MAP, + FEATURE_VALUE_ONE, + RuleTestUtils.MockRuleFeatureType.INSTANCE + ); + assertNull(request.validate()); + request = new UpdateRuleRequest(_ID_ONE, DESCRIPTION_ONE, ATTRIBUTE_MAP, "", RuleTestUtils.MockRuleFeatureType.INSTANCE); + assertNull(request.validate()); + } +} diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleResponseTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleResponseTests.java new file mode 100644 index 0000000000000..4825b28a7c728 --- /dev/null +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/action/UpdateRuleResponseTests.java @@ -0,0 +1,55 @@ +/* + * 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.rule.action; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; + +import static org.opensearch.rule.utils.RuleTestUtils.assertEqualRule; +import static org.opensearch.rule.utils.RuleTestUtils.ruleOne; +import static org.mockito.Mockito.mock; + +public class UpdateRuleResponseTests extends OpenSearchTestCase { + /** + * Test case to verify the serialization and deserialization of UpdateRuleResponse + */ + public void testSerialization() throws IOException { + UpdateRuleResponse response = new UpdateRuleResponse(ruleOne); + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + UpdateRuleResponse otherResponse = new UpdateRuleResponse(streamInput); + assertEqualRule(response.getRule(), otherResponse.getRule(), false); + } + + /** + * Test case to verify the toXContent of GetRuleResponse + */ + public void testToXContent() throws IOException { + UpdateRuleResponse response = new UpdateRuleResponse(ruleOne); + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); + String expected = "{\n" + + " \"id\" : \"e9f35a73-ece2-3fa7-857e-7c1af877fc75\",\n" + + " \"description\" : \"description_1\",\n" + + " \"mock_attribute_one\" : [\n" + + " \"mock_attribute_one\"\n" + + " ],\n" + + " \"mock_feature_type\" : \"feature_value_one\",\n" + + " \"updated_at\" : \"2024-01-26T08:58:57.558Z\"\n" + + "}"; + assertEquals(expected, actual); + } +} diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleTests.java index c0f3a96020a4c..13c6b93fac631 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleTests.java @@ -13,6 +13,7 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.rule.RuleUtils; import org.opensearch.test.AbstractSerializingTestCase; import java.io.IOException; @@ -20,7 +21,6 @@ import java.util.Map; import java.util.Set; -import static org.opensearch.rule.autotagging.Rule._ID_STRING; import static org.opensearch.rule.autotagging.RuleTests.TestAttribute.TEST_ATTRIBUTE_1; import static org.opensearch.rule.autotagging.RuleTests.TestAttribute.TEST_ATTRIBUTE_2; @@ -32,12 +32,7 @@ public class RuleTests extends AbstractSerializingTestCase { public static final String _ID = "test_id"; public static final String FEATURE_VALUE = "feature_value"; public static final TestFeatureType FEATURE_TYPE = TestFeatureType.INSTANCE; - public static final Map> ATTRIBUTE_MAP = Map.of( - TEST_ATTRIBUTE_1, - Set.of("value1"), - TEST_ATTRIBUTE_2, - Set.of("value2") - ); + public static final Map> ATTRIBUTE_MAP = Map.of(TEST_ATTRIBUTE_1, Set.of("value1")); public static final String UPDATED_AT = "2025-02-24T07:42:10.123456Z"; public static final String INVALID_ATTRIBUTE = "invalid_attribute"; public static final String INVALID_FEATURE = "invalid_feature"; @@ -47,7 +42,8 @@ protected Rule createTestInstance() { String description = randomAlphaOfLength(10); String featureValue = randomAlphaOfLength(5); String updatedAt = Instant.now().toString(); - return new Rule("test_id", description, ATTRIBUTE_MAP, FEATURE_TYPE, featureValue, updatedAt); + String id = RuleUtils.computeRuleHash(description, FEATURE_TYPE, ATTRIBUTE_MAP, featureValue); + return new Rule(id, description, ATTRIBUTE_MAP, FEATURE_TYPE, featureValue, updatedAt); } @Override @@ -122,8 +118,9 @@ static Rule buildRule( String updatedAt, String description ) { + String id = RuleUtils.computeRuleHash(description, featureType, attributeListMap, featureValue); return Rule.builder() - .id(_ID) + .id(id) .featureValue(featureValue) .featureType(featureType) .description(description) @@ -146,13 +143,14 @@ public void testValidRule() { public void testToXContent() throws IOException { String updatedAt = Instant.now().toString(); - Rule rule = buildRule(FEATURE_VALUE, FEATURE_TYPE, Map.of(TEST_ATTRIBUTE_1, Set.of("value1")), updatedAt, DESCRIPTION); + Map> map = Map.of(TEST_ATTRIBUTE_1, Set.of("value1")); + Rule rule = buildRule(FEATURE_VALUE, FEATURE_TYPE, map, updatedAt, DESCRIPTION); XContentBuilder builder = JsonXContent.contentBuilder(); - rule.toXContent(builder, new ToXContent.MapParams(Map.of(_ID_STRING, _ID))); + rule.toXContent(builder, ToXContent.EMPTY_PARAMS); assertEquals( - "{\"_id\":\"" - + _ID + "{\"id\":\"" + + RuleUtils.computeRuleHash(DESCRIPTION, FEATURE_TYPE, map, FEATURE_VALUE) + "\",\"description\":\"description\",\"test_attr1\":[\"value1\"],\"test_feature_type\":\"feature_value\",\"updated_at\":\"" + updatedAt + "\"}", diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleValidatorTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleValidatorTests.java index b24f9daa034c6..1b17ede64cca0 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleValidatorTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/autotagging/RuleValidatorTests.java @@ -22,11 +22,12 @@ import static org.opensearch.rule.autotagging.RuleTests.FEATURE_VALUE; import static org.opensearch.rule.autotagging.RuleTests.TestAttribute.TEST_ATTRIBUTE_1; import static org.opensearch.rule.autotagging.RuleTests.UPDATED_AT; +import static org.opensearch.rule.autotagging.RuleTests._ID; public class RuleValidatorTests extends OpenSearchTestCase { public void testValidRule() { - RuleValidator validator = new RuleValidator(DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); + RuleValidator validator = new RuleValidator(_ID, DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); try { validator.validate(); } catch (Exception e) { @@ -41,7 +42,7 @@ public static void validateRule( String updatedAt, String description ) { - RuleValidator validator = new RuleValidator(description, attributeMap, featureValue, updatedAt, featureType); + RuleValidator validator = new RuleValidator(_ID, description, attributeMap, featureValue, updatedAt, featureType); validator.validate(); } @@ -113,8 +114,8 @@ public void testInvalidLabel() { } public void testEqualRuleValidators() { - RuleValidator validator = new RuleValidator(DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); - RuleValidator otherValidator = new RuleValidator(DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); + RuleValidator validator = new RuleValidator(_ID, DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); + RuleValidator otherValidator = new RuleValidator(_ID, DESCRIPTION, ATTRIBUTE_MAP, FEATURE_VALUE, UPDATED_AT, FEATURE_TYPE); assertEquals(validator, otherValidator); } } diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/service/IndexStoredRulePersistenceServiceTests.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/service/IndexStoredRulePersistenceServiceTests.java index eb054ea8124e5..e348d104cfe27 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/service/IndexStoredRulePersistenceServiceTests.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/service/IndexStoredRulePersistenceServiceTests.java @@ -28,14 +28,17 @@ import org.opensearch.core.index.shard.ShardId; import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.query.QueryBuilder; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; -import org.opensearch.rule.DeleteRuleRequest; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.RuleEntityParser; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RuleQueryMapper; +import org.opensearch.rule.RuleUtils; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; +import org.opensearch.rule.action.DeleteRuleRequest; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; import org.opensearch.rule.autotagging.Rule; import org.opensearch.rule.utils.RuleTestUtils; import org.opensearch.search.SearchHit; @@ -56,17 +59,19 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import org.mockito.ArgumentCaptor; import static org.opensearch.rule.XContentRuleParserTests.VALID_JSON; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_MAP; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_VALUE_ONE; +import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_ONE; +import static org.opensearch.rule.utils.RuleTestUtils.DESCRIPTION_TWO; +import static org.opensearch.rule.utils.RuleTestUtils.FEATURE_VALUE_ONE; import static org.opensearch.rule.utils.RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE; -import static org.opensearch.rule.utils.RuleTestUtils.MockRuleFeatureType; import static org.opensearch.rule.utils.RuleTestUtils.TEST_INDEX_NAME; import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.any; @@ -79,7 +84,6 @@ public class IndexStoredRulePersistenceServiceTests extends OpenSearchTestCase { private static final int MAX_VALUES_PER_PAGE = 50; - private Client client; private ClusterService clusterService; private RuleQueryMapper ruleQueryMapper; @@ -93,16 +97,13 @@ public void setUp() throws Exception { super.setUp(); searchRequestBuilder = mock(SearchRequestBuilder.class); client = setUpMockClient(searchRequestBuilder); - rule = mock(Rule.class); - clusterService = mock(ClusterService.class); ClusterState clusterState = mock(ClusterState.class); Metadata metadata = mock(Metadata.class); when(clusterService.state()).thenReturn(clusterState); when(clusterState.metadata()).thenReturn(metadata); when(metadata.hasIndex(TEST_INDEX_NAME)).thenReturn(true); - ruleQueryMapper = mock(RuleQueryMapper.class); ruleEntityParser = mock(RuleEntityParser.class); queryBuilder = mock(QueryBuilder.class); @@ -151,7 +152,7 @@ public void testConcurrentCreateDuplicateRules() throws InterruptedException { CreateRuleRequest createRuleRequest = mock(CreateRuleRequest.class); when(rule.getAttributeMap()).thenReturn(ATTRIBUTE_MAP); - when(rule.getFeatureType()).thenReturn(MockRuleFeatureType.INSTANCE); + when(rule.getFeatureType()).thenReturn(RuleTestUtils.MockRuleFeatureType.INSTANCE); when(createRuleRequest.getRule()).thenReturn(rule); RulePersistenceService rulePersistenceService = new IndexStoredRulePersistenceService( @@ -172,7 +173,7 @@ public void onResponse(Void unused) { synchronized (storedAttributeMaps) { storedAttributeMaps.add(MOCK_RULE_ATTRIBUTE_ONE.getName()); } - listener.onResponse(new CreateRuleResponse("fake-id", rule)); + listener.onResponse(new CreateRuleResponse(rule)); latch.countDown(); } @@ -356,4 +357,155 @@ public void testDeleteRule_notFound() { verify(listener).onFailure(any(ResourceNotFoundException.class)); } + + public void testConcurrentUpdateDuplicateRules() throws InterruptedException { + int threadCount = 10; + CountDownLatch latch = new CountDownLatch(threadCount); + ExecutorService singleThreadExecutor = Executors.newSingleThreadExecutor(); + Set storedAttributeMaps = ConcurrentHashMap.newKeySet(); + + UpdateRuleRequest updateRuleRequest = mock(UpdateRuleRequest.class); + when(updateRuleRequest.getFeatureValue()).thenReturn(FEATURE_VALUE_ONE); + when(updateRuleRequest.getFeatureType()).thenReturn(RuleTestUtils.MockRuleFeatureType.INSTANCE); + when(updateRuleRequest.getAttributeMap()).thenReturn(ATTRIBUTE_MAP); + when(updateRuleRequest.getDescription()).thenReturn(DESCRIPTION_TWO); + + Rule originalRule = mock(Rule.class); + when(originalRule.getId()).thenReturn(_ID_ONE); + when(originalRule.getAttributeMap()).thenReturn(ATTRIBUTE_MAP); + when(originalRule.getFeatureType()).thenReturn(RuleTestUtils.MockRuleFeatureType.INSTANCE); + when(originalRule.getFeatureValue()).thenReturn(FEATURE_VALUE_ONE); + when(originalRule.getDescription()).thenReturn(DESCRIPTION_ONE); + + RulePersistenceService rulePersistenceService = new IndexStoredRulePersistenceService( + TEST_INDEX_NAME, + client, + clusterService, + MAX_VALUES_PER_PAGE, + ruleEntityParser, + ruleQueryMapper + ) { + @Override + public void updateRule(UpdateRuleRequest request, ActionListener listener) { + singleThreadExecutor.execute(() -> { + Rule updatedRule = RuleUtils.composeUpdatedRule(originalRule, request, request.getFeatureType()); + validateNoDuplicateRule(updatedRule, new ActionListener() { + @Override + public void onResponse(Void unused) { + listener.onResponse(new UpdateRuleResponse(updatedRule)); + latch.countDown(); + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + latch.countDown(); + } + }); + }); + } + + public void validateNoDuplicateRule(Rule rule, ActionListener listener) { + synchronized (storedAttributeMaps) { + String key = rule.getAttributeMap().toString(); + if (storedAttributeMaps.contains(key)) { + listener.onFailure(new IllegalArgumentException("Duplicate rule exists with attribute map")); + } else { + storedAttributeMaps.add(key); + listener.onResponse(null); + } + } + } + }; + + class TestListener implements ActionListener { + final AtomicInteger successCount = new AtomicInteger(); + final AtomicInteger failureCount = new AtomicInteger(); + final List failures = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void onResponse(UpdateRuleResponse response) { + successCount.incrementAndGet(); + } + + @Override + public void onFailure(Exception e) { + failureCount.incrementAndGet(); + failures.add(e); + } + } + + TestListener testListener = new TestListener(); + + for (int i = 0; i < threadCount; i++) { + new Thread(() -> rulePersistenceService.updateRule(updateRuleRequest, testListener)).start(); + } + + boolean completed = latch.await(10, TimeUnit.SECONDS); + singleThreadExecutor.shutdown(); + + assertTrue("All update calls should complete", completed); + assertEquals(1, testListener.successCount.get()); + assertEquals(threadCount - 1, testListener.failureCount.get()); + + for (Exception e : testListener.failures) { + assertTrue(e instanceof IllegalArgumentException); + assertTrue(e.getMessage().contains("Duplicate rule")); + } + } + + public void testUpdateRule_RuleNotFound() { + UpdateRuleRequest request = mock(UpdateRuleRequest.class); + when(request.getId()).thenReturn(_ID_ONE); + + SearchHits emptyHits = new SearchHits(new SearchHit[] {}, new TotalHits(0, TotalHits.Relation.EQUAL_TO), 1.0f); + SearchResponse searchResponse = mock(SearchResponse.class); + when(searchResponse.getHits()).thenReturn(emptyHits); + when(searchRequestBuilder.get()).thenReturn(searchResponse); + + AtomicReference failure = new AtomicReference<>(); + rulePersistenceService.updateRule(request, new ActionListener<>() { + @Override + public void onResponse(UpdateRuleResponse updateRuleResponse) { + fail(); + } + + @Override + public void onFailure(Exception e) { + failure.set(e); + } + }); + + assertTrue(failure.get() instanceof ResourceNotFoundException); + assertTrue(failure.get().getMessage().contains(_ID_ONE)); + } + + public void testUpdateRule_ParseFailure() { + UpdateRuleRequest request = mock(UpdateRuleRequest.class); + when(request.getId()).thenReturn(_ID_ONE); + + SearchHit searchHit = new SearchHit(1, _ID_ONE, null, null); + searchHit.sourceRef(new BytesArray(VALID_JSON)); + SearchHits searchHits = new SearchHits(new SearchHit[] { searchHit }, new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f); + SearchResponse searchResponse = mock(SearchResponse.class); + when(searchResponse.getHits()).thenReturn(searchHits); + when(searchRequestBuilder.get()).thenReturn(searchResponse); + when(ruleEntityParser.parse(anyString())).thenThrow(new RuntimeException("Invalid JSON")); + + AtomicReference failure = new AtomicReference<>(); + rulePersistenceService.updateRule(request, new ActionListener<>() { + @Override + public void onResponse(UpdateRuleResponse updateRuleResponse) { + fail(); + } + + @Override + public void onFailure(Exception e) { + failure.set(e); + } + }); + + assertNotNull(failure.get()); + assertTrue(failure.get().getMessage().contains("Invalid JSON")); + } } diff --git a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/utils/RuleTestUtils.java b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/utils/RuleTestUtils.java index 1c10d327a863f..b0a93fa369147 100644 --- a/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/utils/RuleTestUtils.java +++ b/modules/autotagging-commons/common/src/test/java/org/opensearch/rule/utils/RuleTestUtils.java @@ -13,14 +13,15 @@ import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule; +import java.util.Comparator; +import java.util.List; import java.util.Map; import java.util.Set; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; public class RuleTestUtils { - public static final String _ID_ONE = "AgfUO5Ja9yfvhdONlYi3TQ=="; + public static final String _ID_ONE = "e9f35a73-ece2-3fa7-857e-7c1af877fc75"; public static final String ATTRIBUTE_VALUE_ONE = "mock_attribute_one"; public static final String ATTRIBUTE_VALUE_TWO = "mock_attribute_two"; public static final String DESCRIPTION_ONE = "description_1"; @@ -29,7 +30,7 @@ public class RuleTestUtils { public static final String INVALID_ATTRIBUTE = "invalid_attribute"; public static final String SEARCH_AFTER = "search_after"; - public static final String _ID_TWO = "G5iIq84j7eK1qIAAAAIH53=1"; + public static final String _ID_TWO = "b55aa7e6-5aae-38e8-bf43-803599996ffe"; public static final String FEATURE_VALUE_ONE = "feature_value_one"; public static final String FEATURE_VALUE_TWO = "feature_value_two"; public static final String DESCRIPTION_TWO = "description_2"; @@ -63,14 +64,12 @@ public static Map ruleMap() { return Map.of(_ID_ONE, ruleOne, _ID_TWO, ruleTwo); } - public static void assertEqualRules(Map mapOne, Map mapTwo, boolean ruleUpdated) { + public static void assertEqualRules(List mapOne, List mapTwo, boolean ruleUpdated) { assertEquals(mapOne.size(), mapTwo.size()); - for (Map.Entry entry : mapOne.entrySet()) { - String id = entry.getKey(); - assertTrue(mapTwo.containsKey(id)); - Rule one = mapOne.get(id); - Rule two = mapTwo.get(id); - assertEqualRule(one, two, ruleUpdated); + mapOne.sort(Comparator.comparing(Rule::getId)); + mapTwo.sort(Comparator.comparing(Rule::getId)); + for (int i = 0; i < mapOne.size(); i++) { + assertEqualRule(mapOne.get(i), mapTwo.get(i), ruleUpdated); } } diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/RuleFrameworkPlugin.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/RuleFrameworkPlugin.java index cd6197cf890f7..ddccbf2d308e7 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/RuleFrameworkPlugin.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/RuleFrameworkPlugin.java @@ -28,11 +28,14 @@ import org.opensearch.rule.action.TransportCreateRuleAction; import org.opensearch.rule.action.TransportDeleteRuleAction; import org.opensearch.rule.action.TransportGetRuleAction; +import org.opensearch.rule.action.TransportUpdateRuleAction; +import org.opensearch.rule.action.UpdateRuleAction; import org.opensearch.rule.autotagging.AutoTaggingRegistry; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.rest.RestCreateRuleAction; import org.opensearch.rule.rest.RestDeleteRuleAction; import org.opensearch.rule.rest.RestGetRuleAction; +import org.opensearch.rule.rest.RestUpdateRuleAction; import org.opensearch.rule.spi.RuleFrameworkExtension; import org.opensearch.threadpool.ExecutorBuilder; import org.opensearch.threadpool.FixedExecutorBuilder; @@ -78,7 +81,8 @@ public RuleFrameworkPlugin() {} return List.of( new ActionPlugin.ActionHandler<>(GetRuleAction.INSTANCE, TransportGetRuleAction.class), new ActionPlugin.ActionHandler<>(DeleteRuleAction.INSTANCE, TransportDeleteRuleAction.class), - new ActionPlugin.ActionHandler<>(CreateRuleAction.INSTANCE, TransportCreateRuleAction.class) + new ActionPlugin.ActionHandler<>(CreateRuleAction.INSTANCE, TransportCreateRuleAction.class), + new ActionPlugin.ActionHandler<>(UpdateRuleAction.INSTANCE, TransportUpdateRuleAction.class) ); } @@ -92,7 +96,7 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { - return List.of(new RestGetRuleAction(), new RestDeleteRuleAction(), new RestCreateRuleAction()); + return List.of(new RestGetRuleAction(), new RestDeleteRuleAction(), new RestCreateRuleAction(), new RestUpdateRuleAction()); } @Override diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/CreateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/CreateRuleAction.java index fe7b424b31ca4..b61dd272366e0 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/CreateRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/CreateRuleAction.java @@ -9,7 +9,6 @@ package org.opensearch.rule.action; import org.opensearch.action.ActionType; -import org.opensearch.rule.CreateRuleResponse; /** * Action type for creating a Rule diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/GetRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/GetRuleAction.java index aa795d3aa4f72..49369e927aa01 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/GetRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/GetRuleAction.java @@ -9,7 +9,6 @@ package org.opensearch.rule.action; import org.opensearch.action.ActionType; -import org.opensearch.rule.GetRuleResponse; /** * Action type for getting Rules diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportCreateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportCreateRuleAction.java index f808cf7427cbc..9a4bff59ed2a1 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportCreateRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportCreateRuleAction.java @@ -12,8 +12,6 @@ import org.opensearch.action.support.TransportAction; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.rule.RuleRoutingServiceRegistry; diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportDeleteRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportDeleteRuleAction.java index 8bedffe0e89fc..34df52c28bba9 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportDeleteRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportDeleteRuleAction.java @@ -13,7 +13,6 @@ import org.opensearch.action.support.clustermanager.AcknowledgedResponse; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; -import org.opensearch.rule.DeleteRuleRequest; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.tasks.Task; diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportGetRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportGetRuleAction.java index 94321e5d40713..fdd73b3690d8e 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportGetRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportGetRuleAction.java @@ -12,11 +12,10 @@ import org.opensearch.action.support.HandledTransportAction; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.tasks.Task; +import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; /** @@ -26,28 +25,34 @@ public class TransportGetRuleAction extends HandledTransportAction { private final RulePersistenceServiceRegistry rulePersistenceServiceRegistry; + private final ThreadPool threadPool; /** - * Constructor for TransportGetWlmRuleAction + * Constructor for TransportGetRuleAction * @param transportService - a {@link TransportService} object + * @param threadPool - a {@link ThreadPool} object * @param actionFilters - a {@link ActionFilters} object * @param rulePersistenceServiceRegistry - a {@link RulePersistenceServiceRegistry} object */ @Inject public TransportGetRuleAction( TransportService transportService, + ThreadPool threadPool, ActionFilters actionFilters, RulePersistenceServiceRegistry rulePersistenceServiceRegistry ) { super(GetRuleAction.NAME, transportService, actionFilters, GetRuleRequest::new); + this.threadPool = threadPool; this.rulePersistenceServiceRegistry = rulePersistenceServiceRegistry; } @Override protected void doExecute(Task task, GetRuleRequest request, ActionListener listener) { - final RulePersistenceService rulePersistenceService = rulePersistenceServiceRegistry.getRulePersistenceService( - request.getFeatureType() - ); - rulePersistenceService.getRule(request, listener); + threadPool.executor(ThreadPool.Names.GET).execute(() -> { + final RulePersistenceService rulePersistenceService = rulePersistenceServiceRegistry.getRulePersistenceService( + request.getFeatureType() + ); + rulePersistenceService.getRule(request, listener); + }); } } diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportUpdateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportUpdateRuleAction.java new file mode 100644 index 0000000000000..d0f2043d1e196 --- /dev/null +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/TransportUpdateRuleAction.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.rule.action; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.TransportAction; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.action.ActionListener; +import org.opensearch.rule.RulePersistenceService; +import org.opensearch.rule.RulePersistenceServiceRegistry; +import org.opensearch.rule.RuleRoutingServiceRegistry; +import org.opensearch.tasks.Task; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportChannel; +import org.opensearch.transport.TransportException; +import org.opensearch.transport.TransportRequestHandler; +import org.opensearch.transport.TransportService; + +import java.io.IOException; + +import static org.opensearch.rule.RuleFrameworkPlugin.RULE_THREAD_POOL_NAME; + +/** + * Transport action to update Rules + * @opensearch.experimental + */ +public class TransportUpdateRuleAction extends TransportAction { + private final ThreadPool threadPool; + private final RuleRoutingServiceRegistry ruleRoutingServiceRegistry; + private final RulePersistenceServiceRegistry rulePersistenceServiceRegistry; + + /** + * Constructor for TransportUpdateRuleAction + * @param transportService - a {@link TransportService} object + * @param threadPool - a {@link ThreadPool} object + * @param actionFilters - a {@link ActionFilters} object + * @param rulePersistenceServiceRegistry - a {@link RulePersistenceServiceRegistry} object + * @param ruleRoutingServiceRegistry - a {@link RuleRoutingServiceRegistry} object + */ + @Inject + public TransportUpdateRuleAction( + TransportService transportService, + ThreadPool threadPool, + ActionFilters actionFilters, + RulePersistenceServiceRegistry rulePersistenceServiceRegistry, + RuleRoutingServiceRegistry ruleRoutingServiceRegistry + ) { + super(UpdateRuleAction.NAME, actionFilters, transportService.getTaskManager()); + this.rulePersistenceServiceRegistry = rulePersistenceServiceRegistry; + this.ruleRoutingServiceRegistry = ruleRoutingServiceRegistry; + this.threadPool = threadPool; + + transportService.registerRequestHandler( + UpdateRuleAction.NAME, + ThreadPool.Names.SAME, + UpdateRuleRequest::new, + new TransportRequestHandler() { + @Override + public void messageReceived(UpdateRuleRequest request, TransportChannel channel, Task task) { + executeLocally(request, ActionListener.wrap(response -> { + try { + channel.sendResponse(response); + } catch (IOException e) { + logger.error("Failed to send UpdateRuleResponse to transport channel", e); + throw new TransportException("Fail to send", e); + } + }, exception -> { + try { + channel.sendResponse(exception); + } catch (IOException e) { + logger.error("Failed to send exception response to transport channel", e); + throw new TransportException("Fail to send", e); + } + })); + } + } + ); + } + + @Override + protected void doExecute(Task task, UpdateRuleRequest request, ActionListener listener) { + ruleRoutingServiceRegistry.getRuleRoutingService(request.getFeatureType()).handleUpdateRuleRequest(request, listener); + } + + /** + * Executes the update rule operation locally on the dedicated rule thread pool. + * @param request the UpdateRuleRequest + * @param listener listener to handle response or failure + */ + private void executeLocally(UpdateRuleRequest request, ActionListener listener) { + threadPool.executor(RULE_THREAD_POOL_NAME).execute(() -> { + final RulePersistenceService rulePersistenceService = rulePersistenceServiceRegistry.getRulePersistenceService( + request.getFeatureType() + ); + rulePersistenceService.updateRule(request, listener); + }); + } +} diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/UpdateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/UpdateRuleAction.java new file mode 100644 index 0000000000000..8793b258e0a3d --- /dev/null +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/action/UpdateRuleAction.java @@ -0,0 +1,35 @@ +/* + * 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.rule.action; + +import org.opensearch.action.ActionType; + +/** + * Action type for updating Rules + * @opensearch.experimental + */ +public class UpdateRuleAction extends ActionType { + + /** + * An instance of UpdateRuleAction + */ + public static final UpdateRuleAction INSTANCE = new UpdateRuleAction(); + + /** + * Name for UpdateRuleAction + */ + public static final String NAME = "cluster:admin/opensearch/rule/_update"; + + /** + * Default constructor for UpdateRuleAction + */ + private UpdateRuleAction() { + super(NAME, UpdateRuleResponse::new); + } +} diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestCreateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestCreateRuleAction.java index 7a5f45e95a0ba..684691e97aac4 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestCreateRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestCreateRuleAction.java @@ -18,9 +18,9 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; import org.opensearch.rule.action.CreateRuleAction; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule.Builder; import org.opensearch.transport.client.node.NodeClient; @@ -57,8 +57,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli final FeatureType featureType = FeatureType.from(request.param(FEATURE_TYPE)); try (XContentParser parser = request.contentParser()) { Builder builder = Builder.fromXContent(parser, featureType); - CreateRuleRequest createRuleRequest = new CreateRuleRequest(builder.updatedAt(Instant.now().toString()).build()); - + CreateRuleRequest createRuleRequest = new CreateRuleRequest(builder.updatedAt(Instant.now().toString()).id().build()); return channel -> client.execute(CreateRuleAction.INSTANCE, createRuleRequest, createRuleResponse(channel)); } } diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestDeleteRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestDeleteRuleAction.java index bdf25963faf9f..89ce2b9143f70 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestDeleteRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestDeleteRuleAction.java @@ -18,14 +18,16 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; -import org.opensearch.rule.DeleteRuleRequest; import org.opensearch.rule.action.DeleteRuleAction; +import org.opensearch.rule.action.DeleteRuleRequest; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.transport.client.node.NodeClient; import java.util.List; import static org.opensearch.rest.RestRequest.Method.DELETE; +import static org.opensearch.rule.autotagging.Rule.ID_STRING; +import static org.opensearch.rule.rest.RestGetRuleAction.FEATURE_TYPE; /** * Rest action to delete a Rule @@ -45,13 +47,13 @@ public String getName() { @Override public List routes() { - return List.of(new RestHandler.Route(DELETE, "_rules/{featureType}/{_id}")); + return List.of(new RestHandler.Route(DELETE, "_rules/{featureType}/{id}")); } @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { - final String ruleId = request.param("ruleId"); - FeatureType featureType = FeatureType.from(request.param("featureType")); + final String ruleId = request.param(ID_STRING); + FeatureType featureType = FeatureType.from(request.param(FEATURE_TYPE)); DeleteRuleRequest deleteRuleRequest = new DeleteRuleRequest(ruleId, featureType); return channel -> client.execute(DeleteRuleAction.INSTANCE, deleteRuleRequest, deleteRuleResponse(channel)); } diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestGetRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestGetRuleAction.java index f1bd56927c41f..49bdb750b70ee 100644 --- a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestGetRuleAction.java +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestGetRuleAction.java @@ -18,14 +18,13 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.action.GetRuleAction; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; import org.opensearch.rule.autotagging.Attribute; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.transport.client.node.NodeClient; -import java.io.IOException; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -34,7 +33,7 @@ import java.util.Set; import static org.opensearch.rest.RestRequest.Method.GET; -import static org.opensearch.rule.autotagging.Rule._ID_STRING; +import static org.opensearch.rule.autotagging.Rule.ID_STRING; /** * Rest action to get a Rule @@ -63,11 +62,11 @@ public String getName() { @Override public List routes() { - return List.of(new RestHandler.Route(GET, "_rules/{featureType}/"), new RestHandler.Route(GET, "_rules/{featureType}/{_id}")); + return List.of(new RestHandler.Route(GET, "_rules/{featureType}/"), new RestHandler.Route(GET, "_rules/{featureType}/{id}")); } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { final Map> attributeFilters = new HashMap<>(); if (!request.hasParam(FEATURE_TYPE)) { @@ -75,11 +74,8 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } final FeatureType featureType = FeatureType.from(request.param(FEATURE_TYPE)); - final List requestParams = request.params() - .keySet() - .stream() - .filter(key -> !key.equals(FEATURE_TYPE) && !key.equals(_ID_STRING) && !key.equals(SEARCH_AFTER_STRING)) - .toList(); + final Set excludedKeys = Set.of(FEATURE_TYPE, ID_STRING, SEARCH_AFTER_STRING, "pretty"); + final List requestParams = request.params().keySet().stream().filter(key -> !excludedKeys.contains(key)).toList(); for (String attributeName : requestParams) { Attribute attribute = featureType.getAttributeFromName(attributeName); @@ -89,7 +85,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli attributeFilters.put(attribute, parseAttributeValues(request.param(attributeName), attributeName, featureType)); } final GetRuleRequest getRuleRequest = new GetRuleRequest( - request.param(_ID_STRING), + request.param(ID_STRING), attributeFilters, request.param(SEARCH_AFTER_STRING), featureType diff --git a/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestUpdateRuleAction.java b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestUpdateRuleAction.java new file mode 100644 index 0000000000000..98d24655323fe --- /dev/null +++ b/modules/autotagging-commons/src/main/java/org/opensearch/rule/rest/RestUpdateRuleAction.java @@ -0,0 +1,81 @@ +/* + * 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.rule.rest; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; +import org.opensearch.rest.action.RestResponseListener; +import org.opensearch.rule.action.UpdateRuleAction; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; +import org.opensearch.rule.autotagging.FeatureType; +import org.opensearch.rule.autotagging.Rule.Builder; +import org.opensearch.transport.client.node.NodeClient; + +import java.io.IOException; +import java.util.List; + +import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.rule.autotagging.Rule.ID_STRING; +import static org.opensearch.rule.rest.RestGetRuleAction.FEATURE_TYPE; + +/** + * Rest action to update a Rule + * @opensearch.experimental + */ +@ExperimentalApi +public class RestUpdateRuleAction extends BaseRestHandler { + /** + * constructor for RestUpdateRuleAction + */ + public RestUpdateRuleAction() {} + + @Override + public String getName() { + return "update_rule"; + } + + @Override + public List routes() { + return List.of(new RestHandler.Route(PUT, "_rules/{featureType}/{id}")); + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + final FeatureType featureType = FeatureType.from(request.param(FEATURE_TYPE)); + try (XContentParser parser = request.contentParser()) { + Builder builder = Builder.fromXContent(parser, featureType); + UpdateRuleRequest updateRuleRequest = new UpdateRuleRequest( + request.param(ID_STRING), + builder.getDescription(), + builder.getAttributeMap(), + builder.getFeatureValue(), + featureType + ); + return channel -> client.execute(UpdateRuleAction.INSTANCE, updateRuleRequest, updateRuleResponse(channel)); + } + } + + private RestResponseListener updateRuleResponse(final RestChannel channel) { + return new RestResponseListener<>(channel) { + @Override + public RestResponse buildResponse(final UpdateRuleResponse response) throws Exception { + return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)); + } + }; + } +} diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/RuleFrameworkPluginTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/RuleFrameworkPluginTests.java index 4e11d12f9facb..ff5f907ce352c 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/RuleFrameworkPluginTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/RuleFrameworkPluginTests.java @@ -16,7 +16,10 @@ import org.opensearch.plugins.ActionPlugin; import org.opensearch.rest.RestHandler; import org.opensearch.rule.action.GetRuleAction; +import org.opensearch.rule.rest.RestCreateRuleAction; +import org.opensearch.rule.rest.RestDeleteRuleAction; import org.opensearch.rule.rest.RestGetRuleAction; +import org.opensearch.rule.rest.RestUpdateRuleAction; import org.opensearch.test.OpenSearchTestCase; import java.util.List; @@ -28,13 +31,13 @@ public class RuleFrameworkPluginTests extends OpenSearchTestCase { public void testGetActions() { List> handlers = plugin.getActions(); - assertEquals(3, handlers.size()); + assertEquals(4, handlers.size()); assertEquals(GetRuleAction.INSTANCE.name(), handlers.get(0).getAction().name()); } public void testGetRestHandlers() { Settings settings = Settings.EMPTY; - RestHandler handler = plugin.getRestHandlers( + List handlers = plugin.getRestHandlers( settings, mock(org.opensearch.rest.RestController.class), null, @@ -42,8 +45,11 @@ public void testGetRestHandlers() { null, mock(IndexNameExpressionResolver.class), () -> mock(DiscoveryNodes.class) - ).get(0); + ); - assertTrue(handler instanceof RestGetRuleAction); + assertTrue(handlers.get(0) instanceof RestGetRuleAction); + assertTrue(handlers.get(1) instanceof RestDeleteRuleAction); + assertTrue(handlers.get(2) instanceof RestCreateRuleAction); + assertTrue(handlers.get(3) instanceof RestUpdateRuleAction); } } diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportCreateRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportCreateRuleActionTests.java index fa413044c5efc..072e2278bea14 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportCreateRuleActionTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportCreateRuleActionTests.java @@ -10,8 +10,6 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.core.action.ActionListener; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.rule.RuleRoutingService; import org.opensearch.rule.RuleRoutingServiceRegistry; diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportDeleteRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportDeleteRuleActionTests.java index c236362cc449e..d7dd1e4d26c85 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportDeleteRuleActionTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportDeleteRuleActionTests.java @@ -9,7 +9,6 @@ package org.opensearch.rule.action; import org.opensearch.action.support.ActionFilters; -import org.opensearch.rule.DeleteRuleRequest; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.test.OpenSearchTestCase; diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportGetRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportGetRuleActionTests.java index 369af17ed581f..009cabc18ccbd 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportGetRuleActionTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/action/TransportGetRuleActionTests.java @@ -9,12 +9,15 @@ package org.opensearch.rule.action; import org.opensearch.action.support.ActionFilters; -import org.opensearch.rule.GetRuleRequest; import org.opensearch.rule.RulePersistenceService; import org.opensearch.rule.RulePersistenceServiceRegistry; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; @@ -35,8 +38,12 @@ public void testExecute() { when(rulePersistenceServiceRegistry.getRulePersistenceService(any())).thenReturn(rulePersistenceService); doNothing().when(rulePersistenceService).getRule(any(), any()); - sut = new TransportGetRuleAction(transportService, actionFilters, rulePersistenceServiceRegistry); + ThreadPool threadPool = mock(ThreadPool.class); + ExecutorService mockExecutor = Executors.newSingleThreadExecutor(); + when(threadPool.executor(any())).thenReturn(mockExecutor); + sut = new TransportGetRuleAction(transportService, threadPool, actionFilters, rulePersistenceServiceRegistry); sut.doExecute(null, getRuleRequest, null); verify(rulePersistenceService, times(1)).getRule(any(), any()); + mockExecutor.shutdownNow(); } } diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestDeleteRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestDeleteRuleActionTests.java index 4cbaa7ca76420..2997af9e71807 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestDeleteRuleActionTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestDeleteRuleActionTests.java @@ -20,8 +20,6 @@ public void testGetName() { public void testRoutes() { var routes = action.routes(); assertEquals(1, routes.size()); - assertTrue( - routes.stream().anyMatch(r -> r.getMethod().name().equals("DELETE") && r.getPath().equals("_rules/{featureType}/{_id}")) - ); + assertTrue(routes.stream().anyMatch(r -> r.getMethod().name().equals("DELETE") && r.getPath().equals("_rules/{featureType}/{id}"))); } } diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestGetRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestGetRuleActionTests.java index 726924593e2b8..3c5e434c2735e 100644 --- a/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestGetRuleActionTests.java +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestGetRuleActionTests.java @@ -21,6 +21,6 @@ public void testRoutes() { var routes = action.routes(); assertEquals(2, routes.size()); assertTrue(routes.stream().anyMatch(r -> r.getPath().equals("_rules/{featureType}/"))); - assertTrue(routes.stream().anyMatch(r -> r.getPath().equals("_rules/{featureType}/{_id}"))); + assertTrue(routes.stream().anyMatch(r -> r.getPath().equals("_rules/{featureType}/{id}"))); } } diff --git a/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestUpdateRuleActionTests.java b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestUpdateRuleActionTests.java new file mode 100644 index 0000000000000..2f1d4d8d9f61a --- /dev/null +++ b/modules/autotagging-commons/src/test/java/org/opensearch/rule/rest/RestUpdateRuleActionTests.java @@ -0,0 +1,25 @@ +/* + * 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.rule.rest; + +import org.opensearch.test.OpenSearchTestCase; + +public class RestUpdateRuleActionTests extends OpenSearchTestCase { + RestUpdateRuleAction action = new RestUpdateRuleAction();; + + public void testGetName() { + assertEquals("update_rule", action.getName()); + } + + public void testRoutes() { + var routes = action.routes(); + assertEquals(1, routes.size()); + assertTrue(routes.stream().anyMatch(r -> r.getPath().equals("_rules/{featureType}/{id}"))); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java index 369397cc1ae43..9135b12c9cfaf 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java @@ -91,7 +91,6 @@ public class WorkloadManagementPlugin extends Plugin implements ActionPlugin, Sy private static FeatureType featureType; private static RulePersistenceService rulePersistenceService; private static RuleRoutingService ruleRoutingService; - private AutoTaggingActionFilter autoTaggingActionFilter; /** @@ -128,7 +127,6 @@ public Collection createComponents( parser, new IndexBasedRuleQueryMapper() ); - ruleRoutingService = new WorkloadGroupRuleRoutingService(client, clusterService); RefreshBasedSyncMechanism refreshMechanism = new RefreshBasedSyncMechanism( diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/WorkloadGroupRuleRoutingService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/WorkloadGroupRuleRoutingService.java index a70482eb40186..300fe1fcd2b19 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/WorkloadGroupRuleRoutingService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/WorkloadGroupRuleRoutingService.java @@ -12,7 +12,9 @@ import org.apache.logging.log4j.Logger; import org.opensearch.ExceptionsHelper; import org.opensearch.ResourceAlreadyExistsException; +import org.opensearch.ResourceNotFoundException; import org.opensearch.action.ActionListenerResponseHandler; +import org.opensearch.action.ActionRequest; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; import org.opensearch.cluster.ClusterState; @@ -22,11 +24,16 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.plugin.wlm.WorkloadManagementPlugin; -import org.opensearch.rule.CreateRuleRequest; -import org.opensearch.rule.CreateRuleResponse; import org.opensearch.rule.RuleRoutingService; import org.opensearch.rule.action.CreateRuleAction; +import org.opensearch.rule.action.CreateRuleRequest; +import org.opensearch.rule.action.CreateRuleResponse; +import org.opensearch.rule.action.UpdateRuleAction; +import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.action.UpdateRuleResponse; import org.opensearch.transport.TransportService; import org.opensearch.transport.client.Client; @@ -67,8 +74,8 @@ public void handleCreateRuleRequest(CreateRuleRequest request, ActionListener() { @@ -78,7 +85,7 @@ public void onResponse(CreateIndexResponse response) { logger.error("Failed to create index " + indexName); listener.onFailure(new IllegalStateException(indexName + " index creation not acknowledged")); } else { - routeRequest(request, listener, indexName); + routeRequest(CreateRuleAction.NAME, indexName, request, CreateRuleResponse::new, listener); } } @@ -86,7 +93,7 @@ public void onResponse(CreateIndexResponse response) { public void onFailure(Exception e) { Throwable cause = ExceptionsHelper.unwrapCause(e); if (cause instanceof ResourceAlreadyExistsException) { - routeRequest(request, listener, indexName); + routeRequest(CreateRuleAction.NAME, indexName, request, CreateRuleResponse::new, listener); } else { logger.error("Failed to create index {}: {}", indexName, e.getMessage()); listener.onFailure(e); @@ -96,6 +103,19 @@ public void onFailure(Exception e) { } } + @Override + public void handleUpdateRuleRequest(UpdateRuleRequest request, ActionListener listener) { + String indexName = WorkloadManagementPlugin.INDEX_NAME; + try (ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + if (!hasIndex(indexName)) { + logger.error("Index {} not found", indexName); + listener.onFailure(new ResourceNotFoundException("Index " + indexName + " does not exist.")); + } else { + routeRequest(UpdateRuleAction.NAME, indexName, request, UpdateRuleResponse::new, listener); + } + } + } + /** * Creates the backing index if it does not exist, then runs the given success callback. * @param indexName the name of the index to create @@ -107,24 +127,31 @@ private void createIndex(String indexName, ActionListener l } /** - * Routes the CreateRuleRequest to the primary shard node for the given index. + * Routes the request to the primary shard node for the given index. * Executes locally if the current node is the primary. - * @param request the CreateRuleRequest - * @param listener listener to handle response or failure - * @param indexName the index name used to find the primary shard node + * @param actionName + * @param indexName + * @param request + * @param responseReader + * @param listener */ - private void routeRequest(CreateRuleRequest request, ActionListener listener, String indexName) { - Optional optionalPrimaryNode = getPrimaryShardNode(indexName); - if (optionalPrimaryNode.isEmpty()) { + private void routeRequest( + String actionName, + String indexName, + Request request, + Writeable.Reader responseReader, + ActionListener listener + ) { + Optional primaryNodeOpt = getPrimaryShardNode(indexName); + if (primaryNodeOpt.isEmpty()) { listener.onFailure(new IllegalStateException("Primary node for index [" + indexName + "] not found")); return; } - DiscoveryNode primaryNode = optionalPrimaryNode.get(); transportService.sendRequest( - primaryNode, - CreateRuleAction.NAME, + primaryNodeOpt.get(), + actionName, request, - new ActionListenerResponseHandler<>(listener, CreateRuleResponse::new) + new ActionListenerResponseHandler<>(listener, responseReader) ); } @@ -140,4 +167,12 @@ private Optional getPrimaryShardNode(String indexName) { .filter(ShardRouting::assignedToNode) .map(shard -> state.nodes().get(shard.currentNodeId())); } + + /** + * Checks whether the index is present + * @param indexName - the index name to check + */ + private boolean hasIndex(String indexName) { + return clusterService.state().metadata().hasIndex(indexName); + } } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanism.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanism.java index ef0ce150aac07..f367e63aa3f51 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanism.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanism.java @@ -18,11 +18,11 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.plugin.wlm.rule.sync.detect.RuleEvent; import org.opensearch.plugin.wlm.rule.sync.detect.RuleEventClassifier; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.InMemoryRuleProcessingService; import org.opensearch.rule.RuleEntityParser; import org.opensearch.rule.RulePersistenceService; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule; import org.opensearch.threadpool.Scheduler; @@ -123,7 +123,7 @@ synchronized void doRun() { new ActionListener() { @Override public void onResponse(GetRuleResponse response) { - final Set newRules = new HashSet<>(response.getRules().values()); + final Set newRules = new HashSet<>(response.getRules()); ruleEventClassifier.setPreviousRules(lastRunIndexedRules); ruleEventClassifier.getRuleEvents(newRules).forEach(RuleEvent::process); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanismTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanismTests.java index b54f70ea9cf84..cca14c8778a87 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanismTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/rule/sync/RefreshBasedSyncMechanismTests.java @@ -14,11 +14,11 @@ import org.opensearch.plugin.wlm.AutoTaggingActionFilterTests; import org.opensearch.plugin.wlm.WorkloadManagementPlugin; import org.opensearch.plugin.wlm.rule.sync.detect.RuleEventClassifier; -import org.opensearch.rule.GetRuleRequest; -import org.opensearch.rule.GetRuleResponse; import org.opensearch.rule.InMemoryRuleProcessingService; import org.opensearch.rule.RuleEntityParser; import org.opensearch.rule.RulePersistenceService; +import org.opensearch.rule.action.GetRuleRequest; +import org.opensearch.rule.action.GetRuleResponse; import org.opensearch.rule.autotagging.Attribute; import org.opensearch.rule.autotagging.FeatureType; import org.opensearch.rule.autotagging.Rule; @@ -33,12 +33,12 @@ import org.opensearch.wlm.WorkloadManagementSettings; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static org.opensearch.plugin.wlm.rule.sync.detect.RuleEventClassifierTests.getRandomRule; import static org.mockito.ArgumentMatchers.any; @@ -166,7 +166,7 @@ public void test_doRun_RefreshesRules() { .id("test_id") .build(); - when(getRuleResponse.getRules()).thenReturn(Map.of("test_id", rule)); + when(getRuleResponse.getRules()).thenReturn(List.of(rule)); doAnswer(invocation -> { ActionListener listener = invocation.getArgument(1); listener.onResponse(getRuleResponse); @@ -181,10 +181,10 @@ public void test_doRun_RefreshesRules() { @SuppressWarnings("unchecked") public void test_doRun_RefreshesRulesAndCheckInMemoryView() { GetRuleResponse getRuleResponse = mock(GetRuleResponse.class); - Map existingRules = new HashMap<>(); + List existingRules = new ArrayList<>(); for (int i = 0; i < 10; i++) { final String randomRuleId = randomAlphaOfLength(5); - existingRules.put(randomRuleId, getRandomRule(randomRuleId)); + existingRules.add(getRandomRule(randomRuleId)); } when(getRuleResponse.getRules()).thenReturn(existingRules); @@ -197,16 +197,16 @@ public void test_doRun_RefreshesRulesAndCheckInMemoryView() { // marks the first run of service sut.doRun(); - Set previousRules = new HashSet<>(existingRules.values()); + Set previousRules = new HashSet<>(existingRules); Set newRules = new HashSet<>(); int deletionEventCount = 10; // Mark some deletions - for (Map.Entry rule : existingRules.entrySet()) { + for (Rule rule : existingRules) { if (randomBoolean()) { deletionEventCount--; - newRules.add(rule.getValue()); + newRules.add(rule); } } @@ -224,7 +224,7 @@ public void test_doRun_RefreshesRulesAndCheckInMemoryView() { } } - when(getRuleResponse.getRules()).thenReturn(newRules.stream().collect(Collectors.toMap(Rule::getId, rule -> rule))); + when(getRuleResponse.getRules()).thenReturn(new ArrayList<>(newRules)); doAnswer(invocation -> { ActionListener listener = invocation.getArgument(1); listener.onResponse(getRuleResponse);