Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,6 +31,8 @@
*/
@ExperimentalApi
public class RuleUtils {
private static final String PIPE_DELIMITER = "\\|";
private static final String DOT_DELIMITER = ".";

/**
* constructor for RuleUtils
Expand Down Expand Up @@ -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<String, Set<String>> buildAttributeFilters(Rule rule) {
Map<String, Set<String>> attributeFilters = new HashMap<>();

for (Map.Entry<Attribute, Set<String>> entry : rule.getAttributeMap().entrySet()) {
Attribute attribute = entry.getKey();
Set<String> 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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,7 +33,7 @@
@ExperimentalApi
public class GetRuleRequest extends ActionRequest {
private final String id;
private final Map<Attribute, Set<String>> attributeFilters;
private final Map<String, Set<String>> attributeFilters;
private final String searchAfter;
private final FeatureType featureType;

Expand All @@ -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<Attribute, Set<String>> attributeFilters, String searchAfter, FeatureType featureType) {
public GetRuleRequest(String id, Map<String, Set<String>> attributeFilters, String searchAfter, FeatureType featureType) {
this.id = id;
this.attributeFilters = attributeFilters;
this.searchAfter = searchAfter;
Expand All @@ -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();
}

Expand All @@ -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);
}

Expand All @@ -94,7 +93,7 @@ public String getId() {
/**
* attributeFilters getter
*/
public Map<Attribute, Set<String>> getAttributeFilters() {
public Map<String, Set<String>> getAttributeFilters() {
return attributeFilters;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -158,7 +162,8 @@ private void performCardinalityCheck(ActionListener<CreateRuleResponse> listener
* @param listener - listener for validateNoDuplicateRule response
*/
private void validateNoDuplicateRule(Rule rule, ActionListener<Void> listener) {
QueryBuilder query = queryBuilder.from(new GetRuleRequest(null, rule.getAttributeMap(), null, rule.getFeatureType()));
Map<String, Set<String>> 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) {
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -33,24 +33,28 @@ public IndexBasedRuleQueryMapper() {}
@Override
public QueryBuilder from(GetRuleRequest request) {
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
final Map<Attribute, Set<String>> attributeFilters = request.getAttributeFilters();
final Map<String, Set<String>> 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<Attribute, Set<String>> entry : attributeFilters.entrySet()) {
Attribute attribute = entry.getKey();
Map<String, BoolQueryBuilder> groupedQueries = new HashMap<>();
for (Map.Entry<String, Set<String>> entry : attributeFilters.entrySet()) {
String attribute = entry.getKey();
Set<String> 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryBuilder> sut;
Expand All @@ -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<String, Set<String>> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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<String> indexValues = Set.of("my-index");
Set<String> principalValues = Set.of("username|admin", "role|user");

Map<Attribute, Set<String>> attributeMap = new HashMap<>();
attributeMap.put(indexPattern, indexValues);
attributeMap.put(principal, principalValues);

Rule rule = mock(Rule.class);
when(rule.getAttributeMap()).thenReturn(attributeMap);

Map<String, Set<String>> 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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Set<String>> 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());
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -67,7 +66,7 @@ public List<Route> routes() {

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
final Map<Attribute, Set<String>> attributeFilters = new HashMap<>();
final Map<String, Set<String>> attributeFilters = new HashMap<>();

if (!request.hasParam(FEATURE_TYPE)) {
throw new IllegalArgumentException("Invalid route.");
Expand All @@ -77,14 +76,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
final List<String> 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),
Expand Down
Loading