-
Notifications
You must be signed in to change notification settings - Fork 143
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
MicroProfile Config 3.0 Support #23807
Conversation
Wow, that is a nice work! I will first fix my PR and then I will try to help you to fix them (if it would be needed) ;) |
Great work indeed @MattGill98! Maybe though take into consideration alignment with Piranha Cloud:
OmniConfig packages MP Config from Helidon, while JWT is the one I coded at Payara, somewhat repackaged to be reusable (it works in Tomcat too etc when the right dependencies are present). That said, regarding things like Tracing, SmallRye is already reusable out of the box. For Helidon some work may be needed for that. Piranha Cloud did use SmallRye for MP Config before. |
Added CQ for Helidon Config 3.0.0-M1 |
Helidon Config requires JDK 17, Smallrye JDK 8. |
Thanks! Btw, how do you keep track of which dependency has been CQ'ed and which one has not been? Or is it easier to just CQ everything and if it already has been CQ'ed ipzilla will let you know? Is there a tool available that simply CQ's every dependency from a project, or do you manually create the bugs for each one of them? |
They have a query https://dev.eclipse.org/ipzilla/query.cgi for the CQs. |
I try to CQ everything I cannot find the CQ for. There is no tool I am aware of, some questions are complicated, the repository of the source code, all the licenses, description of the dependency, project home page, this information can be difficult to get automatically, I grab them from multiple sources. It is easier when a newer version is CQed, most of the information can be used from the previous CQ... |
This change creates a MP Config TCK runner module to run the TCK against a managed instance of Glassfish. There are existing dependency issues, which means there are a decent proportion of tests that don't run correctly. At this point it's fine to test that the basic deployment of MP Config apps works as expected. Each MP TCK module will have its own instance of Glassfish, but will be passed a common arquillian.xml. The TCK requires a Weld impl to run due to the Arquillian CDI enricher. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
This change implements Smallrye Config as a GF container. Currently Smallrye config and common are packaged together. If other Smallrye libraries are used elsewhere, the common library should be separated out into its own bundle. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
The sniffer can now exit early without type scanning if the archive contains a microprofile-config.properties in the correct location. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
Hamcrest isn't loaded with the applications by default, despite the deployed applications requiring it. This change adds an Arquillian extension to deploy a Hamcrest dependency with the apps. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
The TCK wrongly assumes that, like in CDI versions <4, the 'all' bean discovery mode is assumed. GlassFish is now CDI 4.0 compliant, which means that the 'all' bean discovery mode now needs to be manually specified. To fix this, the arquillian extension now replaces the beans.xml files with custom versions. This will need modifying if the TCK contains any meaningful beans.xml tests. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
When using managed and remote clients, the Arquillian GF connectors throw each deployment exception as a GlassfishClientException, with the body of the actual exception. For the TCK to expect a certain exception type, we need an extension to interpret the response as the correct type. This change adds an extension to convert the "CDI deployment failure" responses to jakarta namespaced DeploymentExceptions, as the server produces. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
This change separated the Arquillian extension behaviours into their own separate classes for readability. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
System properties should ideally be configured via the arquillian container. This isn't possible via the official managed extension, so a workaround is needed. The options were either to configure it via a remote Arquillian extension (which doesn't currently work as Smallrye initialises its config sources before these extensions run), or to run a new test which sets it up. This change creates a new test purely to setup the container programmatically. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
The MP TCK uses ShouldThrowException to check some deployment failures. By default, Arquillian will not apply the extension services to those deployments. The beans.xml processor is still required in some cases though, to force the container to create bean deployments for the app. This change applies the processer during the BeforeDeploy event when the archive is non testable. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
A beans.xml file appears in the TCK deployments in WEB-INF as well as META-INF. This change adds the WEB-INF directory to the extension. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
The OSGI bundle plugin was importing the classes directly based on package. This was a problem, as Smallrye has multi-release JARs which wouldn't have their versioned classes imported. This change adds the Multi-Release instruction to the manifest, as well as imports the Smallrye versioned classes to allow JDK9 compatibility. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
MicroProfile annotations are allowed to be injected into method parameters, but the sniffer code currently tries to cast the parameter to a member (i.e. field or method), which causes a ClassCastException. This change modifies the sniffer code to allow the sniffer to find the class containing the parameter annotation. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
Helidon may be a better fit in the long term for GlassFish. The OSGI includes have been trimmed as much as possible to reduce bundle size, and unnecessary OSGI instructions have been removed. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
Smallrye used a custom file loading mechanism that doesn't have this bug. Since Helidon config uses JarURLConnection to load the microprofile-config.properties file, applications with the same name will return the same cached file contents. Disabling it server-wide would be a big performance hit, so instead this fix disables the cache during the config provider initialisation, and then returns it to the default behaviour. This shouldn't cause performance problems as I expect the file to only be read from the same archive once. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
The config converter to convert config properties to application classes was broken, as the implementation was using the server classloader (for GF this is a bundle classloader). Thankfully Helidon discovers converters using the ServiceLoader, so this change overwrites the default with a class converter that uses the webapp classloader Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
Helidon config causes some deployment errors to present as a "CDI definition failure", rather than a "CDI deployment failure". The Arquillian extension to transform exceptions has been made more generic to allow this. Signed-off-by: Matthew Gill <matthew.gill@live.co.uk>
I've swapped out Smallrye Config for Helidon config - had to make a few small changes (mostly relating to classloading) but in general very little needed changing What else needs completing for this change to make it in? |
Wait for a while with the merge, my yesterday PR broke something - servlet TCK doesn't pass at this moment. |
This change supports the deployment of MicroProfile Config 3.0 compatible applications, and modifies GF full profile to pass the MP Config 3.0.1 TCK by using the Smallrye Config 3.0.0.RC1 implementation.
The Smallrye dependencies are currently inlined in the implementation module, with the
org.glassfish.microprofile.config
andio.smallrye.config
packages exported. If further Smallrye dependencies are used in future, it may be helpful to export the common utilities to their own module to reduce bundle size. I've inlined the dependencies to improve performance rather than loading nested JARs, although I'm happy to change this if required.A sniffer module has been created to enable the respective MP implementations when they are detected, although the module defers the specific deployment steps to Smallrye.
The TCK needed a few modifications which are detailed in the commit messages:
all
bean discovery mode. Since CDI 4.0 this is no longer the case, so all empty beans.xml files are replaced with a correct implementation by an Arquillian extension.GlassFishClientException
s, which fails the deployment assertions. An Arquillian extension is used to cast these exceptions when the message is correct.