TOMEE-4631 - arquillian-tomee-remote: re-register container on every Manager bootstrap (#2817)

RemoteTomEEExtension guarded container registration behind a JVM-static
AtomicBoolean latch. Arquillian instantiates the extension and calls
register() once per Manager bootstrap, so the latch let only the first
Manager in a JVM fork register its DeployableContainer. Any second
Manager in the same fork (e.g. maven-failsafe-plugin's
rerunFailingTestsCount) got an empty ContainerRegistry and failed every
deployment with "DeploymentScenario contains a target (_DEFAULT_) not
matching any defined Container in the registry".

Drop the static latch (and the static lock that only guarded it) and
register unconditionally, matching EmbeddedTomEEExtension. The existing
try/catch(IllegalArgumentException) remains as the guard against a
genuine double-registration within a single builder.

Adds RemoteTomEEExtensionReRegistrationTest covering re-registration
across two Manager bootstraps in the same JVM.

Author: rzo1

arquillian/arquillian-tomee-remote/src/main/java/org/apache/tomee/arquillian/remote/RemoteTomEEExtension.java

@@ -26,40 +26,28 @@
 import org.jboss.arquillian.core.spi.LoadableExtension;
 import org.jboss.arquillian.test.spi.enricher.resource.ResourceProvider;
 
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 public class RemoteTomEEExtension implements LoadableExtension {
 
     private static final String ADAPTER = "tomee-remote";
-    private static final AtomicBoolean registered = new AtomicBoolean(false);
-    private static final ReentrantLock lock = new ReentrantLock();
 
     @Override
     public void register(final ExtensionBuilder builder) {
         if (ArquillianUtil.isCurrentAdapter(ADAPTER)) {
-
-            final ReentrantLock l = lock;
-            l.lock();
-
+            // Register unconditionally (like EmbeddedTomEEExtension): register() runs once per
+            // Manager bootstrap, so a JVM-static latch skipped every Manager after the first in
+            // the same fork and broke failsafe reruns (TOMEE-4631). The try/catch below guards
+            // against a genuine double-registration within a single builder.
             try {
-
-                if (!registered.getAndSet(true)) {
-
-                    try {
-                        builder.observer(RemoteInitialContextObserver.class);
-                        builder.observer(DeploymentExceptionObserver.class);
-                        builder.service(DeployableContainer.class, RemoteTomEEContainer.class)
-                                                    .service(AuxiliaryArchiveAppender.class, RemoteTomEEEJBEnricherArchiveAppender.class)
-                                                    .service(ResourceProvider.class, DeploymentExceptionProvider.class);
-                    } catch (final IllegalArgumentException e) {
-                        Logger.getLogger(RemoteTomEEExtension.class.getName()).log(Level.WARNING, "RemoteTomEEExtension: " + e.getMessage());
-                    }
-                }
-            } finally {
-                l.unlock();
+                builder.observer(RemoteInitialContextObserver.class);
+                builder.observer(DeploymentExceptionObserver.class);
+                builder.service(DeployableContainer.class, RemoteTomEEContainer.class)
+                                            .service(AuxiliaryArchiveAppender.class, RemoteTomEEEJBEnricherArchiveAppender.class)
+                                            .service(ResourceProvider.class, DeploymentExceptionProvider.class);
+            } catch (final IllegalArgumentException e) {
+                Logger.getLogger(RemoteTomEEExtension.class.getName()).log(Level.WARNING, "RemoteTomEEExtension: " + e.getMessage());
             }
         }
     }

arquillian/arquillian-tomee-remote/src/test/java/org/apache/tomee/arquillian/remote/RemoteTomEEExtensionReRegistrationTest.java

@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tomee.arquillian.remote;
+
+import org.jboss.arquillian.container.spi.client.container.DeployableContainer;
+import org.jboss.arquillian.core.spi.LoadableExtension;
+import org.jboss.arquillian.core.spi.LoadableExtension.ExtensionBuilder;
+import org.jboss.arquillian.core.spi.context.Context;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Reproducer for TOMEE-4631.
+ *
+ * <p>{@link RemoteTomEEExtension} guards container registration behind a JVM-static
+ * {@code AtomicBoolean} latch. {@link LoadableExtension#register(ExtensionBuilder)} is invoked
+ * once per Arquillian {@code Manager} bootstrap. When a second {@code Manager} is bootstrapped in
+ * the same JVM fork (the classic trigger being maven-failsafe-plugin's
+ * {@code rerunFailingTestsCount}, which spins up a fresh {@code Manager} to rerun a failed test),
+ * the static flag is already {@code true}, so the second bootstrap skips
+ * {@code builder.service(DeployableContainer.class, ...)} and that {@code Manager} ends up with an
+ * empty {@code ContainerRegistry}. Deployments then fail with:</p>
+ *
+ * <pre>
+ * org.jboss.arquillian.container.test.impl.client.deployment.ValidationException:
+ *   DeploymentScenario contains a target (_DEFAULT_) not matching any defined Container in the
+ *   registry. Please include at least 1 Deployable Container on your Classpath.
+ * </pre>
+ *
+ * <p>The sibling {@code EmbeddedTomEEExtension} has no such latch and registers unconditionally,
+ * which is why every other Arquillian adapter survives a rerun and only arquillian-tomee-remote
+ * does not.</p>
+ *
+ * <p>This test drives {@code register(...)} the same way Arquillian's {@code LoadableExtensionLoader}
+ * does (once per simulated {@code Manager} bootstrap, each with its own {@link ExtensionBuilder})
+ * and asserts the {@link DeployableContainer} is registered for every bootstrap, not just the
+ * first. It failed on the former static-latch implementation and guards the unconditional
+ * registration that replaced it.</p>
+ */
+public class RemoteTomEEExtensionReRegistrationTest {
+
+    @Test
+    public void containerIsReRegisteredOnEveryManagerBootstrap() {
+        // 1st Manager bootstrap (e.g. the initial failsafe run)
+        final RecordingExtensionBuilder first = new RecordingExtensionBuilder();
+        new RemoteTomEEExtension().register(first);
+        assertTrue("Sanity: the first Manager bootstrap must register the DeployableContainer",
+                first.registeredDeployableContainer());
+
+        // 2nd Manager bootstrap in the same JVM (e.g. a failsafe rerunFailingTestsCount rerun)
+        final RecordingExtensionBuilder second = new RecordingExtensionBuilder();
+        new RemoteTomEEExtension().register(second);
+
+        // TOMEE-4631: the static latch makes the second bootstrap skip the DeployableContainer
+        // service, leaving the second Manager's ContainerRegistry empty -> _DEFAULT_ matches nothing.
+        assertTrue("TOMEE-4631: a second Arquillian Manager in the same JVM did NOT get a "
+                        + "DeployableContainer registered; reruns (e.g. failsafe rerunFailingTestsCount) "
+                        + "will fail with 'DeploymentScenario contains a target (_DEFAULT_) not matching "
+                        + "any defined Container in the registry'.",
+                second.registeredDeployableContainer());
+    }
+
+    /**
+     * Minimal {@link ExtensionBuilder} that records which service contracts were registered,
+     * standing in for the real builder backed by Arquillian's {@code ServiceRegistry}.
+     */
+    private static final class RecordingExtensionBuilder implements ExtensionBuilder {
+        private final List<Class<?>> services = new ArrayList<>();
+
+        @Override
+        public <T> ExtensionBuilder service(final Class<T> service, final Class<? extends T> impl) {
+            services.add(service);
+            return this;
+        }
+
+        @Override
+        public <T> ExtensionBuilder override(final Class<T> service, final Class<? extends T> oldImpl,
+                                             final Class<? extends T> newImpl) {
+            return this;
+        }
+
+        @Override
+        public ExtensionBuilder observer(final Class<?> observer) {
+            return this;
+        }
+
+        @Override
+        public ExtensionBuilder context(final Class<? extends Context> context) {
+            return this;
+        }
+
+        boolean registeredDeployableContainer() {
+            return services.contains(DeployableContainer.class);
+        }
+    }
+}