sergehuber commented on code in PR #754: URL: https://github.com/apache/unomi/pull/754#discussion_r3252434867
########## 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: Thanks — agreed. We added YamlUtils.newIdentityVisitedSet() (Collections.newSetFromMap(new IdentityHashMap<>())) and switched all YamlConvertible implementations to use it when creating a new visited set. Javadoc on YamlConvertible now documents identity semantics. Added YamlUtilsTest.testNewIdentityVisitedSetUsesReferenceIdentity() to lock this in. ########## 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: Good catch. deepCopy() now uses an IdentityHashMap-based visiting set and throws IllegalStateException with a clear message if a cyclic Condition graph is detected. Javadoc documents that cyclic graphs are unsupported. Tests: testConditionDeepCopyRejectsCycle and related coverage in ConditionTest / YamlUtilsTest. -- 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]
