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

master branch #479

Merged
merged 16 commits into from Mar 30, 2014
Merged

master branch #479

merged 16 commits into from Mar 30, 2014

Conversation

ronsigal
Copy link
Collaborator

RESTEASY-1008: Validation now works in the presence of CDI.

patriot1burke added a commit that referenced this pull request Mar 30, 2014
@patriot1burke patriot1burke merged commit 5047396 into resteasy:master Mar 30, 2014
@fabian
Copy link
Contributor

fabian commented Apr 1, 2014

Well actually validation requires CDI now.

See: https://github.com/resteasy/Resteasy/pull/479/files#diff-67

@patriot1burke
Copy link
Collaborator

thought it was the other way around?

@gbervik
Copy link
Contributor

gbervik commented Apr 1, 2014

It seems to be because GeneralValidatorImpl references org.jboss.resteasy.cdi.ResteasyCdiExtension

https://github.com/resteasy/Resteasy/pull/479/files#diff-c8eb53d63c13781d8ad411833859fd78R21

@ronsigal
Copy link
Collaborator Author

ronsigal commented Apr 1, 2014

Here's the deal. Normally, validation is done by Resteasy core. But in the presence of CDI, I had to move validation into resteasy-cdi. To distinguish the two cases, I extended GeneralValidator with interface GeneralValidatorCDI, with two new methods, isValidatableFromCDI() and isMethodValidatableFromCDI(). When Resteasy core calls isValidatable() or isMethodValidatable(), it will get the answer false if CDI is active. When the other two methods are called from resteasy-cdi, they will return whatever the answer should be. So the two versions of GeneralValidatorImpl need to know if CDI is active, which is why it depends on resteasy-cdi.

@ronsigal
Copy link
Collaborator Author

ronsigal commented Apr 1, 2014

It's also true that resteasy-cdi depends on validation, since it performs validation. But the dependency is indirect, since it gets a validator by calling a ContextResolver.

@ronsigal
Copy link
Collaborator Author

ronsigal commented Apr 1, 2014

This conversation seems to start in the middle, with "Well actually validation requires CDI now." What's up? Is there a problem?

@gbervik
Copy link
Contributor

gbervik commented Apr 1, 2014

The problem for me was https://github.com/resteasy/Resteasy/pull/479/files#diff-c8eb53d63c13781d8ad411833859fd78R72

I didn't have ResteasyCdiExtension on my classpath and it threw a NoClassDefFoundError. The catch block is for an Exception, not an Error so it caused an issue.

@ronsigal
Copy link
Collaborator Author

ronsigal commented Apr 1, 2014

Ah, Ok. I'll change that to catch a Throwable. Thanks.

@gbervik
Copy link
Contributor

gbervik commented Apr 1, 2014

Isn't there a cleaner way the check can be done? Catching a NoClassDefFoundError seems like a bit of a hack. Even the javadoc for java.lang.Error says "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch"

I'm not familiar with the Resteasy internals, but couldn't ResteasyCdiExtension.observeBeforeBeanDiscovery() update something in core that GeneralValidatorImpl could check? Then validation wouldn't be dependent on CDI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants