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

Allow schema and validation event handler customization in JAXBContextFactory #2084

Merged
merged 7 commits into from Jul 2, 2023

Conversation

virtual-machinist
Copy link
Contributor

The default JAXB behavior is to silently ignore errors caused by input being incorrect, so eg. an Integer field will remain null for an input foo. This isn't always preferred, but to fix that a ValidationEventHandler implementation must be assigned to the created Unmarshaller. It may be as simple as event -> false.

For a more complex validation scenario a Schema may be needed.

However at the moment there is no way to set up either of those in Marshaller and Unmarshaller instances created in JAXB and SOAP encoders/decoders.

The PR addresses this problem by adding the customization options for feign.jaxb.JAXBContextFactory in jaxb and jaxb-jakarta modules. A customized JAXBContextFactory may then be used to address the original issue in #1479 .

@@ -51,6 +78,10 @@ public Unmarshaller createUnmarshaller(Class<?> clazz) throws JAXBException {
public Marshaller createMarshaller(Class<?> clazz) throws JAXBException {
Marshaller marshaller = getContext(clazz).createMarshaller();
setMarshallerProperties(marshaller);
if (marshallerEventHandler != null) {
Copy link
Member

Choose a reason for hiding this comment

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

So, this is piling more and more functionality/code into feign to allow fine tuning... wonder if it would be possible to turn this into an interface that allows user to do whatever they to JAXBContext and pass it to feign in a more transparent way..

@virtual-machinist
Copy link
Contributor Author

virtual-machinist commented Jun 19, 2023 via email

@velo
Copy link
Member

velo commented Jun 20, 2023

How's that sound?

sounds great, would avoid all ifs inside feign, if you keen to do it, I would like to see it

@velo velo merged commit 7e9e547 into OpenFeign:master Jul 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants