sergehuber opened a new pull request, #768: URL: https://github.com/apache/unomi/pull/768
> **Note:** This PR replaces #754, which was accidentally closed when its base branch (`UNOMI-921-itests-es-docker`) was deleted after being merged. All commits, reviews, and fixes from #754 are preserved in this branch — see that PR for the full inline review thread. --- **JIRA:** https://issues.apache.org/jira/browse/UNOMI-920 ### Why this change Core Unomi API objects are deeply nested and interconnected — conditions recurse, rules bundle conditions and actions, metadata types carry parameters, and real graphs can contain **cycles** (for example when inspecting profile ↔ event ↔ session). In many places, **`toString()` is missing**, falls back to **`Object.toString()`**, or uses **ad-hoc concatenation** that is hard to read in logs and breakpoints. That slows down **live debugging, operational triage, and support**, because engineers cannot see structure and field names at a glance. **YAML-formatted** output gives **indentation**, **named fields**, and **readable nesting**, which makes logged or inspected values far easier to scan than opaque object identities or flat string blobs. Together with **explicit circular-reference markers**, we get safe, repeatable dumps suitable for logs and copy/paste into issues or docs. ### What changed - Introduce **`YamlUtils`** (SnakeYaml) with **`YamlConvertible`**, **`YamlMapBuilder`**, identity-based **cycle detection**, optional **depth limits**, and **`$ref: circular`** where the same instance reappears. - Implement **`toYaml` / `toString`** on key **`Item`** / **`MetadataItem`** types (including **`Condition`**, **`ConditionType`**, **`Action`**, **`ActionType`**, **`Rule`**, **`Segment`**, **`Goal`**, **`Scoring`**, **`ScoringElement`**, **`Parameter`**, **`Metadata`**, etc.). - Add **`YamlUtilsTest`** for the helper API and representative usage. ### Supporting changes - **`unomi-api`:** SnakeYAML + test dependencies; **`mockito-core`** version managed in **`unomi-bom`** (`mockito.version`). - **`RESTParameter`:** **`defaultValue`** as **`Object`** to match **`Parameter#getDefaultValue()`**. - **`RulesServiceImpl`:** avoid **`null`** **`defaultValue`** before splitting tracked-condition parameter strings. - **`unomi-itests`:** pin **`awaitility`** to **3.1.6** for Karaf/OSGi (Hamcrest 1.3 bundle), Karaf itests logback exclusions, Hamcrest bundle `test` scope (aligned with `unomi-3-dev`). ### Review history (from #754) All 8 Copilot review comments were addressed in commit `bcf5a6898`: | # | File | Issue | Resolution | |---|------|-------|------------| | 1 | `YamlUtils.java` | Circular-reference tracking used `HashSet` (equality-based), risking false "already visited" hits on types that override `equals` by content | Added `YamlUtils.newIdentityVisitedSet()` (`IdentityHashMap`-backed); all implementations switched to it; javadoc updated; test added | | 2 | `Condition.java` | `deepCopy()` had no cycle guard — self-referential graphs would cause `StackOverflowError` | `deepCopy()` now uses an `IdentityHashMap` visited set and throws `IllegalStateException` with a clear message on cycle detection | | 3 | `Condition.java` | Null guard on `getParameter()` implied `parameterValues` can be null, but `containsParameter`/`setParameter`/`equals`/`hashCode` would NPE | `setParameterValues(null)` normalised to empty `HashMap`; `containsParameter`/`setParameter` made null-safe; tests added | | 4 | `Parameter.java` | Max-depth fallback used a `"validation"` key — copy/paste artifact producing misleading YAML | Fixed to use standard keys (`id`, `type`, `multivalued`, `defaultValue`) with `"<max depth exceeded>"` placeholder | | 5 | `Parameter.java` | `serialVersionUID` changed — could break cross-version Java deserialization | Acknowledged: Unomi stores via JSON/OpenSearch, not Java serialization; added class-level comment documenting this | | 6 | `Item.java` | Same `serialVersionUID` concern for the widely-used `Item` base class | Same rationale and comment as above | | 7 | `pom.xml` | Added `<slf4j.version>` property that was not referenced anywhere | Now wired to `slf4j-api` and `slf4j-simple` in `unomi-api/pom.xml` with a clarifying comment | | 8 | `YamlUtilsTest.java` | Double-brace initialisation creates anonymous inner classes that can capture enclosing instances | Replaced with explicit nested `HashMap` instances | - [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) -- 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]
