From 3ce73e78198edb511e0313c1b9f2f84cdcb106a0 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Tue, 5 May 2026 17:17:56 +0200 Subject: [PATCH 1/6] 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 af8c962206..4d9ed7fbd0 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 de283ebe9a..9836cf40df 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 9a112e7664..ec25c7fa89 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 78a419863a..35ef596a25 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 4833c5a5f1..425d86280c 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 b3505bbe82..29c6927e1c 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 5ff336d9ff..e170c779b5 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 526ff463f7..812ee58628 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 c8a43225c1..e8af9c41fc 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 1b2ff0e88f..12694b3d7e 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 0aedfb1238..63750c0d44 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 6018fb376c..3d82e6e7e0 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 1af5147fbe..bfc5a21ccb 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 4e0d338306..2729ec74d0 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 0000000000..664f63fb79 --- /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 0000000000..c40c066ca6 --- /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 69cb5a0518..17b0f37529 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 98e98e9e56..d8b5d4a5ed 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 25360797d5..d4825e7ec3 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 bc7ab9acf2..d6257c6f09 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 fe3fe8fb02..8b6e7063c9 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 2/6] 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 0b8b7c4278..c823fc4720 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 3/6] 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 542b973b65..39efa7cbe7 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 4/6] 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 e629253cd6..4cb248c54a 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 5/6] 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 4d9ed7fbd0..89d7d56add 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 9836cf40df..4828025885 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 ec25c7fa89..4f650589f2 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 35ef596a25..68cf3b3248 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 425d86280c..7fc7c7453b 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 29c6927e1c..aeaba1bc6f 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 e170c779b5..5da9493496 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 812ee58628..6dc1b90474 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 e8af9c41fc..3d22c00a3c 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 12694b3d7e..d2cac5db85 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 63750c0d44..cd6f751e5d 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 3d82e6e7e0..b32aa8ab08 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 bfc5a21ccb..ab79d132ac 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 2729ec74d0..126279c15f 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 664f63fb79..a3dcd3cd09 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 0000000000..2ed12d0c68 --- /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 0000000000..35e6edf250 --- /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 c40c066ca6..af41505121 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 d4825e7ec3..5a2c70fd02 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 6/6] 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 98e98e9e56..0d0ccde8c5 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 25360797d5..f5c1c2aa58 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} +