Merge pull request #3713 from michpetrov/wfcore-3891

WFCORE-3891: ensure unique names for default formatters

logging/src/main/java/org/jboss/as/logging/formatters/PatternFormatterResourceDefinition.java

@@ -57,6 +57,8 @@
 
     public static final String NAME = "pattern-formatter";
 
+    public static final String DEFAULT_FORMATTER_SUFFIX = "-wfcore-pattern-formatter";
+
     // Pattern formatter options
     public static final PropertyAttributeDefinition COLOR_MAP = PropertyAttributeDefinition.Builder.of("color-map", ModelType.STRING)
             .setAllowExpression(true)
@@ -111,6 +113,9 @@ public boolean isMarshallable(final AttributeDefinition attribute, final ModelNo
         @Override
         public void performRuntime(final OperationContext context, final ModelNode operation, final ModelNode model, final LogContextConfiguration logContextConfiguration) throws OperationFailedException {
             final String name = context.getCurrentAddressValue();
+            if (name.endsWith(DEFAULT_FORMATTER_SUFFIX)) {
+                throw LoggingLogger.ROOT_LOGGER.illegalFormatterName();
+            }
             FormatterConfiguration configuration = logContextConfiguration.getFormatterConfiguration(name);
             if (configuration == null) {
                 LoggingLogger.ROOT_LOGGER.tracef("Adding formatter '%s' at '%s'", name, context.getCurrentAddress());

logging/src/main/java/org/jboss/as/logging/formatters/StructuredFormatterResourceDefinition.java

@@ -369,6 +369,9 @@ public void performRuntime(final OperationContext context, final ModelNode opera
             }
 
             final String name = context.getCurrentAddressValue();
+            if (name.endsWith(PatternFormatterResourceDefinition.DEFAULT_FORMATTER_SUFFIX)) {
+                throw LoggingLogger.ROOT_LOGGER.illegalFormatterName();
+            }
             FormatterConfiguration configuration = logContextConfiguration.getFormatterConfiguration(name);
             final String className = type.getName();
 

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

@@ -58,6 +58,7 @@
 import org.jboss.as.logging.Filters;
 import org.jboss.as.logging.Logging;
 import org.jboss.as.logging.LoggingOperations;
+import org.jboss.as.logging.formatters.PatternFormatterResourceDefinition;
 import org.jboss.as.logging.loggers.RootLoggerResourceDefinition;
 import org.jboss.as.logging.logging.LoggingLogger;
 import org.jboss.as.logging.logmanager.Log4jAppenderHandler;
@@ -621,39 +622,40 @@ private static void handleProperty(final AttributeDefinition attribute, final Op
             configuration.setEncoding(resolvedValue);
         } else if (attribute.getName().equals(FORMATTER.getName())) {
             // The handler name will be used for the name of a formatter for the formatter attribute
-            final String handlerName = configuration.getName();
+            final String defaultFormatterName = configuration.getName() + PatternFormatterResourceDefinition.DEFAULT_FORMATTER_SUFFIX;
             // Get the current model and check for a defined named-formatter attribute
             final Resource resource = context.readResource(PathAddress.EMPTY_ADDRESS);
             final ModelNode m = resource.getModel();
             if (m.hasDefined(NAMED_FORMATTER.getName())) {
                 // If a named-formatter exists in the model and a formatter already exists with the name of the handler
                 // remove the formatter
-                if (logContextConfiguration.getFormatterNames().contains(handlerName)) {
-                    logContextConfiguration.removeFormatterConfiguration(handlerName);
+                if (logContextConfiguration.getFormatterNames().contains(defaultFormatterName)) {
+                    logContextConfiguration.removeFormatterConfiguration(defaultFormatterName);
                 }
             } else {
                 // Create a formatter based on the handlers name
                 final FormatterConfiguration fmtConfig;
-                if (logContextConfiguration.getFormatterNames().contains(handlerName)) {
-                    fmtConfig = logContextConfiguration.getFormatterConfiguration(handlerName);
+                if (logContextConfiguration.getFormatterNames().contains(defaultFormatterName)) {
+                    fmtConfig = logContextConfiguration.getFormatterConfiguration(defaultFormatterName);
                 } else {
-                    fmtConfig = logContextConfiguration.addFormatterConfiguration(null, PatternFormatter.class.getName(), handlerName, PATTERN.getPropertyName());
+                    fmtConfig = logContextConfiguration.addFormatterConfiguration(null, PatternFormatter.class.getName(), defaultFormatterName, PATTERN.getPropertyName());
                 }
                 final String resolvedValue = (resolveValue ? FORMATTER.resolvePropertyValue(context, model) : model.asString());
                 fmtConfig.setPropertyValueString(PATTERN.getPropertyName(), resolvedValue);
-                configuration.setFormatterName(handlerName);
+                configuration.setFormatterName(defaultFormatterName);
             }
         } else if (attribute.getName().equals(NAMED_FORMATTER.getName())) {
             // The name of the handler will be used for a "formatter" if the named-formatter is not defined
             final String handlerName = configuration.getName();
+            final String defaultFormatterName = handlerName + PatternFormatterResourceDefinition.DEFAULT_FORMATTER_SUFFIX;
             final ModelNode valueNode = (resolveValue ? NAMED_FORMATTER.resolveModelAttribute(context, model) : model);
             // Set the formatter if the value is defined
             if (valueNode.isDefined()) {
                 final String resolvedValue = valueNode.asString();
                 configuration.setFormatterName(resolvedValue);
                 // If the formatter was previously defined by the formatter attribute, remove the formatter
-                if (logContextConfiguration.getFormatterNames().contains(handlerName)) {
-                    logContextConfiguration.removeFormatterConfiguration(handlerName);
+                if (logContextConfiguration.getFormatterNames().contains(defaultFormatterName)) {
+                    logContextConfiguration.removeFormatterConfiguration(defaultFormatterName);
                 }
             } else if (configuration.getClassName().equals(SyslogHandler.class.getName())) {
                 // The value shouldn't be defined so we want to remove the current formatter, however a null formatter
@@ -668,15 +670,15 @@ private static void handleProperty(final AttributeDefinition attribute, final Op
             } else {
                 // If the named-formatter was undefined we need to create a formatter based on the formatter attribute
                 final FormatterConfiguration fmtConfig;
-                if (logContextConfiguration.getFormatterNames().contains(handlerName)) {
-                    fmtConfig = logContextConfiguration.getFormatterConfiguration(handlerName);
+                if (logContextConfiguration.getFormatterNames().contains(defaultFormatterName)) {
+                    fmtConfig = logContextConfiguration.getFormatterConfiguration(defaultFormatterName);
                 } else {
-                    fmtConfig = logContextConfiguration.addFormatterConfiguration(null, PatternFormatter.class.getName(), handlerName, PATTERN.getPropertyName());
+                    fmtConfig = logContextConfiguration.addFormatterConfiguration(null, PatternFormatter.class.getName(), defaultFormatterName, PATTERN.getPropertyName());
                 }
                 // Get the current model and set the value of the formatter based on the formatter attribute
                 final Resource resource = context.readResource(PathAddress.EMPTY_ADDRESS);
                 fmtConfig.setPropertyValueString(PATTERN.getPropertyName(), FORMATTER.resolvePropertyValue(context, resource.getModel()));
-                configuration.setFormatterName(handlerName);
+                configuration.setFormatterName(defaultFormatterName);
             }
         } else if (attribute.getName().equals(FILTER_SPEC.getName())) {
             final ModelNode valueNode = (resolveValue ? FILTER_SPEC.resolveModelAttribute(context, model) : model);

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

@@ -33,6 +33,7 @@
 import org.jboss.as.controller.OperationFailedException;
 import org.jboss.as.controller.PathAddress;
 import org.jboss.as.controller.registry.Resource;
+import org.jboss.as.logging.formatters.PatternFormatterResourceDefinition;
 import org.jboss.as.server.deployment.DeploymentUnitProcessingException;
 import org.jboss.as.server.deployment.module.ResourceRoot;
 import org.jboss.logging.BasicLogger;
@@ -976,4 +977,12 @@
      */
     @Message(id = 93, value = "Failed to configure SSL context for %s %s.")
     OperationFailedException failedToConfigureSslContext(@Cause Throwable cause, String resourceName, String resourceValue);
+
+    /**
+     * Creates an exception indicating a formatter is using a reserved name.
+     *
+     * @return an {@link OperationFailedException} for the error
+     */
+    @Message(id = 94, value = "Formatter name cannot end with '" + PatternFormatterResourceDefinition.DEFAULT_FORMATTER_SUFFIX + "'")
+    OperationFailedException illegalFormatterName();
 }

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

@@ -216,6 +216,7 @@ public void testFormatsNoColor() throws Exception {
     public void testCompositeOperations() {
         final ModelNode address = createFileHandlerAddress("FILE").toModelNode();
         final String filename = "test-file.log";
+        final String defaultFormatterName = "FILE" + PatternFormatterResourceDefinition.DEFAULT_FORMATTER_SUFFIX;
 
         // Add the handler
         ModelNode addOp = OperationBuilder.createAddOperation(address)
@@ -237,18 +238,18 @@ public void testCompositeOperations() {
         // Get the log context configuration to validate what has been configured
         final LogContextConfiguration configuration = ConfigurationPersistence.getConfigurationPersistence(LogContext.getLogContext());
         assertNotNull("Expected to find the configuration", configuration);
-        assertFalse("Expected the default formatter named FILE to be removed for the handler FILE",
-                configuration.getFormatterNames().contains("FILE"));
+        assertFalse("Expected the default formatter named " + defaultFormatterName + " to be removed for the handler FILE",
+                configuration.getFormatterNames().contains(defaultFormatterName));
         final HandlerConfiguration handlerConfiguration = configuration.getHandlerConfiguration("FILE");
         assertNotNull("Expected to find the configuration for the FILE handler", configuration);
         assertEquals("Expected the handler named FILE to use the PATTERN formatter", "PATTERN",
                 handlerConfiguration.getFormatterName());
 
         // Undefine the named-formatter to ensure a formatter is created
         executeOperation(kernelServices, SubsystemOperations.createUndefineAttributeOperation(address, "named-formatter"));
-        assertTrue("Expected the default formatter named FILE to be added",
-                configuration.getFormatterNames().contains("FILE"));
-        assertEquals("Expected the handler named FILE to use the FILE formatter", "FILE",
+        assertTrue("Expected the default formatter named " + defaultFormatterName + " to be added",
+                configuration.getFormatterNames().contains(defaultFormatterName));
+        assertEquals("Expected the handler named FILE to use the FILE formatter", defaultFormatterName,
                 handlerConfiguration.getFormatterName());
     }