Skip to content

CompositeCodec constructors require at least one delegate #10189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 16, 2025

Conversation

cppwfs
Copy link
Contributor

@cppwfs cppwfs commented Jul 3, 2025

  • In the current implementation, users could pass in an empty list to the composite codec.
  • This is an CompositeCodec and if we had no codecs, then why have the class.
  • All Nullable's were removed from the delegate attribute.
  • Null returns from the findDelegate have been removed. The default delegate will be returned in the stead.
  • Update the tests to require at least one codec in the delegates

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add your name to the @author list of all the affected classes.

@cppwfs cppwfs changed the title Adjust the behavior of the class so that delegates are required. CompositeCodec constructors require delegates Jul 3, 2025
@cppwfs cppwfs changed the title CompositeCodec constructors require delegates CompositeCodec constructors require at least one delegate Jul 3, 2025
@cppwfs cppwfs force-pushed the SI-CLEANUP-COMPOSITE_CODEC branch from 182d2d9 to 77397a3 Compare July 3, 2025 18:55
@cppwfs cppwfs force-pushed the SI-CLEANUP-COMPOSITE_CODEC branch 2 times, most recently from 1d1e239 to 539236d Compare July 11, 2025 13:53
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And since we are here, please, consider to mention this class in the codec.adoc.
Thanks

@cppwfs cppwfs force-pushed the SI-CLEANUP-COMPOSITE_CODEC branch 2 times, most recently from 8f381d9 to 40070c4 Compare July 11, 2025 21:58
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And add some paragraph with a sample code into codec.adoc, please.

@cppwfs cppwfs force-pushed the SI-CLEANUP-COMPOSITE_CODEC branch from 40070c4 to c71bf37 Compare July 14, 2025 19:41
cppwfs added 10 commits July 15, 2025 15:44
* In the current implementation, users could pass in an empty list to the composite codec.
* This is an CompositeCodec and if we had no codecs, then why have the class.
* All @nullable's were removed from the delegate attribute.
* Null returns from the findDelegate have been removed.   The default delegate will be returned in the stead.
* Update the tests to require at least one codec in the delegates
And also test if a user attempts to an invalid class.
Change the  `ClassUtils.findClosestMatch` `failOnTie` to true.
  In the past when finding the appropriate delegate(codec) for a object and
  their were multiple delegates available the first one was returned.
  The problem is it could return a different delegate on each launch of the `findDelegate`.
  Now with `failOnTie` set to true a `IllegalStateException` is returned.

Remove deprecated constructor from CompositeCodec
…maintenance

Rename field names of foo and foo2 in CompositeCodecTests to professional descriptive names
In the onlyDefaultCodec implementation in the CompositeCodecTests use a class other than a PojoCodec class to simulate a real world case.
Enhance javadoc for CompositeCodec to include information on ClassUtils.findClosestMatch and its effects on codec selection.
Make sure @see is in the correct location in javadoc
Provide example for using CompositeCodec.
Remove @see in CompositeCodec javadocs.   This is handled in the links in the javadoc
Add code snippet to references to classes
Verify no tabs exist in sample
@cppwfs cppwfs force-pushed the SI-CLEANUP-COMPOSITE_CODEC branch from c71bf37 to bd73cd5 Compare July 15, 2025 20:07
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
But would be great to have #10207 reviewed and merged.
So, we could rebase this one before merge to working upstream.

Thanks

@artembilan artembilan merged commit d731e1c into spring-projects:main Jul 16, 2025
2 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants