Skip to content
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

fixes #9577 #9580

Draft
wants to merge 5 commits into
base: 4.0.x
Choose a base branch
from
Draft

fixes #9577 #9580

wants to merge 5 commits into from

Conversation

n0tl3ss
Copy link
Member

@n0tl3ss n0tl3ss commented Jul 19, 2023

  • If serialisation doesn't work a ConversionErrorException will be thrown.

Message from a provided example:

Failed to convert argument [E] for value [null] due to: No deserializable introspection present for type: Constraint constraint. Consider adding Serdeable.Deserializable annotate to type Constraint constraint. Alternatively if you are not in control of the project's source code, you can use @SerdeImport(Constraint.class) to enable deserialization of this type.
image

@graemerocher graemerocher marked this pull request as draft July 19, 2023 14:08
@n0tl3ss n0tl3ss marked this pull request as ready for review July 21, 2023 13:40
json-core/build.gradle Outdated Show resolved Hide resolved
@@ -176,6 +177,9 @@ protected TypeConverter<Map, Object> mapToObjectConverter() {
}
ArgumentBinder binder = this.beanPropertyBinder.get();
ArgumentBinder.BindingResult result = binder.bind(conversionContext, correctKeys(map));
conversionContext.getLastError().ifPresent(error -> {
throw new ConversionErrorException(conversionContext.getArgument(), error);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

this will modify universally for all type conversions that use this type converter which is a breaking change.

We need to modify only the place where the conversion happens to throw this error

Copy link
Contributor

Choose a reason for hiding this comment

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

@n0tl3ss can you address @graemerocher PR feedback? thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sdelamo Offline I told Graeme that I think this is the right place where I should expect error to be thrown. He told me that he will take a look how better to do it.

gradle/libs.versions.toml Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jul 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@graemerocher graemerocher self-assigned this Aug 3, 2023
@graemerocher graemerocher marked this pull request as draft August 3, 2023 13:45
@sdelamo sdelamo modified the milestones: 4.2.0, 4.3.0 Nov 10, 2023
@CLAassistant
Copy link

CLAassistant commented Feb 7, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants