Stabilizing DetachAttachITest

- Refactoring to fix occasional race condition seen on GH CI on Windows
- Added toStrings
- Added logs
- API cleanup
- Few javadocs
- Default jobs.xml provider pushed outside JobManagerService, which generally
  manages all jobfiles.
- JobFileScanner moved to JobManagerService method - not used anywhere else
- JobPersistenceService is responsible for the I/O to the file
- Added client counting to the AttachCommand to prevent race conditions

Signed-off-by: David Matějček <[email protected]>

Author: dmatej

appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/progress/DetachAttachITest.java

@@ -41,6 +41,7 @@
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.stringContainsInOrder;
+import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -95,28 +96,32 @@ public void commandWithProgressStatus() throws Exception {
 
     @Test
     public void detachOnesAttachMulti() throws Exception {
-        ExecutorService pool = Executors.newCachedThreadPool(r -> {
-            Thread result = new Thread(r);
-            result.setDaemon(true);
-            return result;
+        final ExecutorService pool = Executors.newCachedThreadPool(r -> {
+            Thread thread = new Thread(r);
+            thread.setDaemon(true);
+            return thread;
         });
-        final DetachedTerseAsadminResult result = ASADMIN.execDetached("progress-custom", "8x1");
-        assertThat(result, asadminOK());
-        assertNotNull(result.getJobId(), "id");
+        final DetachedTerseAsadminResult jobIdResult = ASADMIN.execDetached("progress-custom", "8x1");
+        assertThat(jobIdResult, asadminOK());
+        assertNotNull(jobIdResult.getJobId(), "id");
         final int attachCount = 3;
-        Collection<Callable<AsadminResult>> attaches = new ArrayList<>(attachCount);
+        final Collection<Callable<AsadminResult>> attaches = new ArrayList<>(attachCount);
         for (int i = 0; i < attachCount; i++) {
-            attaches.add(() -> ASADMIN.exec("attach", result.getJobId()));
+            attaches.add(() -> ASADMIN.exec("attach", jobIdResult.getJobId()));
         }
-        List<Future<AsadminResult>> results = pool.invokeAll(attaches);
-        for (Future<AsadminResult> fRes : results) {
-            AsadminResult res = fRes.get();
-            assertThat(res, asadminOK());
-            assertTrue(res.getStdOut().contains("progress-custom"));
-            List<ProgressMessage> prgs = ProgressMessage.grepProgressMessages(res.getStdOut());
-            assertFalse(prgs.isEmpty());
-            assertThat(prgs.get(0).getValue(), greaterThanOrEqualTo(0));
-            assertEquals(100, prgs.get(prgs.size() - 1).getValue());
+        final List<Future<AsadminResult>> futureResults = pool.invokeAll(attaches);
+        for (Future<AsadminResult> futureResult : futureResults) {
+            final AsadminResult result = futureResult.get();
+            final List<ProgressMessage> prgs = ProgressMessage.grepProgressMessages(result.getStdOut());
+            assertAll(
+                () -> assertThat(result, asadminOK()),
+                () -> assertTrue(result.getStdOut().contains("progress-custom")),
+                () -> assertFalse(prgs.isEmpty(), "progress messages empty")
+            );
+            assertAll(
+                () -> assertThat(prgs.get(0).getValue(), greaterThanOrEqualTo(0)),
+                () -> assertEquals(100, prgs.get(prgs.size() - 1).getValue())
+            );
         }
     }
 }

nucleus/admin/rest/rest-service/src/main/java/org/glassfish/admin/rest/utils/SseCommandHelper.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2013, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -17,7 +18,6 @@
 package org.glassfish.admin.rest.utils;
 
 import com.sun.enterprise.admin.remote.AdminCommandStateImpl;
-import com.sun.enterprise.util.LocalStringManagerImpl;
 import com.sun.enterprise.v3.admin.JobManagerService;
 import com.sun.enterprise.v3.common.PropsFileActionReporter;
 
@@ -27,22 +27,24 @@
 import java.util.logging.Level;
 
 import org.glassfish.admin.rest.RestLogging;
-import org.glassfish.admin.rest.resources.admin.CommandResource;
 import org.glassfish.api.ActionReport;
 import org.glassfish.api.admin.AdminCommandEventBroker;
+import org.glassfish.api.admin.AdminCommandEventBroker.AdminCommandListener;
 import org.glassfish.api.admin.AdminCommandState;
 import org.glassfish.api.admin.CommandRunner;
 import org.glassfish.api.admin.CommandRunner.CommandInvocation;
 import org.glassfish.internal.api.Globals;
 import org.glassfish.jersey.media.sse.EventOutput;
 import org.glassfish.jersey.media.sse.OutboundEvent;
 
+import static org.glassfish.api.admin.AdminCommandState.EVENT_STATE_CHANGED;
+
 /**
  * Provides bridge between CommandInvocation and ReST Response for SSE. Create it and call execute.
  *
  * @author martinmares
  */
-public class SseCommandHelper implements Runnable, AdminCommandEventBroker.AdminCommandListener {
+public class SseCommandHelper implements Runnable, AdminCommandListener {
 
     /**
      * If implementation of this interface is registered then it's process() method is used to convert ActionReport before
@@ -58,8 +60,6 @@ public static interface ActionReportProcessor {
 
     }
 
-    private final static LocalStringManagerImpl strings = new LocalStringManagerImpl(CommandResource.class);
-
     private final CommandRunner.CommandInvocation commandInvocation;
     private final ActionReportProcessor processor;
     private final EventOutput eventOuptut = new EventOutput();
@@ -80,7 +80,7 @@ public void run() {
             actionReport.setFailureCause(thr);
             actionReport.setActionExitCode(ActionReport.ExitCode.FAILURE);
             AdminCommandState acs = new AdminCommandStateImpl(AdminCommandState.State.COMPLETED, actionReport, true, "unknown");
-            onAdminCommandEvent(AdminCommandStateImpl.EVENT_STATE_CHANGED, acs);
+            onAdminCommandEvent(EVENT_STATE_CHANGED, acs);
         } finally {
             try {
                 eventOuptut.close();
@@ -97,7 +97,7 @@ private void unregister() {
     }
 
     private Object process(final String name, Object event) {
-        if (processor != null && AdminCommandStateImpl.EVENT_STATE_CHANGED.equals(name)) {
+        if (processor != null && EVENT_STATE_CHANGED.equals(name)) {
             AdminCommandState acs = (AdminCommandState) event;
             ActionReport report = processor.process(acs.getActionReport(), eventOuptut);
             event = new AdminCommandStateImpl(acs.getState(), report, acs.isOutboundPayloadEmpty(), acs.getId());
@@ -122,8 +122,8 @@ public void onAdminCommandEvent(final String name, Object event) {
             unregister();
             return;
         }
-        if ((event instanceof Number) || (event instanceof CharSequence) || (event instanceof Boolean)) {
-            event = String.valueOf(event);
+        if (event instanceof Number || event instanceof CharSequence || event instanceof Boolean) {
+            event = event.toString();
         }
         event = process(name, event);
         OutboundEvent outEvent = new OutboundEvent.Builder().name(name)

nucleus/admin/util/src/main/java/com/sun/enterprise/admin/remote/AdminCommandStateImpl.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2012, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -17,6 +18,7 @@
 package com.sun.enterprise.admin.remote;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 import org.glassfish.api.ActionReport;
 import org.glassfish.api.admin.AdminCommandState;
@@ -30,54 +32,66 @@ public class AdminCommandStateImpl implements AdminCommandState, Serializable {
 
     private static final long serialVersionUID = 1L;
 
-    protected State state = State.PREPARED;
-    protected ActionReport actionReport;
-    private boolean payloadIsEmpty;
-    protected String id;
-
-    public AdminCommandStateImpl(final State state, final ActionReport actionReport, final boolean payloadIsEmpty, final String id) {
-        this.state = state;
-        this.actionReport = actionReport;
-        this.payloadIsEmpty = payloadIsEmpty;
-        this.id = id;
-    }
+    private final String id;
+    private final boolean payloadIsEmpty;
+    private ActionReport actionReport;
+    private volatile State state;
 
     public AdminCommandStateImpl(String id) {
         this(State.PREPARED, null, true, id);
     }
 
+    public AdminCommandStateImpl(final State state, final ActionReport actionReport, final boolean payloadIsEmpty, final String id) {
+        this.id = id;
+        this.payloadIsEmpty = payloadIsEmpty;
+        this.actionReport = actionReport;
+        this.state = state;
+    }
+
     @Override
-    public State getState() {
-        return this.state;
+    public final String getId() {
+        return this.id;
     }
 
     @Override
-    public void complete(ActionReport actionReport) {
-        this.actionReport = actionReport;
-        if (getState().equals(State.REVERTING)) {
-            setState(State.REVERTED);
-        } else {
-            setState(State.COMPLETED);
-        }
+    public boolean isOutboundPayloadEmpty() {
+        return this.payloadIsEmpty;
     }
 
     @Override
-    public ActionReport getActionReport() {
+    public final ActionReport getActionReport() {
         return this.actionReport;
     }
 
     @Override
-    public boolean isOutboundPayloadEmpty() {
-        return this.payloadIsEmpty;
+    public final State getState() {
+        return this.state;
     }
 
-    @Override
-    public String getId() {
-        return this.id;
+    /**
+     * Sets the state and the action report.
+     *
+     * @param state must not be null.
+     * @param report must not be null.
+     */
+    protected final void setState(State state, ActionReport report) {
+        // The order matters - setStatus is overridden by some children,
+        // following actions depend on the report!
+        this.actionReport = Objects.requireNonNull(report, "report");
+        setState(state);
     }
 
+    /**
+     * Sets the state.
+     *
+     * @param state must not be null
+     */
     protected void setState(State state) {
-        this.state = state;
+        this.state = Objects.requireNonNull(state, "state");
     }
 
+    @Override
+    public String toString() {
+        return super.toString() + "[id=" + id + ", state=" + state + ", report=" + actionReport + "]";
+    }
 }

nucleus/common/glassfish-api/src/main/java/org/glassfish/api/ActionReport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Contributors to the Eclipse Foundation.
+ * Copyright (c) 2024, 2025 Contributors to the Eclipse Foundation.
  * Copyright (c) 1997, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -294,4 +294,10 @@ public String findProperty(String key) {
         }
         return null;
     }
+
+
+    @Override
+    public String toString() {
+        return super.toString() + "[exitCode=" + getActionExitCode() + ", message=" + getMessage() + "]";
+    }
 }

nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/AdminCommandState.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2012, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -33,14 +34,6 @@ public enum State {
 
     State getState();
 
-    /**
-     * Completes whole progress and records result
-     *
-     * @param actionReport result message
-     *
-     */
-    void complete(ActionReport actionReport);
-
     ActionReport getActionReport();
 
     /**

nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/JobManager.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2013, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -21,6 +22,7 @@
 import java.io.Serializable;
 import java.util.Iterator;
 
+import org.glassfish.api.admin.progress.JobInfo;
 import org.glassfish.api.admin.progress.JobInfos;
 import org.jvnet.hk2.annotations.Contract;
 
@@ -92,7 +94,7 @@ public AdminCommandContext getContext() {
      * This method is used to get a job by its id
      *
      * @param id The id to look up the job in the job registry
-     * @return the Job
+     * @return the Job or null
      */
     Job get(String id);
 
@@ -106,7 +108,7 @@ public AdminCommandContext getContext() {
     /**
      * This will get the list of jobs from the job registry which have completed
      *
-     * @return the details of all completed jobs using JobInfos
+     * @return the details of all completed jobs using JobInfos. Never null.
      */
     JobInfos getCompletedJobs(File jobs);
 
@@ -116,22 +118,15 @@ public AdminCommandContext getContext() {
      * @param id the completed Job whose id needs to be looked up
      * @return the completed Job
      */
-    Object getCompletedJobForId(String id);
+    JobInfo getCompletedJobForId(String id, File jobsFile);
 
     /**
-     * This is used to purge a completed job whose id is provided
+     * This is used to purge a completed job.
      *
-     * @param id the id of the Job which needs to be purged
+     * @param job the info about Job
      * @return the new list of completed jobs
      */
-    Object purgeCompletedJobForId(String id);
-
-    /**
-     * This is used to get the jobs file for a job
-     *
-     * @return the location of the job file
-     */
-    File getJobsFile();
+    JobInfos purgeCompletedJobForId(JobInfo job);
 
     /**
      * Stores current command state.

nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/progress/JobInfo.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2013, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -73,4 +74,8 @@ public void setJobsFile(File jobsFile) {
         this.jobFile = jobsFile;
     }
 
+    @Override
+    public String toString() {
+        return "Job[id=" + jobId + ", name=" + jobName + ", exitCode=" + exitCode + "]";
+    }
 }

nucleus/common/glassfish-api/src/main/java/org/glassfish/api/admin/progress/JobInfos.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2025 Contributors to the Eclipse Foundation
  * Copyright (c) 2012, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -31,16 +32,24 @@
 public class JobInfos {
     private List<JobInfo> jobInfoList;
 
+    /**
+     * @return the internal list, never null.
+     */
     @XmlElement(name = "job")
     public List<JobInfo> getJobInfoList() {
-
         return jobInfoList;
     }
 
+    /**
+     * @param jobInfoList can be null, then creates an empty {@link ArrayList}.
+     */
     public void setJobInfoList(List<JobInfo> jobInfoList) {
-        this.jobInfoList = jobInfoList;
+        this.jobInfoList = jobInfoList == null ? new ArrayList<>() : jobInfoList;
     }
 
+    /**
+     * Creates an empty list.
+     */
     public JobInfos() {
         jobInfoList = new ArrayList<>();
     }