Copilot commented on code in PR #754:
URL: https://github.com/apache/unomi/pull/754#discussion_r3239481336
##########
api/src/main/java/org/apache/unomi/api/conditions/Condition.java:
##########
@@ -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<String, Object> toYaml(Set<Object> visited, int maxDepth) {
+ if (maxDepth <= 0) {
+ return YamlMapBuilder.create()
+ .put("type", conditionTypeId != null ? conditionTypeId :
"Condition")
+ .put("parameterValues", "<max depth exceeded>")
+ .build();
+ }
+ if (visited != null && visited.contains(this)) {
+ return circularRef();
+ }
+ final Set<Object> 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() {
Review Comment:
deepCopy() performs unbounded recursion on nested Condition values and does
not guard against cycles (which the PR explicitly calls out as possible in real
graphs). A self-referential/nested cycle in parameterValues will cause infinite
recursion/StackOverflowError. Consider adding identity-based visited tracking
(and/or maxDepth) similar to YAML serialization, or clearly documenting that
deepCopy does not support cyclic graphs.
##########
api/src/main/java/org/apache/unomi/api/Parameter.java:
##########
@@ -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<String, Object> toYaml(Set<Object> visited, int maxDepth) {
+ if (maxDepth <= 0) {
+ return YamlUtils.YamlMapBuilder.create()
+ .put("id", id)
+ .put("validation", "<max depth exceeded>")
Review Comment:
The maxDepth fallback map uses a "validation" key, but Parameter has no
validation field. This looks like a copy/paste artifact and produces misleading
YAML output. Replace it with an appropriate placeholder (e.g.,
defaultValue/type/multivalued) or keep the same keys as the non-depth-limited
branch.
##########
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<String, Object> toYaml(Set<Object> 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<String, Object> toYaml(Set<Object> 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<String, Object> 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<String, Object> 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:
+ * <pre>
+ * return YamlMapBuilder.create()
+ * .mergeObject(super.toYaml(visitedSet))
+ * .putIfNotNull("field", value)
+ * .build();
+ * </pre>
+ *
+ * @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<String, Object> 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<String, Object> 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 <T extends Comparable<T>> List<T> setToSortedList(Set<T>
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 <T, R extends Comparable<R>> List<R> setToSortedList(Set<T>
set, Function<T, R> 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)
Review Comment:
Circular-reference tracking uses plain Set/HashSet in implementations, which
relies on equals()/hashCode(). Many of these API types override equals (e.g.,
Item by itemId, Condition by content), so different instances can be treated as
already-visited and incorrectly emit $ref: circular (or fail to detect true
identity cycles). Use identity-based tracking (e.g.,
Collections.newSetFromMap(new IdentityHashMap<>())) and standardize visited-set
creation in YamlUtils to ensure consistent behavior.
##########
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 {
Review Comment:
serialVersionUID was changed. If any persisted Java-serialized Parameter
instances exist (or cross-version serialization happens), this will break
deserialization compatibility. If compatibility is expected, keep the previous
serialVersionUID; otherwise, please add a brief comment explaining the
intentional compatibility break.
##########
api/src/main/java/org/apache/unomi/api/conditions/Condition.java:
##########
@@ -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;
}
Review Comment:
This null-guard implies parameterValues may legitimately be null (also
reflected by other new null checks), but other methods (containsParameter,
setParameter, equals/hashCode) still assume parameterValues is non-null and can
throw NPE. Either enforce non-null in setParameterValues (convert null to empty
map) or update the remaining methods to handle null consistently.
##########
api/src/main/java/org/apache/unomi/api/Item.java:
##########
@@ -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;
Review Comment:
serialVersionUID was changed. Since Item is widely used and implements
Serializable, this can break deserialization of previously serialized objects
across versions. If Java serialization compatibility is required, retain the
old serialVersionUID (or document why it is safe/intentional to change).
##########
pom.xml:
##########
@@ -80,6 +80,7 @@
<snakeyaml.version>2.3</snakeyaml.version>
<opencsv.version>3.10</opencsv.version>
<log4j.version>2.19.0</log4j.version>
+ <slf4j.version>1.7.36</slf4j.version>
Review Comment:
The slf4j.version property is introduced here but is not referenced
elsewhere in the build (no ${slf4j.version} usages). Consider removing it to
avoid implying the project is managing SLF4J versions via this property when it
currently isn't.
##########
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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> map1 = builder.build();
+ Map<String, Object> 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<String> set = new LinkedHashSet<>(Arrays.asList("zebra", "apple",
"banana"));
+ List<String> 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<String> result = YamlUtils.setToSortedList((Set<String>) null);
+ assertNull("Null set should return null", result);
+ }
+
+ @Test
+ public void testSetToSortedListEmpty() {
+ List<String> result =
YamlUtils.setToSortedList(Collections.<String>emptySet());
+ assertNull("Empty set should return null", result);
+ }
+
+ @Test
+ public void testSetToSortedListWithMapper() {
+ Set<Integer> set = new LinkedHashSet<>(Arrays.asList(3, 1, 2));
+ List<String> 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.<Integer,
String>setToSortedList(Collections.singleton(1), null);
+ fail("Null mapper should throw NullPointerException");
+ } catch (NullPointerException e) {
+ // Expected
+ }
+ }
+
+ @Test
+ public void testSetToSortedListWithMapperNullSet() {
+ List<String> 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<String, Object> map = new LinkedHashMap<>();
+ map.put("test", "value");
+ return map;
+ };
+ Set<Object> 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<Object> list = Arrays.asList("a", "b", "c");
+ Set<Object> 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<String, Object> map = new LinkedHashMap<>();
+ map.put("key", "value");
+ Set<Object> 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<Object> visited = new HashSet<>();
+ Object result = YamlUtils.toYamlValue(null, visited);
+ assertNull("Null should return null", result);
+ }
+
+ @Test
+ public void testToYamlValueWithPrimitive() {
+ Set<Object> visited = new HashSet<>();
+ Object result = YamlUtils.toYamlValue(42, visited);
+ assertEquals("Primitive should remain unchanged", 42, result);
+ }
+
+ @Test
+ public void testCircularRef() {
+ Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> conditionMap = (Map<String, Object>)
result.get("condition");
+ assertNotNull("Condition should be serialized", conditionMap);
+ Map<String, Object> paramValues = (Map<String, Object>)
conditionMap.get("parameterValues");
+ assertNotNull("Parameter values should exist", paramValues);
+ Map<String, Object> circularRef = (Map<String, Object>)
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<String, Object> 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<String, Object> actionMap = (Map<String, Object>) actions.get(0);
+ Map<String, Object> paramValues = (Map<String, Object>)
actionMap.get("parameterValues");
+ assertNotNull("Parameter values should exist", paramValues);
+ Map<String, Object> circularRef = (Map<String, Object>)
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<String, Object> result = rule.toYaml(null);
+ assertNotNull("Rule should serialize to YAML", result);
+
+ // Navigate through the nested structure
+ Map<String, Object> conditionMap = (Map<String, Object>)
result.get("condition");
+ Map<String, Object> paramValues = (Map<String, Object>)
conditionMap.get("parameterValues");
+ Map<String, Object> nestedConditionMap = (Map<String, Object>)
paramValues.get("nested");
+ Map<String, Object> nestedParamValues = (Map<String, Object>)
nestedConditionMap.get("parameterValues");
+ Map<String, Object> circularRef = (Map<String, Object>)
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<String, Object> result = rule.toYaml(null);
+ Map<String, Object> conditionMap = (Map<String, Object>)
result.get("condition");
+ Map<String, Object> paramValues = (Map<String, Object>)
conditionMap.get("parameterValues");
+
+ // All three references should show circular ref
+ for (String key : Arrays.asList("rule1", "rule2", "rule3")) {
+ Map<String, Object> circularRef = (Map<String, Object>)
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<String, Object> result = rule.toYaml(null);
+ Map<String, Object> conditionMap = (Map<String, Object>)
result.get("condition");
+ Map<String, Object> paramValues = (Map<String, Object>)
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<String, Object> circularRef1 = (Map<String, Object>)
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<String, Object> circularRef2 = (Map<String, Object>)
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<String, Object> nestedMap = new HashMap<>();
+ nestedMap.put("level1", new HashMap<String, Object>() {{
+ put("level2", new HashMap<String, Object>() {{
+ put("rule", rule);
+ }});
+ }});
Review Comment:
Avoid double-brace initialization in tests (it creates anonymous inner
classes and can capture enclosing instances). A simple explicit nested Map
construction is clearer and avoids the extra class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
