Skip to content

Conversation

@hferentschik
Copy link
Contributor

Fixing LazyValidatorFactory implementation. Hibernate Validator will always be the default, but other available providers can pass in case they are explicitly requested.

A lot of changes for something quite simple. However, the functional change is only in LazyValidatorFactory, the rest is only test infrastructure.

The LazyValidatorFactory code should be ported to the corresponding factories in the AS 7 codebase.

…d build warning in hibernate-validator-integrationtest
…per to build custom provider jar as well as bundled hibernate validator jars.

Also moving classes into util directory to be easier shared between tests
… list of all bean validation providers.

However, other providers are still being processed and used in case explicitly required by eg validation.xml
…faultValidationProviderResolver

Also making use of the already existing privileged action classes
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering why you use here ArchivePaths.create and in other deployments you use addAsWebInfResource( EmptyAsset.INSTANCE, "beans.xml" ) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular reason at all. The setup is a mix of different examples I found (amongst other in Search). Documentation around Shrinkwrap and Arquillian would need some attention IMO. I will align the examples so that we are consistent within Validator

Copy link
Member

Choose a reason for hiding this comment

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

+1 for both ;-)

@kevinpollet
Copy link
Member

I haven't run the tests but looks good!

Copy link
Member

Choose a reason for hiding this comment

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

Just a thought I think you can use addAsManifestResource and remove the META-INF part.

@hferentschik
Copy link
Contributor Author

@kevinpollet formatted and fixed the logging version. I tried addAsManifestResource, but that did not work. Haven't really inspected the generated artifacts yet to see where files ended up, but just changing the lines and running the tests is leading to test failures.

@hferentschik
Copy link
Contributor Author

I assume that this changes are ok now. Going to merge it into master

@hferentschik
Copy link
Contributor Author

rebased and merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants