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

WELD-1837:Weld SE does not support empty beans.xml files #817

Closed
wants to merge 3 commits into from

Conversation

stefangrossmann
Copy link
Contributor

Weld SE has a problem while scanning bean archives, if the application has multiple archives with an empty beans.xml. Only the last detected archive is considered.

An empty beans.xml is represented with the constant org.jboss.weld.bootstrap.spi.BeansXml.EMPTY_BEANS_XML. The problem is, that implementations of org.jboss.weld.environment.deployment.discovery.BeansXml use this .EMPTY_BEANS_XML as key in a HashMap, which should contain all discovered bean archives. But because the key for jars with an empty beans.xml is always the same, they are not discovered properly.

The approach to solve this problem is to use the beansXmlUrl as key for the Map. I introduced a new helper class "ScanResult" which aggregates the beansXml and beanArchiveRef so that further processing of the scan result is influenced as less as possible.

Remark: I did not test the WebAppBeanArchiveScanner because this is not used in my environment. The DefaultBeanArchiveScanner seams to work correctly after this change.

@weld-tester
Copy link

Triggering build using a merge of 4c14c9f on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/

@weld-tester
Copy link

Build 408 is now running using a merge of 4c14c9f on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/408

@jharting
Copy link
Member

Thanks Stefan, the fix looks very good. Would you mind adding a test for the issue? You can use the following test as an example of an automated Weld SE test with multiple modules: https://github.com/weld/core/blob/master/environments/se/core/src/test/java/org/jboss/weld/environment/se/test/provider/MultiModuleCDIProviderTest.java

@weld-tester
Copy link

1 similar comment
@weld-tester
Copy link

@stefangrossmann
Copy link
Contributor Author

Ok, I will add the test.

@weld-tester
Copy link

Triggering build using a merge of 721a87d on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/

@weld-tester
Copy link

Triggering build using a merge of d3c3e9a on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/

@weld-tester
Copy link

Build 412 is now running using a merge of d3c3e9a on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/412

@weld-tester
Copy link

1 similar comment
@weld-tester
Copy link

@jharting
Copy link
Member

retest this please

@weld-tester
Copy link

Triggering build using a merge of d3c3e9a on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/

@weld-tester
Copy link

Build 414 is now running using a merge of d3c3e9a on branch 2.2:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/Weld-2.x-pull-player-executor-osprey/414

@weld-tester
Copy link

@jharting
Copy link
Member

Merged, thanks!

@jharting jharting closed this Jan 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants