[Fix #1271] Improving CustomObjectMarshaller resolution#1272
[Fix #1271] Improving CustomObjectMarshaller resolution#1272fjtirado merged 1 commit intoserverlessworkflow:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses #1271 by making CustomObjectMarshaller selection deterministic when multiple marshallers are applicable, aiming to choose the “closest” match in the class hierarchy (e.g., prefer Employee over Person/Object for Student extends Employee).
Changes:
- Adds a
Studenttest type (subclassingEmployee) to reproduce the ambiguous-resolution case. - Updates
MarshallingUtilsTestto assert the expected marshaller is chosen forStudent. - Refactors
MarshallingUtils.getCustomMarshallerto consider class “distance” (and adds a cache).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java | Updates marshaller resolution logic and introduces caching for resolved marshallers. |
| impl/core/src/test/java/io/serverlessworkflow/impl/marshaller/MarshallingUtilsTest.java | Expands test coverage to include Student and adjusts priorities/order expectations. |
| impl/core/src/test/java/io/serverlessworkflow/impl/marshaller/Student.java | Adds a subclass used to validate “closest match” marshaller selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractOutputBuffer.java
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractOutputBuffer.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractInputBuffer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractOutputBuffer.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractInputBuffer.java
Outdated
Show resolved
Hide resolved
impl/core/src/test/java/io/serverlessworkflow/impl/marshaller/MarshallingUtilsTest.java
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
e015f79 to
751857e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/core/src/test/java/io/serverlessworkflow/impl/marshaller/SerializableClass.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractInputBuffer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/AbstractOutputBuffer.java
Show resolved
Hide resolved
…tion Signed-off-by: fjtirado <ftirados@redhat.com>
Fix #1271