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

Change ProcessBeanAttributesNotFiredForBuiltinBean to only detect PBA for beans with @Default qualifiers #475

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

manovotn
Copy link
Contributor

Fixes #474
Details are in the linked issue. Note that all built-in beans by definition have to have @Default qualifier.

@manovotn manovotn requested a review from Ladicek June 23, 2023 09:44
@Ladicek
Copy link
Contributor

Ladicek commented Jun 23, 2023

This seems fairly safe to put into 4.0.x.

@manovotn
Copy link
Contributor Author

This seems fairly safe to put into 4.0.x.

Yes, that's the idea.
Any impl that passed it previously is bound to pass it now.

@Ladicek Ladicek merged commit d22bf17 into jakartaee:master Jun 23, 2023
@manovotn manovotn deleted the issue474 branch June 23, 2023 11:57
@manovotn
Copy link
Contributor Author

@arjantijms is there an imminent need for next 4.0 minor release with this? Or are there more problems with TCKs you're seeing and we should address?

@arjantijms
Copy link

arjantijms commented Jun 23, 2023

@manovotn we can work around this in GlassFish for now, as the entire purpose of Jersey producing a qualified HttpServletRequest is not very clear. Maybe @jansupol had his reasons, but we're not seeing it right now. So I've patched the module to not contain that specific producer for now.

There are a couple of issues with upgrading to a CDI TCK higher than 4.0.7, specifically this one:

[ERROR] org.jboss.cdi.tck.tests.full.extensions.lifecycle.bbd.broken.passivatingScope.AddingPassivatingScopeTest.arquillianBeforeClass -- Time elapsed: 25.78 s <<< FAILURE!
java.lang.StackOverflowError
	at org.jboss.arquillian.core.impl.Reflections.getEventPoints(Reflections.java:85)
	at org.jboss.arquillian.core.impl.ExtensionImpl.of(ExtensionImpl.java:59)
	at org.jboss.arquillian.core.impl.ManagerImpl.inject(ManagerImpl.java:184)
	at org.jboss.arquillian.core.impl.InjectorImpl.inject(InjectorImpl.java:54)
	at org.jboss.arquillian.core.impl.loadable.ServiceRegistryLoader.createServiceInstance(ServiceRegistryLoader.java:95)
	at org.jboss.arquillian.core.impl.loadable.ServiceRegistryLoader.all(ServiceRegistryLoader.java:50)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.transform(DeploymentExceptionHandler.java:110)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:92)
        at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)
	at org.jboss.arquillian.container.impl.client.container.DeploymentExceptionHandler.containsType(DeploymentExceptionHandler.java:101)

Before we have located the issue of that happening we can't upgrade to a newer CDI TCK version anyway.

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.

ProcessBeanAttributesNotFiredForBuiltinBean should only be checking for beans with @Default
3 participants