[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss Modules APIs #6247

lvydra · 2024-11-20T14:41:32Z

https://issues.redhat.com/browse/WFCORE-3720

wildfly-ci · 2024-11-20T14:49:41Z

Core -> Full Integration Build 14045 outcome was FAILURE using a merge of 070bc6a
Summary: Compilation error: Compiler (new) Build time: 00:04:35

wildfly-ci · 2024-11-20T14:50:32Z

Core -> WildFly Preview Integration Build 14122 outcome was FAILURE using a merge of 070bc6a
Summary: Compilation error: Compiler (new) Build time: 00:05:27

wildfly-ci · 2024-11-20T14:53:44Z

Core -> Full Integration Build 14341 outcome was FAILURE using a merge of 070bc6a
Summary: Tests passed: 934, ignored: 19; compilation error: Compiler (new) Build time: 00:08:39

bstansberry · 2024-11-26T20:55:58Z

In general ModuleIdentifier.getName() is not a safe replacement for ModuleIdentifier. It does not include any slot value. ModuleIdentifier.toString() provides the String variant of the module id.

bstansberry · 2024-11-26T21:19:21Z

server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java

@@ -74,7 +74,7 @@ public void execute(OperationContext context, ModelNode operation) {
                        moduleIdentifier = deploymentUnit.getAttachment(Attachments.MODULE_IDENTIFIER);
                    }

-                    final ServiceController<?> moduleLoadServiceController = sr.getService(ServiceModuleLoader.moduleServiceName(moduleIdentifier));
+                    final ServiceController<?> moduleLoadServiceController = sr.getService(ServiceModuleLoader.moduleServiceName(moduleIdentifier.getName()));


All the changes like this need to be updated to use toString(), not getName().

bstansberry · 2024-11-26T21:22:07Z

server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java

+        return name.startsWith(MODULE_PREFIX);
+    }
+
+    @Deprecated


Please add forRemoval=true and add javadoc @deprecated pointing to the new method.

lvydra · 2024-11-27T11:15:49Z

Hi @bstansberry, thank you for the review, change requests should be addressed now.

yersan · 2024-11-27T14:44:10Z

server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java

+    public ModuleSpec findModule(String name) throws ModuleLoadException {
+        ServiceController<ModuleDefinition> controller = (ServiceController<ModuleDefinition>) serviceContainer.getService(moduleSpecServiceName(name));


I think this method should have been deprecated for removal calling internally to the one using the String as an argument. Otherwise, any caller currently using the old one will now be using the one provided by the parent class.

yersan · 2024-11-27T14:46:42Z

server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java

-    public static ServiceName moduleResolvedServiceName(ModuleIdentifier identifier) {
-        if (!isDynamicModule(identifier)) {
-            throw ServerLogger.ROOT_LOGGER.missingModulePrefix(identifier, MODULE_PREFIX);
+    public static ServiceName moduleResolvedServiceName(String name) {


This falls into the same category as what was done with ServiceName moduleServiceName(ModuleIdentifier identifier) and should be deprecated as well, although better to get other feedback since I don't know exactly what was the use case where this can be called from other parties than WildFly

yersan · 2024-11-27T14:50:25Z

server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java

+    public static boolean isDynamicModule(String name) {
+        return name.startsWith(MODULE_PREFIX);


This falls into the same category as what was done with ServiceName moduleServiceName(ModuleIdentifier identifier) and should be deprecated as well. Although better to get other feedback since I don't know exactly what was the use case where this can be called from other parties than WildFly

wildfly-ci · 2024-11-27T15:07:00Z

Core -> WildFly Preview Integration Build 14145 outcome was FAILURE using a merge of f2cbe06
Summary: Tests failed: 1 (1 new), passed: 5007, ignored: 85 Build time: 03:34:47

Failed tests

TestSuite: org.eclipse.microprofile.rest.client.tck.sse.ReactiveStreamsPublisherTckTest.stochastic_spec103_mustSignalOnMethodsSequentially: java.lang.NullPointerException: Cannot invoke "org.reactivestreams.Publisher.subscribe(org.reactivestreams.Subscriber)" because "pub" is null
	at org.reactivestreams.tck.PublisherVerification$5$1.run(PublisherVerification.java:257)
	at org.reactivestreams.tck.PublisherVerification.activePublisherTest(PublisherVerification.java:1135)
	at org.reactivestreams.tck.PublisherVerification$5.apply(PublisherVerification.java:251)
	at org.reactivestreams.tck.PublisherVerification$5.apply(PublisherVerification.java:248)
	at org.reactivestreams.tck.PublisherVerification.stochasticTest(PublisherVerification.java:1204)
	at org.reactivestreams.tck.PublisherVerification.stochastic_spec103_mustSignalOnMethodsSequentially(PublisherVerification.java:248)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:141)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:686)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:230)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:63)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:992)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:203)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:154)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:134)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at org.testng.TestRunner.privateRun(TestRunner.java:739)
	at org.testng.TestRunner.run(TestRunner.java:614)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:421)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:413)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:373)
	at org.testng.SuiteRunner.run(SuiteRunner.java:312)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1274)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1208)
	at org.testng.TestNG.runSuites(TestNG.java:1112)
	at org.testng.TestNG.run(TestNG.java:1079)
------- Stderr: -------
java.lang.AssertionError: Mock Sse Server did not start as expected expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:111)
	at org.testng.Assert.failNotEquals(Assert.java:1578)
	at org.testng.Assert.assertTrue(Assert.java:57)
	at org.eclipse.microprofile.rest.client.tck.sse.AbstractSseTest.launchServer(AbstractSseTest.java:62)
	at org.eclipse.microprofile.rest.client.tck.sse.ReactiveStreamsPublisherTckTest.createPublisher(ReactiveStreamsPublisherTckTest.java:106)
	at org.reactivestreams.tck.PublisherVerification.activePublisherTest(PublisherVerification.java:1134)
	at org.reactivestreams.tck.PublisherVerification$5.apply(PublisherVerification.java:251)
	at org.reactivestreams.tck.PublisherVerification$5.apply(PublisherVerification.java:248)
	at org.reactivestreams.tck.PublisherVerification.stochasticTest(PublisherVerification.java:1204)
	at org.reactivestreams.tck.PublisherVerification.stochastic_spec103_mustSignalOnMethodsSequentially(PublisherVerification.java:248)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:141)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:686)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:230)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:63)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:992)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:203)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:154)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:134)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

…Modules APIs

lvydra · 2024-11-28T11:18:35Z

Thanks @yersan, updated.

lvydra force-pushed the WFCORE-3720 branch from 070bc6a to 4a5b02a Compare November 20, 2024 16:12

bstansberry requested changes Nov 26, 2024

View reviewed changes

lvydra force-pushed the WFCORE-3720 branch from 4a5b02a to f2cbe06 Compare November 27, 2024 10:17

yersan reviewed Nov 27, 2024

View reviewed changes

[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss …

2557e47

…Modules APIs

lvydra force-pushed the WFCORE-3720 branch from f2cbe06 to 2557e47 Compare November 28, 2024 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss Modules APIs #6247

[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss Modules APIs #6247

lvydra commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

bstansberry commented Nov 26, 2024

bstansberry Nov 26, 2024

bstansberry Nov 26, 2024

lvydra commented Nov 27, 2024

yersan Nov 27, 2024

yersan Nov 27, 2024

yersan Nov 27, 2024

wildfly-ci commented Nov 27, 2024

lvydra commented Nov 28, 2024

		public ModuleSpec findModule(String name) throws ModuleLoadException {
		ServiceController<ModuleDefinition> controller = (ServiceController<ModuleDefinition>) serviceContainer.getService(moduleSpecServiceName(name));

		public static boolean isDynamicModule(String name) {
		return name.startsWith(MODULE_PREFIX);

[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss Modules APIs #6247

Are you sure you want to change the base?

[WFCORE-3720] Update ServiceModuleLoader to use non-deprecated JBoss Modules APIs #6247

Conversation

lvydra commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

wildfly-ci commented Nov 20, 2024

bstansberry commented Nov 26, 2024

bstansberry Nov 26, 2024

Choose a reason for hiding this comment

bstansberry Nov 26, 2024

Choose a reason for hiding this comment

lvydra commented Nov 27, 2024

yersan Nov 27, 2024

Choose a reason for hiding this comment

yersan Nov 27, 2024

Choose a reason for hiding this comment

yersan Nov 27, 2024

Choose a reason for hiding this comment

wildfly-ci commented Nov 27, 2024

Failed tests

lvydra commented Nov 28, 2024