[WFCORE-2958] Add an addtional runtime step to remove formatters from…

… a handler. In the case of a composite operation the formatter may be changed in a undefine-attribute operation followed by a removal of the formatter in a write-attribute handler for the named-formatter attribute. This causes the commit of the log managers configuration as the handler no longer exists after the write-attribute.

logging/src/main/java/org/jboss/as/logging/HandlerOperations.java

@@ -646,7 +646,7 @@ private static void handleProperty(final AttributeDefinition attribute, final Op
                 configuration.setFormatterName(resolvedValue);
                 // Check the current formatter name, if it's the same name as the handler, remove the old formatter
                 if (!formatterName.equals(resolvedValue) && logContextConfiguration.getFormatterNames().contains(formatterName)) {
-                    logContextConfiguration.removeFormatterConfiguration(formatterName);
+                    addAfterCommitRuntimeStep(context, (ctx, operation) -> logContextConfiguration.removeFormatterConfiguration(formatterName));
                 }
             } else {
                 // Use a formatter only if a named-formatter is not defined or the named-formatter was explicitly undefined
@@ -669,7 +669,7 @@ private static void handleProperty(final AttributeDefinition attribute, final Op
                 configuration.setFormatterName(resolvedValue);
                 // Check the current formatter name, if it's the same name as the handler, remove the old formatter
                 if (!formatterName.equals(resolvedValue) && logContextConfiguration.getFormatterNames().contains(formatterName)) {
-                    logContextConfiguration.removeFormatterConfiguration(formatterName);
+                    addAfterCommitRuntimeStep(context, (ctx, operation) -> logContextConfiguration.removeFormatterConfiguration(formatterName));
                 }
             } else {
                 // If the current formatter name already equals the name defined in the configuration, there is no need to process
@@ -946,4 +946,16 @@ public void execute(final OperationContext context, final ModelNode operation) t
             }, Stage.RUNTIME);
         }
     }
+
+    private static void addAfterCommitRuntimeStep(final OperationContext context, final OperationStepHandler step) {
+        Collection<OperationStepHandler> steps = context.getAttachment(LoggingOperations.AFTER_COMMIT_STEPS);
+        if (steps == null) {
+            steps = new ArrayList<>();
+            final Collection<OperationStepHandler> appearing = context.attachIfAbsent(LoggingOperations.AFTER_COMMIT_STEPS, steps);
+            if (appearing != null) {
+                steps = appearing;
+            }
+        }
+        steps.add(step);
+    }
 }

logging/src/main/java/org/jboss/as/logging/LoggingOperations.java

@@ -24,6 +24,8 @@
 
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP_ADDR;
 
+import java.util.Collection;
+
 import org.jboss.as.controller.AbstractWriteAttributeHandler;
 import org.jboss.as.controller.AttributeDefinition;
 import org.jboss.as.controller.OperationContext;
@@ -48,6 +50,11 @@
  */
 final class LoggingOperations {
 
+    /**
+     * Used to add runtime steps to be executed after a {@linkplain LogContextConfiguration#commit() commit}.
+     */
+    static final AttachmentKey<Collection<OperationStepHandler>> AFTER_COMMIT_STEPS = AttachmentKey.create(Collection.class);
+
     /**
      * Get the address name from the operation.
      *
@@ -79,6 +86,13 @@ public static PathAddress getAddress(final ModelNode operation) {
      */
     static void addCommitStep(final OperationContext context, final ConfigurationPersistence configurationPersistence) {
         context.addStep(new CommitOperationStepHandler(configurationPersistence), Stage.RUNTIME);
+        final Collection<OperationStepHandler> afterCommitSteps = context.getAttachment(AFTER_COMMIT_STEPS);
+        if (afterCommitSteps != null && !afterCommitSteps.isEmpty()) {
+            for (OperationStepHandler step : afterCommitSteps) {
+                context.addStep(step, Stage.RUNTIME);
+            }
+            context.addStep(new CommitOperationStepHandler(configurationPersistence), Stage.RUNTIME);
+        }
     }
 
     private static final class CommitOperationStepHandler implements OperationStepHandler {

logging/src/test/java/org/jboss/as/logging/HandlerOperationsTestCase.java

@@ -24,7 +24,9 @@
 import static org.jboss.as.controller.client.helpers.Operations.createAddOperation;
 import static org.jboss.as.controller.client.helpers.Operations.createRemoveOperation;
 import static org.jboss.as.subsystem.test.SubsystemOperations.OperationBuilder;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.IOException;
@@ -37,6 +39,8 @@
 
 import org.apache.commons.io.FileUtils;
 import org.jboss.as.controller.PathAddress;
+import org.jboss.as.controller.client.Operation;
+import org.jboss.as.controller.client.helpers.Operations;
 import org.jboss.as.controller.client.helpers.Operations.CompositeOperationBuilder;
 import org.jboss.as.controller.services.path.PathResourceDefinition;
 import org.jboss.as.subsystem.test.KernelServices;
@@ -275,6 +279,39 @@ public void testFormatsNoColor() throws Exception {
 
     }
 
+    /**
+     * Tests a composite operation of undefining a {@code formatter} attribute and defining a {@code named-formatter}
+     * attribute in a composite operation. These two specific attributes have strange behavior. If the
+     * {@code named-formatter} is defined it removes the formatter, named the same as the handler, which was created
+     * as part of the {@code undefine-attribute} operation of the {@code formatter} attribute.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testCompositeOperations() throws Exception {
+        final KernelServices kernelServices = boot();
+
+        final ModelNode address = createFileHandlerAddress("FILE").toModelNode();
+        final String filename = "test-file.log";
+
+        // Add the handler
+        ModelNode addOp = OperationBuilder.createAddOperation(address)
+                .addAttribute(CommonAttributes.FILE, createFileValue("jboss.server.log.dir", filename))
+                .build();
+        executeOperation(kernelServices, addOp);
+        final ModelNode patternFormatterAddress = createPatternFormatterAddress("PATTERN").toModelNode();
+        addOp = SubsystemOperations.createAddOperation(patternFormatterAddress);
+        addOp.get(PatternFormatterResourceDefinition.PATTERN.getName()).set("%d{HH:mm:ss,SSS} %-5p [%c] %s%e%n");
+        executeOperation(kernelServices, addOp);
+
+        // Create a composite operation to undefine
+        final Operation op = CompositeOperationBuilder.create()
+                .addStep(Operations.createUndefineAttributeOperation(address, "formatter"))
+                .addStep(Operations.createWriteAttributeOperation(address, "named-formatter", "PATTERN"))
+                .build();
+        executeOperation(kernelServices, op.getOperation());
+    }
+
     private void testAsyncHandler(final KernelServices kernelServices, final String profileName) throws Exception {
         final ModelNode address = createAsyncHandlerAddress(profileName, "async").toModelNode();
         final ModelNode subhandlers = new ModelNode().setEmptyList().add("CONSOLE");