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.

Sign up for GitHub

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

Jump to bottom

Avoid adding the same class to be validated multiple times #446

Merged
merged 1 commit into from
Jan 8, 2019
Merged

Avoid adding the same class to be validated multiple times #446

merged 1 commit into from
Jan 8, 2019

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jan 8, 2019 •
edited
Loading

When using Bean Validation (which have to use the recorder.classProxy),
it makes sense to create the initial set from the names of the classes
that need to be validated and then create the class proxies. Doing it
the other way around negates the use of the Set since each proxy is
different even if the target class is the same

What this PR prevents is the duplicate bytecode like the following (from the input-validation quickstart):

        HashSet var4 = new HashSet();
        ClassLoader var3 = Thread.currentThread().getContextClassLoader();
        Class var5 = Class.forName("org.acme.validation.Book", (boolean)1, var3);
        ((Collection)var4).add(var5);
        ClassLoader var6 = Thread.currentThread().getContextClassLoader();
        Class var7 = Class.forName("org.acme.validation.BookService", (boolean)1, var6);
        ((Collection)var4).add(var7);
        ClassLoader var8 = Thread.currentThread().getContextClassLoader();
        Class var9 = Class.forName("org.acme.validation.Book", (boolean)1, var8);
        ((Collection)var4).add(var9);
        ClassLoader var10 = Thread.currentThread().getContextClassLoader();
        Class var11 = Class.forName("org.acme.validation.BookResource", (boolean)1, var10);
        ((Collection)var4).add(var11);
        ClassLoader var12 = Thread.currentThread().getContextClassLoader();
        Class var13 = Class.forName("org.acme.validation.Book", (boolean)1, var12);
        ((Collection)var4).add(var13);
        ClassLoader var14 = Thread.currentThread().getContextClassLoader();
        Class var15 = Class.forName("org.acme.validation.Book", (boolean)1, var14);
        ((Collection)var4).add(var15);
        ClassLoader var16 = Thread.currentThread().getContextClassLoader();
        Class var17 = Class.forName("org.acme.validation.Book", (boolean)1, var16);
        ((Collection)var4).add(var17);

gsmet
gsmet reviewed Jan 8, 2019
View reviewed changes
...tion/deployment/src/main/java/org/jboss/shamrock/beanvalidation/BeanValidationProcessor.java Outdated
@@ -109,34 +109,38 @@ public void build(ValidatorTemplate template, RecorderContext recorder,
// Also consider elements that are marked with @ValidateOnExecution
consideredAnnotations.add(VALIDATE_ON_EXECUTION);

Set<Class<?>> classesToBeValidated = new HashSet<>();
Set<String> classNamesToBeValidated = new HashSet<>();
Copy link
Member

Choose a reason for hiding this comment

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

@geoand good catch! Could you change it to use a set of DotName? Looks better than using strings all over.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gsmet Of course, I'll fix that now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@geoand geoand force-pushed the validation-duplicate-classes branch from d2ff6ca to 2f63599 Compare January 8, 2019 13:20
@geoand @gsmet
Avoid adding the same class to be validated multiple times
a7cde11 
When using Bean Validation (which have to use the recorder.classProxy),
it makes sense to create the initial set from the names of the classes
that need to be validated and then create the class proxies. Doing it
the other way around negates the use of the Set since each proxy is
different even if the target class is the same
@gsmet gsmet force-pushed the validation-duplicate-classes branch from 2f63599 to a7cde11 Compare January 8, 2019 15:16
@gsmet
Copy link
Member

gsmet commented Jan 8, 2019

I force pushed a minor formatting change to your branch. Merging now, thanks!

@gsmet gsmet merged commit 9905469 into quarkusio:master Jan 8, 2019
@gsmet gsmet added this to the 0.5.0 milestone Jan 8, 2019
@geoand
Copy link
Contributor Author

geoand commented Jan 8, 2019

@gsmet Cool, thanks!

@geoand geoand deleted the validation-duplicate-classes branch January 8, 2019 15:17
maxandersen pushed a commit to maxandersen/quarkus that referenced this pull request Nov 5, 2022
@dependabot
chore(deps): bump qute-core from 1.8.3.Final to 1.9.0.Final (quarkusi…
75860d8 
…o#446)

Bumps [qute-core](https://github.com/quarkusio/quarkus) from 1.8.3.Final to 1.9.0.Final.
- [Release notes](https://github.com/quarkusio/quarkus/releases)
- [Commits](quarkusio/quarkus@1.8.3.Final...1.9.0.Final)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@geoand @gsmet