This is an automated email from the ASF dual-hosted git repository. sergehuber pushed a commit to branch UNOMI-1003-segment-scoring-list-sort in repository https://gitbox.apache.org/repos/asf/unomi.git
commit f6de8939d543e365db4cbc53f0c05a7007786c1f Author: Serge Huber <[email protected]> AuthorDate: Thu Oct 15 07:12:10 2026 +0200 UNOMI-1003: Sort merged segment/scoring lists and fix missing limit After system and tenant items are merged, apply an allowlisted sort and page the result. Treat a missing or negative limit as all, capped by the maximum page size, so POST query no longer fails when limit is unset or -1. --- .../java/org/apache/unomi/itests/SegmentIT.java | 78 ++++++++ .../impl/segments/MergedMetadataListHelper.java | 160 ++++++++++++++++ .../services/impl/segments/SegmentServiceImpl.java | 209 +++++---------------- .../segments/MergedMetadataListHelperTest.java | 155 +++++++++++++++ .../impl/segments/SegmentServiceImplTest.java | 178 ++++++++++++++++++ 5 files changed, 620 insertions(+), 160 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java b/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java index e4bffff64..53b31a0da 100644 --- a/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java @@ -17,8 +17,14 @@ package org.apache.unomi.itests; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.util.EntityUtils; import org.apache.unomi.api.Event; import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.PartialList; import org.apache.unomi.api.Profile; import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.exceptions.BadSegmentConditionException; @@ -793,4 +799,76 @@ public class SegmentIT extends BaseIT { updatedProfile.getScores() == null || !updatedProfile.getScores().containsKey("relative-date-scoring-test")), 1000, 20); } + + @Test + public void testQuerySegmentsWithNoLimitReturnsFullList() throws Exception { + Metadata segmentOneMetadata = new Metadata("query-no-limit-segment-1"); + segmentOneMetadata.setName("Query No Limit Segment 1"); + Segment segmentOne = new Segment(segmentOneMetadata); + Condition segmentOneCondition = new Condition(definitionsService.getConditionType("profilePropertyCondition")); + segmentOneCondition.setParameter("propertyName", "properties.email"); + segmentOneCondition.setParameter("comparisonOperator", "exists"); + segmentOne.setCondition(segmentOneCondition); + segmentService.setSegmentDefinition(segmentOne); + + Metadata segmentTwoMetadata = new Metadata("query-no-limit-segment-2"); + segmentTwoMetadata.setName("Query No Limit Segment 2"); + Segment segmentTwo = new Segment(segmentTwoMetadata); + Condition segmentTwoCondition = new Condition(definitionsService.getConditionType("profilePropertyCondition")); + segmentTwoCondition.setParameter("propertyName", "properties.firstName"); + segmentTwoCondition.setParameter("comparisonOperator", "exists"); + segmentTwo.setCondition(segmentTwoCondition); + segmentService.setSegmentDefinition(segmentTwo); + + keepTrying("Segments should be listed", + () -> segmentService.getSegmentMetadatas(0, 50, null).getList(), + list -> list.stream().anyMatch(m -> "query-no-limit-segment-1".equals(m.getId())) + && list.stream().anyMatch(m -> "query-no-limit-segment-2".equals(m.getId())), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + HttpPost request = new HttpPost(getFullUrl("/cxs/segments/query")); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + try (CloseableHttpResponse response = executeHttpRequest(request)) { + Assert.assertEquals("Segment query with no limit should return 200", 200, response.getStatusLine().getStatusCode()); + String body = EntityUtils.toString(response.getEntity()); + PartialList<?> result = getObjectMapper().readValue(body, PartialList.class); + Assert.assertNotNull(result); + Assert.assertNotNull(result.getList()); + Assert.assertTrue("Should return both created segments", result.getList().size() >= 2); + Assert.assertTrue("totalSize should cover the full list", result.getTotalSize() >= 2); + } + } + + @Test + public void testQueryScoringsWithNoLimitReturnsFullList() throws Exception { + Metadata scoringOneMetadata = new Metadata("query-no-limit-scoring-1"); + scoringOneMetadata.setName("Query No Limit Scoring 1"); + Scoring scoringOne = new Scoring(scoringOneMetadata); + scoringOne.setElements(new ArrayList<>()); + segmentService.setScoringDefinition(scoringOne); + + Metadata scoringTwoMetadata = new Metadata("query-no-limit-scoring-2"); + scoringTwoMetadata.setName("Query No Limit Scoring 2"); + Scoring scoringTwo = new Scoring(scoringTwoMetadata); + scoringTwo.setElements(new ArrayList<>()); + segmentService.setScoringDefinition(scoringTwo); + + keepTrying("Scorings should be listed", + () -> segmentService.getScoringMetadatas(0, 50, null).getList(), + list -> list.stream().anyMatch(m -> "query-no-limit-scoring-1".equals(m.getId())) + && list.stream().anyMatch(m -> "query-no-limit-scoring-2".equals(m.getId())), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + HttpPost request = new HttpPost(getFullUrl("/cxs/scoring/query")); + request.setEntity(new StringEntity("{}", ContentType.APPLICATION_JSON)); + try (CloseableHttpResponse response = executeHttpRequest(request)) { + Assert.assertEquals("Scoring query with no limit should return 200", 200, response.getStatusLine().getStatusCode()); + String body = EntityUtils.toString(response.getEntity()); + PartialList<?> result = getObjectMapper().readValue(body, PartialList.class); + Assert.assertNotNull(result); + Assert.assertNotNull(result.getList()); + Assert.assertTrue("Should return both created scorings", result.getList().size() >= 2); + Assert.assertTrue("totalSize should cover the full list", result.getTotalSize() >= 2); + } + } } diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelper.java b/services/src/main/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelper.java new file mode 100644 index 000000000..9d1a907a2 --- /dev/null +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelper.java @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.services.impl.segments; + +import org.apache.commons.beanutils.PropertyUtils; +import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.MetadataItem; +import org.apache.unomi.api.PartialList; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Sorts and pages merged system+tenant {@link MetadataItem} lists for segment and scoring queries. + * Only a fixed allowlist of sort fields is accepted. A missing or negative limit means "all", + * capped by {@link #DEFAULT_MAX_PAGE_SIZE}. + */ +final class MergedMetadataListHelper { + + static final int DEFAULT_MAX_PAGE_SIZE = 5000; + + private static final Map<String, String> ALLOWED_SORT_FIELDS = new HashMap<>(); + + static { + ALLOWED_SORT_FIELDS.put("id", "metadata.id"); + ALLOWED_SORT_FIELDS.put("metadata.id", "metadata.id"); + ALLOWED_SORT_FIELDS.put("itemId", "itemId"); + ALLOWED_SORT_FIELDS.put("name", "metadata.name"); + ALLOWED_SORT_FIELDS.put("metadata.name", "metadata.name"); + ALLOWED_SORT_FIELDS.put("scope", "metadata.scope"); + ALLOWED_SORT_FIELDS.put("metadata.scope", "metadata.scope"); + ALLOWED_SORT_FIELDS.put("description", "metadata.description"); + ALLOWED_SORT_FIELDS.put("metadata.description", "metadata.description"); + ALLOWED_SORT_FIELDS.put("enabled", "metadata.enabled"); + ALLOWED_SORT_FIELDS.put("metadata.enabled", "metadata.enabled"); + ALLOWED_SORT_FIELDS.put("creationDate", "creationDate"); + ALLOWED_SORT_FIELDS.put("lastModificationDate", "lastModificationDate"); + } + + private MergedMetadataListHelper() { + } + + static int resolveLimit(int limit) { + return resolveLimit(limit, DEFAULT_MAX_PAGE_SIZE); + } + + static int resolveLimit(int limit, int maxPageSize) { + if (limit < 0) { + return maxPageSize; + } + return Math.min(limit, maxPageSize); + } + + static <T extends MetadataItem> PartialList<Metadata> sortAndPage( + Collection<T> mergedItems, int offset, int limit, String sortBy) { + return sortAndPage(mergedItems, offset, limit, sortBy, DEFAULT_MAX_PAGE_SIZE); + } + + static <T extends MetadataItem> PartialList<Metadata> sortAndPage( + Collection<T> mergedItems, int offset, int limit, String sortBy, int maxPageSize) { + List<T> list = new ArrayList<>(mergedItems); + sort(list, sortBy); + + int pageSize = resolveLimit(limit, maxPageSize); + int totalSize = list.size(); + int fromIndex = offset; + if (fromIndex >= totalSize || pageSize == 0) { + return new PartialList<>(new ArrayList<>(), offset, pageSize, totalSize, PartialList.Relation.EQUAL); + } + int toIndex = fromIndex + pageSize; + if (toIndex > totalSize) { + toIndex = totalSize; + } + + List<Metadata> page = new ArrayList<>(toIndex - fromIndex); + for (T item : list.subList(fromIndex, toIndex)) { + page.add(item.getMetadata()); + } + return new PartialList<>(page, offset, pageSize, totalSize, PartialList.Relation.EQUAL); + } + + static void sort(List<? extends MetadataItem> items, String sortBy) { + if (sortBy == null || sortBy.trim().isEmpty()) { + return; + } + + String[] criteria = sortBy.split(","); + String[] propertyPaths = new String[criteria.length]; + boolean[] descending = new boolean[criteria.length]; + + for (int i = 0; i < criteria.length; i++) { + String criterion = criteria[i].trim(); + if (criterion.isEmpty()) { + throw new IllegalArgumentException("Empty sort field in: " + sortBy); + } + String[] parts = criterion.split(":"); + String field = parts[0].trim(); + String propertyPath = ALLOWED_SORT_FIELDS.get(field); + if (propertyPath == null) { + throw new IllegalArgumentException("Unsupported sort field: " + field); + } + propertyPaths[i] = propertyPath; + descending[i] = parts.length > 1 && "desc".equalsIgnoreCase(parts[1].trim()); + } + + items.sort((left, right) -> { + for (int i = 0; i < propertyPaths.length; i++) { + int comparison = compareProperties(left, right, propertyPaths[i]); + if (comparison != 0) { + return descending[i] ? -comparison : comparison; + } + } + return 0; + }); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private static int compareProperties(MetadataItem left, MetadataItem right, String propertyPath) { + Object leftValue = getProperty(left, propertyPath); + Object rightValue = getProperty(right, propertyPath); + if (leftValue == null && rightValue == null) { + return 0; + } + if (leftValue == null) { + return -1; + } + if (rightValue == null) { + return 1; + } + if (!(leftValue instanceof Comparable) || !(rightValue instanceof Comparable)) { + return leftValue.toString().compareTo(rightValue.toString()); + } + return ((Comparable) leftValue).compareTo(rightValue); + } + + private static Object getProperty(MetadataItem item, String propertyPath) { + try { + return PropertyUtils.getProperty(item, propertyPath); + } catch (Exception e) { + return null; + } + } +} diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java index ed1b71911..c0501a65c 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java @@ -279,91 +279,49 @@ public class SegmentServiceImpl extends AbstractMultiTypeCachingService implemen public PartialList<Metadata> getSegmentMetadatas(String scope, int offset, int size, String sortBy) { String currentTenantId = contextManager.getCurrentContext().getTenantId(); - List<Metadata> details = new LinkedList<>(); + Map<String, Segment> merged = new HashMap<>(); // Get system tenant segments first if (!TenantService.SYSTEM_TENANT.equals(currentTenantId)) { contextManager.executeAsSystem(() -> { - Condition systemTenantCondition = new Condition(definitionsService.getConditionType("booleanCondition")); - systemTenantCondition.setParameter("operator", "and"); - List<Condition> systemConditions = new ArrayList<>(); - - Condition systemTenantCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - systemTenantCheck.setParameter("propertyName", "tenantId"); - systemTenantCheck.setParameter("comparisonOperator", "equals"); - systemTenantCheck.setParameter("propertyValue", TenantService.SYSTEM_TENANT); - systemConditions.add(systemTenantCheck); - - if (scope != null) { - Condition systemScopeCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - systemScopeCheck.setParameter("propertyName", "metadata.scope"); - systemScopeCheck.setParameter("comparisonOperator", "equals"); - systemScopeCheck.setParameter("propertyValue", scope); - systemConditions.add(systemScopeCheck); - } - - systemTenantCondition.setParameter("subConditions", systemConditions); - - PartialList<Segment> systemSegments = persistenceService.query(systemTenantCondition, sortBy, Segment.class, 0, -1); + Condition systemTenantCondition = buildTenantScopeCondition(TenantService.SYSTEM_TENANT, scope); + PartialList<Segment> systemSegments = persistenceService.query(systemTenantCondition, null, Segment.class, 0, -1); for (Segment definition : systemSegments.getList()) { - details.add(definition.getMetadata()); + merged.put(definition.getMetadata().getId(), definition); } return null; }); } // Get current tenant segments (will override system segments with same ID) - Condition tenantCondition = new Condition(definitionsService.getConditionType("booleanCondition")); - tenantCondition.setParameter("operator", "and"); - List<Condition> conditions = new ArrayList<>(); + Condition tenantCondition = buildTenantScopeCondition(currentTenantId, scope); + PartialList<Segment> segments = persistenceService.query(tenantCondition, null, Segment.class, 0, -1); + for (Segment definition : segments.getList()) { + merged.put(definition.getMetadata().getId(), definition); + } + return MergedMetadataListHelper.sortAndPage(merged.values(), offset, size, sortBy); + } + + private Condition buildTenantScopeCondition(String tenantId, String scope) { Condition tenantCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); tenantCheck.setParameter("propertyName", "tenantId"); tenantCheck.setParameter("comparisonOperator", "equals"); - tenantCheck.setParameter("propertyValue", currentTenantId); - conditions.add(tenantCheck); + tenantCheck.setParameter("propertyValue", tenantId); - if (scope != null) { - Condition scopeCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - scopeCheck.setParameter("propertyName", "metadata.scope"); - scopeCheck.setParameter("comparisonOperator", "equals"); - scopeCheck.setParameter("propertyValue", scope); - conditions.add(scopeCheck); + if (scope == null) { + return tenantCheck; } - tenantCondition.setParameter("subConditions", conditions); - - PartialList<Segment> segments = persistenceService.query(tenantCondition, sortBy, Segment.class, 0, -1); - Map<String, Metadata> mergedDetails = new HashMap<>(); + Condition scopeCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); + scopeCheck.setParameter("propertyName", "metadata.scope"); + scopeCheck.setParameter("comparisonOperator", "equals"); + scopeCheck.setParameter("propertyValue", scope); - // Add system tenant segments first - for (Metadata metadata : details) { - mergedDetails.put(metadata.getId(), metadata); - } - - // Override with current tenant segments - for (Segment definition : segments.getList()) { - mergedDetails.put(definition.getMetadata().getId(), definition.getMetadata()); - } - - // Convert to list and apply pagination - List<Metadata> finalDetails = new ArrayList<>(mergedDetails.values()); - if (sortBy != null) { - // TODO: Implement sorting of merged results - } - - int totalSize = finalDetails.size(); - int fromIndex = offset; - int toIndex = offset + size; - if (fromIndex >= totalSize) { - return new PartialList<Metadata>(new ArrayList<>(), offset, size, totalSize, PartialList.Relation.EQUAL); - } - if (toIndex > totalSize) { - toIndex = totalSize; - } - finalDetails = finalDetails.subList(fromIndex, toIndex); - - return new PartialList<Metadata>(finalDetails, offset, size, totalSize, PartialList.Relation.EQUAL); + Condition combined = new Condition(definitionsService.getConditionType("booleanCondition")); + combined.setParameter("operator", "and"); + combined.setParameter("subConditions", Arrays.asList(tenantCheck, scopeCheck)); + return combined; } public PartialList<Metadata> getSegmentMetadatas(Query query) { @@ -1598,59 +1556,28 @@ public class SegmentServiceImpl extends AbstractMultiTypeCachingService implemen protected <T extends MetadataItem> PartialList<Metadata> getMetadatas(int offset, int size, String sortBy, Class<T> clazz) { String currentTenantId = contextManager.getCurrentContext().getTenantId(); - List<Metadata> details = new LinkedList<>(); + Map<String, T> merged = new HashMap<>(); // Get system tenant items first if (!TenantService.SYSTEM_TENANT.equals(currentTenantId)) { contextManager.executeAsSystem(() -> { - Condition systemTenantCondition = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - systemTenantCondition.setParameter("propertyName", "tenantId"); - systemTenantCondition.setParameter("comparisonOperator", "equals"); - systemTenantCondition.setParameter("propertyValue", TenantService.SYSTEM_TENANT); - PartialList<T> systemItems = persistenceService.query(systemTenantCondition, sortBy, clazz, 0, -1); + Condition systemTenantCondition = buildTenantScopeCondition(TenantService.SYSTEM_TENANT, null); + PartialList<T> systemItems = persistenceService.query(systemTenantCondition, null, clazz, 0, -1); for (T definition : systemItems.getList()) { - details.add(definition.getMetadata()); + merged.put(definition.getMetadata().getId(), definition); } return null; }); } // Get current tenant items (will override system items with same ID) - Condition tenantCondition = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - tenantCondition.setParameter("propertyName", "tenantId"); - tenantCondition.setParameter("comparisonOperator", "equals"); - tenantCondition.setParameter("propertyValue", currentTenantId); - PartialList<T> items = persistenceService.query(tenantCondition, sortBy, clazz, 0, -1); - Map<String, Metadata> mergedDetails = new HashMap<>(); - - // Add system tenant items first - for (Metadata metadata : details) { - mergedDetails.put(metadata.getId(), metadata); - } - - // Override with current tenant items + Condition tenantCondition = buildTenantScopeCondition(currentTenantId, null); + PartialList<T> items = persistenceService.query(tenantCondition, null, clazz, 0, -1); for (T definition : items.getList()) { - mergedDetails.put(definition.getMetadata().getId(), definition.getMetadata()); + merged.put(definition.getMetadata().getId(), definition); } - // Convert to list and apply pagination - List<Metadata> finalDetails = new ArrayList<>(mergedDetails.values()); - if (sortBy != null) { - // TODO: Implement sorting of merged results - } - - int totalSize = finalDetails.size(); - int fromIndex = offset; - int toIndex = offset + size; - if (fromIndex >= totalSize) { - return new PartialList<Metadata>(new ArrayList<>(), offset, size, totalSize, PartialList.Relation.EQUAL); - } - if (toIndex > totalSize) { - toIndex = totalSize; - } - finalDetails = finalDetails.subList(fromIndex, toIndex); - - return new PartialList<Metadata>(finalDetails, offset, size, totalSize, PartialList.Relation.EQUAL); + return MergedMetadataListHelper.sortAndPage(merged.values(), offset, size, sortBy); } protected <T extends MetadataItem> PartialList<Metadata> getMetadatas(Query query, Class<T> clazz) { @@ -1663,77 +1590,39 @@ public class SegmentServiceImpl extends AbstractMultiTypeCachingService implemen return new PartialList<>(); } - List<Metadata> details = new LinkedList<>(); + Map<String, T> merged = new HashMap<>(); // Get system tenant items first if (!TenantService.SYSTEM_TENANT.equals(currentTenantId)) { contextManager.executeAsSystem(() -> { - Condition systemTenantCondition = new Condition(definitionsService.getConditionType("booleanCondition")); - systemTenantCondition.setParameter("operator", "and"); - List<Condition> systemConditions = new ArrayList<>(); - - Condition systemTenantCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - systemTenantCheck.setParameter("propertyName", "tenantId"); - systemTenantCheck.setParameter("comparisonOperator", "equals"); - systemTenantCheck.setParameter("propertyValue", TenantService.SYSTEM_TENANT); - systemConditions.add(systemTenantCheck); - - systemConditions.add(query.getCondition()); - systemTenantCondition.setParameter("subConditions", systemConditions); - - PartialList<T> systemItems = persistenceService.query(systemTenantCondition, query.getSortby(), clazz, 0, -1); + Condition systemTenantCondition = buildTenantQueryCondition(TenantService.SYSTEM_TENANT, query.getCondition()); + PartialList<T> systemItems = persistenceService.query(systemTenantCondition, null, clazz, 0, -1); for (T definition : systemItems.getList()) { - details.add(definition.getMetadata()); + merged.put(definition.getMetadata().getId(), definition); } return null; }); } // Get current tenant items (will override system items with same ID) - Condition tenantCondition = new Condition(definitionsService.getConditionType("booleanCondition")); - tenantCondition.setParameter("operator", "and"); - List<Condition> conditions = new ArrayList<>(); - - Condition tenantCheck = new Condition(definitionsService.getConditionType("sessionPropertyCondition")); - tenantCheck.setParameter("propertyName", "tenantId"); - tenantCheck.setParameter("comparisonOperator", "equals"); - tenantCheck.setParameter("propertyValue", currentTenantId); - conditions.add(tenantCheck); - - conditions.add(query.getCondition()); - tenantCondition.setParameter("subConditions", conditions); - - PartialList<T> items = persistenceService.query(tenantCondition, query.getSortby(), clazz, 0, -1); - Map<String, Metadata> mergedDetails = new HashMap<>(); - - // Add system tenant items first - for (Metadata metadata : details) { - mergedDetails.put(metadata.getId(), metadata); - } - - // Override with current tenant items + Condition tenantCondition = buildTenantQueryCondition(currentTenantId, query.getCondition()); + PartialList<T> items = persistenceService.query(tenantCondition, null, clazz, 0, -1); for (T definition : items.getList()) { - mergedDetails.put(definition.getMetadata().getId(), definition.getMetadata()); + merged.put(definition.getMetadata().getId(), definition); } - // Convert to list and apply pagination - List<Metadata> finalDetails = new ArrayList<>(mergedDetails.values()); - if (query.getSortby() != null) { - // TODO: Implement sorting of merged results - } + return MergedMetadataListHelper.sortAndPage(merged.values(), query.getOffset(), query.getLimit(), query.getSortby()); + } - int totalSize = finalDetails.size(); - int fromIndex = query.getOffset(); - int toIndex = fromIndex + query.getLimit(); - if (fromIndex >= totalSize) { - return new PartialList<Metadata>(new ArrayList<>(), query.getOffset(), query.getLimit(), totalSize, PartialList.Relation.EQUAL); + private Condition buildTenantQueryCondition(String tenantId, Condition queryCondition) { + Condition tenantCheck = buildTenantScopeCondition(tenantId, null); + if (queryCondition == null) { + return tenantCheck; } - if (toIndex > totalSize) { - toIndex = totalSize; - } - finalDetails = finalDetails.subList(fromIndex, toIndex); - - return new PartialList<Metadata>(finalDetails, query.getOffset(), query.getLimit(), totalSize, PartialList.Relation.EQUAL); + Condition combined = new Condition(definitionsService.getConditionType("booleanCondition")); + combined.setParameter("operator", "and"); + combined.setParameter("subConditions", Arrays.asList(tenantCheck, queryCondition)); + return combined; } diff --git a/services/src/test/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelperTest.java b/services/src/test/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelperTest.java new file mode 100644 index 000000000..c639d3cbe --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/impl/segments/MergedMetadataListHelperTest.java @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.services.impl.segments; + +import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.segments.Segment; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.*; + +public class MergedMetadataListHelperTest { + + @Test + public void testResolveLimitTreatsNegativeAsAllCapped() { + assertEquals(MergedMetadataListHelper.DEFAULT_MAX_PAGE_SIZE, MergedMetadataListHelper.resolveLimit(-1)); + assertEquals(MergedMetadataListHelper.DEFAULT_MAX_PAGE_SIZE, MergedMetadataListHelper.resolveLimit(Integer.MIN_VALUE)); + assertEquals(3, MergedMetadataListHelper.resolveLimit(-1, 3)); + assertEquals(0, MergedMetadataListHelper.resolveLimit(0)); + assertEquals(10, MergedMetadataListHelper.resolveLimit(10)); + assertEquals(5, MergedMetadataListHelper.resolveLimit(100, 5)); + } + + @Test + public void testSortByNameAscendingAndDescending() { + List<Segment> items = Arrays.asList( + segment("c", "Charlie", daysAgo(3)), + segment("a", "Alpha", daysAgo(1)), + segment("b", "Bravo", daysAgo(2)) + ); + + MergedMetadataListHelper.sort(items, "name:asc"); + assertEquals(Arrays.asList("a", "b", "c"), ids(items)); + + MergedMetadataListHelper.sort(items, "metadata.name:desc"); + assertEquals(Arrays.asList("c", "b", "a"), ids(items)); + } + + @Test + public void testSortByCreationDateAscendingAndDescending() { + List<Segment> items = Arrays.asList( + segment("c", "Charlie", daysAgo(3)), + segment("a", "Alpha", daysAgo(1)), + segment("b", "Bravo", daysAgo(2)) + ); + + MergedMetadataListHelper.sort(items, "creationDate:asc"); + assertEquals(Arrays.asList("c", "b", "a"), ids(items)); + + MergedMetadataListHelper.sort(items, "creationDate:desc"); + assertEquals(Arrays.asList("a", "b", "c"), ids(items)); + } + + @Test + public void testUnknownSortFieldIsRefused() { + List<Segment> items = Arrays.asList(segment("a", "Alpha", daysAgo(1))); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> MergedMetadataListHelper.sort(items, "unknownField:asc")); + assertTrue(exception.getMessage().contains("unknownField")); + } + + @Test + public void testPagingVariantsAndMaxCap() { + List<Segment> items = Arrays.asList( + segment("a", "Alpha", daysAgo(1)), + segment("b", "Bravo", daysAgo(2)), + segment("c", "Charlie", daysAgo(3)), + segment("d", "Delta", daysAgo(4)), + segment("e", "Echo", daysAgo(5)) + ); + + PartialList<Metadata> noLimit = MergedMetadataListHelper.sortAndPage(items, 0, Integer.MIN_VALUE, "name:asc", 3); + assertEquals(3, noLimit.getList().size()); + assertEquals(5, noLimit.getTotalSize()); + assertEquals(Arrays.asList("a", "b", "c"), metadataIds(noLimit)); + + PartialList<Metadata> negativeOne = MergedMetadataListHelper.sortAndPage(items, 0, -1, "name:asc", 4); + assertEquals(4, negativeOne.getList().size()); + assertEquals(Arrays.asList("a", "b", "c", "d"), metadataIds(negativeOne)); + + PartialList<Metadata> zero = MergedMetadataListHelper.sortAndPage(items, 0, 0, "name:asc"); + assertEquals(0, zero.getList().size()); + assertEquals(5, zero.getTotalSize()); + + PartialList<Metadata> largerThanTotal = MergedMetadataListHelper.sortAndPage(items, 0, 100, "name:asc"); + assertEquals(5, largerThanTotal.getList().size()); + + PartialList<Metadata> pastEnd = MergedMetadataListHelper.sortAndPage(items, 10, 2, "name:asc"); + assertEquals(0, pastEnd.getList().size()); + assertEquals(5, pastEnd.getTotalSize()); + } + + @Test + public void testPagingThroughAllPagesReturnsEachItemOnce() { + List<Segment> items = Arrays.asList( + segment("a", "Alpha", daysAgo(1)), + segment("b", "Bravo", daysAgo(2)), + segment("c", "Charlie", daysAgo(3)), + segment("d", "Delta", daysAgo(4)), + segment("e", "Echo", daysAgo(5)) + ); + + List<String> collected = new java.util.ArrayList<>(); + int pageSize = 2; + for (int offset = 0; offset < items.size(); offset += pageSize) { + PartialList<Metadata> page = MergedMetadataListHelper.sortAndPage(items, offset, pageSize, "name:asc"); + collected.addAll(metadataIds(page)); + } + assertEquals(Arrays.asList("a", "b", "c", "d", "e"), collected); + } + + private static Segment segment(String id, String name, Date creationDate) { + Segment segment = new Segment(); + segment.setItemId(id); + Metadata metadata = new Metadata(); + metadata.setId(id); + metadata.setName(name); + metadata.setScope("systemscope"); + segment.setMetadata(metadata); + segment.setCreationDate(creationDate); + segment.setLastModificationDate(creationDate); + return segment; + } + + private static Date daysAgo(int days) { + return new Date(System.currentTimeMillis() - days * 24L * 60L * 60L * 1000L); + } + + private static List<String> ids(List<Segment> items) { + return items.stream().map(Segment::getItemId).collect(Collectors.toList()); + } + + private static List<String> metadataIds(PartialList<Metadata> page) { + return page.getList().stream().map(Metadata::getId).collect(Collectors.toList()); + } +} diff --git a/services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java index c7564fa9d..fa03fa8ae 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/segments/SegmentServiceImplTest.java @@ -24,7 +24,9 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.conditions.ConditionType; import org.apache.unomi.api.exceptions.BadSegmentConditionException; +import org.apache.unomi.api.query.Query; import org.apache.unomi.api.rules.Rule; +import org.apache.unomi.api.segments.Scoring; import org.apache.unomi.api.segments.Segment; import org.apache.unomi.api.segments.SegmentsAndScores; import org.apache.unomi.api.services.EventService; @@ -239,6 +241,33 @@ public class SegmentServiceImplTest { return segment; } + private Scoring createTestScoring(String scoringId, String name) { + Scoring scoring = new Scoring(); + scoring.setItemId(scoringId); + scoring.setTenantId(executionContextManager.getCurrentContext().getTenantId()); + + Metadata metadata = new Metadata(); + metadata.setId(scoringId); + metadata.setName(name); + metadata.setScope("systemscope"); + metadata.setEnabled(false); + scoring.setMetadata(metadata); + scoring.setElements(Collections.emptyList()); + return scoring; + } + + private Date daysAgo(int days) { + return new Date(System.currentTimeMillis() - days * 24L * 60L * 60L * 1000L); + } + + private List<String> metadataIds(PartialList<Metadata> page) { + List<String> ids = new ArrayList<>(); + for (Metadata metadata : page.getList()) { + ids.add(metadata.getId()); + } + return ids; + } + private Segment createPastEventSegment(String segmentId, String name, String eventType, int numberOfDays) { Segment segment = new Segment(); segment.setItemId(segmentId); @@ -1160,4 +1189,153 @@ public class SegmentServiceImplTest { return null; }); } + + @Test + public void testMergedSegmentListSortByNameAndDate() { + executionContextManager.executeAsSystem(() -> { + Segment zebra = createTestSegment("sys-zebra", "Zebra"); + zebra.setCreationDate(daysAgo(5)); + Segment middle = createTestSegment("sys-middle", "Middle"); + middle.setCreationDate(daysAgo(3)); + segmentService.setSegmentDefinition(zebra); + segmentService.setSegmentDefinition(middle); + return null; + }); + + executionContextManager.executeAsTenant(TENANT_1, () -> { + Segment alpha = createTestSegment("tenant-alpha", "Alpha"); + alpha.setCreationDate(daysAgo(1)); + segmentService.setSegmentDefinition(alpha); + persistenceService.refresh(); + + PartialList<Metadata> byNameAsc = TestHelper.retryQueryUntilAvailable( + () -> segmentService.getSegmentMetadatas(0, 10, "name:asc"), + 3 + ); + assertEquals(Arrays.asList("tenant-alpha", "sys-middle", "sys-zebra"), metadataIds(byNameAsc)); + + PartialList<Metadata> byNameDesc = segmentService.getSegmentMetadatas(0, 10, "name:desc"); + assertEquals(Arrays.asList("sys-zebra", "sys-middle", "tenant-alpha"), metadataIds(byNameDesc)); + + PartialList<Metadata> byDateAsc = segmentService.getSegmentMetadatas(0, 10, "creationDate:asc"); + assertEquals(Arrays.asList("sys-zebra", "sys-middle", "tenant-alpha"), metadataIds(byDateAsc)); + + PartialList<Metadata> byDateDesc = segmentService.getSegmentMetadatas(0, 10, "creationDate:desc"); + assertEquals(Arrays.asList("tenant-alpha", "sys-middle", "sys-zebra"), metadataIds(byDateDesc)); + return null; + }); + } + + @Test + public void testMergedSegmentListPagingAndQueryLimits() { + executionContextManager.executeAsSystem(() -> { + segmentService.setSegmentDefinition(createTestSegment("sys-a", "A-System")); + segmentService.setSegmentDefinition(createTestSegment("sys-b", "B-System")); + return null; + }); + + executionContextManager.executeAsTenant(TENANT_1, () -> { + segmentService.setSegmentDefinition(createTestSegment("ten-c", "C-Tenant")); + segmentService.setSegmentDefinition(createTestSegment("ten-d", "D-Tenant")); + segmentService.setSegmentDefinition(createTestSegment("ten-e", "E-Tenant")); + persistenceService.refresh(); + + TestHelper.retryQueryUntilAvailable(() -> segmentService.getSegmentMetadatas(0, 10, "name:asc"), 5); + + PartialList<Metadata> page1 = segmentService.getSegmentMetadatas(0, 2, "name:asc"); + PartialList<Metadata> page2 = segmentService.getSegmentMetadatas(2, 2, "name:asc"); + PartialList<Metadata> page3 = segmentService.getSegmentMetadatas(4, 2, "name:asc"); + List<String> allPages = new ArrayList<>(); + allPages.addAll(metadataIds(page1)); + allPages.addAll(metadataIds(page2)); + allPages.addAll(metadataIds(page3)); + assertEquals(Arrays.asList("sys-a", "sys-b", "ten-c", "ten-d", "ten-e"), allPages); + assertEquals(5, page1.getTotalSize()); + + Query noLimit = new Query(); + PartialList<Metadata> noLimitResult = segmentService.getSegmentMetadatas(noLimit); + assertEquals(5, noLimitResult.getList().size()); + assertEquals(5, noLimitResult.getTotalSize()); + + Query limitMinusOne = new Query(); + limitMinusOne.setLimit(-1); + assertEquals(5, segmentService.getSegmentMetadatas(limitMinusOne).getList().size()); + + Query limitZero = new Query(); + limitZero.setLimit(0); + assertEquals(0, segmentService.getSegmentMetadatas(limitZero).getList().size()); + + Query largerThanTotal = new Query(); + largerThanTotal.setLimit(100); + largerThanTotal.setSortby("name:asc"); + assertEquals(5, segmentService.getSegmentMetadatas(largerThanTotal).getList().size()); + + Query pastEnd = new Query(); + pastEnd.setOffset(20); + pastEnd.setLimit(5); + assertEquals(0, segmentService.getSegmentMetadatas(pastEnd).getList().size()); + assertEquals(5, segmentService.getSegmentMetadatas(pastEnd).getTotalSize()); + + assertThrows(IllegalArgumentException.class, + () -> segmentService.getSegmentMetadatas(0, 10, "notARealField:asc")); + return null; + }); + } + + @Test + public void testMergedScoringListSortPagingAndQueryLimits() { + executionContextManager.executeAsSystem(() -> { + Scoring late = createTestScoring("sys-late", "Late"); + late.setCreationDate(daysAgo(4)); + Scoring early = createTestScoring("sys-early", "Early"); + early.setCreationDate(daysAgo(1)); + segmentService.setScoringDefinition(late); + segmentService.setScoringDefinition(early); + return null; + }); + + executionContextManager.executeAsTenant(TENANT_1, () -> { + Scoring mid = createTestScoring("ten-mid", "Mid"); + mid.setCreationDate(daysAgo(2)); + segmentService.setScoringDefinition(mid); + persistenceService.refresh(); + + PartialList<Metadata> byNameAsc = TestHelper.retryQueryUntilAvailable( + () -> segmentService.getScoringMetadatas(0, 10, "name:asc"), + 3 + ); + assertEquals(Arrays.asList("sys-early", "sys-late", "ten-mid"), metadataIds(byNameAsc)); + + PartialList<Metadata> byDateDesc = segmentService.getScoringMetadatas(0, 10, "creationDate:desc"); + assertEquals(Arrays.asList("sys-early", "ten-mid", "sys-late"), metadataIds(byDateDesc)); + + PartialList<Metadata> page1 = segmentService.getScoringMetadatas(0, 2, "name:asc"); + PartialList<Metadata> page2 = segmentService.getScoringMetadatas(2, 2, "name:asc"); + List<String> allPages = new ArrayList<>(); + allPages.addAll(metadataIds(page1)); + allPages.addAll(metadataIds(page2)); + assertEquals(Arrays.asList("sys-early", "sys-late", "ten-mid"), allPages); + + Query noLimit = new Query(); + assertEquals(3, segmentService.getScoringMetadatas(noLimit).getList().size()); + + Query limitMinusOne = new Query(); + limitMinusOne.setLimit(-1); + assertEquals(3, segmentService.getScoringMetadatas(limitMinusOne).getList().size()); + + Query limitZero = new Query(); + limitZero.setLimit(0); + assertEquals(0, segmentService.getScoringMetadatas(limitZero).getList().size()); + + Query pastEnd = new Query(); + pastEnd.setOffset(50); + pastEnd.setLimit(10); + assertEquals(0, segmentService.getScoringMetadatas(pastEnd).getList().size()); + + assertThrows(IllegalArgumentException.class, + () -> segmentService.getScoringMetadatas(0, 10, "properties.score:asc")); + return null; + }); + } } +
