diff --git a/CHANGELOG.md b/CHANGELOG.md index b90ebaec766d3..3f558f68c8ec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add a dynamic setting to change skip_cache_factor and min_frequency for querycache ([#18351](https://github.com/opensearch-project/OpenSearch/issues/18351)) - Add overload constructor for Translog to accept Channel Factory as a parameter ([#18918](https://github.com/opensearch-project/OpenSearch/pull/18918)) - Add subdirectory-aware store module with recovery support ([#19132](https://github.com/opensearch-project/OpenSearch/pull/19132)) +- [Rule-based Auto-tagging] Modify get rule api to suit nested attributes ([#19429](https://github.com/opensearch-project/OpenSearch/pull/19429)) - Field collapsing supports search_after ([#19261](https://github.com/opensearch-project/OpenSearch/pull/19261)) - Add a dynamic cluster setting to control the enablement of the merged segment warmer ([#18929](https://github.com/opensearch-project/OpenSearch/pull/18929)) - Publish transport-grpc-spi exposing QueryBuilderProtoConverter and QueryBuilderProtoConverterRegistry ([#18949](https://github.com/opensearch-project/OpenSearch/pull/18949)) 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 243326f9f28d3..d6083f9233767 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 @@ -17,6 +17,8 @@ import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -29,6 +31,8 @@ */ @ExperimentalApi public class RuleUtils { + private static final String PIPE_DELIMITER = "\\|"; + private static final String DOT_DELIMITER = "."; /** * constructor for RuleUtils @@ -112,4 +116,36 @@ public static Rule composeUpdatedRule(Rule originalRule, UpdateRuleRequest reque Instant.now().toString() ); } + + /** + * Builds a flattened map of attribute filters from a {@link Rule}. + * This method reformats nested or prioritized subfields (e.g., values containing "|" for sub-attributes) + * into top-level attribute keys. For example, an attribute "principal" with value "username|admin" will + * become "principal.username" -> "admin" in the resulting map. Attributes without prioritized subfields + * remain unchanged. + * The resulting map is structured to make querying rules from the index easier. + * @param rule the rule whose attributes are to be flattened + */ + public static Map> buildAttributeFilters(Rule rule) { + Map> attributeFilters = new HashMap<>(); + + for (Map.Entry> entry : rule.getAttributeMap().entrySet()) { + Attribute attribute = entry.getKey(); + Set values = entry.getValue(); + if (hasSubfields(attribute)) { + for (String value : values) { + String[] parts = value.split(PIPE_DELIMITER); + String topLevelAttribute = attribute.getName() + DOT_DELIMITER + parts[0]; + attributeFilters.computeIfAbsent(topLevelAttribute, k -> new HashSet<>()).add(parts[1]); + } + } else { + attributeFilters.put(attribute.getName(), values); + } + } + return attributeFilters; + } + + private static boolean hasSubfields(Attribute attribute) { + return !attribute.getPrioritizedSubfields().isEmpty(); + } } diff --git a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java index e6da349b046c7..2e5c02c2e3db3 100644 --- a/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java +++ b/modules/autotagging-commons/common/src/main/java/org/opensearch/rule/action/GetRuleRequest.java @@ -13,7 +13,6 @@ 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 org.opensearch.rule.autotagging.Rule; import org.opensearch.rule.autotagging.RuleValidator; @@ -34,7 +33,7 @@ @ExperimentalApi public class GetRuleRequest extends ActionRequest { private final String id; - private final Map> attributeFilters; + private final Map> attributeFilters; private final String searchAfter; private final FeatureType featureType; @@ -45,7 +44,7 @@ public class GetRuleRequest extends ActionRequest { * @param searchAfter - The sort value used for pagination. * @param featureType - The feature type related to rule. */ - public GetRuleRequest(String id, Map> attributeFilters, String searchAfter, FeatureType featureType) { + public GetRuleRequest(String id, Map> attributeFilters, String searchAfter, FeatureType featureType) { this.id = id; this.attributeFilters = attributeFilters; this.searchAfter = searchAfter; @@ -60,7 +59,7 @@ public GetRuleRequest(StreamInput in) throws IOException { super(in); id = in.readOptionalString(); featureType = FeatureType.from(in); - attributeFilters = in.readMap(i -> Attribute.from(i, featureType), i -> new HashSet<>(i.readStringList())); + attributeFilters = in.readMap(StreamInput::readString, i -> new HashSet<>(i.readStringList())); searchAfter = in.readOptionalString(); } @@ -80,7 +79,7 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeOptionalString(id); featureType.writeTo(out); - out.writeMap(attributeFilters, (output, attribute) -> attribute.writeTo(output), StreamOutput::writeStringCollection); + out.writeMap(attributeFilters, StreamOutput::writeString, StreamOutput::writeStringCollection); out.writeOptionalString(searchAfter); } @@ -94,7 +93,7 @@ public String getId() { /** * attributeFilters getter */ - public Map> getAttributeFilters() { + public Map> getAttributeFilters() { return attributeFilters; } 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 190816d24c279..4e2e51a81d523 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 @@ -26,6 +26,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.query.QueryBuilder; import org.opensearch.rule.RuleEntityParser; @@ -45,10 +46,13 @@ import org.opensearch.search.sort.SortOrder; import org.opensearch.transport.client.Client; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; /** * This class encapsulates the logic to manage the lifecycle of rules at index level @@ -158,7 +162,8 @@ private void performCardinalityCheck(ActionListener listener * @param listener - listener for validateNoDuplicateRule response */ private void validateNoDuplicateRule(Rule rule, ActionListener listener) { - QueryBuilder query = queryBuilder.from(new GetRuleRequest(null, rule.getAttributeMap(), null, rule.getFeatureType())); + Map> attributeFilters = RuleUtils.buildAttributeFilters(rule); + QueryBuilder query = queryBuilder.from(new GetRuleRequest(null, attributeFilters, null, rule.getFeatureType())); getRuleFromIndex(null, query, null, new ActionListener<>() { @Override public void onResponse(GetRuleResponse getRuleResponse) { @@ -225,6 +230,11 @@ private void getRuleFromIndex(String id, QueryBuilder queryBuilder, String searc if (hasNoResults(id, listener, hits)) return; handleGetRuleResponse(hits, listener); } catch (Exception e) { + if (e instanceof IndexNotFoundException) { + logger.debug("Failed to get rule from index [{}]: index doesn't exist.", indexName); + handleGetRuleResponse(new ArrayList<>(), listener); + return; + } logger.error("Failed to fetch all rules: {}", e.getMessage()); listener.onFailure(e); } 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 5189c4b2ce48b..91856b9098cc2 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 @@ -14,8 +14,8 @@ import org.opensearch.index.query.QueryBuilders; import org.opensearch.rule.RuleQueryMapper; import org.opensearch.rule.action.GetRuleRequest; -import org.opensearch.rule.autotagging.Attribute; +import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -33,24 +33,28 @@ public IndexBasedRuleQueryMapper() {} @Override public QueryBuilder from(GetRuleRequest request) { final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery(); - final Map> attributeFilters = request.getAttributeFilters(); + final Map> attributeFilters = request.getAttributeFilters(); final String id = request.getId(); boolQuery.filter(QueryBuilders.existsQuery(request.getFeatureType().getName())); if (id != null) { return boolQuery.must(QueryBuilders.termQuery("_id", id)); } - for (Map.Entry> entry : attributeFilters.entrySet()) { - Attribute attribute = entry.getKey(); + Map groupedQueries = new HashMap<>(); + for (Map.Entry> entry : attributeFilters.entrySet()) { + String attribute = entry.getKey(); Set values = entry.getValue(); if (values != null && !values.isEmpty()) { - BoolQueryBuilder attributeQuery = QueryBuilders.boolQuery(); + String topLevelAttribute = attribute.contains(".") ? attribute.substring(0, attribute.indexOf('.')) : attribute; + BoolQueryBuilder groupQuery = groupedQueries.computeIfAbsent(topLevelAttribute, k -> QueryBuilders.boolQuery()); for (String value : values) { - attributeQuery.should(QueryBuilders.matchQuery(attribute.getName(), value)); + groupQuery.should(QueryBuilders.termQuery(attribute + ".keyword", value)); } - boolQuery.must(attributeQuery); } } + for (BoolQueryBuilder groupQuery : groupedQueries.values()) { + boolQuery.must(groupQuery); + } return boolQuery; } 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 555ab59b71250..35d3e15d5f010 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 @@ -16,6 +16,9 @@ import org.opensearch.test.OpenSearchTestCase; import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; public class IndexStoredRuleUtilsTests extends OpenSearchTestCase { RuleQueryMapper sut; @@ -37,9 +40,11 @@ public void testBuildGetRuleQuery_WithId() { } public void testBuildGetRuleQuery_WithAttributes() { - QueryBuilder queryBuilder = sut.from( - new GetRuleRequest(null, RuleTestUtils.ATTRIBUTE_MAP, null, RuleTestUtils.MockRuleFeatureType.INSTANCE) - ); + Map> attributeFilters = RuleTestUtils.ATTRIBUTE_MAP.entrySet() + .stream() + .collect(Collectors.toMap(e -> e.getKey().getName(), Map.Entry::getValue)); + + QueryBuilder queryBuilder = sut.from(new GetRuleRequest(null, attributeFilters, null, RuleTestUtils.MockRuleFeatureType.INSTANCE)); assertNotNull(queryBuilder); BoolQueryBuilder query = (BoolQueryBuilder) queryBuilder; assertEquals(1, query.must().size()); 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 17026fdf75ad4..84acd89d13f39 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 @@ -9,15 +9,18 @@ package org.opensearch.rule; import org.opensearch.rule.action.UpdateRuleRequest; +import org.opensearch.rule.autotagging.Attribute; 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.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.TreeMap; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_MAP; import static org.opensearch.rule.utils.RuleTestUtils.ATTRIBUTE_VALUE_ONE; @@ -32,6 +35,8 @@ 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; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RuleUtilsTests extends OpenSearchTestCase { @@ -125,4 +130,34 @@ public void testComposeUpdateAllFields() { assertEquals(FEATURE_VALUE_TWO, updatedRule.getFeatureValue()); assertEquals(RuleTestUtils.MockRuleFeatureType.INSTANCE, updatedRule.getFeatureType()); } + + public void testBuildAttributeFiltersWithMock() { + Attribute indexPattern = mock(Attribute.class); + when(indexPattern.getName()).thenReturn("index_pattern"); + when(indexPattern.getPrioritizedSubfields()).thenReturn(new TreeMap<>()); + + Attribute principal = mock(Attribute.class); + when(principal.getName()).thenReturn("principal"); + when(principal.getPrioritizedSubfields()).thenReturn(new TreeMap<>(Map.of(1, "username", 2, "role"))); + + Set indexValues = Set.of("my-index"); + Set principalValues = Set.of("username|admin", "role|user"); + + Map> attributeMap = new HashMap<>(); + attributeMap.put(indexPattern, indexValues); + attributeMap.put(principal, principalValues); + + Rule rule = mock(Rule.class); + when(rule.getAttributeMap()).thenReturn(attributeMap); + + Map> result = RuleUtils.buildAttributeFilters(rule); + + assertEquals(3, result.size()); + assertTrue(result.containsKey("index_pattern")); + assertEquals(Set.of("my-index"), result.get("index_pattern")); + assertTrue(result.containsKey("principal.username")); + assertEquals(Set.of("admin"), result.get("principal.username")); + assertTrue(result.containsKey("principal.role")); + assertEquals(Set.of("user"), result.get("principal.role")); + } } 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 7a17c26f4818f..8ab40c79fbec1 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 @@ -15,17 +15,26 @@ 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.ATTRIBUTE_VALUE_ONE; import static org.opensearch.rule.utils.RuleTestUtils.SEARCH_AFTER; import static org.opensearch.rule.utils.RuleTestUtils._ID_ONE; public class GetRuleRequestTests extends OpenSearchTestCase { + + private final Map> attributeFilters = Map.of( + RuleTestUtils.MockRuleAttributes.MOCK_RULE_ATTRIBUTE_ONE.getName(), + Set.of(ATTRIBUTE_VALUE_ONE) + ); + /** * Test case to verify the serialization and deserialization of GetRuleRequest */ public void testSerialization() throws IOException { - GetRuleRequest request = new GetRuleRequest(_ID_ONE, ATTRIBUTE_MAP, null, RuleTestUtils.MockRuleFeatureType.INSTANCE); + + GetRuleRequest request = new GetRuleRequest(_ID_ONE, attributeFilters, null, RuleTestUtils.MockRuleFeatureType.INSTANCE); assertEquals(_ID_ONE, request.getId()); assertNull(request.validate()); assertNull(request.getSearchAfter()); @@ -58,9 +67,9 @@ public void testSerializationWithNull() throws IOException { } public void testValidate() { - GetRuleRequest request = new GetRuleRequest("", ATTRIBUTE_MAP, null, RuleTestUtils.MockRuleFeatureType.INSTANCE); + GetRuleRequest request = new GetRuleRequest("", attributeFilters, null, RuleTestUtils.MockRuleFeatureType.INSTANCE); assertThrows(IllegalArgumentException.class, request::validate); - request = new GetRuleRequest(_ID_ONE, ATTRIBUTE_MAP, "", RuleTestUtils.MockRuleFeatureType.INSTANCE); + request = new GetRuleRequest(_ID_ONE, attributeFilters, "", RuleTestUtils.MockRuleFeatureType.INSTANCE); assertThrows(IllegalArgumentException.class, request::validate); } } 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 88511f5823dac..a2c1c10814072 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 @@ -21,7 +21,6 @@ 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; @@ -67,7 +66,7 @@ public List routes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { - final Map> attributeFilters = new HashMap<>(); + final Map> attributeFilters = new HashMap<>(); if (!request.hasParam(FEATURE_TYPE)) { throw new IllegalArgumentException("Invalid route."); @@ -77,14 +76,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli final List attributeParams = request.params() .keySet() .stream() - .filter(key -> featureType.getAllowedAttributesRegistry().containsKey(key)) + .filter(key -> featureType.getAllowedAttributesRegistry().containsKey(key.split("\\.", 2)[0])) .toList(); for (String attributeName : attributeParams) { - Attribute attribute = featureType.getAttributeFromName(attributeName); - if (attribute == null) { - throw new IllegalArgumentException(attributeName + " is not a valid attribute under feature type " + featureType.getName()); - } - attributeFilters.put(attribute, parseAttributeValues(request.param(attributeName), attributeName, featureType)); + attributeFilters.put(attributeName, parseAttributeValues(request.param(attributeName), attributeName, featureType)); } final GetRuleRequest getRuleRequest = new GetRuleRequest( request.param(ID_STRING),