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

Add more options to exclude classes from the default JAXBContext #31940

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

gnieser
Copy link
Contributor

@gnieser gnieser commented Mar 17, 2023

Following this discussion, I would like to thank @Sgitario for his kind assistance and his invitation to try to contribute.

Here is a pull request with:

  • A quarkus.jaxb.exclude-packages to exclude classes from given packages from the default JAXBContext
  • A quarkus.jaxb.exclude-pattern to exclude classes whose names matches the pattern from the default JAXBContext

To reply to @Sgitario 's comment from the discussion:

Though, I would merge the exclude-classes and exclude-packages into exclude-classes (and use the startWith matching).

I'm thinking of cases such as classes named User and UserAccount, a startWith matching may not work as intended, if only the User class is to be excluded for some reason.

Moreover, I would not add pattern matching at least it's essential since we used not to try adding too many superficial configuration options.

The motivation behind pattern matching is to provide adequate tooling for developer using xjc with third-party XSD. The sheer number of classes can grow up pretty quickly and one have little control over them.

I agree having both exclude-packages and pattern matching is somehow redundant. Pattern matching can easily exclude packages.

Of course, I will proceed with what ever option you prefer. :)

Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request!
From my side, I have the same comments as in #31882 (reply in thread).

@quarkus-bot

This comment has been minimized.

@gnieser
Copy link
Contributor Author

gnieser commented Mar 17, 2023

What would you think of dropping pattern matching (exclude-pattern) and changing exclude-classes behavior to the following?

  • if item ends with .* then assume it is a package and perform startsWith matching
  • otherwise assume it is a class and perform equals matching

It would avoid collision when exclusion of a User class would incidentally also exclude a UserAccount class.

@Sgitario
Copy link
Contributor

What would you think of dropping pattern matching (exclude-pattern) and changing exclude-classes behavior to the following?

  • if item ends with .* then assume it is a package and perform startsWith matching
  • otherwise assume it is a class and perform equals matching

It would avoid collision when exclusion of a User class would incidentally also exclude a UserAccount class.

Good idea!

@gnieser
Copy link
Contributor Author

gnieser commented Mar 17, 2023

I've committed these changes.

I should also rebase and squash my two commits and force-push into single one, shouldn't I?

@Sgitario
Copy link
Contributor

Yes, please

@gnieser
Copy link
Contributor Author

gnieser commented Mar 17, 2023

Done, thanks for your patience.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 17, 2023

✔️ 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.

Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

lgtm!

@Sgitario Sgitario changed the title WIP - Add more options to exclude classes from the default JAXBContext Add more options to exclude classes from the default JAXBContext Mar 20, 2023
@Sgitario Sgitario merged commit 203f3d9 into quarkusio:main Mar 20, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Mar 20, 2023
@gnieser gnieser deleted the jaxbcontext-excludes branch March 20, 2023 06:56
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.

None yet

2 participants