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

Avoid classes with incomplete hierarchy in Hibernate Validator #40383

Merged
merged 1 commit into from
May 2, 2024

Conversation

gastaldi
Copy link
Contributor

As explained in quarkiverse/quarkus-poi#102, some classes cannot be loaded because of an incomplete hierarchy

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented May 1, 2024

Thanks for having a look. I spotted an issue but not sure why the tests are failing.

@gastaldi gastaldi force-pushed the class_verify branch 2 times, most recently from 86bfb12 to 93ecb15 Compare May 1, 2024 11:07
@gastaldi gastaldi requested a review from gsmet May 1, 2024 11:09

This comment has been minimized.

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

LGTM.

However, I'd like to point out a runtime class may have a complete class hierarchy at build time, yet have an incomplete one at runtime, because a superclass got excluded. Classes relying on XML parsers are a prime example of this, as we try to exclude XML parsers from runtime but still need them at build time.

So, I suspect this won't solve the problem completely. Still, it's a nice improvement, and maybe it'll fix that specific problem with Apache POI (did you check that?), so let's merge.

@yrodiere
Copy link
Member

yrodiere commented May 2, 2024

so let's merge

Well, let's merge once tests pass :]

@gastaldi
Copy link
Contributor Author

gastaldi commented May 2, 2024

maybe it'll fix that specific problem with Apache POI (did you check that?),

Yeap, I checked that and the error is gone, I'll investigate what's causing the other tests to fail

@gastaldi gastaldi marked this pull request as draft May 2, 2024 13:42
@gastaldi
Copy link
Contributor Author

gastaldi commented May 2, 2024

Apparently generated bytecode isn't visible during this build step. It barfs saying that io.quarkus.it.hibernate.orm.rest.data.panache.BooksResourceJaxRs_953025cf555c51e5ab5649f6462b7b6ff38e89ec isn't available in runtime. Still investigating the best solution, but pointers are welcome ;)

@gastaldi gastaldi requested a review from yrodiere May 2, 2024 14:41
@gastaldi gastaldi marked this pull request as ready for review May 2, 2024 14:41
@gastaldi
Copy link
Contributor Author

gastaldi commented May 2, 2024

Because of #40383 (comment), I adopted a different strategy by performing the check in the Recorder class. Tests are passing locally for me

Copy link

quarkus-bot bot commented May 2, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 4c575b3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

LGTM. As far as I can tell loading these classes would happen anyway without your code, so it should not have much impact on startup time.

Let's merge if this still fixes the problem in POI.

@yrodiere yrodiere dismissed gsmet’s stale review May 2, 2024 16:03

Addressed

@gastaldi
Copy link
Contributor Author

gastaldi commented May 2, 2024

I can confirm this fixes quarkiverse/quarkus-poi#102. Merging

@gastaldi gastaldi merged commit 0968585 into quarkusio:main May 2, 2024
38 of 40 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 2, 2024
@gastaldi gastaldi deleted the class_verify branch May 2, 2024 16:38
@gsmet gsmet modified the milestones: 3.11 - main, 3.10.1 May 10, 2024
@gsmet gsmet modified the milestones: 3.10.1, 3.8.5 May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants