Skip to content

fix #7036: Jackson 2.19.0 update issues #7038

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 11 commits into from
May 14, 2025
Merged

Conversation

r1c4r60
Copy link
Contributor

@r1c4r60 r1c4r60 commented Apr 28, 2025

Description

Fixes #7036
Supersedes closes #7035

This PR upgrades Jackson from 2.18.3 to 2.19.0 and introduces required changes to align with the stricter API and serialization behavior introduced in the new version.

Key Changes

  • UnmatchedFieldTypeModule:
    Updated the BeanSerializerModifier to correctly skip wrapping the property associated with @JsonAnyGetter (commonly additionalProperties) to prevent double-serialization and runtime errors.

  • SettableBeanPropertyDelegating:
    Implemented the missing unwrapped(NameTransformer) method to fully comply with the SettableBeanProperty contract and ensure compatibility with Jackson internals.


✅ Resolved Test Failures

io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModuleTest

  • writeValueAsStringWithAdditionalPropertiesOverridingFields
  • writeValueAsStringWithAdditionalPropertiesOverridingFieldsShouldLogWarning
  • writeValueAsStringWithAdditionalPropertiesOverridingFieldsShouldNotLogWarning

io.fabric8.kubernetes.model.jackson.SettableBeanPropertyDelegatingTest

  • SettableBeanPropertyDelegatingTest$ReflectionTest.allMethodsFromSuperclassAreImplementedByDelegatingClass

io.fabric8.kubernetes.client.utils.SerializationAdditionalPropertiesTest

  • marshalWithAdditionalPropertiesOverridingFields
  • cloneShouldPreserveAdditionalProperties

⚠️ Compatibility Note

This change requires Jackson 2.19.0 or newer.

The updated handling of @JsonAnyGetter serialization and the complete SettableBeanProperty implementation are not backward-compatible with Jackson versions prior to 2.19.0.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

Fixes #7035

@@ -475,7 +477,7 @@ void deserializeSetAndReturnWithExceptionAndNullAnySetter() throws IOException {
class ReflectionTest {

@Test
@DisplayName("all methods from superclass (SettableBeanProperty) are implemented by delegating class (SettableBeanPropertyDelegate)")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SettableBeanPropertyDelegate does not exist.

@@ -530,5 +540,12 @@ private static MethodSignature from(Method m) {
return new MethodSignature(m.getReturnType(), m.getName(), m.getParameterTypes());
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improves how we report findings.

Now:

Screenshot 2025-04-25 at 21 05 42

Note: withSimpleName was just a local test. It wasn't missing.

Previously:

Screenshot 2025-04-25 at 20 57 13

Copy link
Member

Choose a reason for hiding this comment

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

nice

/**
* {@inheritDoc}
*/
@Override
Copy link
Contributor Author

@r1c4r60 r1c4r60 Apr 28, 2025

Choose a reason for hiding this comment

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

Note: This @Override is not backward compatible with Jackson versions prior to 2.19.0.

I do have ready local changes using Reflection and package version checks to restore backward compatibility if needed.

@r1c4r60 r1c4r60 changed the title fix #7036: Jackson 2.19.0 issues fix #7036: Jackson 2.19.0 update issues Apr 28, 2025
@r1c4r60 r1c4r60 marked this pull request as ready for review May 2, 2025 05:53
@wendigo
Copy link

wendigo commented May 13, 2025

When this is going to land?

@andreaTP
Copy link
Member

Hey @wendigo nice to see you around 👋
I guess this needs the attention of @manusa and @shawkins 🙏

@wendigo
Copy link

wendigo commented May 13, 2025

@andreaTP Hi 😃 !

@manusa manusa mentioned this pull request May 13, 2025
r1c4r60 and others added 7 commits May 13, 2025 16:20
…od overrides

Refactored ReflectionTest to not only detect but also clearly report missing method implementations
in SettableBeanPropertyDelegating. Missing methods are now listed with readable signatures,
making similar failures easier to diagnose in the future.
…PropertyDelegating

The missing override was detected when upgrading Jackson from 2.18.3 to 2.19.0,
where unwrapped(NameTransformer) became a required concrete method.
…ySerializer null errors

Fixes an issue in UnmatchedFieldTypeModule where the additionalProperties field
was incorrectly wrapped in a BeanPropertyWriterDelegate during serializer construction.

This resolves the following test failures:
- UnmatchedFieldTypeModuleTest.writeValueAsStringWithAdditionalPropertiesOverridingFields
- UnmatchedFieldTypeModuleTest.writeValueAsStringWithAdditionalPropertiesOverridingFieldsShouldLogWarning
- UnmatchedFieldTypeModuleTest.writeValueAsStringWithAdditionalPropertiesOverridingFieldsShouldNotLogWarning
The previous check used writer.getName().equals("additionalProperties"),
which failed for variations like getAdditionalProperties or when
@JsonProperty was used.

Changes:
- Skip wrapping the any-getter property in BeanSerializerModifier
- Add isAnyGetterWriter(...) to match the correct property by member

Resolved test failures:

io.fabric8.kubernetes.client.utils.SerializationAdditionalPropertiesTest
- marshalWithAdditionalPropertiesOverridingFields
- cloneShouldPreserveAdditionalProperties
Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa added this to the 7.3.0 milestone May 13, 2025 — with automated-tasks
@manusa manusa requested review from andreaTP and metacosm as code owners May 14, 2025 09:13
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
77.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@manusa manusa merged commit 70441ca into fabric8io:main May 14, 2025
17 of 18 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.

Jackson 2.19.0 issues
4 participants