-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Require explicit representation configuration for UUID, BigDecimal, and BigInteger
#5044
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
Conversation
Signed-off-by: Hyunsang Han <gustkd3@gmail.com>
Signed-off-by: Hyunsang Han <gustkd3@gmail.com>
UUID, BigDecimal, and BigInteger (追記ここまで)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way these changes manifest is that application startup always breaks if uuid/big decimal representations aren't configured. That isn't ideal as users that don't need these features are forced to make a configuration change.
It would be much more appropriate to check whether such types are used at all.
A utility that walks the MongoMappingContext and introspects whether entities have a property that uses UUID, BigDecimal, or BigInteger would be less invasive and more targeted. We clearly cannot capture uuid/big decimal usage that operates solely on Documents (no entity usage) and in these cases, applications would only fail during runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would break applications for users that do not use UUIDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way these changes manifest is that application startup always breaks if uuid/big decimal representations aren't configured. That isn't ideal as users that don't need these features are forced to make a configuration change.
You're right. I realize I totally went in the wrong direction here.
Thanks for pointing out that forcing configuration for everyone is too invasive. I’ve been thinking this week about how to make the validation smarter, only checking when these types are actually in use.
Here's what I'm considering:
- Create a
TypeUsageValidatorclass that scans the mapping context to detect which entities actually useBigDecimal/BigInteger/UUID - For each type found in use, delegate to
validateBigDecimalConfiguration,validateBigIntegerConfiguration, andvalidateUuidConfigurationto ensure defaults are explicitly configured (If not, throwing an exception.) - Run this validation in
afterPropertiesSet(), when the mapping context is fully initialized
Does this direction make more sense? If so, I can refine it quickly and reopen the PR. 💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same breaking behavior as for UUIDs. We would also break applications for users that do not use big decimals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
Apologies for not updating the PR during the past week. I’ll make the necessary improvements and update it within this week.
Uh oh!
There was an error while loading. Please reload this page.
Remove implicit defaults for UUID, BigDecimal, and BigInteger representations to force explicit configuration.
MongoConfigurationSupport: Require explicitUuidRepresentationconfiguration inmongoClientSettings()MongoClientFactoryBean: Require explicitUuidRepresentationconfiguration incomputeClientSetting()MongoCustomConversions: Require explicitBigDecimalRepresentationconfiguration (covers both BigDecimal and BigInteger)Resolves: #5037