Skip to content

Return the same default instance if unchanged #296

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 2 commits into from
Apr 22, 2025

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Apr 22, 2025

Why create a separate instance that needs its own caching of adapters, resource bundle lookups, etc?

  • Now the builder will return the same validator instance when no changes in the builder are detected.

Essentially, this change causes the same Validator instance to be returned by the statement Validator.builder().build()

Now the builder will return the exact same validator instance when no changes in the builder are detected
@SentryMan SentryMan added the enhancement New feature or request label Apr 22, 2025
@SentryMan SentryMan added this to the 2.10 milestone Apr 22, 2025
@SentryMan SentryMan requested a review from rbygrave April 22, 2025 02:03
@SentryMan SentryMan self-assigned this Apr 22, 2025
@SentryMan SentryMan enabled auto-merge (squash) April 22, 2025 02:03
@rbygrave
Copy link
Contributor

I don't understand the motivation? Seems like most cases there will be a generated validator so customised? What prompted this PR?

@SentryMan
Copy link
Collaborator Author

I don't understand the motivation? Seems like most cases there will be a generated validator so customised?

If you notice, the check happens before the generated adapters are loaded.

What prompted this PR?

I noticed people accidentally instantiating validators per request with Validator.builder().build().

@SentryMan
Copy link
Collaborator Author

For Jsonb, there are a bunch of places where a default jsonb instance can get created. (inject plugin, jex default adapter, http-client default adapter)

I've also got some internal libraries that create default jsonb instances via SPI

@SentryMan SentryMan merged commit d50a714 into avaje:main Apr 22, 2025
6 checks passed
@rbygrave
Copy link
Contributor

Do you want to release this one to central? [or would you like me to]

@SentryMan
Copy link
Collaborator Author

Nah I got it

@SentryMan SentryMan deleted the default-instance branch April 22, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants