UNOMI-920 Improve debugging with YAML-based toString() for core API types#754
Open
sergehuber wants to merge 10 commits into
Open
UNOMI-920 Improve debugging with YAML-based toString() for core API types#754sergehuber wants to merge 10 commits into
sergehuber wants to merge 10 commits into
Conversation
- 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.
There was a problem hiding this comment.
Pull request overview
This PR introduces a YAML-based debugging representation (toYaml() + YAML-formatted toString()) for core Unomi API model types to make deeply nested graphs (including cycles) easier to inspect in logs and debuggers. It also adjusts supporting dependency management and aligns defaultValue typing across API/REST.
Changes:
- Add
YamlUtils(+ tests) and implement YAML conversion / YAMLtoString()across multiple core API types with cycle detection and depth limiting. - Update
Parameter/RESTParameterdefaultValuetoObjectand harden rule tracked-parameter parsing against null default values. - Update BOM/root/itests poms for SnakeYAML and test-library version management/overrides (Mockito, Awaitility, JUnit Jupiter).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java | Avoid NPE when splitting tracked-condition parameter default values. |
| rest/src/main/java/org/apache/unomi/rest/models/RESTParameter.java | Align REST model defaultValue type with API (Object). |
| pom.xml | Add/adjust shared version properties for new test deps (plus SLF4J property). |
| itests/pom.xml | Override Awaitility for OSGi/Hamcrest compatibility; adjust test dependency exclusions/scopes. |
| bom/pom.xml | Manage Mockito/JUnit Jupiter/Awaitility versions in the BOM. |
| api/src/main/java/org/apache/unomi/api/utils/YamlUtils.java | New YAML builder/formatting utilities and YAML conversion API. |
| api/src/test/java/org/apache/unomi/api/utils/YamlUtilsTest.java | Unit tests for YAML utilities and circular reference behavior. |
| api/src/main/java/org/apache/unomi/api/Item.java | Add YAML conversion + YAML toString() base implementation for Items. |
| api/src/main/java/org/apache/unomi/api/MetadataItem.java | Add YAML conversion + YAML toString() including metadata. |
| api/src/main/java/org/apache/unomi/api/Metadata.java | Add YAML conversion + YAML toString() for metadata. |
| api/src/main/java/org/apache/unomi/api/Parameter.java | Change defaultValue to Object; add YAML conversion + YAML toString(). |
| api/src/main/java/org/apache/unomi/api/conditions/Condition.java | Add YAML conversion + YAML toString(); add deepCopy(); null-safety adjustments. |
| api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java | Add YAML conversion for condition type definitions. |
| api/src/main/java/org/apache/unomi/api/actions/Action.java | Add YAML conversion + YAML toString() for actions. |
| api/src/main/java/org/apache/unomi/api/actions/ActionType.java | Add YAML conversion for action type definitions. |
| api/src/main/java/org/apache/unomi/api/rules/Rule.java | Add YAML conversion for rules with condition/actions/linkage. |
| api/src/main/java/org/apache/unomi/api/segments/Segment.java | Add YAML conversion for segments. |
| api/src/main/java/org/apache/unomi/api/segments/Scoring.java | Add YAML conversion for scoring definitions. |
| api/src/main/java/org/apache/unomi/api/segments/ScoringElement.java | Add YAML conversion + YAML toString() for scoring elements. |
| api/src/main/java/org/apache/unomi/api/goals/Goal.java | Add YAML conversion for goals. |
| api/pom.xml | Add SnakeYAML dependency and test deps for new YAML utility tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
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.
Contributor
Author
|
Thanks for the review. The eight inline points are addressed in bcf5a68 (identity-based visited sets, Condition null/deepCopy hardening, Parameter max-depth YAML fix, slf4j.version wiring, test cleanup/expansion). Unit and integration tests are green on our side. |
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 <elasticsearch.port>9400</elasticsearch.port> 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 <docker-maven-plugin.version>0.48.0</docker-maven-plugin.version> 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.
1 task
sergehuber
force-pushed
the
UNOMI-itests-es-docker
branch
from
df1dc2a to
a2e008c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 toObject.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
YamlUtils(SnakeYaml) withYamlConvertible,YamlMapBuilder, identity-based cycle detection, optional depth limits, and$ref: circularwhere the same instance reappears.toYaml/toStringon keyItem/MetadataItemtypes (includingCondition,ConditionType,Action,ActionType,Rule,Segment,Goal,Scoring,ScoringElement,Parameter,Metadata, etc.).YamlUtilsTestfor the helper API and representative usage.Supporting changes
unomi-api: SnakeYAML + test dependencies;mockito-coreversion managed inunomi-bom(mockito.version).RESTParameter:defaultValueasObjectto matchParameter#getDefaultValue().RulesServiceImpl: avoidnulldefaultValuebefore splitting tracked-condition parameter strings.unomi-itests: pinawaitilityto 3.1.6 for Karaf/OSGi (Hamcrest 1.3 bundle), Karaf itests logback exclusions, Hamcrest bundletestscope (aligned withunomi-3-dev).