NIFI-3065 Addressing review comments, modifying UI, correcting wording

nifi-api/src/main/java/org/apache/nifi/flow/VersionedProcessGroup.java

@@ -47,7 +47,6 @@ public class VersionedProcessGroup extends VersionedComponent {
     private Long defaultBackPressureObjectThreshold;
     private String defaultBackPressureDataSizeThreshold;
 
-    private Boolean logToOwnFile;
     private String logFileSuffix;
 
 
@@ -209,16 +208,7 @@ public void setDefaultBackPressureDataSizeThreshold(final String defaultBackPres
         this.defaultBackPressureDataSizeThreshold = defaultBackPressureDataSizeThreshold;
     }
 
-    @ApiModelProperty(value = "Whether dedicated logging is set for this Process Group.")
-    public Boolean isLogToOwnFile() {
-        return logToOwnFile;
-    }
-
-    public void setLogToOwnFile(final Boolean logToOwnFile) {
-        this.logToOwnFile = logToOwnFile;
-    }
-
-    @ApiModelProperty(value = "The log file suffix for this Process Group.")
+    @ApiModelProperty(value = "The log file suffix for this Process Group for dedicated logging.")
     public String getLogFileSuffix() {
         return logFileSuffix;
     }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/ProcessGroupDTO.java

@@ -39,7 +39,6 @@ public class ProcessGroupDTO extends ComponentDTO {
     private String defaultFlowFileExpiration;
     private Long defaultBackPressureObjectThreshold;
     private String defaultBackPressureDataSizeThreshold;
-    private Boolean logToOwnFile;
     private String logFileSuffix;
 
     private Integer runningCount;
@@ -406,16 +405,7 @@ public void setDefaultBackPressureDataSizeThreshold(final String defaultBackPres
         this.defaultBackPressureDataSizeThreshold = defaultBackPressureDataSizeThreshold;
     }
 
-    @ApiModelProperty(value = "Whether this Process Group should log to a dedicated file.")
-    public Boolean isLogToOwnFile() {
-        return logToOwnFile;
-    }
-
-    public void setLogToOwnFile(final Boolean logToOwnFile) {
-        this.logToOwnFile = logToOwnFile;
-    }
-
-    @ApiModelProperty(value = "`Log file suffix`")
+    @ApiModelProperty(value = "The log file suffix for this Process Group for dedicated logging.")
     public String getLogFileSuffix() {
         return logFileSuffix;
     }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizer.java

@@ -317,7 +317,6 @@ private void synchronize(final ProcessGroup group, final VersionedProcessGroup p
         group.setDefaultBackPressureObjectThreshold(proposed.getDefaultBackPressureObjectThreshold());
         group.setDefaultBackPressureDataSizeThreshold(proposed.getDefaultBackPressureDataSizeThreshold());
 
-        group.setLogToOwnFile(proposed.isLogToOwnFile());
         group.setLogFileSuffix(proposed.getLogFileSuffix());
 
         final VersionedFlowCoordinates remoteCoordinates = proposed.getVersionedFlowCoordinates();
@@ -1798,7 +1797,6 @@ public void synchronizeProcessGroupSettings(final ProcessGroup processGroup, fin
                 groupToUpdate.setComments(proposed.getComments());
                 groupToUpdate.setName(proposed.getName());
                 groupToUpdate.setPosition(new Position(proposed.getPosition().getX(), proposed.getPosition().getY()));
-                groupToUpdate.setLogToOwnFile(proposed.isLogToOwnFile());
                 groupToUpdate.setLogFileSuffix(proposed.getLogFileSuffix());
 
                 if (processGroup == null) {

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java

@@ -214,8 +214,7 @@ public final class StandardProcessGroup implements ProcessGroup {
     private static final String DEFAULT_FLOWFILE_EXPIRATION = "0 sec";
     private static final long DEFAULT_BACKPRESSURE_OBJECT = 10_000L;
     private static final String DEFAULT_BACKPRESSURE_DATA_SIZE = "1 GB";
-
-    private volatile Boolean logToOwnFile = Boolean.FALSE;
+    private static final String VALID_DIRECTORY_NAME_REGEX = "[\\s\\<\\>:\\'\\\"\\/\\\\\\|\\?\\*]";
     private volatile String logFileSuffix;
 
 
@@ -247,7 +246,7 @@ public StandardProcessGroup(final String id, final ControllerServiceProvider ser
         this.defaultFlowFileExpiration = new AtomicReference<>();
         this.defaultBackPressureObjectThreshold = new AtomicReference<>();
         this.defaultBackPressureDataSizeThreshold = new AtomicReference<>();
-        this.logFileSuffix = getName();
+        this.logFileSuffix = null;
 
         // save only the nifi properties needed, and account for the possibility those properties are missing
         if (nifiProperties == null) {
@@ -4426,24 +4425,19 @@ public QueueSize getQueueSize() {
         return new QueueSize(count, contentSize);
     }
 
-    @Override
-    public Boolean isLogToOwnFile() {
-        return logToOwnFile;
-    }
-
-    @Override
-    public void setLogToOwnFile(final Boolean logToOwnFile) {
-        this.logToOwnFile = logToOwnFile;
-    }
-
     @Override
     public String getLogFileSuffix() {
         return logFileSuffix;
     }
 
     @Override
     public void setLogFileSuffix(final String logFileSuffix) {
-        this.logFileSuffix = logFileSuffix;
+        final Pattern pattern = Pattern.compile(VALID_DIRECTORY_NAME_REGEX);
+        if (logFileSuffix != null && pattern.matcher(logFileSuffix).find()) {
+            throw new IllegalArgumentException("Log file suffix can not contain the following characters: space, <, >, :, \', \", /, \\, |, ?, *");
+        } else {
+            this.logFileSuffix = logFileSuffix;
+        }
     }
 
     @Override

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/LoggingContext.java

@@ -19,6 +19,13 @@
 import java.util.Optional;
 
 public interface LoggingContext {
+    /**
+     * @return the log file name suffix. This will be the discriminating value for the dedicated logging.
+     */
     Optional<String> getLogFileSuffix();
+
+    /**
+     * @return The key under which the discriminating value should be exported into the host environment.
+     */
     String getDiscriminatorKey();
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/StandardLoggingContext.java

@@ -24,9 +24,9 @@
 
 public class StandardLoggingContext implements LoggingContext {
     private static final String KEY = "logFileSuffix";
-    private volatile PerProcessGroupLoggable component;
+    private volatile GroupedComponent component;
 
-    public StandardLoggingContext(final PerProcessGroupLoggable component) {
+    public StandardLoggingContext(final GroupedComponent component) {
         this.component = component;
     }
 
@@ -47,7 +47,7 @@ public String getDiscriminatorKey() {
     private Optional<String> getSuffix(final ProcessGroup group) {
         if (group == null) {
             return Optional.empty();
-        } else if (group.isLogToOwnFile()) {
+        } else if (group.getLogFileSuffix() != null) {
             return Optional.of(group.getLogFileSuffix());
         } else if (group.isRootGroup()) {
             return Optional.empty();
@@ -56,7 +56,7 @@ private Optional<String> getSuffix(final ProcessGroup group) {
         }
     }
 
-    public void setComponent(final PerProcessGroupLoggable component) {
+    public void setComponent(final GroupedComponent component) {
         this.component = component;
     }
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/registry/flow/StandardFlowRegistryClientNode.java

@@ -327,7 +327,6 @@ private void populateVersionedContentsRecursively(final FlowRegistryClientUserCo
             group.setDefaultFlowFileExpiration(contents.getDefaultFlowFileExpiration());
             group.setDefaultBackPressureObjectThreshold(contents.getDefaultBackPressureObjectThreshold());
             group.setDefaultBackPressureDataSizeThreshold(contents.getDefaultBackPressureDataSizeThreshold());
-            group.setLogToOwnFile(contents.isLogToOwnFile());
             group.setLogFileSuffix(contents.getLogFileSuffix());
             coordinates.setLatest(snapshot.isLatest());
 

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/registry/flow/mapping/NiFiRegistryFlowMapper.java

@@ -258,7 +258,6 @@ private InstantiatedVersionedProcessGroup mapGroup(final ProcessGroup group, fin
         versionedGroup.setDefaultFlowFileExpiration(group.getDefaultFlowFileExpiration());
         versionedGroup.setDefaultBackPressureObjectThreshold(group.getDefaultBackPressureObjectThreshold());
         versionedGroup.setDefaultBackPressureDataSizeThreshold(group.getDefaultBackPressureDataSizeThreshold());
-        versionedGroup.setLogToOwnFile(group.isLogToOwnFile());
         versionedGroup.setLogFileSuffix(group.getLogFileSuffix());
 
         final ParameterContext parameterContext = group.getParameterContext();

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/FlowDifferenceFilters.java

@@ -74,7 +74,8 @@ public static boolean isEnvironmentalChange(final FlowDifference difference, fin
             || isNewZIndexLabelConfigWithDefaultValue(difference, flowManager)
             || isNewZIndexConnectionConfigWithDefaultValue(difference, flowManager)
             || isRegistryUrlChange(difference)
-            || isParameterContextChange(difference);
+            || isParameterContextChange(difference)
+            || isLogFileSuffixChange(difference);
     }
 
     private static boolean isSensitivePropertyDueToGhosting(final FlowDifference difference, final FlowManager flowManager) {
@@ -529,4 +530,8 @@ private static boolean hasConnection(final VersionedProcessGroup processGroup, f
     private static boolean isParameterContextChange(final FlowDifference flowDifference) {
         return flowDifference.getDifferenceType() == DifferenceType.PARAMETER_CONTEXT_CHANGED;
     }
+
+    private static boolean isLogFileSuffixChange(final FlowDifference flowDifference) {
+        return flowDifference.getDifferenceType() == DifferenceType.LOG_FILE_SUFFIX_CHANGED;
+    }
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/logging/TestStandardLoggingContext.java

@@ -28,10 +28,10 @@
 
 @ExtendWith(MockitoExtension.class)
 class TestStandardLoggingContext {
-    private static final String LOG_FILE_Suffix = "myGroup";
+    private static final String LOG_FILE_SUFFIX = "myGroup";
 
     @Mock
-    private PerProcessGroupLoggable processor;
+    private GroupedComponent processor;
 
     @Mock
     private ProcessGroup processGroup;
@@ -48,7 +48,7 @@ void testComponentWithProcessGroups_WithoutPerProcessGroupLogging_ShouldReturnOp
         //component with pg with no setting returns optional empty
         LoggingContext context = new StandardLoggingContext(processor);
         when(processor.getProcessGroup()).thenReturn(processGroup);
-        when(processGroup.isLogToOwnFile()).thenReturn(Boolean.FALSE, Boolean.FALSE);
+        when(processGroup.getLogFileSuffix()).thenReturn(null, null);
         when(processGroup.isRootGroup()).thenReturn(Boolean.FALSE, Boolean.TRUE);
         when(processGroup.getParent()).thenReturn(processGroup);
 
@@ -59,21 +59,19 @@ void testComponentWithProcessGroups_WithoutPerProcessGroupLogging_ShouldReturnOp
     void testComponentWithProcessGroup_WithPerProcessGroupLogging_ShouldReturnLogFileSuffix() {
         LoggingContext context = new StandardLoggingContext(processor);
         when(processor.getProcessGroup()).thenReturn(processGroup);
-        when(processGroup.isLogToOwnFile()).thenReturn(Boolean.TRUE);
-        when(processGroup.getLogFileSuffix()).thenReturn(LOG_FILE_Suffix);
+        when(processGroup.getLogFileSuffix()).thenReturn(LOG_FILE_SUFFIX);
 
-        assertEquals(LOG_FILE_Suffix, context.getLogFileSuffix().orElse(null));
+        assertEquals(LOG_FILE_SUFFIX, context.getLogFileSuffix().orElse(null));
     }
 
     @Test
     void testComponentWithProcessGroups_WithPerProcessGroupLoggingSetOnParent_ShouldReturnLogFileSuffix() {
         LoggingContext context = new StandardLoggingContext(processor);
         when(processor.getProcessGroup()).thenReturn(processGroup);
-        when(processGroup.isLogToOwnFile()).thenReturn(Boolean.FALSE, Boolean.TRUE);
         when(processGroup.isRootGroup()).thenReturn(Boolean.FALSE);
         when(processGroup.getParent()).thenReturn(processGroup);
-        when(processGroup.getLogFileSuffix()).thenReturn(LOG_FILE_Suffix);
+        when(processGroup.getLogFileSuffix()).thenReturn(null, LOG_FILE_SUFFIX);
 
-        assertEquals(LOG_FILE_Suffix, context.getLogFileSuffix().orElse(null));
+        assertEquals(LOG_FILE_SUFFIX, context.getLogFileSuffix().orElse(null));
     }
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/connectable/Connectable.java

@@ -22,7 +22,7 @@
 import org.apache.nifi.controller.BackoffMechanism;
 import org.apache.nifi.controller.Triggerable;
 import org.apache.nifi.groups.ProcessGroup;
-import org.apache.nifi.logging.PerProcessGroupLoggable;
+import org.apache.nifi.logging.GroupedComponent;
 import org.apache.nifi.processor.ProcessSession;
 import org.apache.nifi.processor.Relationship;
 import org.apache.nifi.scheduling.SchedulingStrategy;
@@ -35,7 +35,7 @@
 /**
  * Represents a connectable component to which or from which data can flow.
  */
-public interface Connectable extends Triggerable, ComponentAuthorizable, Positionable, VersionedComponent, PerProcessGroupLoggable {
+public interface Connectable extends Triggerable, ComponentAuthorizable, Positionable, VersionedComponent, GroupedComponent {
 
     /**
      * @return the unique identifier for this <code>Connectable</code>

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/service/ControllerServiceNode.java

@@ -27,7 +27,7 @@
 import org.apache.nifi.groups.ProcessGroup;
 import org.apache.nifi.logging.ComponentLog;
 import org.apache.nifi.logging.LogLevel;
-import org.apache.nifi.logging.PerProcessGroupLoggable;
+import org.apache.nifi.logging.GroupedComponent;
 import org.apache.nifi.nar.ExtensionManager;
 
 import java.util.List;
@@ -37,7 +37,7 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 
-public interface ControllerServiceNode extends ComponentNode, VersionedComponent, PerProcessGroupLoggable {
+public interface ControllerServiceNode extends ComponentNode, VersionedComponent, GroupedComponent {
 
     /**
      * @return the Process Group that this Controller Service belongs to, or <code>null</code> if the Controller Service

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/groups/ProcessGroup.java

@@ -1235,24 +1235,12 @@ public interface ProcessGroup extends ComponentAuthorizable, Positionable, Versi
     QueueSize getQueueSize();
 
     /**
-     * @return whether dedicated logging is set for the ProcessGroup
-     */
-    Boolean isLogToOwnFile();
-
-    /**
-     * Updates logging setting of this ProcessGroup.
-     *
-     * @param logToOwnFile new logging setting value
-     */
-    void setLogToOwnFile(Boolean logToOwnFile);
-
-    /**
-     * @return the log file suffix of the ProcessGroup
+     * @return the log file suffix of the ProcessGroup for dedicated logging
      */
     String getLogFileSuffix();
 
     /**
-     * Updates the log file suffix of this ProcessGroup.
+     * Updates the log file suffix of this ProcessGroup for dedicated logging
      *
      * @param logFileSuffix new log file suffix
      */

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/logging/PerProcessGroupLoggable.java → nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/logging/GroupedComponent.java

@@ -18,6 +18,6 @@
 
 import org.apache.nifi.groups.ProcessGroup;
 
-public interface PerProcessGroupLoggable {
+public interface GroupedComponent {
     ProcessGroup getProcessGroup();
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardFlowSnippet.java

@@ -533,14 +533,11 @@ public void instantiate(final FlowManager flowManager, final FlowController flow
                 childGroup.setDefaultBackPressureDataSizeThreshold(defaultBackPressureDataSizeThreshold);
             }
 
-            final Boolean logToOwnFile = groupDTO.isLogToOwnFile();
-            if (logToOwnFile != null) {
-                childGroup.setLogToOwnFile(logToOwnFile);
+            final String logFileSuffix = groupDTO.getLogFileSuffix();
+            if (logFileSuffix != null) {
+                childGroup.setLogFileSuffix(logFileSuffix);
             }
 
-            //To avoid log file name conflict we default the log file suffix to the Process Group's name.
-            childGroup.setLogFileSuffix(groupDTO.getName());
-
             // If this Process Group is 'top level' then we do not set versioned component ID's.
             // We do this only if this component is the child of a Versioned Component.
             if (!topLevel) {

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/XmlFlowSynchronizer.java

@@ -1292,7 +1292,6 @@ private void updateProcessGroup(final ProcessGroup group, final ProcessGroupDTO
         final String defaultFlowFileExpiration = dto.getDefaultFlowFileExpiration();
         final Long defaultBackPressureObjectThreshold = dto.getDefaultBackPressureObjectThreshold();
         final String defaultBackPressureDataSizeThreshold = dto.getDefaultBackPressureDataSizeThreshold();
-        final Boolean logToOwnFile = dto.isLogToOwnFile();
         final String logFileSuffix = dto.getLogFileSuffix();
 
         if (name != null) {
@@ -1336,10 +1335,6 @@ private void updateProcessGroup(final ProcessGroup group, final ProcessGroupDTO
             group.setDefaultBackPressureDataSizeThreshold(defaultBackPressureDataSizeThreshold);
         }
 
-        if (logToOwnFile != null) {
-            group.setLogToOwnFile(logToOwnFile);
-        }
-
         if (logFileSuffix != null) {
             group.setLogFileSuffix(logFileSuffix);
         }
@@ -1478,7 +1473,6 @@ private ProcessGroup addProcessGroup(final FlowController controller, final Proc
         processGroup.setDefaultBackPressureObjectThreshold(processGroupDTO.getDefaultBackPressureObjectThreshold());
         processGroup.setDefaultBackPressureDataSizeThreshold(processGroupDTO.getDefaultBackPressureDataSizeThreshold());
 
-        processGroup.setLogToOwnFile(processGroupDTO.isLogToOwnFile());
         processGroup.setLogFileSuffix(processGroupDTO.getLogFileSuffix());
 
         final String parameterContextId = getString(processGroupElement, "parameterContextId");

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/FlowFromDOMFactory.java

@@ -296,7 +296,6 @@ public static ProcessGroupDTO getProcessGroup(final String parentId, final Eleme
         dto.setDefaultFlowFileExpiration(getString(element, "defaultFlowFileExpiration"));
         dto.setDefaultBackPressureObjectThreshold(getLong(element, "defaultBackPressureObjectThreshold"));
         dto.setDefaultBackPressureDataSizeThreshold(getString(element, "defaultBackPressureDataSizeThreshold"));
-        dto.setLogToOwnFile(getBoolean(element, "logToOwnFile"));
         dto.setLogFileSuffix(getString(element, "logFileSuffix"));
 
         final Map<String, String> variables = new HashMap<>();

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/StandardFlowSerializer.java

@@ -256,7 +256,6 @@ private void addProcessGroup(final Element parentElement, final ProcessGroup gro
         addTextElement(element, "defaultFlowFileExpiration", group.getDefaultFlowFileExpiration());
         addTextElement(element, "defaultBackPressureObjectThreshold", group.getDefaultBackPressureObjectThreshold());
         addTextElement(element, "defaultBackPressureDataSizeThreshold", group.getDefaultBackPressureDataSizeThreshold());
-        addTextElement(element, "logToOwnFile", String.valueOf(group.isLogToOwnFile()));
         addTextElement(element, "logFileSuffix", group.getLogFileSuffix());
 
         final VersionControlInformation versionControlInfo = group.getVersionControlInformation();

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/fingerprint/FingerprintFactory.java

@@ -392,7 +392,6 @@ StringBuilder addProcessGroupFingerprint(final StringBuilder builder, final Elem
         appendFirstValue(builder, DomUtils.getChildNodesByTagName(processGroupElem, "defaultFlowFileExpiration"));
         appendFirstValue(builder, DomUtils.getChildNodesByTagName(processGroupElem, "defaultBackPressureObjectThreshold"));
         appendFirstValue(builder, DomUtils.getChildNodesByTagName(processGroupElem, "defaultBackPressureDataSizeThreshold"));
-        appendFirstValue(builder, DomUtils.getChildNodesByTagName(processGroupElem, "logToOwnFile"));
         appendFirstValue(builder, DomUtils.getChildNodesByTagName(processGroupElem, "logFileSuffix"));
 
         final Element versionControlInfo = DomUtils.getChild(processGroupElem, "versionControlInformation");