This is an automated email from the ASF dual-hosted git repository.
bamaer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 68b1e51233 Issue #8521 : Honor a disabled linter and file or script
action sources (#8635)
68b1e51233 is described below
commit 68b1e51233172cb161f00415513ee8acc3bbdf63
Author: Matt Casters <[email protected]>
AuthorDate: Mon Sep 28 09:26:44 2026 +0200
Issue #8521 : Honor a disabled linter and file or script action sources
(#8635)
* Issue #8521 : Honor a disabled linter and file or script action sources
Switching the linter off left Explorer colors in place and still linted
the open file. SQL, Shell and Set Variables also reported a missing
script or variable list when the action was using the other source.
* Issue #8521 : Drop lint results that arrive after the linter is switched
off
updateResultsForFile ignores findings while the linter is disabled, so an
in-flight pass cannot refill the Problems tab. Turning the linter back on
attaches the Problems bar to editors that were opened while it was off.
---
.../actions/setvariables/ActionSetVariables.java | 37 +++----
.../messages/messages_en_US.properties | 1 +
.../setvariables/ActionSetVariablesCheckTest.java | 81 ++++++++++++++
.../hop/workflow/actions/shell/ActionShell.java | 24 +++--
.../shell/messages/messages_en_US.properties | 1 +
.../actions/shell/ActionShellCheckTest.java | 86 +++++++++++++++
.../apache/hop/workflow/actions/sql/ActionSql.java | 28 +++--
.../actions/sql/messages/messages_en_US.properties | 1 +
.../workflow/actions/sql/ActionSqlCheckTest.java | 78 ++++++++++++++
.../org/apache/hop/lint/BackgroundLintService.java | 12 ++-
.../org/apache/hop/lint/EditorLintSupport.java | 9 ++
.../org/apache/hop/lint/LintResultsManager.java | 5 +
.../org/apache/hop/lint/LintStatusFilePainter.java | 107 +++++++++++++++---
.../org/apache/hop/lint/LinterConfigPlugin.java | 56 ++++++++++
.../hop/lint/PipelineAfterOpenLintExtension.java | 7 +-
.../hop/lint/WorkflowAfterOpenLintExtension.java | 7 +-
.../org/apache/hop/lint/DisabledLinterTest.java | 119 +++++++++++++++++++++
17 files changed, 605 insertions(+), 54 deletions(-)
diff --git
a/plugins/actions/setvariables/src/main/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariables.java
b/plugins/actions/setvariables/src/main/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariables.java
index fc12db0242..e1af116474 100644
---
a/plugins/actions/setvariables/src/main/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariables.java
+++
b/plugins/actions/setvariables/src/main/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariables.java
@@ -24,6 +24,7 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
+import org.apache.hop.core.CheckResult;
import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.Result;
import org.apache.hop.core.annotations.Action;
@@ -44,10 +45,6 @@ import org.apache.hop.resource.ResourceReference;
import org.apache.hop.workflow.WorkflowMeta;
import org.apache.hop.workflow.action.ActionBase;
import org.apache.hop.workflow.action.IAction;
-import org.apache.hop.workflow.action.validator.AbstractFileValidator;
-import org.apache.hop.workflow.action.validator.ActionValidatorUtils;
-import org.apache.hop.workflow.action.validator.AndValidator;
-import org.apache.hop.workflow.action.validator.ValidatorContext;
import org.apache.hop.workflow.engine.IWorkflowEngine;
/** This defines a 'Set variables' action. */
@@ -353,22 +350,26 @@ public class ActionSetVariables extends ActionBase
implements Cloneable, IAction
WorkflowMeta workflowMeta,
IVariables variables,
IHopMetadataProvider metadataProvider) {
- boolean res =
- ActionValidatorUtils.andValidator()
- .validate(
- this,
- "variableName",
- remarks,
-
AndValidator.putValidators(ActionValidatorUtils.notNullValidator()));
-
- if (!res) {
- return;
+ // Variables come from the list, from a properties file, or from both.
Either source is enough.
+ if (Utils.isEmpty(filename) && !hasNamedVariable()) {
+ remarks.add(
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG,
"ActionSetVariables.NoVariablesSpecified"),
+ this));
}
+ }
- ValidatorContext ctx = new ValidatorContext();
- AbstractFileValidator.putVariableSpace(ctx, getVariables());
- AndValidator.putValidators(
- ctx, ActionValidatorUtils.notNullValidator(),
ActionValidatorUtils.fileExistsValidator());
+ private boolean hasNamedVariable() {
+ if (variableDefinitions == null) {
+ return false;
+ }
+ for (VariableDefinition definition : variableDefinitions) {
+ if (definition != null && !Utils.isEmpty(definition.getName())) {
+ return true;
+ }
+ }
+ return false;
}
@Override
diff --git
a/plugins/actions/setvariables/src/main/resources/org/apache/hop/workflow/actions/setvariables/messages/messages_en_US.properties
b/plugins/actions/setvariables/src/main/resources/org/apache/hop/workflow/actions/setvariables/messages/messages_en_US.properties
index 8cc26b1f04..ddb63f081c 100644
---
a/plugins/actions/setvariables/src/main/resources/org/apache/hop/workflow/actions/setvariables/messages/messages_en_US.properties
+++
b/plugins/actions/setvariables/src/main/resources/org/apache/hop/workflow/actions/setvariables/messages/messages_en_US.properties
@@ -30,6 +30,7 @@ ActionSetVariables.FileVariableType.Label=Variable scope
ActionSetVariables.keyword=variable,environment,parameter,assign,property
ActionSetVariables.Log.SetVariableToValue=Set variable {0} to value [{1}]
ActionSetVariables.Name=Set variables
+ActionSetVariables.NoVariablesSpecified=Specify the variables to set, or a
properties file that contains them.
ActionSetVariables.Name.Default=Set variables
ActionSetVariables.Name.Label=Action name
ActionSetVariables.Settings.Label=Settings
diff --git
a/plugins/actions/setvariables/src/test/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariablesCheckTest.java
b/plugins/actions/setvariables/src/test/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariablesCheckTest.java
new file mode 100644
index 0000000000..336f8c3acc
--- /dev/null
+++
b/plugins/actions/setvariables/src/test/java/org/apache/hop/workflow/actions/setvariables/ActionSetVariablesCheckTest.java
@@ -0,0 +1,81 @@
+/*
+ * 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.hop.workflow.actions.setvariables;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.ICheckResult;
+import
org.apache.hop.workflow.actions.setvariables.ActionSetVariables.VariableDefinition;
+import
org.apache.hop.workflow.actions.setvariables.ActionSetVariables.VariableType;
+import org.junit.jupiter.api.Test;
+
+/** Variables are listed on the action or read from a properties file. Either
source is valid. */
+class ActionSetVariablesCheckTest {
+
+ @Test
+ void aPropertiesFileDoesNotRequireListedVariables() {
+ ActionSetVariables action = new ActionSetVariables();
+ action.setFilename("${PROJECT_HOME}/retail.properties");
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void listedVariablesDoNotRequireAPropertiesFile() {
+ ActionSetVariables action = new ActionSetVariables();
+ action
+ .getVariableDefinitions()
+ .add(new VariableDefinition("RETAIL_CSV_WAVE", "1",
VariableType.CURRENT_WORKFLOW));
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void anEmptyVariableRowIsIgnoredWhenAPropertiesFileIsSet() {
+ ActionSetVariables action = new ActionSetVariables();
+ action.setFilename("${PROJECT_HOME}/retail.properties");
+ action.getVariableDefinitions().add(new VariableDefinition("", "",
VariableType.JVM));
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void nothingToSetIsReported() {
+ ActionSetVariables action = new ActionSetVariables();
+
+ List<String> errors = errors(action);
+ assertEquals(1, errors.size());
+ assertTrue(errors.get(0).contains("properties file"));
+ assertTrue(errors.stream().noneMatch(text ->
text.contains("variableName")));
+ }
+
+ private static List<String> errors(ActionSetVariables action) {
+ List<ICheckResult> remarks = new ArrayList<>();
+ action.check(remarks, null, null, null);
+ List<String> errors = new ArrayList<>();
+ for (ICheckResult remark : remarks) {
+ if (remark.getType() == ICheckResult.TYPE_RESULT_ERROR) {
+ errors.add(remark.getText());
+ }
+ }
+ return errors;
+ }
+}
diff --git
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
index 99a82f0aa7..65021c75da 100644
---
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
+++
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
@@ -30,6 +30,7 @@ import lombok.Getter;
import lombok.Setter;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.vfs2.FileObject;
+import org.apache.hop.core.CheckResult;
import org.apache.hop.core.Const;
import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.Result;
@@ -665,12 +666,23 @@ public class ActionShell extends ActionBase implements
ILegacyXml {
ctx, ActionValidatorUtils.notBlankValidator(),
ActionValidatorUtils.fileExistsValidator());
ActionValidatorUtils.andValidator().validate(this, "workDirectory",
remarks, ctx);
- ActionValidatorUtils.andValidator()
- .validate(
- this,
- "filename",
- remarks,
-
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ // The script is either typed on the Script tab or read from a file.
+ if (insertScript) {
+ if (StringUtils.isBlank(script)) {
+ remarks.add(
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG, "ActionShell.NoScriptSpecified"),
+ this));
+ }
+ } else {
+ ActionValidatorUtils.andValidator()
+ .validate(
+ this,
+ "filename",
+ remarks,
+
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ }
if (setLogfile) {
ActionValidatorUtils.andValidator()
diff --git
a/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
b/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
index 3f904ff854..8af24d7ff8 100644
---
a/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
+++
b/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
@@ -45,6 +45,7 @@ ActionShell.LogfileExtension.Label=Extension of logfile
ActionShell.Loglevel.Label=Log level
ActionShell.LogSettings.Group.Label=Logging settings
ActionShell.NoScriptFileSpecified=Please specify script filename\!
+ActionShell.NoScriptSpecified=Please specify the script to execute.
ActionShell.ReferencedObject.Description=Shell script
ActionShell.Name=Shell
ActionShell.Name.Label=Action name
diff --git
a/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/ActionShellCheckTest.java
b/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/ActionShellCheckTest.java
new file mode 100644
index 0000000000..565874780b
--- /dev/null
+++
b/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/ActionShellCheckTest.java
@@ -0,0 +1,86 @@
+/*
+ * 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.hop.workflow.actions.shell;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.ICheckResult;
+import org.junit.jupiter.api.Test;
+
+/** A shell action runs a script file or the text on the Script tab, not both.
*/
+class ActionShellCheckTest {
+
+ @Test
+ void aScriptOnTheScriptTabDoesNotRequireAFilename() {
+ ActionShell action = shellWithWorkingDirectory();
+ action.setInsertScript(true);
+ action.setScript("echo hello");
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void aScriptFileDoesNotRequireScriptText() {
+ ActionShell action = shellWithWorkingDirectory();
+ action.setFilename("${PROJECT_HOME}/bootstrap-retail-work.py");
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void aMissingScriptIsReportedWhenTheScriptTabIsUsed() {
+ ActionShell action = shellWithWorkingDirectory();
+ action.setInsertScript(true);
+
+ List<String> errors = errors(action);
+ assertEquals(1, errors.size());
+ assertTrue(errors.get(0).contains("script to execute"));
+ }
+
+ @Test
+ void aMissingFilenameIsReportedWhenAFileIsUsed() {
+ ActionShell action = shellWithWorkingDirectory();
+
+ List<String> errors = errors(action);
+ assertEquals(1, errors.size());
+ assertTrue(errors.get(0).contains("filename"));
+ }
+
+ private static ActionShell shellWithWorkingDirectory() {
+ ActionShell action = new ActionShell();
+ // The working directory is checked on its own. Give it a value so these
tests see only the
+ // script-or-file result.
+ action.setWorkDirectory(".");
+ return action;
+ }
+
+ private static List<String> errors(ActionShell action) {
+ List<ICheckResult> remarks = new ArrayList<>();
+ action.check(remarks, null, action, null);
+ List<String> errors = new ArrayList<>();
+ for (ICheckResult remark : remarks) {
+ if (remark.getType() == ICheckResult.TYPE_RESULT_ERROR) {
+ errors.add(remark.getText());
+ }
+ }
+ return errors;
+ }
+}
diff --git
a/plugins/actions/sql/src/main/java/org/apache/hop/workflow/actions/sql/ActionSql.java
b/plugins/actions/sql/src/main/java/org/apache/hop/workflow/actions/sql/ActionSql.java
index dec3fd538e..1e5858d3ef 100644
---
a/plugins/actions/sql/src/main/java/org/apache/hop/workflow/actions/sql/ActionSql.java
+++
b/plugins/actions/sql/src/main/java/org/apache/hop/workflow/actions/sql/ActionSql.java
@@ -24,7 +24,9 @@ import java.io.InputStreamReader;
import java.util.List;
import lombok.Getter;
import lombok.Setter;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.vfs2.FileObject;
+import org.apache.hop.core.CheckResult;
import org.apache.hop.core.Const;
import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.Result;
@@ -48,8 +50,6 @@ import org.apache.hop.resource.ResourceReference;
import org.apache.hop.workflow.WorkflowMeta;
import org.apache.hop.workflow.action.ActionBase;
import org.apache.hop.workflow.action.IAction;
-import org.apache.hop.workflow.action.validator.ActionValidatorUtils;
-import org.apache.hop.workflow.action.validator.AndValidator;
/** This defines an SQL action. */
@Action(
@@ -237,12 +237,24 @@ public class ActionSql extends ActionBase implements
Cloneable, IAction {
WorkflowMeta workflowMeta,
IVariables variables,
IHopMetadataProvider metadataProvider) {
- ActionValidatorUtils.andValidator()
- .validate(
- this,
- "SQL",
- remarks,
-
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ // The statement is either typed in or read from a file. Only the source
in use has to be set.
+ if (sqlFromFile) {
+ if (StringUtils.isBlank(sqlFilename)) {
+ remarks.add(
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG, "ActionSQL.NoSQLFileSpecified"),
+ this));
+ }
+ return;
+ }
+ if (StringUtils.isBlank(sql)) {
+ remarks.add(
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG, "ActionSQL.NoSQLSpecified"),
+ this));
+ }
}
@Override
diff --git
a/plugins/actions/sql/src/main/resources/org/apache/hop/workflow/actions/sql/messages/messages_en_US.properties
b/plugins/actions/sql/src/main/resources/org/apache/hop/workflow/actions/sql/messages/messages_en_US.properties
index c912da207b..4dadbe05b4 100644
---
a/plugins/actions/sql/src/main/resources/org/apache/hop/workflow/actions/sql/messages/messages_en_US.properties
+++
b/plugins/actions/sql/src/main/resources/org/apache/hop/workflow/actions/sql/messages/messages_en_US.properties
@@ -35,6 +35,7 @@ ActionSQL.Name.Default=SQL
ActionSQL.Name.Label=Action name
ActionSQL.NoDatabaseConnection=No database connection is defined.
ActionSQL.NoSQLFileSpecified=Please specify SQL filename\!
+ActionSQL.NoSQLSpecified=Please specify the SQL to execute.
ActionSQL.ReferencedObject.Description=SQL file
ActionSQL.Position.Label=Line {0} Column {1}
ActionSQL.Script.Label=SQL Script\:
diff --git
a/plugins/actions/sql/src/test/java/org/apache/hop/workflow/actions/sql/ActionSqlCheckTest.java
b/plugins/actions/sql/src/test/java/org/apache/hop/workflow/actions/sql/ActionSqlCheckTest.java
new file mode 100644
index 0000000000..d9e0c3a4ce
--- /dev/null
+++
b/plugins/actions/sql/src/test/java/org/apache/hop/workflow/actions/sql/ActionSqlCheckTest.java
@@ -0,0 +1,78 @@
+/*
+ * 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.hop.workflow.actions.sql;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.ICheckResult;
+import org.junit.jupiter.api.Test;
+
+/** SQL comes from the script field or from a file. Checking the unused one is
a false positive. */
+class ActionSqlCheckTest {
+
+ @Test
+ void sqlFromAFileDoesNotRequireTheScript() {
+ ActionSql action = new ActionSql();
+ action.setSqlFromFile(true);
+ action.setSqlFilename("${PROJECT_HOME}/create-staging-views.sql");
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void aTypedScriptDoesNotRequireAFile() {
+ ActionSql action = new ActionSql();
+ action.setSql("drop table staging_customer");
+
+ assertTrue(errors(action).isEmpty());
+ }
+
+ @Test
+ void aMissingFileIsReportedWhenSqlComesFromAFile() {
+ ActionSql action = new ActionSql();
+ action.setSqlFromFile(true);
+
+ List<String> errors = errors(action);
+ assertEquals(1, errors.size());
+ assertTrue(errors.get(0).contains("SQL filename"));
+ }
+
+ @Test
+ void aMissingScriptIsReportedWhenSqlIsTyped() {
+ ActionSql action = new ActionSql();
+
+ List<String> errors = errors(action);
+ assertEquals(1, errors.size());
+ assertTrue(errors.get(0).contains("SQL to execute"));
+ }
+
+ private static List<String> errors(ActionSql action) {
+ List<ICheckResult> remarks = new ArrayList<>();
+ action.check(remarks, null, null, null);
+ List<String> errors = new ArrayList<>();
+ for (ICheckResult remark : remarks) {
+ if (remark.getType() == ICheckResult.TYPE_RESULT_ERROR) {
+ errors.add(remark.getText());
+ }
+ }
+ return errors;
+ }
+}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/BackgroundLintService.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/BackgroundLintService.java
index 63753adb0c..9b101a71a9 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/BackgroundLintService.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/BackgroundLintService.java
@@ -118,7 +118,9 @@ public class BackgroundLintService {
}
public void scheduleGraphLint(HopGuiAbstractGraph graph, boolean force) {
- if (graph == null) {
+ // Opening or editing a file schedules this even when the linter is
switched off. Without the
+ // check, that one open file is still linted and stays marked in the
Explorer.
+ if (!isEnabled() || graph == null) {
return;
}
String graphId = graph.getId();
@@ -192,6 +194,10 @@ public class BackgroundLintService {
*/
private void lintGraphInternal(HopGuiAbstractGraph graph, boolean force) {
String graphId = graph.getId();
+ if (!isEnabled()) {
+ deferredGenerations.remove(graphId);
+ return;
+ }
String filename = LintEditorGraphHelper.getFilename(graph);
if (!LintEditorGraphHelper.isLintableFilename(filename)) {
deferredGenerations.remove(graphId);
@@ -210,6 +216,10 @@ public class BackgroundLintService {
submit(
() -> {
try {
+ // Switched off while this pass was waiting or running: do not
publish what it found.
+ if (!isEnabled()) {
+ return;
+ }
HopGui hopGui = HopGui.peekInstance();
IHopMetadataProvider metadataProvider =
hopGui != null ? hopGui.getMetadataProvider() : null;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/EditorLintSupport.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/EditorLintSupport.java
index 2a7527adea..6bb3888da7 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/EditorLintSupport.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/EditorLintSupport.java
@@ -27,6 +27,9 @@ final class EditorLintSupport {
private EditorLintSupport() {}
static void onNewGraph(HopGuiAbstractGraph graph) {
+ if (!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
+ }
if (graph == null || graph.isDisposed()) {
return;
}
@@ -35,6 +38,9 @@ final class EditorLintSupport {
}
static void onGraphUpdate(HopGuiAbstractGraph graph) {
+ if (!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
+ }
if (graph == null || graph.isDisposed()) {
return;
}
@@ -53,6 +59,9 @@ final class EditorLintSupport {
}
static void onFileSaved(String filename) {
+ if (!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
+ }
if (!LintEditorGraphHelper.isLintableFilename(filename)) {
return;
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
index 619dce6c01..cbf89c2792 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
@@ -143,6 +143,11 @@ public class LintResultsManager {
/** Update results for a single file without clearing other file results. */
public synchronized void updateResultsForFile(String filePath,
List<LintResult> results) {
+ // One choke point for every producer. An in-flight pass must not publish
after the linter was
+ // switched off and clearResults() has already run.
+ if (!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
+ }
String normalizedPath = LintPathUtils.normalizePath(filePath);
// Capture the previous results for this file so we only notify (and
trigger canvas/Explorer
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
index ec02dde8e9..c45ededfe5 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintStatusFilePainter.java
@@ -54,6 +54,15 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
private static final String BASE_ICON_KEY = "lintBaseIcon";
private static final String APPLIED_STATUS_KEY = "lintAppliedStatus";
+ /**
+ * The file this item was painted for.
+ *
+ * <p>The Explorer calls this painter when it builds a tree item, not on
every redraw. Remembering
+ * the path is what lets a later result change, or switching the linter off,
update the item that
+ * is already on screen.
+ */
+ private static final String LINT_PATH_KEY = "lintPath";
+
/** Size of the status badge, matching the small icons the Explorer tree
draws. */
private static final int BADGE_SIZE = 12;
@@ -130,19 +139,60 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
}
/**
- * Repaint lint icons in the Explorer. Prefers a lightweight tree redraw
(which re-runs this
- * painter while preserving selection and expansion) and only falls back to
a full perspective
- * refresh when we have not painted a tree yet.
+ * Whether Explorer files should carry a lint mark.
+ *
+ * <p>The canvas overlays already follow this switch. The Explorer used to
keep painting from
+ * whatever the last run had stored, so a file stayed red after the linter
was switched off.
+ */
+ static boolean decoratesExplorerFiles() {
+ try {
+ return LinterConfigPlugin.getInstance().isLinterEnabled();
+ } catch (Exception e) {
+ return true;
+ }
+ }
+
+ /**
+ * Re-apply lint marks on the items already in the Explorer.
+ *
+ * <p>A redraw of the tree does not ask this painter again: the color and
icon were set when the
+ * item was created. Walking those items keeps the selection and the
expanded folders, and is what
+ * clears a mark when the linter is switched off or a file no longer has
findings.
*/
public void repaintExplorerIcons() {
- Tree tree = lastPaintedTree;
- if (tree != null && !tree.isDisposed()) {
- tree.redraw();
+ // Callers hop to the UI thread first. Doing it again here loops when this
thread has no
+ // display to report, which is how a dead Hop Web session answers.
+ try {
+ Tree tree = lastPaintedTree;
+ if (tree != null && !tree.isDisposed()) {
+ reapply(tree);
+ return;
+ }
+ ExplorerPerspective perspective = HopGui.getExplorerPerspective();
+ if (perspective != null) {
+ perspective.refresh();
+ }
+ } catch (Exception e) {
+ log.logError("Error repainting explorer lint icons: " + e.getMessage(),
e);
+ }
+ }
+
+ private void reapply(Tree tree) {
+ for (TreeItem item : tree.getItems()) {
+ reapply(tree, item);
+ }
+ }
+
+ private void reapply(Tree tree, TreeItem item) {
+ if (item == null || item.isDisposed()) {
return;
}
- ExplorerPerspective perspective = HopGui.getExplorerPerspective();
- if (perspective != null) {
- perspective.refresh();
+ String path = (String) item.getData(LINT_PATH_KEY);
+ if (path != null) {
+ applyResolvedStatus(tree, item, item.getText(), path);
+ }
+ for (TreeItem child : item.getItems()) {
+ reapply(tree, child);
}
}
@@ -156,18 +206,45 @@ public class LintStatusFilePainter implements
IExplorerFilePaintListener {
return;
}
- LintStatus status = resolveStatus(absolutePath);
- if (status == LintStatus.UNKNOWN) {
- return;
- }
-
- applyStatusStyle(tree, treeItem, name, absolutePath, status);
+ treeItem.setData(LINT_PATH_KEY, absolutePath);
+ applyResolvedStatus(tree, treeItem, name, absolutePath);
} catch (Exception e) {
log.logError(
"Error painting lint status for file " + path + "/" + name + ": " +
e.getMessage(), e);
}
}
+ private void applyResolvedStatus(Tree tree, TreeItem treeItem, String name,
String absolutePath) {
+ if (!decoratesExplorerFiles()) {
+ clearStatusStyle(tree, treeItem);
+ return;
+ }
+ LintStatus status = resolveStatus(absolutePath);
+ if (status == LintStatus.UNKNOWN) {
+ clearStatusStyle(tree, treeItem);
+ return;
+ }
+ applyStatusStyle(tree, treeItem, name, absolutePath, status);
+ }
+
+ /** Put the item back the way the Explorer built it, when a lint mark is no
longer called for. */
+ private void clearStatusStyle(Tree tree, TreeItem treeItem) {
+ if (treeItem.getData(APPLIED_STATUS_KEY) == null) {
+ return;
+ }
+ org.eclipse.swt.graphics.Color darkGray =
tree.getDisplay().getSystemColor(SWT.COLOR_DARK_GRAY);
+ org.eclipse.swt.graphics.Color current = treeItem.getForeground();
+ if (current == null || !current.equals(darkGray)) {
+ treeItem.setForeground(null);
+ }
+ Image base = (Image) treeItem.getData(BASE_ICON_KEY);
+ if (base != null && !base.isDisposed()) {
+ treeItem.setImage(base);
+ }
+ treeItem.setData(APPLIED_STATUS_KEY, null);
+ treeItem.setData("lintTooltip", null);
+ }
+
private void applyStatusStyle(
Tree tree, TreeItem treeItem, String name, String absolutePath,
LintStatus status) {
org.eclipse.swt.graphics.Color systemDarkGray =
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
index dbad946f4a..9f5585efde 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
@@ -39,7 +39,10 @@ import org.apache.hop.ui.core.gui.GuiCompositeWidgets;
import org.apache.hop.ui.core.gui.IGuiPluginCompositeWidgetsListener;
import org.apache.hop.ui.core.widget.TextVar;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.file.shared.HopGuiAbstractGraph;
+import org.apache.hop.ui.hopgui.perspective.TabItemHandler;
import
org.apache.hop.ui.hopgui.perspective.configuration.tabs.ConfigPluginOptionsTab;
+import org.apache.hop.ui.hopgui.perspective.explorer.ExplorerPerspective;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Control;
import picocli.CommandLine;
@@ -130,6 +133,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
*/
@Override
public void persistContents(GuiCompositeWidgets compositeWidgets) {
+ boolean enabledBefore = isLinterEnabled();
for (String widgetId : compositeWidgets.getWidgetsMap().keySet()) {
Control control = compositeWidgets.getWidgetsMap().get(widgetId);
switch (widgetId) {
@@ -158,6 +162,9 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
}
}
saveToHopConfig();
+ if (enabledBefore != isLinterEnabled()) {
+ applyEnabledState();
+ }
}
/**
@@ -188,6 +195,55 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
return options;
}
+ /**
+ * Drop marks the Explorer and open editors are still showing, or lint the
project again.
+ *
+ * <p>Called after the new value has been saved. Reading the option back has
to see it: the
+ * Explorer painter and the background service load a fresh instance rather
than this one.
+ */
+ void applyEnabledState() {
+ if (!isLinterEnabled()) {
+ LintResultsManager.getInstance().clearResults();
+ try {
+ LintProblemsBarManager.getInstance().refreshAllOpenEditors();
+ } catch (Exception | LinkageError e) {
+ log.logDetailed("No open editor to clear lint marks from: " +
e.getMessage());
+ }
+ return;
+ }
+ try {
+ HopGui hopGui = HopGui.peekInstance();
+ if (hopGui == null) {
+ return;
+ }
+ attachOpenEditors();
+ BackgroundLintService.getInstance()
+ .lintProjectAsync(getProjectPath(), hopGui.getMetadataProvider(),
hopGui.getVariables());
+ } catch (Exception | LinkageError e) {
+ log.logDetailed("Could not lint the project after enabling the linter: "
+ e.getMessage());
+ }
+ }
+
+ /**
+ * Editors opened while the linter was off never registered a Problems bar.
Re-enabling lints the
+ * project, but the bar only comes back once those graphs are attached.
+ */
+ private static void attachOpenEditors() {
+ try {
+ ExplorerPerspective perspective = HopGui.getExplorerPerspective();
+ if (perspective == null) {
+ return;
+ }
+ for (TabItemHandler item : perspective.getItems()) {
+ if (item.getTypeHandler() instanceof HopGuiAbstractGraph graph &&
!graph.isDisposed()) {
+ EditorLintSupport.onNewGraph(graph);
+ }
+ }
+ } catch (Exception | LinkageError e) {
+ log.logDetailed("Could not attach the problems bar to open editors: " +
e.getMessage());
+ }
+ }
+
private static void putIfSet(Map<String, Object> options, String key, Object
value) {
if (value != null) {
options.put(key, value);
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineAfterOpenLintExtension.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineAfterOpenLintExtension.java
index 434149c519..e5b5db9959 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineAfterOpenLintExtension.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineAfterOpenLintExtension.java
@@ -34,9 +34,10 @@ public class PipelineAfterOpenLintExtension implements
IExtensionPoint<PipelineM
org.apache.hop.core.variables.IVariables variables,
PipelineMeta pipelineMeta)
throws HopException {
- if (pipelineMeta != null) {
-
LintEditorGraphHelper.scheduleAttachForFilename(pipelineMeta.getFilename());
-
BackgroundLintService.getInstance().scheduleFileLint(pipelineMeta.getFilename(),
true);
+ if (pipelineMeta == null ||
!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
}
+
LintEditorGraphHelper.scheduleAttachForFilename(pipelineMeta.getFilename());
+
BackgroundLintService.getInstance().scheduleFileLint(pipelineMeta.getFilename(),
true);
}
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/WorkflowAfterOpenLintExtension.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/WorkflowAfterOpenLintExtension.java
index 5368127f9f..1b267e7329 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/WorkflowAfterOpenLintExtension.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/WorkflowAfterOpenLintExtension.java
@@ -34,9 +34,10 @@ public class WorkflowAfterOpenLintExtension implements
IExtensionPoint<WorkflowM
org.apache.hop.core.variables.IVariables variables,
WorkflowMeta workflowMeta)
throws HopException {
- if (workflowMeta != null) {
-
LintEditorGraphHelper.scheduleAttachForFilename(workflowMeta.getFilename());
-
BackgroundLintService.getInstance().scheduleFileLint(workflowMeta.getFilename(),
true);
+ if (workflowMeta == null ||
!LinterConfigPlugin.getInstance().isLinterEnabled()) {
+ return;
}
+
LintEditorGraphHelper.scheduleAttachForFilename(workflowMeta.getFilename());
+
BackgroundLintService.getInstance().scheduleFileLint(workflowMeta.getFilename(),
true);
}
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/DisabledLinterTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/DisabledLinterTest.java
new file mode 100644
index 0000000000..16d5acaf59
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/DisabledLinterTest.java
@@ -0,0 +1,119 @@
+/*
+ * 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.hop.lint;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+import java.util.List;
+import org.apache.hop.ui.hopgui.file.shared.HopGuiAbstractGraph;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Switching the linter off has to stop marking files.
+ *
+ * <p>The canvas overlays already followed the switch. The Explorer and an
open editor did not: the
+ * file that was open kept being linted, and a file that had already been
marked stayed red.
+ */
+class DisabledLinterTest {
+
+ @AfterEach
+ void restore() {
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(true);
+ config.saveToHopConfig();
+ LintResultsManager.getInstance().clearResults();
+ }
+
+ @Test
+ void switchingTheLinterOffClearsStoredFindings() {
+ LintResultsManager manager = LintResultsManager.getInstance();
+ manager.updateResultsForFile(
+ "/tmp/example.hwf",
+ List.of(
+ new LintResult(
+ "HOP-CHECK", "SQL", "ERROR", "SQL cannot be null or blank.",
"/tmp/example.hwf")));
+ assertFalse(manager.getAllResults().isEmpty());
+
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(false);
+ config.applyEnabledState();
+
+ assertTrue(manager.getAllResults().isEmpty(), "a disabled linter must not
leave a file marked");
+ }
+
+ @Test
+ void anInFlightUpdateDoesNotPublishAfterTheLinterIsOff() {
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(false);
+ config.saveToHopConfig();
+
+ LintResultsManager manager = LintResultsManager.getInstance();
+ manager.updateResultsForFile(
+ "/tmp/example.hwf",
+ List.of(new LintResult("HOP-CHECK", "SQL", "ERROR", "too late",
"/tmp/example.hwf")));
+
+ assertTrue(
+ manager.getAllResults().isEmpty(),
+ "a pass that finishes after the switch must not put findings back");
+ }
+
+ @Test
+ void leavingTheLinterOnKeepsStoredFindings() {
+ LintResultsManager manager = LintResultsManager.getInstance();
+ manager.updateResultsForFile(
+ "/tmp/example.hwf",
+ List.of(new LintResult("HOP-CHECK", "SQL", "ERROR", "still there",
"/tmp/example.hwf")));
+
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(true);
+ config.applyEnabledState();
+
+ assertFalse(manager.getAllResults().isEmpty());
+ }
+
+ @Test
+ void theExplorerStopsDecoratingWhenTheLinterIsOff() {
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(false);
+ config.saveToHopConfig();
+ assertFalse(LintStatusFilePainter.decoratesExplorerFiles());
+
+ config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(true);
+ config.saveToHopConfig();
+ assertTrue(LintStatusFilePainter.decoratesExplorerFiles());
+ }
+
+ @Test
+ void anOpenEditorIsNotLintedWhenTheLinterIsOff() {
+ LinterConfigPlugin config = LinterConfigPlugin.getInstance();
+ config.setLinterEnabled(false);
+ config.saveToHopConfig();
+
+ HopGuiAbstractGraph graph = mock(HopGuiAbstractGraph.class);
+ EditorLintSupport.onGraphUpdate(graph);
+ EditorLintSupport.onNewGraph(graph);
+ BackgroundLintService.getInstance().scheduleGraphLint(graph, true);
+
+ verifyNoInteractions(graph);
+ }
+}
