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

fix #4109: moving template handling to a custom deserializer #4292

Merged
merged 2 commits into from Aug 23, 2022

Conversation

shawkins
Copy link
Contributor

@shawkins shawkins commented Jul 22, 2022

Description

This addresses #4109 and is related to #3972. It is also a replacement for #4034 - since the idea of subclassing ObjectMapper is unpalatable, the other way to do this is with a custom deserializer. But to keep the same unmatched logic, we need to move that to model-common, rather than client-api.

If it's acceptable, then I can further add a changelog about moving the unmatched static methods, etc. The unmatched test references thing from the api, so it can't be moved to model-common as is - but it could of course be further refactored to move the tests.

With this change we also don't need to have the parameter handling logic so deeply coupled with the Serialization logic - that is we don't have to do string substitution on the string prior to parsing. That could easily be done as post processing logic in the TemplateOperationsImpl.

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

@manusa
Copy link
Member

manusa commented Jul 25, 2022

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

@shawkins
Copy link
Contributor Author

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

Other than changing the package, it does not. You can of course leave the old ones in place, but deprecated if you want.

@manusa
Copy link
Member

manusa commented Jul 25, 2022

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

Other than changing the package, it does not. You can of course leave the old ones in place, but deprecated if you want.

I just gave it a quick look and the changes in the Serialization class made me think of this. I'll try to review properly later

@sunix sunix self-requested a review August 16, 2022 13:05
@shawkins
Copy link
Contributor Author

@sunix I mentioned something about backporting on the call - I was mixed up. #4263 is the one that should be considered for backport, not this one.

Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa manusa added this to the 6.1.0 milestone Aug 23, 2022
@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

78.4% 78.4% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit 0d553ce into fabric8io:master Aug 23, 2022
@manusa manusa linked an issue Aug 23, 2022 that may be closed by this pull request
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.

deserialize value of type int from String
2 participants