From 3ce73e78198edb511e0313c1b9f2f84cdcb106a0 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Tue, 5 May 2026 17:17:56 +0200 Subject: [PATCH 01/16] UNOMI-920 Improve toString() methods with YAML formatting for debugging - Add YamlUtils (SnakeYaml) with YamlConvertible, YamlMapBuilder, circular reference detection (identity-based visited sets), and recursion depth limits. - Implement YAML-backed toString()/toYaml() on core API types extending Item / MetadataItem (Condition, ConditionType, Action, ActionType, Rule, Segment, Goal, Scoring, ScoringElement, Parameter, Metadata, etc.). - Add YamlUtilsTest coverage for builder, toYamlValue, and representative rules. Build and integration alignment: - unomi-api: snakeyaml + test dependencies; manage mockito-core version in BOM. - RESTParameter: use Object for defaultValue to match Parameter#getDefaultValue(). - RulesServiceImpl: avoid NPE when tracked parameter defaultValue is null before split. - itests: override awaitility to 3.1.6 for OSGi (Hamcrest 1.3 bundle); Karaf itests common logback exclusions; hamcrest bundle scope test. --- api/pom.xml | 22 + .../main/java/org/apache/unomi/api/Item.java | 62 +- .../java/org/apache/unomi/api/Metadata.java | 46 +- .../org/apache/unomi/api/MetadataItem.java | 57 +- .../java/org/apache/unomi/api/Parameter.java | 64 +- .../org/apache/unomi/api/actions/Action.java | 48 +- .../apache/unomi/api/actions/ActionType.java | 40 +- .../unomi/api/conditions/Condition.java | 113 +++- .../unomi/api/conditions/ConditionType.java | 43 +- .../java/org/apache/unomi/api/goals/Goal.java | 44 +- .../java/org/apache/unomi/api/rules/Rule.java | 46 +- .../apache/unomi/api/segments/Scoring.java | 38 +- .../unomi/api/segments/ScoringElement.java | 47 +- .../apache/unomi/api/segments/Segment.java | 40 +- .../org/apache/unomi/api/utils/YamlUtils.java | 319 +++++++++ .../apache/unomi/api/utils/YamlUtilsTest.java | 610 ++++++++++++++++++ bom/pom.xml | 20 + itests/pom.xml | 22 +- pom.xml | 4 + .../unomi/rest/models/RESTParameter.java | 6 +- .../services/impl/rules/RulesServiceImpl.java | 10 +- 21 files changed, 1656 insertions(+), 45 deletions(-) create mode 100644 api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java create mode 100644 api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java diff --git a/api/pom.xml b/api/pom.xml index af8c96220..4d9ed7fbd 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -73,6 +73,28 @@ jackson-databind provided + + org.yaml + snakeyaml + + + + + junit + junit + test + + + org.mockito + mockito-core + test + + + + org.slf4j + slf4j-simple + test + diff --git a/api/src/main/java/org/apache/unomi/api/Item.java b/api/src/main/java/org/apache/unomi/api/Item.java index de283ebe9..9836cf40d 100644 --- a/api/src/main/java/org/apache/unomi/api/Item.java +++ b/api/src/main/java/org/apache/unomi/api/Item.java @@ -17,14 +17,18 @@ package org.apache.unomi.api; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.Serializable; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; + /** * A context server tracked entity. All tracked entities need to extend this class so as to provide the minimal information the context server needs to be able to track such * entities and operate on them. Items are persisted according to their type (structure) and identifier (identity). Of note, all Item subclasses must define a @@ -36,10 +40,10 @@ * though scopes could span across sites depending on the desired analysis granularity). Scopes allow clients accessing the context server to filter data. The context server * defines a built-in scope ({@link Metadata#SYSTEM_SCOPE}) that clients can use to share data across scopes. */ -public abstract class Item implements Serializable { +public abstract class Item implements Serializable, YamlConvertible { private static final Logger LOGGER = LoggerFactory.getLogger(Item.class.getName()); - private static final long serialVersionUID = 7446061538573517071L; + private static final long serialVersionUID = 1217180125083162915L; private static final Map itemTypeCache = new ConcurrentHashMap<>(); @@ -150,4 +154,54 @@ public Object getSystemMetadata(String key) { public void setSystemMetadata(String key, Object value) { systemMetadata.put(key, value); } + + /** + * Converts this item to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this item + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("itemId", itemId) + .put("itemType", itemType) + .put("systemMetadata", "") + .build(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + // Check if already visited - if so, we're being called from a child class via super.toYaml() + // OR it's a real circular reference. We can't distinguish, but since child classes + // (like Rule, ConditionType, etc.) all check for circular refs before calling super, + // if we're already visited here, it's safe to assume it's a super call, not a circular ref. + // If Item is directly serialized and encounters itself, the check would happen at the + // top level before nested processing, so this should be safe. + boolean alreadyVisited = visitedSet.contains(this); + if (!alreadyVisited) { + // First time seeing this object - add it to track for circular references + visitedSet.add(this); + } + try { + return YamlMapBuilder.create() + .put("itemId", itemId) // Always include, even if null, to reflect actual state + .put("itemType", itemType) // Always include, even if null, to reflect actual state + .putIfNotNull("scope", scope) + .putIfNotNull("version", version) + .putIfNotNull("systemMetadata", systemMetadata != null && !systemMetadata.isEmpty() ? toYamlValue(systemMetadata, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + // Only remove if we added it (i.e., if it wasn't already visited) + if (!alreadyVisited) { + visitedSet.remove(this); + } + } + } + + @Override + public String toString() { + Map map = toYaml(); + return YamlUtils.format(map); + } } diff --git a/api/src/main/java/org/apache/unomi/api/Metadata.java b/api/src/main/java/org/apache/unomi/api/Metadata.java index 9a112e766..ec25c7fa8 100644 --- a/api/src/main/java/org/apache/unomi/api/Metadata.java +++ b/api/src/main/java/org/apache/unomi/api/Metadata.java @@ -17,16 +17,24 @@ package org.apache.unomi.api; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; + import java.io.Serializable; +import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; +import static org.apache.unomi.api.utils.YamlUtils.circularRef; + /** * A class providing information about context server entities. * * @see MetadataItem */ -public class Metadata implements Comparable, Serializable { +public class Metadata implements Comparable, Serializable, YamlConvertible { private static final long serialVersionUID = 7446061538573517071L; @@ -279,5 +287,41 @@ public int hashCode() { return result; } + /** + * Converts this metadata to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this metadata + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .putIfNotNull("id", id) + .putIfNotNull("name", name) + .putIfNotNull("description", description) + .putIfNotNull("scope", scope) + .putIfNotEmpty("tags", tags) + .putIfNotEmpty("systemTags", systemTags) + .putIf("enabled", true, enabled) + .putIf("missingPlugins", true, missingPlugins) + .putIf("hidden", true, hidden) + .putIf("readOnly", true, readOnly) + .build(); + } finally { + visitedSet.remove(this); + } + } + @Override + public String toString() { + Map map = toYaml(); + return YamlUtils.format(map); + } } diff --git a/api/src/main/java/org/apache/unomi/api/MetadataItem.java b/api/src/main/java/org/apache/unomi/api/MetadataItem.java index 78a419863..35ef596a2 100644 --- a/api/src/main/java/org/apache/unomi/api/MetadataItem.java +++ b/api/src/main/java/org/apache/unomi/api/MetadataItem.java @@ -17,8 +17,16 @@ package org.apache.unomi.api; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; + import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlTransient; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A superclass for all {@link Item}s that bear {@link Metadata}. @@ -31,7 +39,7 @@ public MetadataItem() { } public MetadataItem(Metadata metadata) { - super(metadata.getId()); + super(metadata != null ? metadata.getId() : null); this.metadata = metadata; } @@ -54,7 +62,52 @@ public void setMetadata(Metadata metadata) { @XmlTransient public String getScope() { - return metadata.getScope(); + if (metadata != null) { + return metadata.getScope(); + } + return scope; + } + + /** + * Converts this metadata item to a Map structure for YAML output. + * Merges fields from Item parent class and adds metadata field. + * Subclasses should override this method, call super.toYaml(visited), and add their specific fields. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this metadata item + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("metadata", "") + .build(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + // Check if already visited - if so, we're being called from a child class via super.toYaml() + // In that case, skip the circular reference check and just proceed + boolean alreadyVisited = visitedSet.contains(this); + if (!alreadyVisited) { + // Only check for circular references if this is the first time we're seeing this object + visitedSet.add(this); + } + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("metadata", metadata != null ? toYamlValue(metadata, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + // Only remove if we added it (i.e., if it wasn't already visited) + if (!alreadyVisited) { + visitedSet.remove(this); + } + } } + + @Override + public String toString() { + Map map = toYaml(); + return YamlUtils.format(map); + } } diff --git a/api/src/main/java/org/apache/unomi/api/Parameter.java b/api/src/main/java/org/apache/unomi/api/Parameter.java index 4833c5a5f..425d86280 100644 --- a/api/src/main/java/org/apache/unomi/api/Parameter.java +++ b/api/src/main/java/org/apache/unomi/api/Parameter.java @@ -17,20 +17,27 @@ package org.apache.unomi.api; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; + import java.io.Serializable; +import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A representation of a condition parameter, to be used in the segment building UI to either select parameters from a * choicelist or to enter a specific value. */ -public class Parameter implements Serializable { +public class Parameter implements Serializable, YamlConvertible { - private static final long serialVersionUID = 7446061538573517071L; + private static final long serialVersionUID = 6019392686888941547L; - String id; - String type; - boolean multivalued = false; - String defaultValue = null; + private String id; + private String type; + private boolean multivalued; + private Object defaultValue; public Parameter() { } @@ -45,14 +52,26 @@ public String getId() { return id; } + public void setId(String id) { + this.id = id; + } + public String getType() { return type; } + public void setType(String type) { + this.type = type; + } + public boolean isMultivalued() { return multivalued; } + public void setMultivalued(boolean multivalued) { + this.multivalued = multivalued; + } + /** * @param choiceListInitializerFilter a reference to a choicelist * @deprecated As of version 1.1.0-incubating @@ -62,11 +81,40 @@ public void setChoiceListInitializerFilter(String choiceListInitializerFilter) { // Avoid errors when deploying old definitions } - public String getDefaultValue() { + public Object getDefaultValue() { return defaultValue; } - public void setDefaultValue(String defaultValue) { + public void setDefaultValue(Object defaultValue) { this.defaultValue = defaultValue; } + + /** + * Converts this parameter to a Map structure for YAML output. + * Implements YamlConvertible interface. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this parameter + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlUtils.YamlMapBuilder.create() + .put("id", id) + .put("validation", "") + .build(); + } + return YamlUtils.YamlMapBuilder.create() + .putIfNotNull("id", id) + .putIfNotNull("type", type) + .putIf("multivalued", true, multivalued) + .putIfNotNull("defaultValue", defaultValue) + .build(); + } + + @Override + public String toString() { + return YamlUtils.format(toYaml()); + } + } diff --git a/api/src/main/java/org/apache/unomi/api/actions/Action.java b/api/src/main/java/org/apache/unomi/api/actions/Action.java index b3505bbe8..29c6927e1 100644 --- a/api/src/main/java/org/apache/unomi/api/actions/Action.java +++ b/api/src/main/java/org/apache/unomi/api/actions/Action.java @@ -18,18 +18,26 @@ package org.apache.unomi.api.actions; import org.apache.unomi.api.rules.Rule; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlTransient; import java.io.Serializable; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * An action that can be executed as a consequence of a {@link Rule} being triggered. An action is characterized by its associated {@link * ActionType} and parameter values. */ -public class Action implements Serializable { +public class Action implements Serializable, YamlConvertible { private ActionType actionType; private String actionTypeId; private Map parameterValues = new HashMap<>(); @@ -117,4 +125,42 @@ public void setParameter(String name, Object value) { parameterValues.put(name, value); } + /** + * Converts this action to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this action + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("type", actionTypeId != null ? actionTypeId : "Action") + .put("parameterValues", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + YamlMapBuilder builder = YamlMapBuilder.create() + .put("type", actionTypeId != null ? actionTypeId : "Action"); + if (parameterValues != null && !parameterValues.isEmpty()) { + builder.put("parameterValues", toYamlValue(parameterValues, visitedSet, maxDepth - 1)); + } + return builder.build(); + } finally { + visitedSet.remove(this); + } + } + + @Override + public String toString() { + Map map = toYaml(); + return YamlUtils.format(map); + } + } diff --git a/api/src/main/java/org/apache/unomi/api/actions/ActionType.java b/api/src/main/java/org/apache/unomi/api/actions/ActionType.java index 5ff336d9f..e170c779b 100644 --- a/api/src/main/java/org/apache/unomi/api/actions/ActionType.java +++ b/api/src/main/java/org/apache/unomi/api/actions/ActionType.java @@ -20,14 +20,18 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Parameter; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; -import java.util.ArrayList; -import java.util.List; +import java.util.*; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A type definition for {@link Action}s. */ -public class ActionType extends MetadataItem { +public class ActionType extends MetadataItem implements YamlConvertible { public static final String ITEM_TYPE = "actionType"; private static final long serialVersionUID = -3522958600710010935L; @@ -101,4 +105,34 @@ public boolean equals(Object o) { public int hashCode() { return itemId.hashCode(); } + + /** + * Converts this action type to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this action type + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("parameters", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("actionExecutor", actionExecutor) + .putIfNotEmpty("parameters", parameters != null ? (Collection) toYamlValue(parameters, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + visitedSet.remove(this); + } + } } diff --git a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java index 526ff463f..812ee5862 100644 --- a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java +++ b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java @@ -17,16 +17,22 @@ package org.apache.unomi.api.conditions; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; + import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlTransient; import java.io.Serializable; -import java.util.HashMap; -import java.util.Map; +import java.util.*; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A set of elements that can be evaluated. */ -public class Condition implements Serializable { +public class Condition implements Serializable, YamlConvertible { private static final long serialVersionUID = 7584522402785053206L; ConditionType conditionType; @@ -65,7 +71,9 @@ public ConditionType getConditionType() { */ public void setConditionType(ConditionType conditionType) { this.conditionType = conditionType; - this.conditionTypeId = conditionType.getItemId(); + if (conditionType != null) { + this.conditionTypeId = conditionType.getItemId(); + } } /** @@ -123,7 +131,7 @@ public boolean containsParameter(String name) { * @return the value of the specified parameter or {@code null} if no such parameter exists */ public Object getParameter(String name) { - return parameterValues.get(name); + return parameterValues != null ? parameterValues.get(name) : null; } /** @@ -156,13 +164,96 @@ public int hashCode() { return result; } + /** + * Converts this condition to a Map structure for YAML output with depth limiting. + * Implements YamlConvertible interface with circular reference detection and depth limiting + * to prevent StackOverflowError from extremely deep nested structures. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @param maxDepth maximum recursion depth (prevents StackOverflowError from deep nesting) + * @return a Map representation of this condition + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("type", conditionTypeId != null ? conditionTypeId : "Condition") + .put("parameterValues", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + YamlMapBuilder builder = YamlMapBuilder.create() + .put("type", conditionTypeId != null ? conditionTypeId : "Condition"); + if (parameterValues != null && !parameterValues.isEmpty()) { + builder.put("parameterValues", toYamlValue(parameterValues, visitedSet, maxDepth - 1)); + } + return builder.build(); + } finally { + visitedSet.remove(this); + } + } + + /** + * Creates a deep copy of this condition, including all nested conditions in parameter values. + * Recursively copies all nested conditions to avoid sharing references. + * + * @return a deep copy of this condition + */ + public Condition deepCopy() { + Condition copied = new Condition(); + if (this.conditionType != null) { + copied.setConditionType(this.conditionType); + } else if (this.conditionTypeId != null) { + copied.setConditionTypeId(this.conditionTypeId); + } + + // Deep copy parameter values + Map copiedParams = new HashMap<>(); + if (this.parameterValues != null) { + for (Map.Entry entry : this.parameterValues.entrySet()) { + Object value = entry.getValue(); + if (value instanceof Condition) { + // Recursively deep copy nested condition + copiedParams.put(entry.getKey(), ((Condition) value).deepCopy()); + } else if (value instanceof Collection) { + // Deep copy collection - preserve the collection type if possible + Collection collection = (Collection) value; + Collection copiedCollection; + if (collection instanceof List) { + copiedCollection = new ArrayList<>(); + } else { + // Fallback to ArrayList for other collection types + copiedCollection = new ArrayList<>(); + } + for (Object item : collection) { + if (item instanceof Condition) { + // Recursively deep copy nested condition + copiedCollection.add(((Condition) item).deepCopy()); + } else { + // Not a condition, add as-is (for non-condition values in collections) + copiedCollection.add(item); + } + } + copiedParams.put(entry.getKey(), copiedCollection); + } else { + // Primitive or other non-condition value, copy as-is + copiedParams.put(entry.getKey(), value); + } + } + } + copied.setParameterValues(copiedParams); + + return copied; + } + @Override public String toString() { - final StringBuilder sb = new StringBuilder("Condition{"); - sb.append("conditionType=").append(conditionType); - sb.append(", conditionTypeId='").append(conditionTypeId).append('\''); - sb.append(", parameterValues=").append(parameterValues); - sb.append('}'); - return sb.toString(); + Map map = toYaml(); + return YamlUtils.format(map); } } diff --git a/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java b/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java index c8a43225c..e8af9c41f 100644 --- a/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java +++ b/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java @@ -20,10 +20,14 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Parameter; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import javax.xml.bind.annotation.XmlElement; -import java.util.ArrayList; -import java.util.List; +import java.util.*; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * ConditionTypes define new conditions that can be applied to items (for example to decide whether a rule needs to be triggered or if a profile is considered as taking part in a @@ -31,7 +35,7 @@ * optimized by coding it. They may also be defined as combination of other conditions. A simple condition could be: “User is male”, while a more generic condition with * parameters may test whether a given property has a specific value: “User property x has value y”. */ -public class ConditionType extends MetadataItem { +public class ConditionType extends MetadataItem implements YamlConvertible { public static final String ITEM_TYPE = "conditionType"; private static final long serialVersionUID = -6965481691241954969L; @@ -142,4 +146,37 @@ public boolean equals(Object o) { public int hashCode() { return itemId.hashCode(); } + + /** + * Converts this condition type to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this condition type + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("parentCondition", "") + .put("parameters", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("conditionEvaluator", conditionEvaluator) + .putIfNotNull("queryBuilder", queryBuilder) + .putIfNotNull("parentCondition", parentCondition != null ? toYamlValue(parentCondition, visitedSet, maxDepth - 1) : null) + .putIfNotEmpty("parameters", parameters != null ? (Collection) toYamlValue(parameters, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + visitedSet.remove(this); + } + } } diff --git a/api/src/main/java/org/apache/unomi/api/goals/Goal.java b/api/src/main/java/org/apache/unomi/api/goals/Goal.java index 1b2ff0e88..12694b3d7 100644 --- a/api/src/main/java/org/apache/unomi/api/goals/Goal.java +++ b/api/src/main/java/org/apache/unomi/api/goals/Goal.java @@ -21,6 +21,15 @@ import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.campaigns.Campaign; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A tracked activity / action that can be accomplished by site (scope) visitors. These are tracked in general because they relate to specific business objectives or are @@ -32,7 +41,7 @@ *
  • audience filtering: any visitor is considered for scope-level goals while campaign-level goals only consider visitors who match the campaign's conditions * */ -public class Goal extends MetadataItem { +public class Goal extends MetadataItem implements YamlConvertible { public static final String ITEM_TYPE = "goal"; private static final long serialVersionUID = 6131648013470949983L; private Condition startEvent; @@ -87,4 +96,37 @@ public String getCampaignId() { public void setCampaignId(String campaignId) { this.campaignId = campaignId; } + + /** + * Converts this goal to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this goal + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("startEvent", "") + .put("targetEvent", "") + .put("campaignId", campaignId) + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("startEvent", startEvent != null ? toYamlValue(startEvent, visitedSet, maxDepth - 1) : null) + .putIfNotNull("targetEvent", targetEvent != null ? toYamlValue(targetEvent, visitedSet, maxDepth - 1) : null) + .putIfNotNull("campaignId", campaignId) + .build(); + } finally { + visitedSet.remove(this); + } + } } diff --git a/api/src/main/java/org/apache/unomi/api/rules/Rule.java b/api/src/main/java/org/apache/unomi/api/rules/Rule.java index 0aedfb123..63750c0d4 100644 --- a/api/src/main/java/org/apache/unomi/api/rules/Rule.java +++ b/api/src/main/java/org/apache/unomi/api/rules/Rule.java @@ -20,10 +20,15 @@ import org.apache.unomi.api.*; import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; -import java.util.List; +import java.util.*; import java.util.stream.Collectors; +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; + /** * A conditional set of actions to be executed in response to incoming events. Triggering of rules is guarded by a condition: the rule is only triggered if the associated * condition ({@link #getCondition()}) is satisfied. Once a rule triggers, a list of actions ({@link #getActions()} can be performed as consequences. @@ -34,7 +39,7 @@ * We could also specify a priority for our rule in case it needs to be executed before other ones when similar conditions match. This is accomplished using the * {@link #getPriority()} property. */ -public class Rule extends MetadataItem { +public class Rule extends MetadataItem implements YamlConvertible { /** * The Rule ITEM_TYPE. @@ -197,4 +202,41 @@ public int getPriority() { public void setPriority(int priority) { this.priority = priority; } + + /** + * Converts this rule to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this rule + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("condition", "") + .put("actions", "") + .put("priority", priority) + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("condition", condition != null ? toYamlValue(condition, visitedSet, maxDepth - 1) : null) + .putIfNotEmpty("actions", actions != null ? (Collection) toYamlValue(actions, visitedSet, maxDepth - 1) : null) + .putIfNotEmpty("linkedItems", linkedItems) + .putIf("raiseEventOnlyOnceForProfile", true, raiseEventOnlyOnceForProfile) + .putIf("raiseEventOnlyOnceForSession", true, raiseEventOnlyOnceForSession) + .putIf("raiseEventOnlyOnce", true, raiseEventOnlyOnce) + .putIf("priority", priority, priority != 0) + .build(); + } finally { + visitedSet.remove(this); + } + } } diff --git a/api/src/main/java/org/apache/unomi/api/segments/Scoring.java b/api/src/main/java/org/apache/unomi/api/segments/Scoring.java index 6018fb376..3d82e6e7e 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/Scoring.java +++ b/api/src/main/java/org/apache/unomi/api/segments/Scoring.java @@ -21,14 +21,19 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Profile; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; -import java.util.List; +import java.util.*; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A set of conditions associated with a value to assign to {@link Profile}s when matching so that the associated users can be scored along that * dimension. Each {@link ScoringElement} is evaluated and matching profiles' scores are incremented with the associated value. */ -public class Scoring extends MetadataItem { +public class Scoring extends MetadataItem implements YamlConvertible { /** * The Scoring ITEM_TYPE. * @@ -71,4 +76,33 @@ public void setElements(List elements) { this.elements = elements; } + /** + * Converts this scoring to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this scoring + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("elements", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotEmpty("elements", elements != null ? (Collection) toYamlValue(elements, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + visitedSet.remove(this); + } + } + } diff --git a/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java b/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java index 1af5147fb..bfc5a21cc 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java +++ b/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java @@ -18,13 +18,22 @@ package org.apache.unomi.api.segments; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import java.io.Serializable; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A scoring dimension along profiles can be evaluated and associated value to be assigned. */ -public class ScoringElement implements Serializable { +public class ScoringElement implements Serializable, YamlConvertible { private Condition condition; private int value; @@ -69,4 +78,40 @@ public int getValue() { public void setValue(int value) { this.value = value; } + + /** + * Converts this scoring element to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this scoring element + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("condition", "") + .put("value", value) + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .putIfNotNull("condition", condition != null ? toYamlValue(condition, visitedSet, maxDepth - 1) : null) + .put("value", value) + .build(); + } finally { + visitedSet.remove(this); + } + } + + @Override + public String toString() { + Map map = toYaml(); + return YamlUtils.format(map); + } } diff --git a/api/src/main/java/org/apache/unomi/api/segments/Segment.java b/api/src/main/java/org/apache/unomi/api/segments/Segment.java index 4e0d33830..2729ec74d 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/Segment.java +++ b/api/src/main/java/org/apache/unomi/api/segments/Segment.java @@ -22,13 +22,22 @@ import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Profile; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; +import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.unomi.api.utils.YamlUtils.circularRef; +import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; /** * A dynamically evaluated group of similar profiles in order to categorize the associated users. To be considered part of a given segment, users must satisfies * the segment’s condition. If they match, users are automatically added to the segment. Similarly, if at any given point during, they cease to satisfy the segment’s condition, * they are automatically removed from it. */ -public class Segment extends MetadataItem { +public class Segment extends MetadataItem implements YamlConvertible { /** * The Segment ITEM_TYPE. @@ -72,4 +81,33 @@ public void setCondition(Condition condition) { this.condition = condition; } + /** + * Converts this segment to a Map structure for YAML output. + * Implements YamlConvertible interface with circular reference detection. + * + * @param visited set of already visited objects to prevent infinite recursion (may be null) + * @return a Map representation of this segment + */ + @Override + public Map toYaml(Set visited, int maxDepth) { + if (maxDepth <= 0) { + return YamlMapBuilder.create() + .put("condition", "") + .build(); + } + if (visited != null && visited.contains(this)) { + return circularRef(); + } + final Set visitedSet = visited != null ? visited : new HashSet<>(); + visitedSet.add(this); + try { + return YamlMapBuilder.create() + .mergeObject(super.toYaml(visitedSet, maxDepth)) + .putIfNotNull("condition", condition != null ? toYamlValue(condition, visitedSet, maxDepth - 1) : null) + .build(); + } finally { + visitedSet.remove(this); + } + } + } diff --git a/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java new file mode 100644 index 000000000..664f63fb7 --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java @@ -0,0 +1,319 @@ +/* + * 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.api.utils; + +import org.yaml.snakeyaml.DumperOptions; +import org.yaml.snakeyaml.Yaml; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * YAML utilities using SnakeYaml with fluent API wrapper. + * Provides utilities for building YAML structures and formatting them via SnakeYaml. + */ +public class YamlUtils { + // SnakeYaml instance with configured options + private static final Yaml YAML_INSTANCE; + + static { + DumperOptions options = new DumperOptions(); + options.setIndent(2); + options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); + options.setPrettyFlow(true); + options.setDefaultScalarStyle(DumperOptions.ScalarStyle.PLAIN); + YAML_INSTANCE = new Yaml(options); + } + + /** + * Interface for objects that can convert themselves to YAML Map structures. + */ + public interface YamlConvertible { + /** + * Converts this object to a Map structure for YAML output with depth limiting. + * This method accepts an optional visited set to detect circular references and a max depth + * to prevent StackOverflowError from extremely deep nested structures. + * + * @param visited optional set of visited objects to detect circular references (may be null) + * @param maxDepth maximum recursion depth (prevents StackOverflowError from deep nesting) + * @return a Map representation of this object + */ + Map toYaml(Set visited, int maxDepth); + + /** + * Converts this object to a Map structure for YAML output. + * This method accepts an optional visited set to detect circular references. + * Uses a default max depth of 20 to prevent StackOverflowError. + * + * @param visited optional set of visited objects to detect circular references (may be null) + * @return a Map representation of this object + */ + default Map toYaml(Set visited) { + return toYaml(visited, 20); + } + + /** + * Converts this object to a Map structure for YAML output. + * This is a convenience method that calls toYaml(null, 20). + * + * @return a Map representation of this object + */ + default Map toYaml() { + return toYaml(null, 20); + } + } + + /** + * Fluent builder for creating YAML Map structures. + * Provides chaining methods to avoid repeating the map variable. + */ + public static class YamlMapBuilder { + private final Map map; + + private YamlMapBuilder() { + this.map = new LinkedHashMap<>(); + } + + /** + * Creates a new builder instance. + * + * @return a new YamlMapBuilder + */ + public static YamlMapBuilder create() { + return new YamlMapBuilder(); + } + + /** + * Adds a field if the value is not null. + * + * @param key the key (must not be null) + * @param value the value (only added if not null) + * @return this builder for chaining + * @throws NullPointerException if key is null + */ + public YamlMapBuilder putIfNotNull(String key, Object value) { + if (key == null) { + throw new NullPointerException("Key must not be null"); + } + if (value != null) { + map.put(key, value); + } + return this; + } + + /** + * Adds a field if the condition is true. + * + * @param key the key (must not be null) + * @param value the value (only added if condition is true) + * @param condition the condition + * @return this builder for chaining + * @throws NullPointerException if key is null + */ + public YamlMapBuilder putIf(String key, Object value, boolean condition) { + if (key == null) { + throw new NullPointerException("Key must not be null"); + } + if (condition) { + map.put(key, value); + } + return this; + } + + /** + * Adds a field unconditionally. + * + * @param key the key (must not be null) + * @param value the value + * @return this builder for chaining + * @throws NullPointerException if key is null + */ + public YamlMapBuilder put(String key, Object value) { + if (key == null) { + throw new NullPointerException("Key must not be null"); + } + map.put(key, value); + return this; + } + + /** + * Adds a field if the collection is not null and not empty. + * + * @param key the key (must not be null) + * @param collection the collection (only added if not null and not empty) + * @return this builder for chaining + * @throws NullPointerException if key is null + */ + public YamlMapBuilder putIfNotEmpty(String key, java.util.Collection collection) { + if (key == null) { + throw new NullPointerException("Key must not be null"); + } + if (collection != null && !collection.isEmpty()) { + map.put(key, collection); + } + return this; + } + + /** + * Merges all fields from a Map into this builder. + * This is useful for inheritance where subclasses want to include parent class fields. + * + * Usage in subclasses: + * 
    +         * return YamlMapBuilder.create()
+         *     .mergeObject(super.toYaml(visitedSet))
+         *     .putIfNotNull("field", value)
+         *     .build();
+         *
    + * + * @param objectMap the Map containing fields to merge (may be null, in which case nothing is merged) + * @return this builder for chaining + */ + public YamlMapBuilder mergeObject(Map objectMap) { + if (objectMap != null) { + objectMap.forEach(map::put); + } + return this; + } + + /** + * Builds and returns a defensive copy of the map. + * + * @return a new LinkedHashMap containing the built entries + */ + public Map build() { + return new LinkedHashMap<>(map); + } + } + + /** + * Converts a Set to a sorted List for YAML output. + * + * @param set the set to convert + * @return a sorted list, or null if the set is null or empty + */ + public static > List setToSortedList(Set set) { + if (set == null || set.isEmpty()) { + return null; + } + return set.stream().sorted().collect(Collectors.toList()); + } + + /** + * Converts a Set to a sorted List using a mapper function. + * + * @param set the set to convert + * @param mapper the mapper function (must not be null) + * @return a sorted list, or null if the set is null or empty + * @throws NullPointerException if mapper is null + */ + public static > List setToSortedList(Set set, Function mapper) { + if (mapper == null) { + throw new NullPointerException("Mapper function must not be null"); + } + if (set == null || set.isEmpty()) { + return null; + } + return set.stream().map(mapper).sorted().collect(Collectors.toList()); + } + + /** + * Converts a value to YAML-compatible format, handling nested structures. + * For objects that implement YamlConvertible, circular reference detection is + * handled by passing the visited set to their toYaml() implementation. + * + * @param value the value to convert + * @param visited set of visited objects for circular reference detection (may be null) + * @return the converted value + */ + public static Object toYamlValue(Object value, Set visited) { + return toYamlValue(value, visited, Integer.MAX_VALUE); + } + + /** + * Converts a value to YAML-compatible format with depth limiting to prevent StackOverflowError. + * For objects that implement YamlConvertible, circular reference detection is + * handled by passing the visited set to their toYaml() implementation. + * + * @param value the value to convert + * @param visited set of visited objects for circular reference detection (may be null) + * @param maxDepth maximum recursion depth (prevents StackOverflowError from deep nesting) + * @return the converted value, or a placeholder if max depth exceeded + */ + public static Object toYamlValue(Object value, Set visited, int maxDepth) { + if (maxDepth <= 0) { + return ""; + } + if (value == null) { + return null; + } + if (value instanceof YamlConvertible) { + // For YamlConvertible, get the Map and then process it as a Map to ensure sorting + // Pass maxDepth - 1 to the toYaml method to continue depth limiting + Map result = ((YamlConvertible) value).toYaml(visited, maxDepth - 1); + // Process the result as a Map to ensure it's sorted (this handles both sorting and recursive processing) + return toYamlValue(result, visited, maxDepth - 1); + } + if (value instanceof List) { + return ((List) value).stream() + .map(item -> toYamlValue(item, visited, maxDepth - 1)) + .collect(Collectors.toList()); + } + if (value instanceof Map) { + Map inputMap = (Map) value; + Map result = new LinkedHashMap<>(); + + if (!inputMap.isEmpty()) { + // Sort entries alphabetically by key string representation + inputMap.entrySet().stream() + .sorted((e1, e2) -> String.valueOf(e1.getKey()).compareTo(String.valueOf(e2.getKey()))) + .forEach(entry -> + result.put(String.valueOf(entry.getKey()), toYamlValue(entry.getValue(), visited, maxDepth - 1))); + } + return result; + } + return value; + } + + + /** + * Formats a value as YAML using SnakeYaml. + * This is a convenience method that delegates to SnakeYaml. + * + * @param value the value to format + * @return YAML string representation + */ + public static String format(Object value) { + return YAML_INSTANCE.dump(value); + } + + /** + * Creates a circular reference marker map. + * + * @return a map indicating a circular reference + */ + public static Map circularRef() { + return YamlMapBuilder.create() + .put("$ref", "circular") + .build(); + } +} diff --git a/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java b/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java new file mode 100644 index 000000000..c40c066ca --- /dev/null +++ b/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java @@ -0,0 +1,610 @@ +/* + * 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.api.utils; + +import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.actions.Action; +import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.rules.Rule; +import org.junit.Test; + +import java.util.*; + +import static org.junit.Assert.*; + +/** + * Unit tests for YamlUtils fluent API. + * Tests focus on our fluent API, not SnakeYaml's implementation. + */ +public class YamlUtilsTest { + + @Test + public void testYamlMapBuilderCreate() { + YamlUtils.YamlMapBuilder builder = YamlUtils.YamlMapBuilder.create(); + assertNotNull("Builder should be created", builder); + } + + @Test + public void testYamlMapBuilderPut() { + Map map = YamlUtils.YamlMapBuilder.create() + .put("key1", "value1") + .put("key2", 42) + .build(); + assertEquals("First value should be set", "value1", map.get("key1")); + assertEquals("Second value should be set", 42, map.get("key2")); + } + + @Test + public void testYamlMapBuilderPutIfNotNull() { + Map map = YamlUtils.YamlMapBuilder.create() + .putIfNotNull("key1", "value1") + .putIfNotNull("key2", null) + .putIfNotNull("key3", "value3") + .build(); + assertEquals("Non-null value should be set", "value1", map.get("key1")); + assertFalse("Null value should not be set", map.containsKey("key2")); + assertEquals("Another non-null value should be set", "value3", map.get("key3")); + } + + @Test + public void testYamlMapBuilderPutIf() { + Map map = YamlUtils.YamlMapBuilder.create() + .putIf("key1", "value1", true) + .putIf("key2", "value2", false) + .putIf("key3", "value3", true) + .build(); + assertEquals("Value with true condition should be set", "value1", map.get("key1")); + assertFalse("Value with false condition should not be set", map.containsKey("key2")); + assertEquals("Another value with true condition should be set", "value3", map.get("key3")); + } + + @Test + public void testYamlMapBuilderPutIfNotEmpty() { + Map map = YamlUtils.YamlMapBuilder.create() + .putIfNotEmpty("key1", Arrays.asList("a", "b")) + .putIfNotEmpty("key2", Collections.emptyList()) + .putIfNotEmpty("key3", null) + .putIfNotEmpty("key4", Arrays.asList("c")) + .build(); + assertTrue("Non-empty collection should be set", map.containsKey("key1")); + assertFalse("Empty collection should not be set", map.containsKey("key2")); + assertFalse("Null collection should not be set", map.containsKey("key3")); + assertTrue("Another non-empty collection should be set", map.containsKey("key4")); + } + + @Test + public void testYamlMapBuilderChaining() { + Map map = YamlUtils.YamlMapBuilder.create() + .put("a", 1) + .putIfNotNull("b", "value") + .putIf("c", 3, true) + .putIfNotEmpty("d", Arrays.asList(1, 2)) + .build(); + assertEquals("All valid entries should be added", 4, map.size()); + } + + @Test + public void testYamlMapBuilderNullKeyThrowsException() { + try { + YamlUtils.YamlMapBuilder.create().put(null, "value"); + fail("Null key should throw NullPointerException"); + } catch (NullPointerException e) { + // Expected + } + } + + @Test + public void testYamlMapBuilderNullKeyInPutIfNotNull() { + try { + YamlUtils.YamlMapBuilder.create().putIfNotNull(null, "value"); + fail("Null key in putIfNotNull should throw NullPointerException"); + } catch (NullPointerException e) { + // Expected + } + } + + @Test + public void testYamlMapBuilderNullKeyInPutIf() { + try { + YamlUtils.YamlMapBuilder.create().putIf(null, "value", true); + fail("Null key in putIf should throw NullPointerException"); + } catch (NullPointerException e) { + // Expected + } + } + + @Test + public void testYamlMapBuilderNullKeyInPutIfNotEmpty() { + try { + YamlUtils.YamlMapBuilder.create().putIfNotEmpty(null, Arrays.asList(1)); + fail("Null key in putIfNotEmpty should throw NullPointerException"); + } catch (NullPointerException e) { + // Expected + } + } + + @Test + public void testYamlMapBuilderBuildReturnsNewMap() { + YamlUtils.YamlMapBuilder builder = YamlUtils.YamlMapBuilder.create(); + builder.put("key", "value"); + Map map1 = builder.build(); + Map map2 = builder.build(); + assertNotSame("Each build() should return a new map", map1, map2); + assertEquals("Both maps should have same content", map1, map2); + } + + @Test + public void testSetToSortedList() { + Set set = new LinkedHashSet<>(Arrays.asList("zebra", "apple", "banana")); + List result = YamlUtils.setToSortedList(set); + assertNotNull("Result should not be null", result); + assertEquals("Set should be converted to sorted list", Arrays.asList("apple", "banana", "zebra"), result); + } + + @Test + public void testSetToSortedListNull() { + List result = YamlUtils.setToSortedList((Set) null); + assertNull("Null set should return null", result); + } + + @Test + public void testSetToSortedListEmpty() { + List result = YamlUtils.setToSortedList(Collections.emptySet()); + assertNull("Empty set should return null", result); + } + + @Test + public void testSetToSortedListWithMapper() { + Set set = new LinkedHashSet<>(Arrays.asList(3, 1, 2)); + List result = YamlUtils.setToSortedList(set, String::valueOf); + assertNotNull("Result should not be null", result); + assertEquals("Set should be converted to sorted list using mapper", Arrays.asList("1", "2", "3"), result); + } + + @Test + public void testSetToSortedListWithMapperNull() { + try { + YamlUtils.setToSortedList(Collections.singleton(1), null); + fail("Null mapper should throw NullPointerException"); + } catch (NullPointerException e) { + // Expected + } + } + + @Test + public void testSetToSortedListWithMapperNullSet() { + List result = YamlUtils.setToSortedList(null, String::valueOf); + assertNull("Null set should return null even with mapper", result); + } + + @Test + public void testToYamlValueWithYamlConvertible() { + YamlUtils.YamlConvertible convertible = (visited, maxDepth) -> { + Map map = new LinkedHashMap<>(); + map.put("test", "value"); + return map; + }; + Set visited = new HashSet<>(); + Object result = YamlUtils.toYamlValue(convertible, visited); + assertTrue("YamlConvertible should be converted to Map", result instanceof Map); + Map map = (Map) result; + assertEquals("Converted map should contain test value", "value", map.get("test")); + } + + @Test + public void testToYamlValueWithList() { + List list = Arrays.asList("a", "b", "c"); + Set visited = new HashSet<>(); + Object result = YamlUtils.toYamlValue(list, visited); + assertTrue("List should remain a List", result instanceof List); + assertEquals("List should be unchanged", list, result); + } + + @Test + public void testToYamlValueWithMap() { + Map map = new LinkedHashMap<>(); + map.put("key", "value"); + Set visited = new HashSet<>(); + Object result = YamlUtils.toYamlValue(map, visited); + assertTrue("Map should remain a Map", result instanceof Map); + assertEquals("Map should contain key-value", "value", ((Map) result).get("key")); + } + + @Test + public void testToYamlValueWithNull() { + Set visited = new HashSet<>(); + Object result = YamlUtils.toYamlValue(null, visited); + assertNull("Null should return null", result); + } + + @Test + public void testToYamlValueWithPrimitive() { + Set visited = new HashSet<>(); + Object result = YamlUtils.toYamlValue(42, visited); + assertEquals("Primitive should remain unchanged", 42, result); + } + + @Test + public void testCircularRef() { + Map result = YamlUtils.circularRef(); + assertNotNull("circularRef should return a map", result); + assertEquals("Should contain $ref: circular", "circular", result.get("$ref")); + assertEquals("Should have only one entry", 1, result.size()); + } + + @Test + public void testFormatBasic() { + // Just verify format() works - we don't test SnakeYaml's output format + Map map = new LinkedHashMap<>(); + map.put("key", "value"); + String result = YamlUtils.format(map); + assertNotNull("Format should return a string", result); + assertTrue("Format should contain key", result.contains("key")); + assertTrue("Format should contain value", result.contains("value")); + } + + // ========== Circular Reference Detection Tests ========== + + @Test + public void testRuleInheritanceChainNoCircularRef() { + // Test that Rule -> MetadataItem -> Item inheritance chain doesn't produce false circular refs + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + metadata.setScope("systemscope"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("testCondition"); + rule.setCondition(condition); + + Map result = rule.toYaml(null); + assertNotNull("Rule should serialize to YAML", result); + assertFalse("Should not contain circular reference marker", result.containsKey("$ref")); + assertTrue("Should contain condition", result.containsKey("condition")); + assertTrue("Should contain itemId from Item parent", result.containsKey("itemId")); + assertTrue("Should contain metadata from MetadataItem parent", result.containsKey("metadata")); + } + + @Test + public void testRuleWithCircularReferenceInCondition() { + // Test that a real circular reference (Rule referenced in condition's parameterValues) is detected + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("testCondition"); + // Create a circular reference: condition's parameterValues contains the rule itself + condition.getParameterValues().put("referencedRule", rule); + rule.setCondition(condition); + + Map result = rule.toYaml(null); + assertNotNull("Rule should serialize to YAML", result); + assertTrue("Should contain condition", result.containsKey("condition")); + + // Check that the circular reference is detected in the condition's parameterValues + Map conditionMap = (Map) result.get("condition"); + assertNotNull("Condition should be serialized", conditionMap); + Map paramValues = (Map) conditionMap.get("parameterValues"); + assertNotNull("Parameter values should exist", paramValues); + Map circularRef = (Map) paramValues.get("referencedRule"); + assertNotNull("Circular reference should be detected", circularRef); + assertEquals("Should contain circular reference marker", "circular", circularRef.get("$ref")); + } + + @Test + public void testRuleWithCircularReferenceInActions() { + // Test circular reference in actions list + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Action action = new Action(); + action.setActionTypeId("testAction"); + // Create circular reference: action's parameterValues contains the rule + action.getParameterValues().put("triggeringRule", rule); + rule.setActions(Collections.singletonList(action)); + + Map result = rule.toYaml(null); + assertNotNull("Rule should serialize to YAML", result); + assertTrue("Should contain actions", result.containsKey("actions")); + + List actions = (List) result.get("actions"); + assertNotNull("Actions list should exist", actions); + assertEquals("Should have one action", 1, actions.size()); + + Map actionMap = (Map) actions.get(0); + Map paramValues = (Map) actionMap.get("parameterValues"); + assertNotNull("Parameter values should exist", paramValues); + Map circularRef = (Map) paramValues.get("triggeringRule"); + assertNotNull("Circular reference should be detected", circularRef); + assertEquals("Should contain circular reference marker", "circular", circularRef.get("$ref")); + } + + @Test + public void testNestedCircularReference() { + // Test nested circular reference: Rule -> Condition -> nested Condition -> Rule + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Condition outerCondition = new Condition(); + outerCondition.setConditionTypeId("outerCondition"); + + Condition nestedCondition = new Condition(); + nestedCondition.setConditionTypeId("nestedCondition"); + // Nested condition references the rule + nestedCondition.getParameterValues().put("ruleRef", rule); + + // Outer condition contains nested condition + outerCondition.getParameterValues().put("nested", nestedCondition); + rule.setCondition(outerCondition); + + Map result = rule.toYaml(null); + assertNotNull("Rule should serialize to YAML", result); + + // Navigate through the nested structure + Map conditionMap = (Map) result.get("condition"); + Map paramValues = (Map) conditionMap.get("parameterValues"); + Map nestedConditionMap = (Map) paramValues.get("nested"); + Map nestedParamValues = (Map) nestedConditionMap.get("parameterValues"); + Map circularRef = (Map) nestedParamValues.get("ruleRef"); + + assertNotNull("Circular reference should be detected in nested structure", circularRef); + assertEquals("Should contain circular reference marker", "circular", circularRef.get("$ref")); + } + + @Test + public void testMultipleCircularReferences() { + // Test multiple circular references to the same object + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("testCondition"); + // Multiple references to the same rule + condition.getParameterValues().put("rule1", rule); + condition.getParameterValues().put("rule2", rule); + condition.getParameterValues().put("rule3", rule); + rule.setCondition(condition); + + Map result = rule.toYaml(null); + Map conditionMap = (Map) result.get("condition"); + Map paramValues = (Map) conditionMap.get("parameterValues"); + + // All three references should show circular ref + for (String key : Arrays.asList("rule1", "rule2", "rule3")) { + Map circularRef = (Map) paramValues.get(key); + assertNotNull("Circular reference should be detected for " + key, circularRef); + assertEquals("Should contain circular reference marker for " + key, "circular", circularRef.get("$ref")); + } + } + + + @Test + public void testCircularReferenceInList() { + // Test circular reference in a list + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("testCondition"); + // List containing the rule itself + condition.getParameterValues().put("ruleList", Arrays.asList(rule, "other", rule)); + rule.setCondition(condition); + + Map result = rule.toYaml(null); + Map conditionMap = (Map) result.get("condition"); + Map paramValues = (Map) conditionMap.get("parameterValues"); + List ruleList = (List) paramValues.get("ruleList"); + + assertNotNull("Rule list should exist", ruleList); + assertEquals("List should have 3 elements", 3, ruleList.size()); + + // First element should be circular ref + Map circularRef1 = (Map) ruleList.get(0); + assertEquals("First element should be circular ref", "circular", circularRef1.get("$ref")); + + // Second element should be string + assertEquals("Second element should be string", "other", ruleList.get(1)); + + // Third element should also be circular ref + Map circularRef2 = (Map) ruleList.get(2); + assertEquals("Third element should be circular ref", "circular", circularRef2.get("$ref")); + } + + @Test + public void testCircularReferenceInNestedMap() { + // Test circular reference in nested map structure + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("testCondition"); + // Nested map containing the rule + Map nestedMap = new HashMap<>(); + nestedMap.put("level1", new HashMap() {{ + put("level2", new HashMap() {{ + put("rule", rule); + }}); + }}); + condition.getParameterValues().put("nested", nestedMap); + rule.setCondition(condition); + + Map result = rule.toYaml(null); + Map conditionMap = (Map) result.get("condition"); + Map paramValues = (Map) conditionMap.get("parameterValues"); + Map nested = (Map) paramValues.get("nested"); + Map level1 = (Map) nested.get("level1"); + Map level2 = (Map) level1.get("level2"); + Map circularRef = (Map) level2.get("rule"); + + assertNotNull("Circular reference should be detected in nested map", circularRef); + assertEquals("Should contain circular reference marker", "circular", circularRef.get("$ref")); + } + + @Test + public void testNoFalseCircularRefInInheritance() { + // Test that inheritance chain (Rule -> MetadataItem -> Item) doesn't create false circular refs + // This is the main bug we're fixing + Rule rule = new Rule(); + rule.setItemId("test-rule"); + Metadata metadata = new Metadata("test-rule"); + metadata.setScope("systemscope"); + rule.setMetadata(metadata); + + Condition condition = new Condition(); + condition.setConditionTypeId("unavailableConditionType"); + condition.getParameterValues().put("comparisonOperator", "equals"); + condition.getParameterValues().put("propertyName", "testProperty"); + condition.getParameterValues().put("propertyValue", "testValue"); + rule.setCondition(condition); + + Action action = new Action(); + action.setActionTypeId("test"); + rule.setActions(Collections.singletonList(action)); + + Map result = rule.toYaml(null); + + // Should NOT contain $ref: circular at the top level + assertNotNull("Rule should serialize", result); + assertFalse("Should not have false circular reference at top level", + result.containsKey("$ref") && "circular".equals(result.get("$ref"))); + + // Should contain all expected fields from inheritance chain + assertTrue("Should contain itemId from Item", result.containsKey("itemId")); + assertTrue("Should contain itemType from Item", result.containsKey("itemType")); + assertEquals("itemType should be 'rule'", "rule", result.get("itemType")); + assertTrue("Should contain metadata from MetadataItem", result.containsKey("metadata")); + assertTrue("Should contain condition", result.containsKey("condition")); + assertTrue("Should contain actions", result.containsKey("actions")); + + // Verify condition structure + Map conditionMap = (Map) result.get("condition"); + assertNotNull("Condition should be present", conditionMap); + assertEquals("Condition should have correct type", "unavailableConditionType", conditionMap.get("type")); + + // Verify actions structure + List actions = (List) result.get("actions"); + assertNotNull("Actions should be present", actions); + assertEquals("Should have one action", 1, actions.size()); + } + + @Test + public void testItemTypeIsAlwaysIncluded() { + // Test that itemType is always included in YAML output, even if null + // This reflects the actual state of the object + Rule rule = new Rule(); + Metadata metadata = new Metadata("test-id"); + metadata.setScope("systemscope"); + rule.setMetadata(metadata); + + Map result = rule.toYaml(null); + + // itemType should always be present in output (set in Item constructor for Rule) + assertTrue("itemType should be included", result.containsKey("itemType")); + assertEquals("itemType should be 'rule'", "rule", result.get("itemType")); + + // itemId should also always be included + assertTrue("itemId should be included", result.containsKey("itemId")); + } + + @Test + public void testItemIdAndItemTypeIncludedEvenWhenNull() { + // Test that itemId and itemType are always included, even when null + // This ensures YAML output reflects the actual state of the object + Rule rule = new Rule(); + // Explicitly set itemId and itemType to null to test null handling + rule.setItemId(null); + rule.setItemType(null); + + Map result = rule.toYaml(null); + + // Both should be included even if null + assertTrue("itemId should be included even when null", result.containsKey("itemId")); + assertNull("itemId should be null", result.get("itemId")); + + assertTrue("itemType should be included even when null", result.containsKey("itemType")); + assertNull("itemType should be null", result.get("itemType")); + } + + @Test + public void testItemIdFromMetadata() { + // Test that itemId is set from metadata and included in YAML + Rule rule = new Rule(); + Metadata metadata = new Metadata("test-rule-id"); + metadata.setScope("systemscope"); + rule.setMetadata(metadata); + + Map result = rule.toYaml(null); + + // itemId should be set from metadata.getId() + assertTrue("itemId should be included when set from metadata", result.containsKey("itemId")); + assertEquals("itemId should match metadata id", "test-rule-id", result.get("itemId")); + } + + @Test + public void testVisitedSetIsSharedCorrectly() { + // Test that visited set is properly shared across nested calls + Rule rule1 = new Rule(); + rule1.setItemId("rule1"); + rule1.setMetadata(new Metadata("rule1")); + + Rule rule2 = new Rule(); + rule2.setItemId("rule2"); + rule2.setMetadata(new Metadata("rule2")); + + // rule1 references rule2, rule2 references rule1 (mutual circular reference) + Condition condition1 = new Condition(); + condition1.setConditionTypeId("test"); + condition1.getParameterValues().put("otherRule", rule2); + rule1.setCondition(condition1); + + Condition condition2 = new Condition(); + condition2.setConditionTypeId("test"); + condition2.getParameterValues().put("otherRule", rule1); + rule2.setCondition(condition2); + + // Serialize rule1 - should detect circular ref when it encounters rule2 which references rule1 + Map result1 = rule1.toYaml(null); + assertNotNull("Rule1 should serialize", result1); + + Map conditionMap1 = (Map) result1.get("condition"); + Map paramValues1 = (Map) conditionMap1.get("parameterValues"); + Map rule2Ref = (Map) paramValues1.get("otherRule"); + + // rule2 should be serialized, but when it tries to reference rule1, it should detect circular ref + assertNotNull("Rule2 reference should exist", rule2Ref); + // rule2 itself should be fully serialized (not circular), but its condition's otherRule should be circular + Map conditionMap2 = (Map) rule2Ref.get("condition"); + assertNotNull("Rule2's condition should exist", conditionMap2); + Map paramValues2 = (Map) conditionMap2.get("parameterValues"); + Map rule1CircularRef = (Map) paramValues2.get("otherRule"); + assertNotNull("Circular reference to rule1 should be detected", rule1CircularRef); + assertEquals("Should contain circular reference marker", "circular", rule1CircularRef.get("$ref")); + } +} diff --git a/bom/pom.xml b/bom/pom.xml index 69cb5a051..17b0f3752 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -503,6 +503,26 @@ junit ${junit.version} + + org.junit.jupiter + junit-jupiter + ${junit-jupiter.version} + + + org.mockito + mockito-core + ${mockito.version} + + + org.mockito + mockito-junit-jupiter + ${mockito.version} + + + org.awaitility + awaitility + ${awaitility.version} + diff --git a/itests/pom.xml b/itests/pom.xml index 98e98e9e5..d8b5d4a5e 100644 --- a/itests/pom.xml +++ b/itests/pom.xml @@ -32,6 +32,8 @@ elasticsearch false itests-opensearch + + true @@ -43,6 +45,14 @@ pom import + + + + + org.awaitility + awaitility + 3.1.6 + @@ -53,6 +63,16 @@ common ${karaf.version} test + + + ch.qos.logback + logback-classic + + + ch.qos.logback + logback-core + + @@ -105,7 +125,7 @@ org.apache.servicemix.bundles org.apache.servicemix.bundles.hamcrest 1.3_1 - runtime + test org.apache.httpcomponents diff --git a/pom.xml b/pom.xml index 25360797d..d4825e7ec 100644 --- a/pom.xml +++ b/pom.xml @@ -80,6 +80,7 @@ 2.3 3.10 2.19.0 + 1.7.36 9.12.2 2.4.0 2.12.7 @@ -102,6 +103,9 @@ 4.5.14 4.4.16 4.13.2 + 5.8.2 + 4.5.1 + 4.2.0 2.2.1 4.3.4 1.6.0 diff --git a/rest/src/main/java/org/apache/unomi/rest/models/RESTParameter.java b/rest/src/main/java/org/apache/unomi/rest/models/RESTParameter.java index bc7ab9acf..d6257c6f0 100644 --- a/rest/src/main/java/org/apache/unomi/rest/models/RESTParameter.java +++ b/rest/src/main/java/org/apache/unomi/rest/models/RESTParameter.java @@ -26,7 +26,7 @@ public class RESTParameter { private String id; private String type; private boolean multivalued = false; - private String defaultValue = null; + private Object defaultValue = null; public String getId() { return id; @@ -52,11 +52,11 @@ public void setMultivalued(boolean multivalued) { this.multivalued = multivalued; } - public String getDefaultValue() { + public Object getDefaultValue() { return defaultValue; } - public void setDefaultValue(String defaultValue) { + public void setDefaultValue(Object defaultValue) { this.defaultValue = defaultValue; } diff --git a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java index fe3fe8fb0..8b6e7063c 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java @@ -450,7 +450,15 @@ public Set getTrackedConditions(Item source) { trackedCondition.getConditionType().getParameters().forEach(parameter -> { try { if (TRACKED_PARAMETER.equals(parameter.getId())) { - Arrays.stream(StringUtils.split(parameter.getDefaultValue(), ",")).forEach(trackedParameter -> { + // Parameter#getDefaultValue is Object; null must not call toString() (NPE) or be passed to split. + Object defaultValue = parameter.getDefaultValue(); + if (defaultValue == null) { + LOGGER.debug( + "Skipping tracked parameter mapping: parameter id={} has null defaultValue for condition type {}", + parameter.getId(), trackedCondition.getConditionType().getItemId()); + return; + } + Arrays.stream(StringUtils.split(defaultValue.toString(), ",")).forEach(trackedParameter -> { String[] param = StringUtils.split(StringUtils.trim(trackedParameter), ":"); trackedParameters.put(StringUtils.trim(param[1]), trackedCondition.getParameter(StringUtils.trim(param[0]))); }); From dd35f3e44d831b2e16f74a85da63b266df13b44e Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 15 May 2026 08:10:13 +0200 Subject: [PATCH 02/16] Add manual dispatch to CodeQL Java workflow --- .github/workflows/codeql-analysis-java.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/codeql-analysis-java.yml b/.github/workflows/codeql-analysis-java.yml index 0b8b7c427..c823fc472 100644 --- a/.github/workflows/codeql-analysis-java.yml +++ b/.github/workflows/codeql-analysis-java.yml @@ -17,6 +17,7 @@ on: pull_request: # The branches below must be a subset of the branches above branches: [ master ] + workflow_dispatch: schedule: - cron: '38 1 * * 0' From 86d14d0cbe3db0d80cdd485e52935fc61513c58e Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 15 May 2026 08:10:25 +0200 Subject: [PATCH 03/16] Add manual dispatch to CodeQL JavaScript workflow --- .github/workflows/codeql-analysis-javascript.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/codeql-analysis-javascript.yml b/.github/workflows/codeql-analysis-javascript.yml index 542b973b6..39efa7cbe 100644 --- a/.github/workflows/codeql-analysis-javascript.yml +++ b/.github/workflows/codeql-analysis-javascript.yml @@ -17,6 +17,7 @@ on: pull_request: # The branches below must be a subset of the branches above branches: [ master ] + workflow_dispatch: schedule: - cron: '38 1 * * 0' From 22de38c24b4e693f4859997caa800ccbc232c6fa Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 15 May 2026 19:40:22 +0200 Subject: [PATCH 04/16] CI: always publish IT JUnit report and update check on re-run Run mikepenz/action-junit-report after every integration matrix leg (if: always), with update_check and per-matrix check_name, so a green re-run replaces the stale red JUnit check. Use fail_on_failure=false and require_tests=false; continue-on-error so missing XML cannot fail the job. --- .github/workflows/unomi-ci-build-tests.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unomi-ci-build-tests.yml b/.github/workflows/unomi-ci-build-tests.yml index e629253cd..4cb248c54 100644 --- a/.github/workflows/unomi-ci-build-tests.yml +++ b/.github/workflows/unomi-ci-build-tests.yml @@ -79,8 +79,15 @@ jobs: itests/target/exam/**/data/log itests/target/elasticsearch0/data itests/target/elasticsearch0/logs + # Always publish so a later "re-run failed jobs" pass updates the check to green. + # Previously `if: failure()` left a stale red "JUnit Test Report" when ITs passed on re-run. - name: Publish Test Report uses: mikepenz/action-junit-report@v3 - if: failure() + if: always() + continue-on-error: true with: report_paths: 'itests/target/failsafe-reports/TEST-*.xml' + check_name: 'JUnit Test Report (${{ matrix.search-engine }})' + update_check: true + fail_on_failure: false + require_tests: false From bcf5a68989dfa2a93206a50af6fe598d24920212 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 16 May 2026 08:33:02 +0200 Subject: [PATCH 05/16] UNOMI-920: Address PR review feedback for YAML toString support Use identity-based visited sets for circular-reference detection, harden Condition deepCopy and parameter handling, fix Parameter max-depth YAML, wire slf4j.version in unomi-api, and expand unit test coverage. --- api/pom.xml | 2 + .../main/java/org/apache/unomi/api/Item.java | 5 +- .../java/org/apache/unomi/api/Metadata.java | 3 +- .../org/apache/unomi/api/MetadataItem.java | 3 +- .../java/org/apache/unomi/api/Parameter.java | 11 +- .../org/apache/unomi/api/actions/Action.java | 3 +- .../apache/unomi/api/actions/ActionType.java | 3 +- .../unomi/api/conditions/Condition.java | 95 ++++++---- .../unomi/api/conditions/ConditionType.java | 3 +- .../java/org/apache/unomi/api/goals/Goal.java | 4 +- .../java/org/apache/unomi/api/rules/Rule.java | 3 +- .../apache/unomi/api/segments/Scoring.java | 3 +- .../unomi/api/segments/ScoringElement.java | 3 +- .../apache/unomi/api/segments/Segment.java | 4 +- .../org/apache/unomi/api/utils/YamlUtils.java | 17 +- .../org/apache/unomi/api/ParameterTest.java | 81 +++++++++ .../unomi/api/conditions/ConditionTest.java | 171 ++++++++++++++++++ .../apache/unomi/api/utils/YamlUtilsTest.java | 145 +++++++++++++-- pom.xml | 1 + 19 files changed, 485 insertions(+), 75 deletions(-) create mode 100644 api/src/test/java/org/apache/unomi/api/ParameterTest.java create mode 100644 api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java diff --git a/api/pom.xml b/api/pom.xml index 4d9ed7fbd..89d7d56ad 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -66,6 +66,7 @@ org.slf4j slf4j-api + ${slf4j.version} provided @@ -93,6 +94,7 @@ org.slf4j slf4j-simple + ${slf4j.version} test diff --git a/api/src/main/java/org/apache/unomi/api/Item.java b/api/src/main/java/org/apache/unomi/api/Item.java index 9836cf40d..482802588 100644 --- a/api/src/main/java/org/apache/unomi/api/Item.java +++ b/api/src/main/java/org/apache/unomi/api/Item.java @@ -43,6 +43,9 @@ public abstract class Item implements Serializable, YamlConvertible { private static final Logger LOGGER = LoggerFactory.getLogger(Item.class.getName()); + /** + * Java serialization version; Unomi does not rely on Java serialization of this type as a cross-version persistence contract. + */ private static final long serialVersionUID = 1217180125083162915L; private static final Map itemTypeCache = new ConcurrentHashMap<>(); @@ -171,7 +174,7 @@ public Map toYaml(Set visited, int maxDepth) { .put("systemMetadata", "") .build(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); // Check if already visited - if so, we're being called from a child class via super.toYaml() // OR it's a real circular reference. We can't distinguish, but since child classes // (like Rule, ConditionType, etc.) all check for circular refs before calling super, diff --git a/api/src/main/java/org/apache/unomi/api/Metadata.java b/api/src/main/java/org/apache/unomi/api/Metadata.java index ec25c7fa8..4f650589f 100644 --- a/api/src/main/java/org/apache/unomi/api/Metadata.java +++ b/api/src/main/java/org/apache/unomi/api/Metadata.java @@ -22,7 +22,6 @@ import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import java.io.Serializable; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -299,7 +298,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/MetadataItem.java b/api/src/main/java/org/apache/unomi/api/MetadataItem.java index 35ef596a2..68cf3b324 100644 --- a/api/src/main/java/org/apache/unomi/api/MetadataItem.java +++ b/api/src/main/java/org/apache/unomi/api/MetadataItem.java @@ -22,7 +22,6 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlTransient; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -83,7 +82,7 @@ public Map toYaml(Set visited, int maxDepth) { .put("metadata", "") .build(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); // Check if already visited - if so, we're being called from a child class via super.toYaml() // In that case, skip the circular reference check and just proceed boolean alreadyVisited = visitedSet.contains(this); diff --git a/api/src/main/java/org/apache/unomi/api/Parameter.java b/api/src/main/java/org/apache/unomi/api/Parameter.java index 425d86280..7fc7c7453 100644 --- a/api/src/main/java/org/apache/unomi/api/Parameter.java +++ b/api/src/main/java/org/apache/unomi/api/Parameter.java @@ -24,14 +24,15 @@ import java.util.Map; import java.util.Set; -import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; - /** * A representation of a condition parameter, to be used in the segment building UI to either select parameters from a * choicelist or to enter a specific value. */ public class Parameter implements Serializable, YamlConvertible { + /** + * Java serialization version; Unomi does not rely on Java serialization of this type as a cross-version persistence contract. + */ private static final long serialVersionUID = 6019392686888941547L; private String id; @@ -100,8 +101,10 @@ public void setDefaultValue(Object defaultValue) { public Map toYaml(Set visited, int maxDepth) { if (maxDepth <= 0) { return YamlUtils.YamlMapBuilder.create() - .put("id", id) - .put("validation", "") + .putIfNotNull("id", id) + .putIfNotNull("type", type) + .putIf("multivalued", true, multivalued) + .putIfNotNull("defaultValue", "") .build(); } return YamlUtils.YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/actions/Action.java b/api/src/main/java/org/apache/unomi/api/actions/Action.java index 29c6927e1..aeaba1bc6 100644 --- a/api/src/main/java/org/apache/unomi/api/actions/Action.java +++ b/api/src/main/java/org/apache/unomi/api/actions/Action.java @@ -26,7 +26,6 @@ import javax.xml.bind.annotation.XmlTransient; import java.io.Serializable; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -143,7 +142,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { YamlMapBuilder builder = YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/actions/ActionType.java b/api/src/main/java/org/apache/unomi/api/actions/ActionType.java index e170c779b..5da949349 100644 --- a/api/src/main/java/org/apache/unomi/api/actions/ActionType.java +++ b/api/src/main/java/org/apache/unomi/api/actions/ActionType.java @@ -20,6 +20,7 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Parameter; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; @@ -123,7 +124,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java index 812ee5862..6dc1b9047 100644 --- a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java +++ b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java @@ -24,7 +24,13 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlTransient; import java.io.Serializable; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; import static org.apache.unomi.api.utils.YamlUtils.circularRef; import static org.apache.unomi.api.utils.YamlUtils.toYamlValue; @@ -111,7 +117,7 @@ public Map getParameterValues() { * @param parameterValues a Map containing the parameter name - value pairs for this profile */ public void setParameterValues(Map parameterValues) { - this.parameterValues = parameterValues; + this.parameterValues = parameterValues != null ? parameterValues : new HashMap<>(); } /** @@ -121,7 +127,7 @@ public void setParameterValues(Map parameterValues) { * @return {@code true} if this condition contains a parameter with the specified name, {@code false} otherwise */ public boolean containsParameter(String name) { - return parameterValues.containsKey(name); + return parameterValues != null && parameterValues.containsKey(name); } /** @@ -142,6 +148,9 @@ public Object getParameter(String name) { * @param value the value of the parameter */ public void setParameter(String name, Object value) { + if (parameterValues == null) { + parameterValues = new HashMap<>(); + } parameterValues.put(name, value); } @@ -184,7 +193,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { YamlMapBuilder builder = YamlMapBuilder.create() @@ -203,52 +212,58 @@ public Map toYaml(Set visited, int maxDepth) { * Recursively copies all nested conditions to avoid sharing references. * * @return a deep copy of this condition + * @throws IllegalStateException if the condition graph contains a cycle through nested {@link Condition} values */ public Condition deepCopy() { - Condition copied = new Condition(); - if (this.conditionType != null) { - copied.setConditionType(this.conditionType); - } else if (this.conditionTypeId != null) { - copied.setConditionTypeId(this.conditionTypeId); + return deepCopy(new IdentityHashMap<>()); + } + + private Condition deepCopy(IdentityHashMap copying) { + if (copying.put(this, Boolean.TRUE) != null) { + throw new IllegalStateException("Cyclic Condition graph: cannot deepCopy()"); } + try { + Condition copied = new Condition(); + if (this.conditionType != null) { + copied.setConditionType(this.conditionType); + } else if (this.conditionTypeId != null) { + copied.setConditionTypeId(this.conditionTypeId); + } - // Deep copy parameter values - Map copiedParams = new HashMap<>(); - if (this.parameterValues != null) { - for (Map.Entry entry : this.parameterValues.entrySet()) { - Object value = entry.getValue(); - if (value instanceof Condition) { - // Recursively deep copy nested condition - copiedParams.put(entry.getKey(), ((Condition) value).deepCopy()); - } else if (value instanceof Collection) { - // Deep copy collection - preserve the collection type if possible - Collection collection = (Collection) value; - Collection copiedCollection; - if (collection instanceof List) { - copiedCollection = new ArrayList<>(); - } else { - // Fallback to ArrayList for other collection types - copiedCollection = new ArrayList<>(); - } - for (Object item : collection) { - if (item instanceof Condition) { - // Recursively deep copy nested condition - copiedCollection.add(((Condition) item).deepCopy()); + // Deep copy parameter values + Map copiedParams = new HashMap<>(); + if (this.parameterValues != null) { + for (Map.Entry entry : this.parameterValues.entrySet()) { + Object value = entry.getValue(); + if (value instanceof Condition) { + copiedParams.put(entry.getKey(), ((Condition) value).deepCopy(copying)); + } else if (value instanceof Collection) { + Collection collection = (Collection) value; + Collection copiedCollection; + if (collection instanceof List) { + copiedCollection = new ArrayList<>(); } else { - // Not a condition, add as-is (for non-condition values in collections) - copiedCollection.add(item); + copiedCollection = new ArrayList<>(); + } + for (Object item : collection) { + if (item instanceof Condition) { + copiedCollection.add(((Condition) item).deepCopy(copying)); + } else { + copiedCollection.add(item); + } } + copiedParams.put(entry.getKey(), copiedCollection); + } else { + copiedParams.put(entry.getKey(), value); } - copiedParams.put(entry.getKey(), copiedCollection); - } else { - // Primitive or other non-condition value, copy as-is - copiedParams.put(entry.getKey(), value); } } - } - copied.setParameterValues(copiedParams); + copied.setParameterValues(copiedParams); - return copied; + return copied; + } finally { + copying.remove(this); + } } @Override diff --git a/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java b/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java index e8af9c41f..3d22c00a3 100644 --- a/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java +++ b/api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java @@ -20,6 +20,7 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Parameter; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; @@ -165,7 +166,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/goals/Goal.java b/api/src/main/java/org/apache/unomi/api/goals/Goal.java index 12694b3d7..d2cac5db8 100644 --- a/api/src/main/java/org/apache/unomi/api/goals/Goal.java +++ b/api/src/main/java/org/apache/unomi/api/goals/Goal.java @@ -21,10 +21,10 @@ import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.campaigns.Campaign; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -116,7 +116,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/rules/Rule.java b/api/src/main/java/org/apache/unomi/api/rules/Rule.java index 63750c0d4..cd6f751e5 100644 --- a/api/src/main/java/org/apache/unomi/api/rules/Rule.java +++ b/api/src/main/java/org/apache/unomi/api/rules/Rule.java @@ -20,6 +20,7 @@ import org.apache.unomi.api.*; import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; @@ -222,7 +223,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/segments/Scoring.java b/api/src/main/java/org/apache/unomi/api/segments/Scoring.java index 3d82e6e7e..b32aa8ab0 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/Scoring.java +++ b/api/src/main/java/org/apache/unomi/api/segments/Scoring.java @@ -21,6 +21,7 @@ import org.apache.unomi.api.Metadata; import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Profile; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; @@ -93,7 +94,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java b/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java index bfc5a21cc..ab79d132a 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java +++ b/api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java @@ -23,7 +23,6 @@ import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; import java.io.Serializable; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -97,7 +96,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/segments/Segment.java b/api/src/main/java/org/apache/unomi/api/segments/Segment.java index 2729ec74d..126279c15 100644 --- a/api/src/main/java/org/apache/unomi/api/segments/Segment.java +++ b/api/src/main/java/org/apache/unomi/api/segments/Segment.java @@ -22,10 +22,10 @@ import org.apache.unomi.api.MetadataItem; import org.apache.unomi.api.Profile; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.utils.YamlUtils; import org.apache.unomi.api.utils.YamlUtils.YamlConvertible; import org.apache.unomi.api.utils.YamlUtils.YamlMapBuilder; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -98,7 +98,7 @@ public Map toYaml(Set visited, int maxDepth) { if (visited != null && visited.contains(this)) { return circularRef(); } - final Set visitedSet = visited != null ? visited : new HashSet<>(); + final Set visitedSet = visited != null ? visited : YamlUtils.newIdentityVisitedSet(); visitedSet.add(this); try { return YamlMapBuilder.create() diff --git a/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java index 664f63fb7..a3dcd3cd0 100644 --- a/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java +++ b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java @@ -20,6 +20,8 @@ import org.yaml.snakeyaml.DumperOptions; import org.yaml.snakeyaml.Yaml; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -53,7 +55,9 @@ public interface YamlConvertible { * This method accepts an optional visited set to detect circular references and a max depth * to prevent StackOverflowError from extremely deep nested structures. * - * @param visited optional set of visited objects to detect circular references (may be null) + * @param visited optional set of visited objects to detect circular references (may be null). + * When non-null, use identity semantics (e.g. {@link YamlUtils#newIdentityVisitedSet()}) + * so cycles are detected by object identity, not {@code equals}. * @param maxDepth maximum recursion depth (prevents StackOverflowError from deep nesting) * @return a Map representation of this object */ @@ -236,6 +240,17 @@ public static > List setToSortedList(Set set, F return set.stream().map(mapper).sorted().collect(Collectors.toList()); } + /** + * Creates an empty {@link Set} suitable for {@link YamlConvertible#toYaml(Set, int)} visited tracking. + * The set uses reference identity ({@link IdentityHashMap}), not {@link Object#equals(Object) equals}, + * so distinct object graphs are not mistaken for cycles when types override equality. + * + * @return a new modifiable identity-based set + */ + public static Set newIdentityVisitedSet() { + return Collections.newSetFromMap(new IdentityHashMap<>()); + } + /** * Converts a value to YAML-compatible format, handling nested structures. * For objects that implement YamlConvertible, circular reference detection is diff --git a/api/src/test/java/org/apache/unomi/api/ParameterTest.java b/api/src/test/java/org/apache/unomi/api/ParameterTest.java new file mode 100644 index 000000000..2ed12d0c6 --- /dev/null +++ b/api/src/test/java/org/apache/unomi/api/ParameterTest.java @@ -0,0 +1,81 @@ +/* + * 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.api; + +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link Parameter} YAML and accessors. + */ +public class ParameterTest { + + @Test + public void testToYamlMaxDepthZeroIncludesExpectedKeys() { + Parameter p = new Parameter(); + p.setId("pid"); + p.setType("string"); + p.setMultivalued(true); + p.setDefaultValue("def"); + Map y = p.toYaml(null, 0); + assertEquals("pid", y.get("id")); + assertEquals("string", y.get("type")); + assertTrue(y.containsKey("multivalued")); + assertEquals("", y.get("defaultValue")); + } + + @Test + public void testToYamlMaxDepthZeroAddsDefaultValueTruncationMarkerEvenWhenUnset() { + Parameter p = new Parameter(); + p.setMultivalued(false); + Map y = p.toYaml(null, 0); + assertFalse(y.containsKey("id")); + assertFalse(y.containsKey("type")); + assertFalse(y.containsKey("multivalued")); + assertEquals("", y.get("defaultValue")); + } + + @Test + public void testToYamlNormalPath() { + Parameter p = new Parameter("id1", "number", false); + p.setDefaultValue(42); + Map y = p.toYaml(null, 10); + assertEquals("id1", y.get("id")); + assertEquals("number", y.get("type")); + assertFalse(y.containsKey("multivalued")); + assertEquals(42, y.get("defaultValue")); + } + + @Test + public void testToYamlMultivaluedTrueAddsFlag() { + Parameter p = new Parameter("i", "t", true); + Map y = p.toYaml(null, 10); + assertEquals(Boolean.TRUE, y.get("multivalued")); + } + + @Test + public void testToStringIsNonEmptyYaml() { + Parameter p = new Parameter("x", "boolean", false); + String s = p.toString(); + assertNotNull(s); + assertTrue(s.length() > 0); + assertTrue(s.contains("x")); + } +} diff --git a/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java b/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java new file mode 100644 index 000000000..35e6edf25 --- /dev/null +++ b/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java @@ -0,0 +1,171 @@ +/* + * 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.api.conditions; + +import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.utils.YamlUtils; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link Condition} behavior (parameters, YAML, deep copy). + */ +public class ConditionTest { + + @Test + public void testSetParameterValuesNullReplacesWithEmptyMap() { + Condition c = new Condition(); + c.setConditionTypeId("t"); + c.getParameterValues().put("k", "v"); + c.setParameterValues(null); + assertNotNull(c.getParameterValues()); + assertTrue(c.getParameterValues().isEmpty()); + assertFalse(c.containsParameter("k")); + assertNull(c.getParameter("k")); + } + + @Test + public void testSetParameterAfterClearingParameterValues() { + Condition c = new Condition(); + c.setConditionTypeId("t"); + c.setParameterValues(null); + c.setParameter("x", 1); + assertEquals(Integer.valueOf(1), c.getParameter("x")); + assertTrue(c.containsParameter("x")); + } + + @Test + public void testToYamlMaxDepthZeroUsesPlaceholder() { + Condition c = new Condition(); + c.setConditionTypeId("myType"); + c.getParameterValues().put("p", "v"); + Map y = c.toYaml(null, 0); + assertEquals("myType", y.get("type")); + assertEquals("", y.get("parameterValues")); + } + + @Test + public void testToYamlMaxDepthZeroDefaultTypeWhenIdMissing() { + Condition c = new Condition(); + Map y = c.toYaml(null, 0); + assertEquals("Condition", y.get("type")); + } + + @Test + public void testToYamlWhenAlreadyVisitedReturnsCircularMarker() { + Condition c = new Condition(); + c.setConditionTypeId("t"); + Set visited = YamlUtils.newIdentityVisitedSet(); + visited.add(c); + Map y = c.toYaml(visited, 5); + assertEquals("circular", y.get("$ref")); + } + + @Test + public void testToYamlOmitsParameterValuesWhenEmpty() { + Condition c = new Condition(); + c.setConditionTypeId("onlyType"); + Map y = c.toYaml(null, 10); + assertFalse(y.containsKey("parameterValues")); + } + + @Test + public void testToStringUsesYamlFormat() { + Condition c = new Condition(); + c.setConditionTypeId("ctype"); + String s = c.toString(); + assertNotNull(s); + assertTrue(s.contains("ctype")); + } + + @Test + public void testDeepCopyPreservesConditionTypeIdOnly() { + Condition c = new Condition(); + c.setConditionTypeId("idOnly"); + Condition copy = c.deepCopy(); + assertNotSame(c, copy); + assertEquals("idOnly", copy.getConditionTypeId()); + assertNull(copy.getConditionType()); + } + + @Test + public void testDeepCopyPreservesConditionTypeReference() { + ConditionType ct = new ConditionType(new Metadata("meta-ct")); + ct.setItemId("evaluatorType"); + Condition c = new Condition(ct); + Condition copy = c.deepCopy(); + assertSame(ct, copy.getConditionType()); + assertEquals("evaluatorType", copy.getConditionTypeId()); + } + + @Test + public void testDeepCopyNestedConditionInSetBecomesArrayList() { + Condition inner = new Condition(); + inner.setConditionTypeId("inner"); + Condition outer = new Condition(); + outer.setConditionTypeId("outer"); + Set nested = new LinkedHashSet<>(); + nested.add(inner); + outer.getParameterValues().put("conds", nested); + + Condition copy = outer.deepCopy(); + Object copiedVal = copy.getParameterValues().get("conds"); + assertTrue(copiedVal instanceof ArrayList); + @SuppressWarnings("unchecked") + Collection col = (Collection) copiedVal; + assertEquals(1, col.size()); + Condition copyInner = col.iterator().next(); + assertNotSame(inner, copyInner); + assertEquals("inner", copyInner.getConditionTypeId()); + } + + @Test(expected = IllegalStateException.class) + public void testDeepCopyRejectsSelfReferenceInParameterMap() { + Condition c = new Condition(); + c.setConditionTypeId("self"); + c.getParameterValues().put("me", c); + c.deepCopy(); + } + + @Test(expected = IllegalStateException.class) + public void testDeepCopyRejectsSelfInSingletonCollection() { + Condition c = new Condition(); + c.setConditionTypeId("self"); + c.getParameterValues().put("list", Collections.singletonList(c)); + c.deepCopy(); + } + + @Test + public void testEqualsAndHashCode() { + Condition a = new Condition(); + a.setConditionTypeId("t"); + a.getParameterValues().put("k", 1); + Condition b = new Condition(); + b.setConditionTypeId("t"); + b.getParameterValues().put("k", 1); + assertEquals(a, b); + assertEquals(a.hashCode(), b.hashCode()); + } +} diff --git a/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java b/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java index c40c066ca..af4150512 100644 --- a/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java +++ b/api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java @@ -198,7 +198,7 @@ public void testToYamlValueWithYamlConvertible() { map.put("test", "value"); return map; }; - Set visited = new HashSet<>(); + Set visited = YamlUtils.newIdentityVisitedSet(); Object result = YamlUtils.toYamlValue(convertible, visited); assertTrue("YamlConvertible should be converted to Map", result instanceof Map); Map map = (Map) result; @@ -208,7 +208,7 @@ public void testToYamlValueWithYamlConvertible() { @Test public void testToYamlValueWithList() { List list = Arrays.asList("a", "b", "c"); - Set visited = new HashSet<>(); + Set visited = YamlUtils.newIdentityVisitedSet(); Object result = YamlUtils.toYamlValue(list, visited); assertTrue("List should remain a List", result instanceof List); assertEquals("List should be unchanged", list, result); @@ -218,7 +218,7 @@ public void testToYamlValueWithList() { public void testToYamlValueWithMap() { Map map = new LinkedHashMap<>(); map.put("key", "value"); - Set visited = new HashSet<>(); + Set visited = YamlUtils.newIdentityVisitedSet(); Object result = YamlUtils.toYamlValue(map, visited); assertTrue("Map should remain a Map", result instanceof Map); assertEquals("Map should contain key-value", "value", ((Map) result).get("key")); @@ -226,14 +226,14 @@ public void testToYamlValueWithMap() { @Test public void testToYamlValueWithNull() { - Set visited = new HashSet<>(); + Set visited = YamlUtils.newIdentityVisitedSet(); Object result = YamlUtils.toYamlValue(null, visited); assertNull("Null should return null", result); } @Test public void testToYamlValueWithPrimitive() { - Set visited = new HashSet<>(); + Set visited = YamlUtils.newIdentityVisitedSet(); Object result = YamlUtils.toYamlValue(42, visited); assertEquals("Primitive should remain unchanged", 42, result); } @@ -446,12 +446,12 @@ public void testCircularReferenceInNestedMap() { Condition condition = new Condition(); condition.setConditionTypeId("testCondition"); // Nested map containing the rule + Map level2 = new HashMap<>(); + level2.put("rule", rule); + Map level1 = new HashMap<>(); + level1.put("level2", level2); Map nestedMap = new HashMap<>(); - nestedMap.put("level1", new HashMap() {{ - put("level2", new HashMap() {{ - put("rule", rule); - }}); - }}); + nestedMap.put("level1", level1); condition.getParameterValues().put("nested", nestedMap); rule.setCondition(condition); @@ -459,9 +459,9 @@ public void testCircularReferenceInNestedMap() { Map conditionMap = (Map) result.get("condition"); Map paramValues = (Map) conditionMap.get("parameterValues"); Map nested = (Map) paramValues.get("nested"); - Map level1 = (Map) nested.get("level1"); - Map level2 = (Map) level1.get("level2"); - Map circularRef = (Map) level2.get("rule"); + Map nestedLevel1 = (Map) nested.get("level1"); + Map nestedLevel2 = (Map) nestedLevel1.get("level2"); + Map circularRef = (Map) nestedLevel2.get("rule"); assertNotNull("Circular reference should be detected in nested map", circularRef); assertEquals("Should contain circular reference marker", "circular", circularRef.get("$ref")); @@ -607,4 +607,123 @@ public void testVisitedSetIsSharedCorrectly() { assertNotNull("Circular reference to rule1 should be detected", rule1CircularRef); assertEquals("Should contain circular reference marker", "circular", rule1CircularRef.get("$ref")); } + + @Test + public void testConditionDeepCopyCopiesNestedConditions() { + Condition inner = new Condition(); + inner.setConditionTypeId("inner"); + Condition outer = new Condition(); + outer.setConditionTypeId("outer"); + outer.getParameterValues().put("c", inner); + + Condition copy = outer.deepCopy(); + assertNotSame(outer, copy); + Condition copyInner = (Condition) copy.getParameterValues().get("c"); + assertNotSame(inner, copyInner); + assertEquals("inner", copyInner.getConditionTypeId()); + } + + @Test(expected = IllegalStateException.class) + public void testConditionDeepCopyRejectsCycle() { + Condition a = new Condition(); + a.setConditionTypeId("a"); + Condition b = new Condition(); + b.setConditionTypeId("b"); + a.getParameterValues().put("child", b); + b.getParameterValues().put("child", a); + a.deepCopy(); + } + + @Test + public void testNewIdentityVisitedSetUsesReferenceIdentity() { + Set visited = YamlUtils.newIdentityVisitedSet(); + String a = new String("x"); + String b = new String("x"); + assertTrue(visited.add(a)); + assertTrue(visited.add(b)); + assertEquals("equal strings are distinct objects for identity set", 2, visited.size()); + } + + @Test + public void testYamlMapBuilderMergeObjectNullIsNoOp() { + Map map = YamlUtils.YamlMapBuilder.create() + .put("k", "v") + .mergeObject(null) + .build(); + assertEquals(1, map.size()); + assertEquals("v", map.get("k")); + } + + @Test + public void testYamlMapBuilderMergeObjectCopiesEntries() { + Map extra = new LinkedHashMap<>(); + extra.put("a", 1); + extra.put("b", 2); + Map map = YamlUtils.YamlMapBuilder.create() + .put("z", 0) + .mergeObject(extra) + .build(); + assertEquals(3, map.size()); + assertEquals(Integer.valueOf(0), map.get("z")); + assertEquals(Integer.valueOf(1), map.get("a")); + assertEquals(Integer.valueOf(2), map.get("b")); + } + + @Test + public void testToYamlValueMaxDepthZeroReturnsPlaceholder() { + assertEquals("", YamlUtils.toYamlValue("anything", null, 0)); + } + + @Test + public void testToYamlValueEmptyMapWithDepth() { + @SuppressWarnings("unchecked") + Map out = (Map) YamlUtils.toYamlValue(Collections.emptyMap(), null, 5); + assertNotNull(out); + assertTrue(out.isEmpty()); + } + + @Test + public void testToYamlValueSortsMapKeysLexicographically() { + Map in = new LinkedHashMap<>(); + in.put("z", 1); + in.put("a", 2); + @SuppressWarnings("unchecked") + Map out = (Map) YamlUtils.toYamlValue(in, null, 10); + assertEquals(Arrays.asList("a", "z"), new ArrayList<>(out.keySet())); + } + + @Test + public void testToYamlValueNonStringMapKeysBecomeStrings() { + Map in = new HashMap<>(); + in.put(10, "ten"); + in.put(2, "two"); + @SuppressWarnings("unchecked") + Map out = (Map) YamlUtils.toYamlValue(in, null, 10); + assertTrue(out.keySet().stream().allMatch(k -> k instanceof String)); + assertEquals("two", out.get("2")); + assertEquals("ten", out.get("10")); + } + + @Test + public void testToYamlValueTwoArgDelegatesToUnboundedDepth() { + Condition c = new Condition(); + c.setConditionTypeId("c"); + c.getParameterValues().put("n", 1); + Object result = YamlUtils.toYamlValue(c, YamlUtils.newIdentityVisitedSet()); + assertTrue(result instanceof Map); + } + + @Test + public void testYamlConvertibleDefaultToYamlWithVisitedOnly() { + YamlUtils.YamlConvertible convertible = new YamlUtils.YamlConvertible() { + @Override + public Map toYaml(Set visited, int maxDepth) { + Map m = new LinkedHashMap<>(); + m.put("depth", maxDepth); + return m; + } + }; + Map map = convertible.toYaml(null); + assertEquals(Integer.valueOf(20), map.get("depth")); + } } diff --git a/pom.xml b/pom.xml index d4825e7ec..5a2c70fd0 100644 --- a/pom.xml +++ b/pom.xml @@ -80,6 +80,7 @@ 2.3 3.10 2.19.0 + 1.7.36 9.12.2 2.4.0 From df1dc2a8f1f568ddead960bb679d56af2fa8b2ac Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 18 May 2026 15:04:49 +0200 Subject: [PATCH 06/16] UNOMI-880 (split A0): migrate Elasticsearch integration tests to Docker Replace the com.github.alexcojocaru:elasticsearch-maven-plugin (binary download + forked JVM) with io.fabric8:docker-maven-plugin so the elasticsearch profile of itests runs ES in a Docker container, mirroring how the opensearch profile already runs OpenSearch. itests/pom.xml (elasticsearch profile) * Add an 9400 property and pass it through the failsafe systemPropertyVariables so tests resolve the HTTP port from a single source. * Replace the elasticsearch-maven-plugin block with a docker-maven-plugin block that starts/stops a docker.elastic.co/elasticsearch/elasticsearch container, binds target/snapshots_repository to /tmp/snapshots_repository, and waits on the HTTP port before the integration-test phase. * Add a chmod -R ugo+rwx on snapshots_repository in the antrun unzip step: the ES container runs as UID 1000, so on Linux CI the bind-mounted snapshot repo otherwise hits access_denied during repository verify. pom.xml (root) * Declare 0.48.0 and add the pluginManagement entry so the elasticsearch profile (and any future user of the plugin) inherits a single version. This is phase A0 of the PR #757 stack split (see docs/PR-757-stack-extraction-tracker.md). It is intentionally small and self-contained: no test/code changes, only the test infrastructure switch. --- itests/pom.xml | 91 ++++++++++++++++++++++++++++++++++---------------- pom.xml | 6 ++++ 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/itests/pom.xml b/itests/pom.xml index 98e98e9e5..0d0ccde8c 100644 --- a/itests/pom.xml +++ b/itests/pom.xml @@ -196,6 +196,14 @@ + + + + + + @@ -210,6 +218,9 @@ elasticsearch + + 9400 + true @@ -229,6 +240,7 @@ foo elasticsearch + ${elasticsearch.port} @@ -247,43 +259,64 @@ - com.github.alexcojocaru - elasticsearch-maven-plugin - - 6.29 + io.fabric8 + docker-maven-plugin - - contextElasticSearchITests - 9500 - 9400 - ${elasticsearch.test.version} - true - 120 - - -Xms4g -Xmx4g - - - - false - ${project.build.directory}/snapshots_repository - false - OPTIONS,HEAD,GET,POST,PUT,DELETE - Authorization,X-Requested-With,X-Auth-Token,Content-Type,Content-Length - - + itests-elasticsearch + + + docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version} + elasticsearch + + + ${elasticsearch.port}:9200 + + + single-node + -Xms8g -Xmx8g -Dcluster.default.index.settings.number_of_replicas=0 + false + false + /tmp/snapshots_repository + false + + + + ${project.build.directory}/snapshots_repository:/tmp/snapshots_repository + + + + + http://localhost:${elasticsearch.port} + GET + 200 + + + + ${project.build.directory}/elasticsearch-port.properties + + + - + + + remove-existing-container + pre-integration-test + + stop + remove + + + start-elasticsearch pre-integration-test - runforked + start + + true + stop-elasticsearch diff --git a/pom.xml b/pom.xml index 25360797d..f5c1c2aa5 100644 --- a/pom.xml +++ b/pom.xml @@ -146,6 +146,7 @@ 3.21.0 0.16.1 1.0-m5.1 + 0.48.0 v16.20.2 v1.22.19 @@ -865,6 +866,11 @@ dependency-check-maven ${dependency-check.plugin.version} + + io.fabric8 + docker-maven-plugin + ${docker-maven-plugin.version} + From a2e008cc9896abb252717994e554a1408706a8e8 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 18 May 2026 15:04:49 +0200 Subject: [PATCH 07/16] UNOMI-921: Replace elasticsearch-maven-plugin with Docker-based Elasticsearch in integration tests Implements the core configuration switch from UNOMI-921: https://issues.apache.org/jira/browse/UNOMI-921 Replace the com.github.alexcojocaru:elasticsearch-maven-plugin (binary download + forked JVM) with io.fabric8:docker-maven-plugin in the elasticsearch profile of itests, mirroring how the opensearch profile already runs OpenSearch in a Docker container. itests/pom.xml (elasticsearch profile) * Add an 9400 property and pass it through the failsafe systemPropertyVariables so tests resolve the HTTP port from a single source (unchanged from the previous 9400). * Replace the elasticsearch-maven-plugin block with a docker-maven-plugin block that runs docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version}, binds target/snapshots_repository to /tmp/snapshots_repository, and waits on the HTTP port before the integration-test phase. Heap aligned to 8GB (-Xms8g -Xmx8g) to match the OpenSearch configuration and the ES 9 recommendation. Discovery=single-node, replicas=0, xpack.ml and xpack.security disabled. * Container lifecycle matches OpenSearch exactly: pre-integration-test runs stop+remove then start (with showLogs); post-integration-test runs stop only -- container is kept around for inspection. * Add a chmod -R ugo+rwx on snapshots_repository in the antrun unzip step: the ES container runs as UID 1000, so on Linux CI the bind-mounted snapshot repo otherwise hits access_denied during repository verify. pom.xml (root) * Declare 0.48.0 and add the pluginManagement entry so the elasticsearch profile (and any future user of the plugin) inherits a single version. Scope kept minimal for the PR #757 stack split: only the test infrastructure switch lives here. The follow-up UNOMI-921 acceptance items below ship in the platform PR (P) once it lands: * Remove BaseIT.fixDefaultTemplateIfNeeded() and the call in checkSearchEngine() (no longer needed with Docker). * Migrate16xToCurrentVersionIT: replace hardcoded ES_BASE_URL = "http://localhost:9400" with dynamic getSearchPort(). * Drop the comments referring to the elasticsearch-maven-plugin template-override workaround. See docs/PR-757-stack-extraction-tracker.md for the full split plan and how this PR fits in the stack. --- itests/pom.xml | 91 ++++++++++++++++++++++++++++++++++---------------- pom.xml | 6 ++++ 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/itests/pom.xml b/itests/pom.xml index 98e98e9e5..0d0ccde8c 100644 --- a/itests/pom.xml +++ b/itests/pom.xml @@ -196,6 +196,14 @@ + + + + + + @@ -210,6 +218,9 @@ elasticsearch + + 9400 + true @@ -229,6 +240,7 @@ foo elasticsearch + ${elasticsearch.port} @@ -247,43 +259,64 @@ - com.github.alexcojocaru - elasticsearch-maven-plugin - - 6.29 + io.fabric8 + docker-maven-plugin - - contextElasticSearchITests - 9500 - 9400 - ${elasticsearch.test.version} - true - 120 - - -Xms4g -Xmx4g - - - - false - ${project.build.directory}/snapshots_repository - false - OPTIONS,HEAD,GET,POST,PUT,DELETE - Authorization,X-Requested-With,X-Auth-Token,Content-Type,Content-Length - - + itests-elasticsearch + + + docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version} + elasticsearch + + + ${elasticsearch.port}:9200 + + + single-node + -Xms8g -Xmx8g -Dcluster.default.index.settings.number_of_replicas=0 + false + false + /tmp/snapshots_repository + false + + + + ${project.build.directory}/snapshots_repository:/tmp/snapshots_repository + + + + + http://localhost:${elasticsearch.port} + GET + 200 + + + + ${project.build.directory}/elasticsearch-port.properties + + + - + + + remove-existing-container + pre-integration-test + + stop + remove + + + start-elasticsearch pre-integration-test - runforked + start + + true + stop-elasticsearch diff --git a/pom.xml b/pom.xml index 25360797d..f5c1c2aa5 100644 --- a/pom.xml +++ b/pom.xml @@ -146,6 +146,7 @@ 3.21.0 0.16.1 1.0-m5.1 + 0.48.0 v16.20.2 v1.22.19 @@ -865,6 +866,11 @@ dependency-check-maven ${dependency-check.plugin.version} + + io.fabric8 + docker-maven-plugin + ${docker-maven-plugin.version} + From 944c39bdf91238e937eac739c532eb7083e39db9 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 23 May 2026 10:31:07 +0200 Subject: [PATCH 08/16] UNOMI-937: Add build.sh CI mode for non-interactive GitHub Actions Non-interactive prompts when CI, GITHUB_ACTIONS, or BUILD_NON_INTERACTIVE is set. Add --ci (no Karaf, no Maven build cache) and MAVEN_EXTRA_OPTS for matrix ports. --- build.sh | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/build.sh b/build.sh index 19dd5d0f6..80435acc8 100755 --- a/build.sh +++ b/build.sh @@ -222,13 +222,23 @@ print_progress() { fi } +# Non-interactive when run from CI or when explicitly requested (e.g. GitHub Actions). +is_non_interactive() { + [ -n "${CI:-}" ] || [ -n "${GITHUB_ACTIONS:-}" ] || [ "${BUILD_NON_INTERACTIVE:-}" = "true" ] +} + # Function to prompt for continuation prompt_continue() { local prompt_text="$1" if [ -z "$prompt_text" ]; then prompt_text="Continue?" fi - + + if is_non_interactive; then + print_status "info" "Non-interactive mode: continuing ($prompt_text)" + return 0 + fi + read -p "$prompt_text (y/N) " -n 1 -r echo if [[ ! $REPLY =~ ^[Yy]$ ]]; then @@ -296,6 +306,7 @@ EOF echo -e " ${CYAN}--it-debug-port PORT${NC} Set integration test debug port" echo -e " ${CYAN}--it-debug-suspend${NC} Suspend integration test until debugger connects" echo -e " ${CYAN}--skip-migration-tests${NC} Skip migration-related tests" + echo -e " ${CYAN}--ci${NC} CI mode: no Karaf, no Maven build cache, non-interactive" else cat << "EOF" _ _ _____ _ ____ @@ -329,6 +340,7 @@ EOF echo " --it-debug-port PORT Set integration test debug port" echo " --it-debug-suspend Suspend integration test until debugger connects" echo " --skip-migration-tests Skip migration-related tests" + echo " --ci CI mode: no Karaf, no Maven build cache, non-interactive" fi echo @@ -459,6 +471,11 @@ while [ "$1" != "" ]; do --skip-migration-tests) SKIP_MIGRATION_TESTS=true ;; + --ci) + NO_KARAF=true + USE_MAVEN_CACHE=false + BUILD_NON_INTERACTIVE=true + ;; *) echo "Unknown option: $1" usage @@ -784,10 +801,14 @@ if [ "$MAVEN_OFFLINE" = true ]; then # Warn if purge cache is enabled with offline mode if [ "$PURGE_MAVEN_CACHE" = true ]; then echo "Warning: Purging Maven cache while in offline mode may cause build failures" - read -p "Continue anyway? (y/N) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - exit 1 + if is_non_interactive; then + print_status "warning" "Non-interactive mode: continuing despite purge + offline" + else + read -p "Continue anyway? (y/N) " -n 1 -r + echo + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + exit 1 + fi fi fi fi @@ -797,13 +818,22 @@ if [ "$USE_MAVEN_CACHE" = false ]; then MVN_OPTS="$MVN_OPTS -Dmaven.build.cache.enabled=false" fi +# Extra Maven options (e.g. CI matrix ports: -Delasticsearch.port=9400) +if [ -n "${MAVEN_EXTRA_OPTS:-}" ]; then + MVN_OPTS="$MVN_OPTS $MAVEN_EXTRA_OPTS" +fi + # Verify Maven settings if [ ! -f ~/.m2/settings.xml ]; then echo "Warning: Maven settings.xml not found at ~/.m2/settings.xml" - read -p "Continue anyway? (y/N) " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - exit 1 + if is_non_interactive; then + print_status "info" "Non-interactive mode: continuing without ~/.m2/settings.xml" + else + read -p "Continue anyway? (y/N) " -n 1 -r + echo + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + exit 1 + fi fi fi From eb294da69a3e5b28cd31cc75c2f6a260350b4ba0 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 23 May 2026 10:32:18 +0200 Subject: [PATCH 09/16] UNOMI-937: Harden flaky integration tests with keepTrying polling Replace fixed sleeps in GraphQLListIT and poll after patch refresh in PatchIT. --- .../java/org/apache/unomi/itests/PatchIT.java | 24 ++++++++++++----- .../unomi/itests/graphql/GraphQLListIT.java | 26 ++++++++++++------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/PatchIT.java b/itests/src/test/java/org/apache/unomi/itests/PatchIT.java index 098a03ad2..1c9cd5a43 100644 --- a/itests/src/test/java/org/apache/unomi/itests/PatchIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/PatchIT.java @@ -49,7 +49,10 @@ public void testPatch() throws IOException { profileService.refresh(); - newCompany = profileService.getPropertyType("company"); + newCompany = keepTrying("Failed waiting for patched property type", + () -> profileService.getPropertyType("company"), + pt -> pt != null && "foo".equals(pt.getDefaultValue()), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); Assert.assertEquals("foo", newCompany.getDefaultValue()); } finally { profileService.setPropertyType(company); @@ -68,7 +71,10 @@ public void testOverride() throws IOException { profileService.refresh(); - newGender = profileService.getPropertyType("gender"); + newGender = keepTrying("Failed waiting for patched property type", + () -> profileService.getPropertyType("gender"), + pt -> pt != null && "foo".equals(pt.getDefaultValue()), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); Assert.assertEquals("foo", newGender.getDefaultValue()); } finally { profileService.setPropertyType(gender); @@ -86,8 +92,8 @@ public void testRemove() throws IOException, InterruptedException { profileService.refresh(); - PropertyType newIncome = profileService.getPropertyType("income"); - Assert.assertNull(newIncome); + waitForNullValue("Failed waiting for property type removal", + () -> profileService.getPropertyType("income"), DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); } finally { profileService.setPropertyType(income); } @@ -105,7 +111,10 @@ public void testPatchOnConditionType() throws IOException, InterruptedException definitionsService.refresh(); - ConditionType newFormCondition = definitionsService.getConditionType("formEventCondition"); + ConditionType newFormCondition = keepTrying("Failed waiting for patched condition type", + () -> definitionsService.getConditionType("formEventCondition"), + ct -> ct != null && !ct.getMetadata().getSystemTags().contains("profileTags"), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); Assert.assertFalse(newFormCondition.getMetadata().getSystemTags().contains("profileTags")); } finally { definitionsService.setConditionType(formCondition); @@ -124,7 +133,10 @@ public void testPatchOnActionType() throws IOException, InterruptedException { definitionsService.refresh(); - ActionType newMailAction = definitionsService.getActionType("sendMailAction"); + ActionType newMailAction = keepTrying("Failed waiting for patched action type", + () -> definitionsService.getActionType("sendMailAction"), + at -> at != null && !at.getMetadata().getSystemTags().contains("availableToEndUser"), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); Assert.assertFalse(newMailAction.getMetadata().getSystemTags().contains("availableToEndUser")); } finally { definitionsService.setActionType(mailAction); diff --git a/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java b/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java index 96a6eb986..757904b72 100644 --- a/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java @@ -77,15 +77,23 @@ public void testCRUD() throws Exception { refreshPersistence(UserList.class); - Thread.sleep(6000); - - try (CloseableHttpResponse response = post("graphql/list/find-lists.json")) { - final ResponseContext context = ResponseContext.parse(response.getEntity()); - - Assert.assertEquals(1, ((Integer) context.getValue("data.cdp.findLists.totalCount")).intValue()); - Assert.assertEquals("testListId", context.getValue("data.cdp.findLists.edges[0].node.id")); - Assert.assertEquals(profile.getItemId(), context.getValue("data.cdp.findLists.edges[0].node.active.edges[0].node.cdp_profileIDs[0].id")); - } + final ResponseContext findListsContext = keepTrying("Failed waiting for profile in list query", + () -> { + try (CloseableHttpResponse response = post("graphql/list/find-lists.json")) { + return ResponseContext.parse(response.getEntity()); + } + }, + context -> { + Integer totalCount = (Integer) context.getValue("data.cdp.findLists.totalCount"); + if (totalCount == null || totalCount != 1) { + return false; + } + Object profileId = context.getValue("data.cdp.findLists.edges[0].node.active.edges[0].node.cdp_profileIDs[0].id"); + return profile.getItemId().equals(profileId); + }, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + Assert.assertEquals("testListId", findListsContext.getValue("data.cdp.findLists.edges[0].node.id")); try (CloseableHttpResponse response = post("graphql/list/delete-list.json")) { final ResponseContext context = ResponseContext.parse(response.getEntity()); From c2f30010c6a1701f331eac783bde4ee1dc56df41 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 23 May 2026 10:32:18 +0200 Subject: [PATCH 10/16] UNOMI-937: Add waitForProfileProperty helper for async rule updates Poll profile properties after events in PropertiesUpdateActionIT. --- .../test/java/org/apache/unomi/itests/BaseIT.java | 8 ++++++++ .../unomi/itests/PropertiesUpdateActionIT.java | 14 +++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java index 1c1bd6f8c..676639b0d 100644 --- a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java @@ -467,6 +467,14 @@ protected T keepTrying(String failMessage, Supplier call, Predicate pr return value; } + protected void waitForProfileProperty(String profileId, String propertyName, Object expected) + throws InterruptedException { + keepTrying("Profile " + profileId + " property " + propertyName + " not updated", + () -> profileService.load(profileId), + profile -> profile != null && java.util.Objects.equals(expected, profile.getProperty(propertyName)), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + } + protected void waitForNullValue(String failMessage, Supplier call, int timeout, int retries) throws InterruptedException { int count = 0; while (call.get() != null) { diff --git a/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java b/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java index a1e340680..e07d2bd5c 100644 --- a/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java @@ -78,7 +78,7 @@ public void setUp() throws InterruptedException { } @Test - public void testUpdateProperties_CurrentProfile() { + public void testUpdateProperties_CurrentProfile() throws InterruptedException { Profile profile = profileService.load(PROFILE_TARGET_TEST_ID); Assert.assertNull(profile.getProperty("firstName")); @@ -92,16 +92,13 @@ public void testUpdateProperties_CurrentProfile() { updateProperties.setProperty(UpdatePropertiesAction.TARGET_ID_KEY, PROFILE_TARGET_TEST_ID); updateProperties.setProperty(UpdatePropertiesAction.TARGET_TYPE_KEY, "profile"); - int changes = eventService.send(updateProperties); - - LOGGER.info("Changes of the event : {}", changes); + eventService.send(updateProperties); - Assert.assertTrue(changes > 0); - Assert.assertEquals("UPDATED FIRST NAME CURRENT PROFILE", profile.getProperty("firstName")); + waitForProfileProperty(PROFILE_TARGET_TEST_ID, "firstName", "UPDATED FIRST NAME CURRENT PROFILE"); } @Test - public void testUpdateProperties_NotCurrentProfile() { + public void testUpdateProperties_NotCurrentProfile() throws InterruptedException { Profile profile = profileService.load(PROFILE_TARGET_TEST_ID); Profile profileToUpdate = profileService.load(PROFILE_TEST_ID); Assert.assertNull(profileToUpdate.getProperty("firstName")); @@ -117,8 +114,7 @@ public void testUpdateProperties_NotCurrentProfile() { updateProperties.setProperty(UpdatePropertiesAction.TARGET_TYPE_KEY, "profile"); eventService.send(updateProperties); - profileToUpdate = profileService.load(PROFILE_TEST_ID); - Assert.assertEquals("UPDATED FIRST NAME", profileToUpdate.getProperty("firstName")); + waitForProfileProperty(PROFILE_TEST_ID, "firstName", "UPDATED FIRST NAME"); } @Test From 2611dea64f026ea2897bf5c9ef7235d3407f4e28 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 23 May 2026 10:32:18 +0200 Subject: [PATCH 11/16] UNOMI-937: Run GitHub Actions unit and IT jobs via build.sh Align CI with local developer workflow; pass matrix ports via MAVEN_EXTRA_OPTS. --- .github/workflows/unomi-ci-build-tests.yml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/unomi-ci-build-tests.yml b/.github/workflows/unomi-ci-build-tests.yml index e629253cd..2533a8b23 100644 --- a/.github/workflows/unomi-ci-build-tests.yml +++ b/.github/workflows/unomi-ci-build-tests.yml @@ -31,7 +31,7 @@ jobs: sudo apt-get install -y graphviz dot -V - name: Build and Unit tests - run: mvn -U -ntp -e clean install + run: ./build.sh --ci integration-tests: name: Execute integration tests @@ -54,16 +54,22 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' + - name: Install GraphViz + run: | + sudo apt-get update + sudo apt-get install -y graphviz + dot -V - name: Integration tests + env: + MAVEN_EXTRA_OPTS: >- + -Dopensearch.port=${{ matrix.port }} + -Delasticsearch.port=${{ matrix.port }} run: | - FLAGS="-Pintegration-tests" if [ "${{ matrix.search-engine }}" = "opensearch" ]; then - # Trigger OpenSearch profile activation via property; do not pass any -P profile toggles - FLAGS="$FLAGS -Duse.opensearch=true" + ./build.sh --ci --integration-tests --use-opensearch + else + ./build.sh --ci --integration-tests fi - mvn -ntp clean install $FLAGS \ - -Dopensearch.port=${{ matrix.port }} \ - -Delasticsearch.port=${{ matrix.port }} - name: Archive code coverage logs uses: actions/upload-artifact@v4 if: false # UNOMI-746 Reactivate if necessary From 88da78ad2be9f541b480e423a72021b85778e1b0 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 18 May 2026 15:04:49 +0200 Subject: [PATCH 12/16] UNOMI-921: Replace elasticsearch-maven-plugin with Docker-based Elasticsearch in integration tests Implements the core configuration switch from UNOMI-921: https://issues.apache.org/jira/browse/UNOMI-921 Replace the com.github.alexcojocaru:elasticsearch-maven-plugin (binary download + forked JVM) with io.fabric8:docker-maven-plugin in the elasticsearch profile of itests, mirroring how the opensearch profile already runs OpenSearch in a Docker container. itests/pom.xml (elasticsearch profile) * Add an 9400 property and pass it through the failsafe systemPropertyVariables so tests resolve the HTTP port from a single source (unchanged from the previous 9400). * Replace the elasticsearch-maven-plugin block with a docker-maven-plugin block that runs docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version}, binds target/snapshots_repository to /tmp/snapshots_repository, and waits on the HTTP port before the integration-test phase. Heap aligned to 8GB (-Xms8g -Xmx8g) to match the OpenSearch configuration and the ES 9 recommendation. Discovery=single-node, replicas=0, xpack.ml and xpack.security disabled. * Container lifecycle matches OpenSearch exactly: pre-integration-test runs stop+remove then start (with showLogs); post-integration-test runs stop only -- container is kept around for inspection. * Add a chmod -R ugo+rwx on snapshots_repository in the antrun unzip step: the ES container runs as UID 1000, so on Linux CI the bind-mounted snapshot repo otherwise hits access_denied during repository verify. pom.xml (root) * Declare 0.48.0 and add the pluginManagement entry so the elasticsearch profile (and any future user of the plugin) inherits a single version. Scope kept minimal for the PR #757 stack split: only the test infrastructure switch lives here. The follow-up UNOMI-921 acceptance items below ship in the platform PR (P) once it lands: * Remove BaseIT.fixDefaultTemplateIfNeeded() and the call in checkSearchEngine() (no longer needed with Docker). * Migrate16xToCurrentVersionIT: replace hardcoded ES_BASE_URL = "http://localhost:9400" with dynamic getSearchPort(). * Drop the comments referring to the elasticsearch-maven-plugin template-override workaround. See docs/PR-757-stack-extraction-tracker.md for the full split plan and how this PR fits in the stack. --- itests/pom.xml | 91 ++++++++++++++++++++++++++++++++++---------------- pom.xml | 6 ++++ 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/itests/pom.xml b/itests/pom.xml index 98e98e9e5..0d0ccde8c 100644 --- a/itests/pom.xml +++ b/itests/pom.xml @@ -196,6 +196,14 @@ + + + + + + @@ -210,6 +218,9 @@ elasticsearch + + 9400 + true @@ -229,6 +240,7 @@ foo elasticsearch + ${elasticsearch.port} @@ -247,43 +259,64 @@ - com.github.alexcojocaru - elasticsearch-maven-plugin - - 6.29 + io.fabric8 + docker-maven-plugin - - contextElasticSearchITests - 9500 - 9400 - ${elasticsearch.test.version} - true - 120 - - -Xms4g -Xmx4g - - - - false - ${project.build.directory}/snapshots_repository - false - OPTIONS,HEAD,GET,POST,PUT,DELETE - Authorization,X-Requested-With,X-Auth-Token,Content-Type,Content-Length - - + itests-elasticsearch + + + docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version} + elasticsearch + + + ${elasticsearch.port}:9200 + + + single-node + -Xms8g -Xmx8g -Dcluster.default.index.settings.number_of_replicas=0 + false + false + /tmp/snapshots_repository + false + + + + ${project.build.directory}/snapshots_repository:/tmp/snapshots_repository + + + + + http://localhost:${elasticsearch.port} + GET + 200 + + + + ${project.build.directory}/elasticsearch-port.properties + + + - + + + remove-existing-container + pre-integration-test + + stop + remove + + + start-elasticsearch pre-integration-test - runforked + start + + true + stop-elasticsearch diff --git a/pom.xml b/pom.xml index 174e68308..f7e85f403 100644 --- a/pom.xml +++ b/pom.xml @@ -146,6 +146,7 @@ 3.21.0 0.16.1 1.0-m5.1 + 0.48.0 v16.20.2 v1.22.19 @@ -867,6 +868,11 @@ dependency-check-maven ${dependency-check.plugin.version} + + io.fabric8 + docker-maven-plugin + ${docker-maven-plugin.version} + From 6e1221a02abe740a7c30e533b9c4e94ad1d7b894 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 29 May 2026 19:39:18 +0200 Subject: [PATCH 13/16] Fix GraphQLListIT compilation: restore LOGGER and catch block lost in stack sync --- .../org/apache/unomi/itests/graphql/GraphQLListIT.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java b/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java index 757904b72..60a9a8507 100644 --- a/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java @@ -21,10 +21,14 @@ import org.apache.unomi.lists.UserList; import org.junit.Assert; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Objects; public class GraphQLListIT extends BaseGraphQLIT { + private static final Logger LOGGER = LoggerFactory.getLogger(GraphQLListIT.class); + @Test public void testCRUD() throws Exception { Profile persistedProfile = null; @@ -81,9 +85,15 @@ public void testCRUD() throws Exception { () -> { try (CloseableHttpResponse response = post("graphql/list/find-lists.json")) { return ResponseContext.parse(response.getEntity()); + } catch (Exception e) { + LOGGER.warn("find-lists poll attempt failed: {}", e.getMessage()); + return null; } }, context -> { + if (context == null) { + return false; + } Integer totalCount = (Integer) context.getValue("data.cdp.findLists.totalCount"); if (totalCount == null || totalCount != 1) { return false; From 934c969a0e8661160b81559e1d9e48714c7164d7 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 29 May 2026 19:49:45 +0200 Subject: [PATCH 14/16] Fix YamlUtils thread safety, Condition null-safety, depth decrement, and deepCopy collection type --- .../apache/unomi/api/conditions/Condition.java | 15 ++++++--------- .../org/apache/unomi/api/utils/YamlUtils.java | 18 +++++++----------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java index 6dc1b9047..248d1c43a 100644 --- a/api/src/main/java/org/apache/unomi/api/conditions/Condition.java +++ b/api/src/main/java/org/apache/unomi/api/conditions/Condition.java @@ -28,8 +28,10 @@ import java.util.Collection; import java.util.HashMap; import java.util.IdentityHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import static org.apache.unomi.api.utils.YamlUtils.circularRef; @@ -158,19 +160,14 @@ public void setParameter(String name, Object value) { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - Condition condition = (Condition) o; - - if (!conditionTypeId.equals(condition.conditionTypeId)) return false; - return parameterValues.equals(condition.parameterValues); - + return Objects.equals(conditionTypeId, condition.conditionTypeId) + && Objects.equals(parameterValues, condition.parameterValues); } @Override public int hashCode() { - int result = conditionTypeId.hashCode(); - result = 31 * result + parameterValues.hashCode(); - return result; + return Objects.hash(conditionTypeId, parameterValues); } /** @@ -243,7 +240,7 @@ private Condition deepCopy(IdentityHashMap copying) { if (collection instanceof List) { copiedCollection = new ArrayList<>(); } else { - copiedCollection = new ArrayList<>(); + copiedCollection = new LinkedHashSet<>(); } for (Object item : collection) { if (item instanceof Condition) { diff --git a/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java index a3dcd3cd0..0755fbe6d 100644 --- a/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java +++ b/api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java @@ -34,17 +34,15 @@ * Provides utilities for building YAML structures and formatting them via SnakeYaml. */ public class YamlUtils { - // SnakeYaml instance with configured options - private static final Yaml YAML_INSTANCE; - - static { + // ThreadLocal because SnakeYAML's Yaml is not thread-safe + private static final ThreadLocal YAML_INSTANCE = ThreadLocal.withInitial(() -> { DumperOptions options = new DumperOptions(); options.setIndent(2); options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); options.setPrettyFlow(true); options.setDefaultScalarStyle(DumperOptions.ScalarStyle.PLAIN); - YAML_INSTANCE = new Yaml(options); - } + return new Yaml(options); + }); /** * Interface for objects that can convert themselves to YAML Map structures. @@ -282,11 +280,9 @@ public static Object toYamlValue(Object value, Set visited, int maxDepth return null; } if (value instanceof YamlConvertible) { - // For YamlConvertible, get the Map and then process it as a Map to ensure sorting - // Pass maxDepth - 1 to the toYaml method to continue depth limiting + // toYaml already decrements depth; pass maxDepth through unchanged for the map-sorting pass Map result = ((YamlConvertible) value).toYaml(visited, maxDepth - 1); - // Process the result as a Map to ensure it's sorted (this handles both sorting and recursive processing) - return toYamlValue(result, visited, maxDepth - 1); + return toYamlValue(result, visited, maxDepth); } if (value instanceof List) { return ((List) value).stream() @@ -318,7 +314,7 @@ public static Object toYamlValue(Object value, Set visited, int maxDepth * @return YAML string representation */ public static String format(Object value) { - return YAML_INSTANCE.dump(value); + return YAML_INSTANCE.get().dump(value); } /** From ad4f1bdc8b76d3c527e2fe455af19a90a2d09f5c Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 29 May 2026 20:14:48 +0200 Subject: [PATCH 15/16] Fix ConditionTest: expect LinkedHashSet after deepCopy of Set parameter, rename test --- .../java/org/apache/unomi/api/conditions/ConditionTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java b/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java index 35e6edf25..0fff914cf 100644 --- a/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java +++ b/api/src/test/java/org/apache/unomi/api/conditions/ConditionTest.java @@ -20,7 +20,6 @@ import org.apache.unomi.api.utils.YamlUtils; import org.junit.Test; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; @@ -121,7 +120,7 @@ public void testDeepCopyPreservesConditionTypeReference() { } @Test - public void testDeepCopyNestedConditionInSetBecomesArrayList() { + public void testDeepCopyNestedConditionInSetPreservesSetType() { Condition inner = new Condition(); inner.setConditionTypeId("inner"); Condition outer = new Condition(); @@ -132,7 +131,7 @@ public void testDeepCopyNestedConditionInSetBecomesArrayList() { Condition copy = outer.deepCopy(); Object copiedVal = copy.getParameterValues().get("conds"); - assertTrue(copiedVal instanceof ArrayList); + assertTrue(copiedVal instanceof LinkedHashSet); @SuppressWarnings("unchecked") Collection col = (Collection) copiedVal; assertEquals(1, col.size()); From f3af2ce8f274b50b1eab69b5ae56731f08382cea Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Fri, 29 May 2026 21:54:49 +0200 Subject: [PATCH 16/16] Revert testUpdateProperties_CurrentProfile to in-memory assertion UpdatePropertiesAction only updates event.getProfile() in-memory for the current-profile path; persistence timing is not synchronous so waitForProfileProperty reliably times out. Original comment on this test explicitly called this out. --- .../apache/unomi/itests/PropertiesUpdateActionIT.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java b/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java index e07d2bd5c..51368c503 100644 --- a/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/PropertiesUpdateActionIT.java @@ -78,7 +78,7 @@ public void setUp() throws InterruptedException { } @Test - public void testUpdateProperties_CurrentProfile() throws InterruptedException { + public void testUpdateProperties_CurrentProfile() { Profile profile = profileService.load(PROFILE_TARGET_TEST_ID); Assert.assertNull(profile.getProperty("firstName")); @@ -92,9 +92,11 @@ public void testUpdateProperties_CurrentProfile() throws InterruptedException { updateProperties.setProperty(UpdatePropertiesAction.TARGET_ID_KEY, PROFILE_TARGET_TEST_ID); updateProperties.setProperty(UpdatePropertiesAction.TARGET_TYPE_KEY, "profile"); - eventService.send(updateProperties); - - waitForProfileProperty(PROFILE_TARGET_TEST_ID, "firstName", "UPDATED FIRST NAME CURRENT PROFILE"); + int changes = eventService.send(updateProperties); + Assert.assertTrue("eventService.send() reported no changes — action may not have fired", changes > 0); + // UpdatePropertiesAction updates event.getProfile() in-memory for the current-profile path; + // persistence is not guaranteed synchronous here so we assert the in-memory state directly. + Assert.assertEquals("UPDATED FIRST NAME CURRENT PROFILE", profile.getProperty("firstName")); } @Test