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 e7956b4af8 Fixes #8536 : Let the linter address Hop's own verify
checks individu… (#8590)
e7956b4af8 is described below
commit e7956b4af8cf0f5054017b0104cab6e06e8a3744
Author: Bart Maertens <[email protected]>
AuthorDate: Sat Sep 26 14:35:35 2026 +0200
Fixes #8536 : Let the linter address Hop's own verify checks individu…
(#8590)
* Fixes #8536 : Let the linter address Hop's own verify checks individually
- Report lint findings in the editor as the linter's, the way the command
line does
- Match a messageKey whose message has values filled in
- Keep a check's own error code as its rule id; suppressions and baselines
also accept the classifying rule or the code
- Cap severity with the blanket native rule; a narrowed rule sets it
- Table input no longer repeats the missing-connection remark
* Issue #8536 : Address review on #8590
- Report a connection the transform cannot resolve. The pipeline check
leaves a
name that still holds a variable alone, because at design time it cannot
be
decided; the transform tried to load it and got nothing, so it says so,
under
CONNECTION_NOT_RESOLVED.
- Claim baseline entries under every finding's own rule id before falling
back
to an alias, so a coded finding cannot take the entry a codeless remark
on the
same transform matches exactly.
- Bring hop-lint.yml.example up to date: a coded check is reported under its
code and answers to HOP-CHECK, and the blanket rule's severity is a cap.
- Add integration test 0002, which was missing: a check is addressed by its
own
code, the others are left alone, a configuration naming the classifying
rule
still covers it, a rule narrowed by a message key with values matches,
and a
named check reports at the severity the project gave it.
* Issue #8536 : Keep the blanket rule as a name when a rule narrows
A narrowed rule takes the finding's rule id, so the rule that covers every
remark stopped naming it. A suppression or a baseline entry written against
HOP-CHECK before the project named that check then quietly stopped applying:
naming one check brought back findings the project had already accepted. A
finding now answers to the narrowed id, its own error code and the blanket
rule, in that order.
Also repoint the hops in integration test 0002 at its own action. They were
copied from 0001 and named an action this workflow does not have, so Start
had
no outgoing hop and the assertions never ran.
* Issue #8536 : Report an unassigned connection under its own code
---
.../metadata/util/HopMetadataPropertyWalker.java | 52 +++-
.../util/HopMetadataPropertyWalkerTest.java | 50 ++++
.../modules/ROOT/pages/linting/lint-rules.adoc | 8 +-
.../ReferencedDatabaseConnectionChecker.java | 12 +-
.../ReferencedDatabaseConnectionCheckerTest.java | 15 +
.../lint/files/lint-alias-with-narrowed-rule.yml | 31 ++
.../lint/files/lint-raise-by-message-key.yml | 26 ++
.../lint/files/lint-suppress-by-alias.yml | 20 ++
.../lint/files/lint-suppress-by-code.yml | 20 ++
.../lint/files/lint-unresolved-is-fatal.yml | 25 ++
.../lint/main-0002-lint-rule-granularity.hwf | 311 +++++++++++++++++++++
integration-tests/lint/subject/mixed-remarks.hpl | 85 ++++++
.../lint/subject/unresolved-connection.hpl | 90 ++++++
.../main/java/org/apache/hop/lint/HopLinter.java | 16 +-
.../java/org/apache/hop/lint/LintBaseline.java | 67 ++++-
.../apache/hop/lint/LintCheckResultAdapter.java | 27 +-
.../main/java/org/apache/hop/lint/LintPolicy.java | 6 +-
.../main/java/org/apache/hop/lint/LintResult.java | 66 +++++
.../org/apache/hop/lint/NativeCheckClassifier.java | 106 ++++++-
.../hop/lint/PipelineVerifyLintExtension.java | 7 +-
.../misc/lint/src/main/resources/hop-lint-core.yml | 6 +-
.../lint/src/main/resources/hop-lint.yml.example | 16 +-
.../java/org/apache/hop/lint/LintBaselineTest.java | 75 +++++
.../hop/lint/LintCheckResultAdapterTest.java | 118 ++++++++
.../java/org/apache/hop/lint/LintPolicyTest.java | 41 +++
.../hop/lint/LintSuppressionInEditorTest.java | 55 +++-
.../apache/hop/lint/NativeCheckClassifierTest.java | 102 ++++++-
.../transforms/tableinput/TableInputMeta.java | 46 ++-
.../tableinput/messages/messages_en_US.properties | 1 +
.../transforms/tableinput/TableInputMetaTest.java | 128 +++++++++
30 files changed, 1547 insertions(+), 81 deletions(-)
diff --git
a/core/src/main/java/org/apache/hop/metadata/util/HopMetadataPropertyWalker.java
b/core/src/main/java/org/apache/hop/metadata/util/HopMetadataPropertyWalker.java
index d1f0d93801..a4e9889045 100644
---
a/core/src/main/java/org/apache/hop/metadata/util/HopMetadataPropertyWalker.java
+++
b/core/src/main/java/org/apache/hop/metadata/util/HopMetadataPropertyWalker.java
@@ -48,7 +48,8 @@ public final class HopMetadataPropertyWalker {
*
* @param type the annotated property type
* @param key the serialised key, or the field name when no key is set
- * @param value the raw (unresolved) string value, never null
+ * @param value the raw (unresolved) string value, null only for a field
left unset and only when
+ * unset fields were asked for
*/
public record StringProperty(HopMetadataPropertyType type, String key,
String value) {}
@@ -60,6 +61,25 @@ public final class HopMetadataPropertyWalker {
* @return the matching properties, possibly empty
*/
public static List<StringProperty> collectStrings(Object root,
HopMetadataPropertyType type) {
+ return collectStrings(root, type, false);
+ }
+
+ /**
+ * Collect every string field annotated with {@code type} under {@code
root}, optionally including
+ * the fields that are left unset.
+ *
+ * <p>A field that was never given a value is null, not empty: that is the
default on a new
+ * transform or action. Callers that only want names to work with can ignore
those, but a check
+ * that reports an unset property has to see them, so it asks for them here
and gets a {@link
+ * StringProperty} with a null value.
+ *
+ * @param root the object to walk, may be null
+ * @param type the property type to collect
+ * @param includeUnset whether to also report annotated string fields that
are null
+ * @return the matching properties, possibly empty
+ */
+ public static List<StringProperty> collectStrings(
+ Object root, HopMetadataPropertyType type, boolean includeUnset) {
List<StringProperty> collected = new ArrayList<>();
if (root == null || type == null) {
return collected;
@@ -70,7 +90,8 @@ public final class HopMetadataPropertyWalker {
(field, node, property, value) ->
collected.add(new StringProperty(type, serialisedKey(property,
field), value)),
0,
- java.util.Collections.newSetFromMap(new IdentityHashMap<Object,
Boolean>()));
+ java.util.Collections.newSetFromMap(new IdentityHashMap<Object,
Boolean>()),
+ includeUnset);
return collected;
}
@@ -108,12 +129,14 @@ public final class HopMetadataPropertyWalker {
}
},
0,
- java.util.Collections.newSetFromMap(new IdentityHashMap<Object,
Boolean>()));
+ java.util.Collections.newSetFromMap(new IdentityHashMap<Object,
Boolean>()),
+ false);
return changed[0];
}
@FunctionalInterface
private interface StringFieldHandler {
+ /** Handle one annotated string field. {@code value} is null for a field
that is unset. */
void handle(Field field, Object node, HopMetadataProperty property, String
value);
}
@@ -122,7 +145,8 @@ public final class HopMetadataPropertyWalker {
HopMetadataPropertyType type,
StringFieldHandler handler,
int depth,
- Set<Object> visited) {
+ Set<Object> visited,
+ boolean includeUnset) {
if (node == null || depth > MAX_DEPTH || !isMetadataObject(node) ||
!visited.add(node)) {
return;
}
@@ -136,12 +160,19 @@ public final class HopMetadataPropertyWalker {
}
Object value = readField(field, node);
if (value == null) {
+ // Nothing to descend into, but the field itself is still of interest
when the caller wants
+ // to know about the properties that were left unset.
+ if (includeUnset
+ && property.hopMetadataPropertyType() == type
+ && field.getType() == String.class) {
+ handler.handle(field, node, property, null);
+ }
continue;
}
if (property.hopMetadataPropertyType() == type && value instanceof
String stringValue) {
handler.handle(field, node, property, stringValue);
}
- descend(value, type, handler, depth, visited);
+ descend(value, type, handler, depth, visited, includeUnset);
}
}
@@ -150,27 +181,28 @@ public final class HopMetadataPropertyWalker {
HopMetadataPropertyType type,
StringFieldHandler handler,
int depth,
- Set<Object> visited) {
+ Set<Object> visited,
+ boolean includeUnset) {
if (value instanceof Collection<?> collection) {
for (Object element : collection) {
- walk(element, type, handler, depth + 1, visited);
+ walk(element, type, handler, depth + 1, visited, includeUnset);
}
return;
}
if (value instanceof Map<?, ?> map) {
for (Object element : map.values()) {
- walk(element, type, handler, depth + 1, visited);
+ walk(element, type, handler, depth + 1, visited, includeUnset);
}
return;
}
if (value.getClass().isArray()) {
int length = Array.getLength(value);
for (int i = 0; i < length; i++) {
- walk(Array.get(value, i), type, handler, depth + 1, visited);
+ walk(Array.get(value, i), type, handler, depth + 1, visited,
includeUnset);
}
return;
}
- walk(value, type, handler, depth + 1, visited);
+ walk(value, type, handler, depth + 1, visited, includeUnset);
}
private static String serialisedKey(HopMetadataProperty property, Field
field) {
diff --git
a/core/src/test/java/org/apache/hop/metadata/util/HopMetadataPropertyWalkerTest.java
b/core/src/test/java/org/apache/hop/metadata/util/HopMetadataPropertyWalkerTest.java
index bb5665df13..4cb20b7ac3 100644
---
a/core/src/test/java/org/apache/hop/metadata/util/HopMetadataPropertyWalkerTest.java
+++
b/core/src/test/java/org/apache/hop/metadata/util/HopMetadataPropertyWalkerTest.java
@@ -73,6 +73,17 @@ class HopMetadataPropertyWalkerTest {
String connection = "hidden";
}
+ static class UnsetConnectionMeta {
+ @HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.RDBMS_CONNECTION)
+ String connection;
+
+ @HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.RDBMS_CONNECTION)
+ String other = "set";
+
+ @HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.VFS_SFTP_CONNECTION)
+ String sftp;
+ }
+
static class SftpAndRdbmsMeta {
@HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.RDBMS_CONNECTION)
String rdbms = "Warehouse";
@@ -123,6 +134,45 @@ class HopMetadataPropertyWalkerTest {
assertTrue(found.isEmpty());
}
+ /** A field that was never set is null, and by default a collect is about
values, not fields. */
+ @Test
+ void unsetConnectionFieldsAreSkippedByDefault() {
+ List<StringProperty> found =
+ HopMetadataPropertyWalker.collectStrings(
+ new UnsetConnectionMeta(),
HopMetadataPropertyType.RDBMS_CONNECTION);
+
+ assertEquals(1, found.size());
+ assertEquals("set", found.get(0).value());
+ }
+
+ /**
+ * A check that reports an unset property has to see the null fields, and
only those of the type
+ * it asked for.
+ */
+ @Test
+ void unsetConnectionFieldsAreCollectedWhenAskedFor() {
+ List<StringProperty> found =
+ HopMetadataPropertyWalker.collectStrings(
+ new UnsetConnectionMeta(),
HopMetadataPropertyType.RDBMS_CONNECTION, true);
+
+ assertEquals(2, found.size());
+ assertTrue(
+ found.stream().anyMatch(p -> "connection".equals(p.key()) && p.value()
== null),
+ found.toString());
+ assertTrue(found.stream().anyMatch(p -> "set".equals(p.value())));
+ }
+
+ /** Collecting unset fields must not start descending into nulls or lose the
set ones. */
+ @Test
+ void unsetCollectionStillWalksNestedObjects() {
+ List<StringProperty> found =
+ HopMetadataPropertyWalker.collectStrings(
+ new NestedMeta(), HopMetadataPropertyType.RDBMS_CONNECTION, true);
+
+ assertEquals(
+ List.of("primary", "second", "third"),
found.stream().map(StringProperty::value).toList());
+ }
+
@Test
void nullRootYieldsNothing() {
assertTrue(
diff --git a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
index f13e57e555..523eb6366b 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
@@ -239,11 +239,11 @@ rules:
severity: WARNING
----
-`severity` is what the linter reports the remark as, whatever severity the
transform gave it, and `enabled: false` drops it.
+`severity` on a rule that covers every remark is a cap: a remark above it is
lowered to it, and one below it keeps the severity the transform gave it, so a
comment is never reported as a warning. On a rule narrowed with `appliesTo` or
`messageKey`, `severity` is what the linter reports those remarks as, higher or
lower than the transform gave them. `enabled: false` drops the remarks the rule
covers.
Two optional keys narrow a rule to less than every remark:
* `appliesTo` — the plugin ids of the transforms or actions it covers, as for
any other rule.
-* `messageKey` — one single check, named as `<i18n package>:<key>` for the
message it prints, the same form Hop's plugin annotations use. The key is
resolved through the plugin's own message bundle, so the rule keeps matching in
every language rather than depending on the English wording. A key that no
longer resolves matches nothing rather than everything.
+* `messageKey` — one single check, named as `<i18n package>:<key>` for the
message it prints, the same form Hop's plugin annotations use. The key is
resolved through the plugin's own message bundle, so the rule keeps matching in
every language rather than depending on the English wording. For a check that
fills values into its message, such as a connection or field name, the rule
matches whatever values the check filled in. A key that no longer resolves
matches nothing rather than everything.
The most specific rule wins — one naming the check beats one naming only the
plugin, which beats the blanket rule — so a pack can hold a general policy and
an exception to it, and the order of the file does not decide which applies.
@@ -256,7 +256,9 @@ rules:
severity: ERROR
----
-Findings from a native rule carry that rule's id, so they suppress by id like
any other:
+Findings from a native rule carry that rule's id, so they suppress by id like
any other.
+A check that sets an error code of its own, such as
`CONNECTION_DOES_NOT_EXIST`, is reported under that code instead, unless a rule
naming its plugin or its message matched it.
+A suppression or baseline entry can name either the code or the native rule
that classified the finding, and one naming the code keeps working when a rule
naming that check is added later:
[source,yaml]
----
diff --git
a/engine/src/main/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionChecker.java
b/engine/src/main/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionChecker.java
index 5058bd3f4f..c9f32d15bd 100644
---
a/engine/src/main/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionChecker.java
+++
b/engine/src/main/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionChecker.java
@@ -51,6 +51,13 @@ public final class ReferencedDatabaseConnectionChecker {
public static final String ERROR_NOT_ASSIGNED = "CONNECTION_NOT_ASSIGNED";
public static final String ERROR_DOES_NOT_EXIST =
"CONNECTION_DOES_NOT_EXIST";
+ /**
+ * The name still holds a variable after resolving, so nothing can be looked
up. This checker
+ * never reports it: at design time such a name cannot be decided. A
transform that went on to
+ * load the connection anyway, and got nothing, knows more and may report it
under this code.
+ */
+ public static final String ERROR_NOT_RESOLVED = "CONNECTION_NOT_RESOLVED";
+
/**
* The connection could not be looked up at all, so nothing is known about
it. Reported at INFO:
* it says something about the metadata being unreadable, not about the file
being linted.
@@ -146,9 +153,12 @@ public final class ReferencedDatabaseConnectionChecker {
return remarks;
}
+ // Ask for the unset fields too. A connection that was never assigned is
null, not empty - that
+ // is the field default on a new transform or action - and it is exactly
what ERROR_NOT_ASSIGNED
+ // is about, so it has to be seen here rather than left to each
transform's own check.
for (StringProperty property :
HopMetadataPropertyWalker.collectStrings(
- metadataObject, HopMetadataPropertyType.RDBMS_CONNECTION)) {
+ metadataObject, HopMetadataPropertyType.RDBMS_CONNECTION, true)) {
ICheckResult remark =
checkConnectionName(
property.value(), ownerKind, ownerName, source, variables,
serializer);
diff --git
a/engine/src/test/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionCheckerTest.java
b/engine/src/test/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionCheckerTest.java
index af06745537..dc370b612b 100644
---
a/engine/src/test/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionCheckerTest.java
+++
b/engine/src/test/java/org/apache/hop/metadata/validation/ReferencedDatabaseConnectionCheckerTest.java
@@ -169,6 +169,21 @@ class ReferencedDatabaseConnectionCheckerTest {
ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED,
remarks.get(0).getErrorCode());
}
+ /**
+ * An unassigned connection is null, not empty: that is the field default on
a new transform. The
+ * walker used to skip null fields, so this warning was never reported and
the transform's own
+ * check was left saying the same thing without a code.
+ */
+ @Test
+ void unassignedConnectionIsAWarning() {
+ List<ICheckResult> remarks = check(new ConnMeta(null), "Read sales");
+
+ assertEquals(1, remarks.size());
+ assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED,
remarks.get(0).getErrorCode());
+ assertEquals(ICheckResult.TYPE_RESULT_WARNING, remarks.get(0).getType());
+ }
+
@Test
void nestedListConnectionsAreChecked() {
List<ICheckResult> remarks =
diff --git a/integration-tests/lint/files/lint-alias-with-narrowed-rule.yml
b/integration-tests/lint/files/lint-alias-with-narrowed-rule.yml
new file mode 100644
index 0000000000..5cc55a3464
--- /dev/null
+++ b/integration-tests/lint/files/lint-alias-with-narrowed-rule.yml
@@ -0,0 +1,31 @@
+# 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.
+
+
+# A configuration written before the project named any check, plus a rule that
+# names one. Naming a check must not quietly undo the suppression already in
+# force: the finding answers to the rule that covers every remark as well as to
+# the narrowed rule and to its own code.
+suppress:
+ - rule: HOP-CHECK
+ reason: "Written before the project named any check"
+rules:
+ UNRESOLVED-CONNECTION-IS-FATAL:
+ type: native
+ enabled: true
+ severity: ERROR
+ name: "Unresolved database connection"
+ appliesTo: TableInput
+ messageKey:
"org.apache.hop.pipeline.transforms.tableinput:TableInputMeta.CheckResult.ConnectionNotResolved"
diff --git a/integration-tests/lint/files/lint-raise-by-message-key.yml
b/integration-tests/lint/files/lint-raise-by-message-key.yml
new file mode 100644
index 0000000000..0cfbf6acc5
--- /dev/null
+++ b/integration-tests/lint/files/lint-raise-by-message-key.yml
@@ -0,0 +1,26 @@
+# 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.
+
+# A native rule narrowed to one check by the message it prints. The message
+# takes values, so this only matches once a resolved pattern is matched rather
+# than compared literally. A narrowed rule sets the severity outright, where
the
+# blanket rule only caps it.
+rules:
+ MISSING-CONNECTION-IS-FATAL:
+ type: native
+ enabled: true
+ severity: ERROR
+ name: "Missing database connection"
+ messageKey:
"org.apache.hop.metadata.validation:ReferencedDatabaseConnectionChecker.DoesNotExist"
diff --git a/integration-tests/lint/files/lint-suppress-by-alias.yml
b/integration-tests/lint/files/lint-suppress-by-alias.yml
new file mode 100644
index 0000000000..1f9bbeb22a
--- /dev/null
+++ b/integration-tests/lint/files/lint-suppress-by-alias.yml
@@ -0,0 +1,20 @@
+# 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.
+
+# A configuration written before a check reported its own code named the rule
+# that classified it. It has to keep working.
+suppress:
+ - rule: HOP-CHECK
+ reason: "Written before the check reported its own code"
diff --git a/integration-tests/lint/files/lint-suppress-by-code.yml
b/integration-tests/lint/files/lint-suppress-by-code.yml
new file mode 100644
index 0000000000..02a8305a35
--- /dev/null
+++ b/integration-tests/lint/files/lint-suppress-by-code.yml
@@ -0,0 +1,20 @@
+# 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.
+
+# Issue #8536: one check addressed by its own error code, leaving every other
+# remark from Hop's own verify alone.
+suppress:
+ - rule: CONNECTION_DOES_NOT_EXIST
+ reason: "The connection is created by the deployment, not by the project"
diff --git a/integration-tests/lint/files/lint-unresolved-is-fatal.yml
b/integration-tests/lint/files/lint-unresolved-is-fatal.yml
new file mode 100644
index 0000000000..b5062ce248
--- /dev/null
+++ b/integration-tests/lint/files/lint-unresolved-is-fatal.yml
@@ -0,0 +1,25 @@
+# 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.
+
+# The blanket rule caps Hop's own remarks at warning, so a team that wants a
+# connection it cannot resolve to fail the build names that check and says so.
+rules:
+ UNRESOLVED-CONNECTION-IS-FATAL:
+ type: native
+ enabled: true
+ severity: ERROR
+ name: "Unresolved database connection"
+ appliesTo: TableInput
+ messageKey:
"org.apache.hop.pipeline.transforms.tableinput:TableInputMeta.CheckResult.ConnectionNotResolved"
diff --git a/integration-tests/lint/main-0002-lint-rule-granularity.hwf
b/integration-tests/lint/main-0002-lint-rule-granularity.hwf
new file mode 100644
index 0000000000..dab454e8cf
--- /dev/null
+++ b/integration-tests/lint/main-0002-lint-rule-granularity.hwf
@@ -0,0 +1,311 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<workflow>
+ <name>main-0002-lint-rule-granularity</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8536: a project can raise, lower or silence one of Hop's
own verify checks instead of all of them.</description>
+ <extended_description/>
+ <workflow_version/>
+ <created_user>-</created_user>
+ <created_date>2026/09/09 08:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/09/09 08:00:00.000</modified_date>
+ <parameters>
+ </parameters>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <DayOfMonth>1</DayOfMonth>
+ <hour>12</hour>
+ <intervalMinutes>60</intervalMinutes>
+ <intervalSeconds>0</intervalSeconds>
+ <minutes>0</minutes>
+ <repeat>N</repeat>
+ <schedulerType>0</schedulerType>
+ <weekDay>1</weekDay>
+ <parallel>N</parallel>
+ <xloc>96</xloc>
+ <yloc>96</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Lint with rule granularity configurations</name>
+ <description>Runs hop lint over one subject with several configurations
and checks the rule ids and severities reported.</description>
+ <type>SHELL</type>
+ <attributes/>
+ <filename/>
+ <work_directory>${PROJECT_HOME}</work_directory>
+ <arg_from_previous>N</arg_from_previous>
+ <exec_per_row>N</exec_per_row>
+ <set_logfile>N</set_logfile>
+ <logfile/>
+ <set_append_logfile>N</set_append_logfile>
+ <logext/>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <insertScript>Y</insertScript>
+ <script>#!/bin/bash
+# Issue #8536: a project can address one of Hop's own verify checks.
+#
+# Every remark from Hop's check() methods used to be reported under one rule
id, HOP-CHECK, so a
+# project could raise, lower or silence all of them or none of them. A check
that reports an error
+# code is now reported under that code, and still answers to the rule that
classified it, so a
+# configuration written before this keeps working.
+#
+# Note on ${} below: Hop resolves its own variables in this script before
running it, so only Hop
+# variables are written with braces. Shell variables are written without them.
+
+set -u
+
+PROJECT_DIR="${PROJECT_HOME}"
+
+# Finding the hop CLI: hop-run.sh and hop-gui.sh both start the JVM from the
Hop installation, so
+# ${user.dir} is the install folder - /opt/hop in the test image, the client
folder when this
+# workflow is run from Hop GUI.
+HOP_CLI="${user.dir}/hop"
+if [ ! -x "$HOP_CLI" ]; then
+ echo "FAIL: no hop CLI at $HOP_CLI. The linter cannot be tested without it."
+ exit 1
+fi
+echo "Using hop CLI: $HOP_CLI"
+
+export HOP_CONFIG_FOLDER="$PROJECT_DIR"
+
+REPORT_DIR="$PROJECT_DIR/output"
+mkdir -p "$REPORT_DIR"
+
+failures=0
+
+# Run hop lint over $1 into report $2, optionally with the configuration $3.
hop lint exits 1 when
+# a finding reaches --fail-on, so only an exit code above 1 means the linter
failed to run.
+run_lint() {
+ subject="$1"
+ report="$2"
+ config="${3:-}"
+ rm -f "$report"
+ if [ -n "$config" ]; then
+ "$HOP_CLI" lint "$subject" -c "$PROJECT_DIR/files/$config" --format json
--output "$report"
+ else
+ "$HOP_CLI" lint "$subject" --format json --output "$report"
+ fi
+ rc=$?
+ if [ $rc -gt 1 ]; then
+ echo "FAIL: hop lint could not lint $subject (exit code $rc)"
+ return 1
+ fi
+ if [ ! -s "$report" ]; then
+ echo "FAIL: hop lint wrote no report for $subject"
+ return 1
+ fi
+ return 0
+}
+
+# The rule ids a report holds, sorted, on one line.
+rule_ids() {
+ grep '"ruleId"' "$1" | sed -e 's/.*"ruleId" : "//' -e 's/".*//' | sort | tr
'\n' ' '
+}
+
+# $1 label, $2 report, $3 the rule ids expected, sorted and space separated
with a trailing space.
+expect_ids() {
+ actual=$(rule_ids "$2")
+ if [ "$actual" != "$3" ]; then
+ echo "FAIL: $1"
+ echo " expected rule ids : $3"
+ echo " actual rule ids : $actual"
+ echo "----- report -----"
+ cat "$2"
+ echo "------------------"
+ failures=$((failures + 1))
+ return 1
+ fi
+ echo "OK: $1"
+ return 0
+}
+
+# The subject holds one coded check (the connection does not exist), one coded
comment (the
+# transform can start without input), and two findings from the linter's own
rules.
+MIXED="$PROJECT_DIR/subject/mixed-remarks.hpl"
+
+echo "=== A check that has an error code is reported under it ==="
+REPORT="$REPORT_DIR/lint-8536-default.json"
+if run_lint "$MIXED" "$REPORT"; then
+ expect_ids "coded checks report under their own code" "$REPORT" \
+ "CAN_START_WITHOUT_INPUT CONNECTION_DOES_NOT_EXIST TRANS-002 TRANS-002 "
+ # The blanket native rule caps a remark, it does not set it: a comment stays
a comment. Reported
+ # at the rule's own severity this would be a warning, and the summary would
say info 0.
+ if grep -q '"info" : 1' "$REPORT"; then
+ echo "OK: the blanket rule caps severity rather than setting it."
+ else
+ echo "FAIL: the comment-level check was not reported at info."
+ echo "----- report -----"
+ cat "$REPORT"
+ echo "------------------"
+ failures=$((failures + 1))
+ fi
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== One check is addressed by its code, and the others are left alone
==="
+REPORT="$REPORT_DIR/lint-8536-by-code.json"
+if run_lint "$MIXED" "$REPORT" "lint-suppress-by-code.yml"; then
+ expect_ids "suppressing one code silences that check only" "$REPORT" \
+ "CAN_START_WITHOUT_INPUT TRANS-002 TRANS-002 "
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== A configuration naming the classifying rule still covers the check
==="
+REPORT="$REPORT_DIR/lint-8536-by-alias.json"
+if run_lint "$MIXED" "$REPORT" "lint-suppress-by-alias.yml"; then
+ # HOP-CHECK still means every remark from Hop's own checks, which is what it
meant before.
+ expect_ids "suppressing HOP-CHECK silences Hop's checks, not the linter's
rules" "$REPORT" \
+ "TRANS-002 TRANS-002 "
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== A rule narrowed by messageKey names a check whose message takes
values ==="
+REPORT="$REPORT_DIR/lint-8536-by-message-key.json"
+if run_lint "$PROJECT_DIR/subject/missing-connection.hwf" "$REPORT"
"lint-raise-by-message-key.yml"; then
+ if expect_ids "the narrowed rule reports the check" "$REPORT"
"MISSING-CONNECTION-IS-FATAL "; then
+ # A narrowed rule is a decision about that check, so its severity is
reported as it stands
+ # rather than capped.
+ if grep -q '"errors" : 1' "$REPORT"; then
+ echo "OK: a narrowed rule sets the severity."
+ else
+ echo "FAIL: the narrowed rule did not raise the check to error."
+ echo "----- report -----"
+ cat "$REPORT"
+ echo "------------------"
+ failures=$((failures + 1))
+ fi
+ fi
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== A connection the transform cannot resolve is still reported ==="
+# The pipeline check leaves a name that still holds a variable alone: at
design time it cannot be
+# decided. The transform tried to load it and got nothing, so it is the only
thing that can say so.
+UNRESOLVED="$PROJECT_DIR/subject/unresolved-connection.hpl"
+REPORT="$REPORT_DIR/lint-8536-unresolved.json"
+if run_lint "$UNRESOLVED" "$REPORT"; then
+ if grep -q '"ruleId" : "CONNECTION_NOT_RESOLVED"' "$REPORT"; then
+ echo "OK: the unresolved connection is reported under its own code."
+ else
+ echo "FAIL: nothing reported the connection this transform could not
resolve."
+ echo "----- report -----"
+ cat "$REPORT"
+ echo "------------------"
+ failures=$((failures + 1))
+ fi
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== That check can be made to fail a build ==="
+# The blanket rule caps Hop's own remarks at warning, so a project that wants
this one to stop a
+# build names the check and sets its severity. Naming one check is what this
issue is about.
+REPORT="$REPORT_DIR/lint-8536-unresolved-fatal.json"
+if run_lint "$UNRESOLVED" "$REPORT" "lint-unresolved-is-fatal.yml"; then
+ if grep -q '"ruleId" : "UNRESOLVED-CONNECTION-IS-FATAL"' "$REPORT" \
+ && grep -q '"errors" : 1' "$REPORT"; then
+ echo "OK: a named check reports at the severity the project gave it."
+ else
+ echo "FAIL: the project could not raise that one check to error."
+ echo "----- report -----"
+ cat "$REPORT"
+ echo "------------------"
+ failures=$((failures + 1))
+ fi
+else
+ failures=$((failures + 1))
+fi
+
+echo "=== Naming a check does not undo a suppression already in force ==="
+# The narrowed rule takes the rule id, so without the blanket rule kept as
another name a
+# suppression written before the project named that check would quietly stop
applying.
+REPORT="$REPORT_DIR/lint-8536-alias-with-narrowed-rule.json"
+if run_lint "$UNRESOLVED" "$REPORT" "lint-alias-with-narrowed-rule.yml"; then
+ if grep -q "CONNECTION" "$REPORT" || grep -q
"UNRESOLVED-CONNECTION-IS-FATAL" "$REPORT"; then
+ echo "FAIL: naming the check brought back a finding the project had
accepted."
+ echo "----- report -----"
+ cat "$REPORT"
+ echo "------------------"
+ failures=$((failures + 1))
+ else
+ echo "OK: the suppression still holds after the project named the check."
+ fi
+else
+ failures=$((failures + 1))
+fi
+
+if [ $failures -ne 0 ]; then
+ echo "$failures lint assertion(s) failed."
+ exit 1
+fi
+
+echo "All lint assertions passed."
+exit 0
+</script>
+ <loglevel>Basic</loglevel>
+ <parallel>N</parallel>
+ <nr>0</nr>
+ <xloc>320</xloc>
+ <yloc>96</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Abort workflow</name>
+ <description/>
+ <type>ABORT</type>
+ <attributes/>
+ <always_log_rows>N</always_log_rows>
+ <message>The Hop linter cannot address Hop's own verify checks
individually. See issue #8536.</message>
+ <parallel>N</parallel>
+ <xloc>560</xloc>
+ <yloc>208</yloc>
+ <attributes_hac/>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Lint with rule granularity configurations</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ </hop>
+ <hop>
+ <from>Lint with rule granularity configurations</from>
+ <to>Abort workflow</to>
+ <enabled>Y</enabled>
+ <evaluation>N</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ </hops>
+ <notepads>
+ </notepads>
+ <attributes/>
+</workflow>
diff --git a/integration-tests/lint/subject/mixed-remarks.hpl
b/integration-tests/lint/subject/mixed-remarks.hpl
new file mode 100644
index 0000000000..47f66d5fcc
--- /dev/null
+++ b/integration-tests/lint/subject/mixed-remarks.hpl
@@ -0,0 +1,85 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+ <info>
+ <name>mixed-remarks</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8536: one remark carries an error code, the other does
not. Suppressing the coded one by its code must leave the other in
place.</description>
+ <extended_description/>
+ <pipeline_version/>
+ <pipeline_type>Normal</pipeline_type>
+ <parameters>
+ </parameters>
+ <capture_transform_performance>N</capture_transform_performance>
+
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+ <created_user>-</created_user>
+ <created_date>2026/09/25 10:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/09/25 10:00:00.000</modified_date>
+ </info>
+ <notepads>
+ </notepads>
+ <order>
+ </order>
+ <transform>
+ <name>Read from NoSuchConnection</name>
+ <type>TableInput</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <connection>NoSuchConnection</connection>
+ <sql>SELECT 1</sql>
+ <limit>0</limit>
+ <execute_each_row>N</execute_each_row>
+ <variables_active>N</variables_active>
+ <lazy_conversion_active>N</lazy_conversion_active>
+ <attributes/>
+ <GUI>
+ <xloc>192</xloc>
+ <yloc>96</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Orphan</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>400</xloc>
+ <yloc>96</yloc>
+ </GUI>
+ </transform>
+ <transform_error_handling>
+ </transform_error_handling>
+ <attributes/>
+</pipeline>
diff --git a/integration-tests/lint/subject/unresolved-connection.hpl
b/integration-tests/lint/subject/unresolved-connection.hpl
new file mode 100644
index 0000000000..f5731b0ca9
--- /dev/null
+++ b/integration-tests/lint/subject/unresolved-connection.hpl
@@ -0,0 +1,90 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+ <info>
+ <name>unresolved-connection</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8536: the connection name still holds a variable at
check time. The pipeline check leaves such a name alone, so the transform is
the only thing that can report it.</description>
+ <extended_description/>
+ <pipeline_version/>
+ <pipeline_type>Normal</pipeline_type>
+ <parameters>
+ </parameters>
+ <capture_transform_performance>N</capture_transform_performance>
+
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+ <created_user>-</created_user>
+ <created_date>2026/09/25 10:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/09/25 10:00:00.000</modified_date>
+ </info>
+ <notepads>
+ </notepads>
+ <order>
+ <hop>
+ <from>Read from a variable</from>
+ <to>Next</to>
+ <enabled>Y</enabled>
+ </hop>
+ </order>
+ <transform>
+ <name>Read from a variable</name>
+ <type>TableInput</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <connection>${DB_CONN}</connection>
+ <sql>SELECT 1</sql>
+ <limit>0</limit>
+ <execute_each_row>N</execute_each_row>
+ <variables_active>N</variables_active>
+ <lazy_conversion_active>N</lazy_conversion_active>
+ <attributes/>
+ <GUI>
+ <xloc>192</xloc>
+ <yloc>96</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Next</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>400</xloc>
+ <yloc>96</yloc>
+ </GUI>
+ </transform>
+ <transform_error_handling>
+ </transform_error_handling>
+ <attributes/>
+</pipeline>
diff --git a/plugins/misc/lint/src/main/java/org/apache/hop/lint/HopLinter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/HopLinter.java
index f1e9d5098a..23b41b9f77 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/HopLinter.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/HopLinter.java
@@ -511,11 +511,10 @@ public class HopLinter {
List<LintResult> results = new ArrayList<>(fromNativeRemarks(remarks,
fileName));
if (shouldIncludeLintInPipelineVerify()) {
- results.addAll(
- LintCheckResultAdapter.fromCheckResults(
- LintCheckResultAdapter.toCheckResults(
- runPolicyRules(pipelineMeta, fileName), pipelineMeta),
- fileName));
+ // As they are, the way the command line reports them. Round tripping
them through Hop's own
+ // remarks would report each one as Hop's, under the source's name and
with the rule id
+ // prefixed to the message, and keep deduplication from telling the two
apart.
+ results.addAll(runPolicyRules(pipelineMeta, fileName));
}
return applyPolicy(results, fileName);
@@ -540,11 +539,8 @@ public class HopLinter {
List<LintResult> results = new ArrayList<>(fromNativeRemarks(remarks,
fileName));
if (shouldIncludeLintInWorkflowVerify()) {
- results.addAll(
- LintCheckResultAdapter.fromCheckResults(
- WorkflowCheckResultAdapter.toCheckResults(
- runPolicyRules(workflowMeta, fileName), workflowMeta),
- fileName));
+ // As they are, for the same reason as the pipeline path above.
+ results.addAll(runPolicyRules(workflowMeta, fileName));
}
return applyPolicy(results, fileName);
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintBaseline.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintBaseline.java
index dc6078c823..2f02eef0e8 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintBaseline.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintBaseline.java
@@ -24,6 +24,7 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
@@ -123,18 +124,7 @@ public final class LintBaseline {
if (accepted.isEmpty()) {
return results;
}
- Map<String, Integer> remaining = new LinkedHashMap<>(accepted);
- List<LintResult> fresh = new ArrayList<>();
- for (LintResult result : results) {
- String fingerprint = fingerprint(result, projectRoot);
- Integer left = remaining.get(fingerprint);
- if (left != null && left > 0) {
- remaining.put(fingerprint, left - 1);
- continue;
- }
- fresh.add(result);
- }
- return fresh;
+ return unclaimed(new LinkedHashMap<>(accepted), results, projectRoot);
}
/**
@@ -143,19 +133,66 @@ public final class LintBaseline {
*/
public int countStaleEntries(List<LintResult> results, Path projectRoot) {
Map<String, Integer> remaining = new LinkedHashMap<>(accepted);
+ unclaimed(remaining, results, projectRoot);
+ return remaining.values().stream().mapToInt(Integer::intValue).sum();
+ }
+
+ /**
+ * The findings with no recorded occurrence left, taking the entries they
claim out of {@code
+ * remaining}.
+ *
+ * <p>In two passes, because a finding answers to more than one rule id. A
coded finding also
+ * answers to the rule that classified it, and claiming under that alias in
the same pass would
+ * let it take an entry recorded for a different, codeless remark on the
same transform - which
+ * would then be reported as new. Every finding gets its own id first, and
only what is left over
+ * falls back to an alias.
+ */
+ private static List<LintResult> unclaimed(
+ Map<String, Integer> remaining, List<LintResult> results, Path
projectRoot) {
+ List<LintResult> fresh = new ArrayList<>();
for (LintResult result : results) {
- String fingerprint = fingerprint(result, projectRoot);
+ if (!claim(remaining, result, projectRoot, true)) {
+ fresh.add(result);
+ }
+ }
+ List<LintResult> unmatched = new ArrayList<>();
+ for (LintResult result : fresh) {
+ if (!claim(remaining, result, projectRoot, false)) {
+ unmatched.add(result);
+ }
+ }
+ return unmatched;
+ }
+
+ /**
+ * Take one recorded occurrence of this finding, if one is left.
+ *
+ * <p>A baseline written before a check reported its own error code recorded
the finding under the
+ * native rule that classified it, so a finding answers to that alias as
well as to its own id.
+ * {@code ownIdOnly} is what keeps the two apart across the passes above.
+ */
+ private static boolean claim(
+ Map<String, Integer> remaining, LintResult result, Path root, boolean
ownIdOnly) {
+ List<String> ruleIds =
+ ownIdOnly ? Collections.singletonList(result.getRuleId()) :
result.getRuleIds();
+ for (String ruleId : ruleIds) {
+ String fingerprint = fingerprint(ruleId, result, root);
Integer left = remaining.get(fingerprint);
if (left != null && left > 0) {
remaining.put(fingerprint, left - 1);
+ return true;
}
}
- return remaining.values().stream().mapToInt(Integer::intValue).sum();
+ return false;
}
static String fingerprint(LintResult result, Path projectRoot) {
+ return fingerprint(result.getRuleId(), result, projectRoot);
+ }
+
+ private static String fingerprint(String ruleId, LintResult result, Path
projectRoot) {
String file = LintPolicy.relativise(result.getFileName(), projectRoot);
String source = result.getSource() != null ? result.getSource().getName()
: "";
- return result.getRuleId() + "|" + file + "|" + (source != null ? source :
"");
+ return ruleId + "|" + file + "|" + (source != null ? source : "");
}
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCheckResultAdapter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCheckResultAdapter.java
index cbd82b3380..4b7c59487b 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCheckResultAdapter.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCheckResultAdapter.java
@@ -101,6 +101,7 @@ public final class LintCheckResultAdapter {
String severity = LintSeverity.fromCheckResultType(remark.getType());
String ruleId = remark.getErrorCode();
+ List<String> aliasRuleIds = new ArrayList<>();
if (classifier != null && !classifier.isEmpty()) {
NativeCheckClassifier.Classification classification =
classifier.classify(remark);
@@ -110,8 +111,27 @@ public final class LintCheckResultAdapter {
return null;
}
severity = classification.severity();
- if (!Utils.isEmpty(classification.ruleId())) {
- ruleId = classification.ruleId();
+ String classifyingRule = classification.ruleId();
+ if (!Utils.isEmpty(classifyingRule)) {
+ if (Utils.isEmpty(ruleId)) {
+ ruleId = classifyingRule;
+ } else if (classification.narrowed()) {
+ // A rule naming the plugin or the check is the more specific id.
The code stays an
+ // alias, so a suppression written against it survives the project
adding that rule.
+ aliasRuleIds.add(ruleId);
+ ruleId = classifyingRule;
+ } else {
+ // A check with its own error code keeps it, so a project can
address that one check.
+ // The blanket rule names every remark, and taking its id would
collapse them all into
+ // one. It is kept alongside, so what a project wrote against it
still applies.
+ aliasRuleIds.add(classifyingRule);
+ }
+ }
+ // A narrowed rule took the id above, so the rule covering every remark
would stop naming
+ // this finding. It is kept as the last name, after the check's own
code, so a suppression
+ // written against either one still applies.
+ if (!Utils.isEmpty(classification.blanketRuleId())) {
+ aliasRuleIds.add(classification.blanketRuleId());
}
}
@@ -130,7 +150,8 @@ public final class LintCheckResultAdapter {
remark.getText(),
fileName,
sourceRef,
- LintResult.Origin.HOP_NATIVE);
+ LintResult.Origin.HOP_NATIVE,
+ aliasRuleIds);
}
private static String formatCheckText(LintResult lintResult) {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicy.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicy.java
index 31f54767cc..cde1da608d 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicy.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicy.java
@@ -141,8 +141,10 @@ public final class LintPolicy {
String relative = relativise(result.getFileName(), projectRoot);
String sourceName = result.getSource() != null ?
result.getSource().getName() : null;
for (Suppression suppression : suppressions) {
- if (suppression.matches(result.getRuleId(), relative, sourceName)) {
- return true;
+ for (String ruleId : result.getRuleIds()) {
+ if (suppression.matches(ruleId, relative, sourceName)) {
+ return true;
+ }
}
}
return false;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResult.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResult.java
index b40b62c23d..bc74abd849 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResult.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResult.java
@@ -16,6 +16,10 @@
*/
package org.apache.hop.lint;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
/** Data class to hold the result of a single linting violation. */
public class LintResult {
@@ -31,6 +35,7 @@ public class LintResult {
private final String fileName;
private final LintSourceRef source;
private final Origin origin;
+ private final List<String> aliasRuleIds;
public LintResult(
String ruleId, String ruleName, String severity, String message, String
fileName) {
@@ -45,6 +50,35 @@ public class LintResult {
String fileName,
LintSourceRef source,
Origin origin) {
+ this(ruleId, ruleName, severity, message, fileName, source, origin,
List.of());
+ }
+
+ /**
+ * @param aliasRuleId the other id a native remark answers to: the rule that
classified it when it
+ * is reported under its own error code, or that error code when a
narrowed rule named it;
+ * null otherwise
+ */
+ public LintResult(
+ String ruleId,
+ String ruleName,
+ String severity,
+ String message,
+ String fileName,
+ LintSourceRef source,
+ Origin origin,
+ String aliasRuleId) {
+ this(ruleId, ruleName, severity, message, fileName, source, origin,
aliasList(aliasRuleId));
+ }
+
+ public LintResult(
+ String ruleId,
+ String ruleName,
+ String severity,
+ String message,
+ String fileName,
+ LintSourceRef source,
+ Origin origin,
+ List<String> aliasRuleIds) {
this.ruleId = ruleId;
this.ruleName = ruleName;
this.severity = severity;
@@ -52,6 +86,7 @@ public class LintResult {
this.fileName = fileName;
this.source = source;
this.origin = origin != null ? origin : Origin.LINT;
+ this.aliasRuleIds = aliasRuleIds == null ? Collections.emptyList() :
List.copyOf(aliasRuleIds);
}
public String getRuleId() {
@@ -82,6 +117,37 @@ public class LintResult {
return origin;
}
+ /**
+ * The other id this finding answers to, or null.
+ *
+ * <p>A native remark with its own error code is reported under that code,
and answers to the rule
+ * that classified it too, so what a project wrote against {@code HOP-CHECK}
still applies. When a
+ * rule naming the check wins instead, the finding answers to the error code
as well, so a
+ * suppression written against the code keeps working after a project adds
such a rule.
+ */
+ public String getAliasRuleId() {
+ return aliasRuleIds.isEmpty() ? null : aliasRuleIds.get(0);
+ }
+
+ /** The rule ids this finding answers to: its own, then its alias, if any. */
+ public List<String> getRuleIds() {
+ if (aliasRuleIds.isEmpty()) {
+ return Collections.singletonList(ruleId);
+ }
+ List<String> ids = new ArrayList<>();
+ ids.add(ruleId);
+ for (String alias : aliasRuleIds) {
+ if (alias != null && !alias.equalsIgnoreCase(ruleId) &&
!ids.contains(alias)) {
+ ids.add(alias);
+ }
+ }
+ return List.copyOf(ids);
+ }
+
+ private static List<String> aliasList(String aliasRuleId) {
+ return aliasRuleId == null ? List.of() : List.of(aliasRuleId);
+ }
+
@Override
public String toString() {
StringBuilder sb =
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/NativeCheckClassifier.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/NativeCheckClassifier.java
index dee87b80ab..a3571965e3 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/NativeCheckClassifier.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/NativeCheckClassifier.java
@@ -18,6 +18,8 @@ package org.apache.hop.lint;
import java.util.ArrayList;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.ICheckResultSource;
import org.apache.hop.core.util.Utils;
@@ -43,8 +45,16 @@ import org.apache.hop.workflow.action.ActionMeta;
*/
public final class NativeCheckClassifier {
- /** What a matching rule says should happen to a remark. */
- public record Classification(String severity, String ruleId) {}
+ /**
+ * What a matching rule says should happen to a remark.
+ *
+ * @param narrowed whether the rule names a plugin or a check, rather than
every remark
+ */
+ public record Classification(
+ String severity, String ruleId, boolean narrowed, String blanketRuleId)
{}
+
+ /** Where MessageFormat left a value out: {@code {0}}, {@code {1}}, and so
on. */
+ private static final Pattern PLACEHOLDER = Pattern.compile("\\{\\d+\\}");
private final List<CustomLintRule> rules;
@@ -78,12 +88,43 @@ public final class NativeCheckClassifier {
}
CustomLintRule match = bestMatch(remark);
if (match == null) {
- return new
Classification(LintSeverity.fromCheckResultType(remark.getType()), null);
+ return new Classification(
+ LintSeverity.fromCheckResultType(remark.getType()), null, false,
null);
}
if (!match.isEnabled()) {
return null;
}
- return new Classification(match.getSeverity(), match.generateRuleId());
+ boolean narrowed = isNarrowed(match);
+ String severity =
+ narrowed
+ ? match.getSeverity()
+ : capped(LintSeverity.fromCheckResultType(remark.getType()),
match.getSeverity());
+ // A narrowed rule takes the id, so the rule that covers every remark
would otherwise stop
+ // naming this finding, and a project's existing suppression of it would
quietly lapse the
+ // moment that project named the check. It is carried along as another
name.
+ return new Classification(
+ severity, match.generateRuleId(), narrowed, narrowed ? blanketRuleId()
: null);
+ }
+
+ /**
+ * The remark's own severity, lowered to the cap if it is above it.
+ *
+ * <p>A rule covering every remark caps them: it cannot know that any one of
them deserves more
+ * than the transform gave it, so a comment stays a comment. A rule naming a
plugin or a check is
+ * a decision about those remarks, and its severity is reported as it stands.
+ */
+ private static String capped(String severity, String cap) {
+ return rank(severity) > rank(cap) ? cap : severity;
+ }
+
+ private static int rank(String severity) {
+ if ("ERROR".equalsIgnoreCase(severity)) {
+ return 2;
+ }
+ if ("WARNING".equalsIgnoreCase(severity)) {
+ return 1;
+ }
+ return 0;
}
/**
@@ -106,6 +147,25 @@ public final class NativeCheckClassifier {
return bestScore < 0 ? null : best;
}
+ /**
+ * The id of the rule that covers every remark, or null when no such rule is
in force.
+ *
+ * <p>A rule naming neither a plugin nor a check applies to anything put in
front of it, so the
+ * first one is the one a blanket configuration was written against.
+ */
+ private String blanketRuleId() {
+ for (CustomLintRule rule : rules) {
+ if (!isNarrowed(rule)) {
+ return rule.generateRuleId();
+ }
+ }
+ return null;
+ }
+
+ private static boolean isNarrowed(CustomLintRule rule) {
+ return !rule.getAppliesTo().isEmpty() ||
!Utils.isEmpty(rule.getMessageKey());
+ }
+
/** How specifically the rule matches, or -1 when it does not apply. */
private static int score(CustomLintRule rule, ICheckResult remark) {
int score = 0;
@@ -134,6 +194,10 @@ public final class NativeCheckClassifier {
* is not installed, or the key was renamed — matches nothing rather than
everything, so a stale
* rule loses its narrowing instead of silencing every remark.
*
+ * <p>Most checks fill values into their message. Resolved without them, the
message keeps a
+ * {@code {0}} where each value goes, so it is matched as a pattern: the
words must appear in
+ * order, and each placeholder stands for whatever the check filled in.
+ *
* @param text the remark as the transform built it, usually a heading
followed by detail lines
* @param messageKey {@code <i18n package>:<key>}, the same form Hop's own
plugin annotations use
* @param bundleClass the class whose class loader holds the bundle, or null
@@ -152,7 +216,39 @@ public final class NativeCheckClassifier {
if (Utils.isEmpty(message)) {
return false;
}
- return text.contains(message.trim());
+ Pattern pattern = patternOf(message.trim());
+ return pattern != null && pattern.matcher(text).find();
+ }
+
+ /**
+ * The resolved message as a pattern, or null when it holds no words to
match.
+ *
+ * <p>BaseMessages formats every message, with or without values, so quoting
is already undone and
+ * a value that was not given is printed as {@code {n}}. A message made of
placeholders alone
+ * would match every remark, and is refused for the same reason as an
unresolved key.
+ */
+ private static Pattern patternOf(String message) {
+ StringBuilder regex = new StringBuilder();
+ boolean hasWords = false;
+ int start = 0;
+ Matcher placeholder = PLACEHOLDER.matcher(message);
+ while (placeholder.find()) {
+ hasWords |= appendLiteral(regex, message.substring(start,
placeholder.start()));
+ // Lazy, and across lines: values such as an exception message often
carry line breaks.
+ regex.append("(?s:.*?)");
+ start = placeholder.end();
+ }
+ hasWords |= appendLiteral(regex, message.substring(start));
+ return hasWords ? Pattern.compile(regex.toString()) : null;
+ }
+
+ /** Append the text as a literal, and say whether it holds a letter or
digit. */
+ private static boolean appendLiteral(StringBuilder regex, String literal) {
+ if (literal.isEmpty()) {
+ return false;
+ }
+ regex.append(Pattern.quote(literal));
+ return literal.codePoints().anyMatch(Character::isLetterOrDigit);
}
/**
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineVerifyLintExtension.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineVerifyLintExtension.java
index 35ad6e23ef..0e27764705 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineVerifyLintExtension.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PipelineVerifyLintExtension.java
@@ -81,10 +81,9 @@ public class PipelineVerifyLintExtension implements
IExtensionPoint<CheckTransfo
LintCheckResultAdapter.toCheckResults(policyResults, pipelineMeta);
extension.getRemarks().addAll(policyRemarks);
- // Through the same conversion as before. This view has always reported
a policy finding as
- // Hop's own verify output renders it, and reporting it differently here
would leave the
- // Problems bar disagreeing with the background lint about the same file.
- results.addAll(LintCheckResultAdapter.fromCheckResults(policyRemarks,
fileName));
+ // As they are, not read back from the remarks: those carry Hop's
origin, the source's name
+ // and a message prefixed with the rule id, which is not how the command
line reports them.
+ results.addAll(policyResults);
List<LintResult> verifyViewResults =
LintResultDeduplicator.deduplicate(results);
LintResultsManager.getInstance().updateResultsForFile(fileName,
verifyViewResults);
LintProblemsBarManager.getInstance().updateProblemsBar(fileName);
diff --git a/plugins/misc/lint/src/main/resources/hop-lint-core.yml
b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
index c44898e300..423f93a716 100644
--- a/plugins/misc/lint/src/main/resources/hop-lint-core.yml
+++ b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
@@ -158,9 +158,9 @@ rules:
description: >
Every remark Hop's own transform and action checks produce. They work
from the row stream
inferred at design time, which is right for a plain pipeline and wrong
wherever fields
- arrive at runtime, so the linter reports them as warnings rather than
letting them fail a
- build. Set severity to ERROR to treat them as the transform meant them,
or disable this
- rule to leave Hop's verify remarks to the Verify button.
+ arrive at runtime, so the linter caps them at warning rather than
letting them fail a
+ build; a comment stays a comment. Set severity to ERROR to treat them as
the transform meant
+ them, or disable this rule to leave Hop's verify remarks to the Verify
button.
# No narrowed rule ships enabled or disabled here. A check that is simply
wrong is fixed in the
# transform, which is what #8294 did for the two in Select Values; silencing
it from a rule pack
diff --git a/plugins/misc/lint/src/main/resources/hop-lint.yml.example
b/plugins/misc/lint/src/main/resources/hop-lint.yml.example
index 52c40e0ab0..834595f17f 100644
--- a/plugins/misc/lint/src/main/resources/hop-lint.yml.example
+++ b/plugins/misc/lint/src/main/resources/hop-lint.yml.example
@@ -38,11 +38,17 @@ rules:
severity: ERROR
parameters: {}
- # Hop's own transform and action checks — everything reported as [HOP-CHECK].
- # The core pack reports them as warnings, because check() works from the row
- # stream Hop infers at design time and is wrong wherever fields arrive at
- # runtime. Put the severity back to ERROR to treat them as the transform
- # meant them, or set enabled: false to leave them to the Verify button.
+ # Hop's own transform and action checks. A check that reports an error code
+ # is reported under that code, and answers to HOP-CHECK as well, so a rule or
+ # a suppression written against either one holds. A check with no code is
+ # reported as [HOP-CHECK].
+ #
+ # The severity here is a cap, not a setting: check() works from the row
stream
+ # Hop infers at design time and is wrong wherever fields arrive at runtime,
so
+ # a remark is never raised above this, and a comment stays a comment. Raise
it
+ # to ERROR to let checks report as the transform meant them, or set
+ # enabled: false to leave them to the Verify button. A rule naming a plugin
+ # (appliesTo) or one check (messageKey) sets the severity outright instead.
HOP-CHECK:
enabled: true
severity: WARNING
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintBaselineTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintBaselineTest.java
index 81b4403221..3a6fb329b4 100644
--- a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintBaselineTest.java
+++ b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintBaselineTest.java
@@ -153,4 +153,79 @@ public class LintBaselineTest {
assertEquals(1, LintBaseline.read(baselineFile).filter(results,
ROOT).size());
}
+
+ /**
+ * A baseline written while every native remark was reported as HOP-CHECK
still covers a check
+ * that is now reported under its own error code.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void aBaselineRecordedUnderTheClassifyingRuleStillApplies(@TempDir
Path dir)
+ throws IOException {
+ Path baselineFile = dir.resolve("baseline.json");
+ LintBaseline.write(baselineFile, List.of(finding("HOP-CHECK", "a.hpl",
"Table input")), ROOT);
+ LintBaseline baseline = LintBaseline.read(baselineFile);
+ List<LintResult> now = List.of(missingConnection("a.hpl", "Table input"));
+
+ assertEquals(List.of(), baseline.filter(now, ROOT));
+ assertEquals(0, baseline.countStaleEntries(now, ROOT));
+ }
+
+ /** New baselines record the check under its own code. */
+ @Test
+ public void aNewBaselineRecordsTheChecksOwnCode(@TempDir Path dir) throws
IOException {
+ Path baselineFile = dir.resolve("baseline.json");
+ LintBaseline.write(baselineFile, List.of(missingConnection("a.hpl", "Table
input")), ROOT);
+
+ assertTrue(
+ Files.readString(baselineFile, StandardCharsets.UTF_8)
+ .contains("\"CONNECTION_DOES_NOT_EXIST|a.hpl|Table input\""));
+ }
+
+ /**
+ * A coded finding must not take a baseline entry that a codeless remark
matches exactly.
+ *
+ * <p>A coded finding answers to its own code and to HOP-CHECK, so with both
kinds on one
+ * transform and one HOP-CHECK entry recorded, whichever finding was looked
at first took it.
+ * Claiming every own id before any alias gives the entry to the remark that
matches it exactly,
+ * and the coded finding - which has no entry of its own - is the one
reported as new.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void
aCodedFindingDoesNotTakeAnEntryACodelessRemarkMatchesExactly(@TempDir Path dir)
+ throws IOException {
+ Path baselineFile = dir.resolve("baseline.json");
+ LintBaseline.write(baselineFile, List.of(finding("HOP-CHECK", "a.hpl",
"Table input")), ROOT);
+ LintBaseline baseline = LintBaseline.read(baselineFile);
+
+ // The coded finding is looked at first, which is what let it reach for
the alias.
+ List<LintResult> now =
+ List.of(
+ missingConnection("a.hpl", "Table input"),
+ finding("HOP-CHECK", "a.hpl", "Table input"));
+
+ List<LintResult> fresh = baseline.filter(now, ROOT);
+
+ assertEquals(1, fresh.size(), fresh.toString());
+ assertEquals(
+ "CONNECTION_DOES_NOT_EXIST",
+ fresh.get(0).getRuleId(),
+ "the recorded HOP-CHECK remark should have kept its own entry");
+ assertEquals(0, baseline.countStaleEntries(now, ROOT));
+ }
+
+ /** Hop's missing-connection check, reported under its own code after
HOP-CHECK classified it. */
+ private LintResult missingConnection(String relativePath, String sourceName)
{
+ return new LintResult(
+ "CONNECTION_DOES_NOT_EXIST",
+ sourceName,
+ "WARNING",
+ "Database connection 'warehouse' assigned on transform 'Table input'
does not exist",
+ ROOT.resolve(relativePath).toString(),
+ LintSourceRef.transform(sourceName),
+ LintResult.Origin.HOP_NATIVE,
+ "HOP-CHECK");
+ }
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCheckResultAdapterTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCheckResultAdapterTest.java
index 08e56708ed..d3f8e4079d 100644
---
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCheckResultAdapterTest.java
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCheckResultAdapterTest.java
@@ -21,7 +21,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Arrays;
import java.util.List;
+import org.apache.hop.core.CheckResult;
+import org.apache.hop.core.ICheckResult;
+import org.apache.hop.metadata.validation.ReferencedDatabaseConnectionChecker;
import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
import org.junit.jupiter.api.Test;
public class LintCheckResultAdapterTest {
@@ -81,4 +85,118 @@ public class LintCheckResultAdapterTest {
org.junit.jupiter.api.Assertions.assertNotNull(check);
org.junit.jupiter.api.Assertions.assertNull(check.getSourceInfo());
}
+
+ /**
+ * A check that sets its own error code is reported under it. The blanket
rule names every remark,
+ * so taking its id instead left a project unable to tell one check from
another.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void aChecksOwnErrorCodeSurvivesTheBlanketRule() {
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING")));
+
+ LintResult result =
+ LintCheckResultAdapter.fromCheckResult(
+ missingConnectionRemark(), "/tmp/test.hpl", classifier);
+
+ assertEquals("CONNECTION_DOES_NOT_EXIST", result.getRuleId());
+ assertEquals("HOP-CHECK", result.getAliasRuleId());
+ assertEquals("WARNING", result.getSeverity(), "the blanket rule still sets
the severity");
+ }
+
+ @Test
+ public void
aRemarkWithoutAnErrorCodeIsReportedUnderTheRuleThatClassifiedIt() {
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING")));
+
+ LintResult result =
+ LintCheckResultAdapter.fromCheckResult(
+ new CheckResult(ICheckResult.TYPE_RESULT_ERROR, "boom",
tableInput()),
+ "/tmp/test.hpl",
+ classifier);
+
+ assertEquals("HOP-CHECK", result.getRuleId());
+ assertEquals(List.of("HOP-CHECK"), result.getRuleIds());
+ }
+
+ /** A rule a project wrote for one plugin or one check is more specific than
a code. */
+ @Test
+ public void aNarrowedRuleWinsOverTheErrorCode() {
+ CustomLintRule tableInput = nativeRule("HOP-CHECK-TABLEINPUT", "ERROR");
+ tableInput.setAppliesTo(List.of("TableInput"));
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING"),
tableInput));
+
+ LintResult result =
+ LintCheckResultAdapter.fromCheckResult(
+ missingConnectionRemark(), "/tmp/test.hpl", classifier);
+
+ assertEquals("HOP-CHECK-TABLEINPUT", result.getRuleId());
+ assertEquals("ERROR", result.getSeverity());
+ assertEquals(
+ "CONNECTION_DOES_NOT_EXIST",
+ result.getAliasRuleId(),
+ "a suppression naming the code must survive the project adding this
rule");
+ }
+
+ /**
+ * Naming one check must not quietly undo a suppression the project already
had.
+ *
+ * <p>The narrowed rule takes the id, so the rule covering every remark
stopped naming the finding
+ * and a {@code suppress: HOP-CHECK} written beforehand no longer matched it.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void aNarrowedRuleKeepsTheBlanketRuleAsAName() {
+ CustomLintRule tableInput = nativeRule("HOP-CHECK-TABLEINPUT", "ERROR");
+ tableInput.setAppliesTo(List.of("TableInput"));
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING"),
tableInput));
+
+ LintResult result =
+ LintCheckResultAdapter.fromCheckResult(
+ missingConnectionRemark(), "/tmp/test.hpl", classifier);
+
+ assertEquals(
+ List.of("HOP-CHECK-TABLEINPUT", "CONNECTION_DOES_NOT_EXIST",
"HOP-CHECK"),
+ result.getRuleIds(),
+ "the finding must still answer to the rule that covers every remark");
+ }
+
+ /** With no narrowed rule in force the blanket rule is the alias, and is not
repeated. */
+ @Test
+ public void theBlanketRuleIsNamedOnceWhenNoRuleNarrows() {
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING")));
+
+ LintResult result =
+ LintCheckResultAdapter.fromCheckResult(
+ missingConnectionRemark(), "/tmp/test.hpl", classifier);
+
+ assertEquals(List.of("CONNECTION_DOES_NOT_EXIST", "HOP-CHECK"),
result.getRuleIds());
+ }
+
+ private static ICheckResult missingConnectionRemark() {
+ return new CheckResult(
+ ICheckResult.TYPE_RESULT_WARNING,
+ ReferencedDatabaseConnectionChecker.ERROR_DOES_NOT_EXIST,
+ "Database connection 'warehouse' assigned on transform 'Table input'
does not exist",
+ tableInput());
+ }
+
+ private static TransformMeta tableInput() {
+ return new TransformMeta("TableInput", "Table input", null);
+ }
+
+ private static CustomLintRule nativeRule(String id, String severity) {
+ CustomLintRule rule = new CustomLintRule();
+ rule.setId(id);
+ rule.setType(CustomLintRule.TYPE_NATIVE);
+ rule.setSeverity(severity);
+ rule.setEnabled(true);
+ return rule;
+ }
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintPolicyTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintPolicyTest.java
index 22f422402a..6be3837a2e 100644
--- a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintPolicyTest.java
+++ b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintPolicyTest.java
@@ -165,4 +165,45 @@ public class LintPolicyTest {
private String relative(LintResult result) {
return LintPolicy.relativise(result.getFileName(), ROOT);
}
+
+ /**
+ * A check with its own error code is reported under it, and a suppression
can name that one
+ * check. One written against HOP-CHECK before the check had a code still
applies.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void aSuppressionMayNameTheCheckOrTheRuleThatClassifiedIt() {
+ LintResult finding = missingConnection("a.hpl", "Table input");
+
+ assertTrue(
+ new LintPolicy(
+ List.of(),
+ List.of(new
LintPolicy.Suppression("CONNECTION_DOES_NOT_EXIST", null, null, "ok")))
+ .isSuppressed(finding, ROOT));
+ assertTrue(
+ new LintPolicy(
+ List.of(), List.of(new LintPolicy.Suppression("HOP-CHECK",
null, null, "ok")))
+ .isSuppressed(finding, ROOT),
+ "an existing suppression naming HOP-CHECK must keep working");
+ assertFalse(
+ new LintPolicy(
+ List.of(),
+ List.of(new
LintPolicy.Suppression("CONNECTION_DOES_NOT_EXIST", null, null, "ok")))
+ .isSuppressed(finding("HOP-CHECK", "a.hpl", "Table input"), ROOT),
+ "naming one check must leave Hop's other remarks alone");
+ }
+
+ /** Hop's missing-connection check, reported under its own code after
HOP-CHECK classified it. */
+ private LintResult missingConnection(String relativePath, String sourceName)
{
+ return new LintResult(
+ "CONNECTION_DOES_NOT_EXIST",
+ sourceName,
+ "WARNING",
+ "Database connection 'warehouse' assigned on transform 'Table input'
does not exist",
+ ROOT.resolve(relativePath).toString(),
+ LintSourceRef.transform(sourceName),
+ LintResult.Origin.HOP_NATIVE,
+ "HOP-CHECK");
+ }
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressionInEditorTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressionInEditorTest.java
index c04a348a3a..920bbd78b6 100644
---
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressionInEditorTest.java
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressionInEditorTest.java
@@ -18,6 +18,7 @@
package org.apache.hop.lint;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.nio.charset.StandardCharsets;
@@ -28,6 +29,7 @@ import org.apache.hop.core.variables.IVariables;
import org.apache.hop.core.variables.Variables;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
+import org.apache.hop.pipeline.transforms.missing.Missing;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
@@ -46,8 +48,10 @@ public class LintSuppressionInEditorTest {
@TempDir private Path projectDir;
/**
- * Two transforms with no hop between them. Hop's own check reports each as
unused, with no
- * transform plugin needed, so the finding under test is a native remark
rather than a lint rule.
+ * Two transforms with no hop between them, whose plugins are not installed.
Hop's own check
+ * reports each as unused, which {@code TRANS-002} says too, so
deduplication keeps the lint
+ * finding. It also reports each plugin as missing, which no lint rule says:
that remark is the
+ * native finding under test.
*/
private PipelineMeta pipelineWithUnusedTransforms(String fileName) {
PipelineMeta pipelineMeta = new PipelineMeta();
@@ -57,11 +61,13 @@ public class LintSuppressionInEditorTest {
TransformMeta source = new TransformMeta();
source.setName("Fonte Sql");
source.setTransformPluginId("TableInput");
+ source.setTransform(new Missing("Fonte Sql", "TableInput"));
pipelineMeta.addTransform(source);
TransformMeta target = new TransformMeta();
target.setName("Salva S3");
target.setTransformPluginId("TextFileOutput");
+ target.setTransform(new Missing("Salva S3", "TextFileOutput"));
pipelineMeta.addTransform(target);
return pipelineMeta;
@@ -173,7 +179,7 @@ public class LintSuppressionInEditorTest {
/**
* The blanket native rule names no plugin and no message, so it matches
every check result put in
- * front of it. The linter's own findings travel through {@code
ICheckResult} on this path, and
+ * front of it. The verify tab shows the linter's own findings as {@code
ICheckResult}s, and
* classifying them along with Hop's remarks would rename every one of them
to {@code HOP-CHECK} -
* silently collapsing the rule ids a project writes its suppressions
against.
*/
@@ -191,6 +197,49 @@ public class LintSuppressionInEditorTest {
"the orphaned-transform finding on Salva S3 lost its rule id: " +
results);
}
+ /**
+ * A lint rule's finding is the linter's, in the editor as on the command
line.
+ *
+ * <p>It was not: the editor turned policy findings into Hop remarks and
read them back, which
+ * reported each as Hop's own, named after the transform rather than the
rule, and with {@code
+ * [TRANS-002] Orphaned Transform: } written into its message.
+ */
+ @Test
+ public void policyFindingsAreReportedAsTheLintersInTheEditor() throws
Exception {
+ List<LintResult> orphaned =
+ lintAsEditor().stream().filter(r ->
"TRANS-002".equals(r.getRuleId())).toList();
+
+ assertEquals(2, orphaned.size(), "expected one finding per transform: " +
orphaned);
+ for (LintResult result : orphaned) {
+ assertEquals(LintResult.Origin.LINT, result.getOrigin(),
result.toString());
+ assertEquals("Orphaned Transform", result.getRuleName(),
result.toString());
+ assertFalse(result.getMessage().startsWith("["), result.toString());
+ }
+ }
+
+ /**
+ * The editor and the linter's own entry point report the same findings for
the same pipeline.
+ *
+ * <p>They did not: the editor labelled its lint findings as Hop's, so
deduplication could not see
+ * that Hop's "not used" remark and {@code TRANS-002} were one finding, and
reported both.
+ */
+ @Test
+ public void editorReportsWhatTheLinterReports() throws Exception {
+ String fileName = projectDir.resolve("template.hpl").toString();
+ List<LintResult> linter =
+ new HopLinter()
+ .lintHopObject(pipelineWithUnusedTransforms(fileName), fileName,
null, variables);
+
+ assertEquals(describe(linter), describe(lintAsEditor()));
+ }
+
+ private List<String> describe(List<LintResult> results) {
+ return results.stream()
+ .map(r -> r.getOrigin() + " " + r.getRuleId() + " " + sourceName(r) +
" " + r.getMessage())
+ .sorted()
+ .toList();
+ }
+
private long countOfRule(List<LintResult> results, String ruleId) {
return results.stream().filter(r -> ruleId.equals(r.getRuleId())).count();
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/NativeCheckClassifierTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/NativeCheckClassifierTest.java
index 4ef31e8c39..cb98909311 100644
---
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/NativeCheckClassifierTest.java
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/NativeCheckClassifierTest.java
@@ -48,6 +48,8 @@ public class NativeCheckClassifierTest {
private static final String SELECT_VALUES_PACKAGE =
"org.apache.hop.pipeline.transforms.selectvalues";
+ private static final String VALIDATION_PACKAGE =
"org.apache.hop.metadata.validation";
+
@Test
public void remarksAreUntouchedWhenNoRuleSpeaksAboutThem() {
NativeCheckClassifier classifier = new NativeCheckClassifier(List.of());
@@ -74,6 +76,37 @@ public class NativeCheckClassifierTest {
assertEquals("HOP-CHECK", classification.ruleId(), "the finding stays
suppressible by id");
}
+ /**
+ * The blanket rule caps; it does not raise. A comment such as "this
transform can start without
+ * incoming hops" was reported as a warning on every source transform in a
project.
+ */
+ @Test
+ public void theBlanketRuleNeverRaisesARemark() {
+ NativeCheckClassifier warnings =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "WARNING",
true)));
+ NativeCheckClassifier errors =
+ new NativeCheckClassifier(List.of(nativeRule("HOP-CHECK", "ERROR",
true)));
+
+ assertEquals(
+ "INFO", warnings.classify(remark(ICheckResult.TYPE_RESULT_COMMENT, "a
source")).severity());
+ assertEquals(
+ "WARNING",
+ errors.classify(remark(ICheckResult.TYPE_RESULT_WARNING, "a
warning")).severity(),
+ "putting the severity back must not turn warnings into errors");
+ }
+
+ /** A rule naming a plugin or a check is a decision about it, so it may
raise as well as lower. */
+ @Test
+ public void aNarrowedRuleSetsTheSeverity() {
+ CustomLintRule raised = nativeRule("HOP-CHECK-SV", "ERROR", true);
+ raised.setAppliesTo(List.of("SelectValues"));
+ NativeCheckClassifier classifier = new
NativeCheckClassifier(List.of(raised));
+
+ assertEquals(
+ "ERROR",
+ classifier.classify(remark(ICheckResult.TYPE_RESULT_COMMENT, "a
comment")).severity());
+ }
+
@Test
public void aProjectCanPutTheRemarksBackToTheSeverityTheTransformMeant() {
NativeCheckClassifier classifier =
@@ -167,8 +200,8 @@ public class NativeCheckClassifierTest {
}
/**
- * The message key is resolved through the bundle rather than matched as a
pattern, which is what
- * lets a rule name a check without naming the English words it happens to
use.
+ * The message key is resolved through the bundle rather than written out as
English text, which
+ * is what lets a rule name a check without naming the words it happens to
use.
*/
@Test
public void aMessageKeyIsResolvedAgainstThePluginsOwnBundle() {
@@ -188,6 +221,71 @@ public class NativeCheckClassifierTest {
}
}
+ /**
+ * A check that fills values into its message can be named by its key.
+ *
+ * <p>It could not: the key resolved with its placeholders still in it,
which no printed remark
+ * contains, so the rule matched nothing and said nothing about why.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8536";>#8536</a>
+ */
+ @Test
+ public void aMessageKeyMatchesACheckThatFillsInValues() {
+ String doesNotExist =
+ BaseMessages.getString(
+ VALIDATION_PACKAGE,
+ "ReferencedDatabaseConnectionChecker.DoesNotExist",
+ "warehouse",
+ "transform",
+ "Table input");
+ String notVerified =
+ BaseMessages.getString(
+ VALIDATION_PACKAGE,
+ "ReferencedDatabaseConnectionChecker.NotVerified",
+ "warehouse",
+ "transform",
+ "Table input",
+ "no such file" + Const.CR + "\tat somewhere");
+ String key = VALIDATION_PACKAGE +
":ReferencedDatabaseConnectionChecker.DoesNotExist";
+
+ assertTrue(NativeCheckClassifier.printsMessage(doesNotExist, key, null),
doesNotExist);
+ assertFalse(
+ NativeCheckClassifier.printsMessage(notVerified, key, null),
+ "the words after the values tell the two checks apart: " +
notVerified);
+ assertTrue(
+ NativeCheckClassifier.printsMessage(
+ notVerified,
+ VALIDATION_PACKAGE +
":ReferencedDatabaseConnectionChecker.NotVerified",
+ null),
+ "a value spanning lines still matches: " + notVerified);
+ }
+
+ @Test
+ public void aRuleNamingACheckThatFillsInValuesWinsOverTheBlanketOne() {
+ CustomLintRule missingConnection =
nativeRule("HOP-CHECK-MISSING-CONNECTION", "ERROR", true);
+ missingConnection.setMessageKey(
+ VALIDATION_PACKAGE +
":ReferencedDatabaseConnectionChecker.DoesNotExist");
+ NativeCheckClassifier classifier =
+ new NativeCheckClassifier(
+ List.of(nativeRule("HOP-CHECK", "WARNING", true),
missingConnection));
+
+ NativeCheckClassifier.Classification classification =
+ classifier.classify(
+ remark(
+ ICheckResult.TYPE_RESULT_WARNING,
+ BaseMessages.getString(
+ VALIDATION_PACKAGE,
+ "ReferencedDatabaseConnectionChecker.DoesNotExist",
+ "warehouse",
+ "transform",
+ "Table input"),
+ "TableInput"));
+
+ assertNotNull(classification);
+ assertEquals("HOP-CHECK-MISSING-CONNECTION", classification.ruleId());
+ assertEquals("ERROR", classification.severity());
+ }
+
/**
* The transform from the issue, checked by Select Values itself.
*
diff --git
a/plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMeta.java
b/plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMeta.java
index 2faa8eed5f..e97bed2f31 100644
---
a/plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMeta.java
+++
b/plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMeta.java
@@ -38,6 +38,7 @@ import org.apache.hop.core.row.IRowMeta;
import org.apache.hop.core.row.IValueMeta;
import org.apache.hop.core.row.RowDataUtil;
import org.apache.hop.core.row.RowMeta;
+import org.apache.hop.core.util.StringUtil;
import org.apache.hop.core.util.Utils;
import org.apache.hop.core.variables.IVariables;
import org.apache.hop.core.vfs.HopVfs;
@@ -45,6 +46,7 @@ import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.metadata.api.HopMetadataProperty;
import org.apache.hop.metadata.api.HopMetadataPropertyType;
import org.apache.hop.metadata.api.IHopMetadataProvider;
+import org.apache.hop.metadata.validation.ReferencedDatabaseConnectionChecker;
import org.apache.hop.pipeline.DatabaseImpact;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.BaseTransformMeta;
@@ -313,19 +315,24 @@ public class TableInputMeta extends
BaseTransformMeta<TableInput, TableInputData
DatabaseMeta databaseMeta = null;
- try {
- databaseMeta =
-
metadataProvider.getSerializer(DatabaseMeta.class).load(variables.resolve(connection));
- } catch (HopException e) {
- cr =
- new CheckResult(
- ICheckResult.TYPE_RESULT_ERROR,
- BaseMessages.getString(
- PKG,
- "TableInputMeta.CheckResult.DatabaseMetaError",
- variables.resolve(connection)),
- transformMeta);
- remarks.add(cr);
+ String resolvedConnection = variables.resolve(connection);
+ // An unset connection is null, the field default on a new transform.
Loading it would only
+ // raise "you need to specify the name...", and the remark that came out
of that said the same
+ // thing as the pipeline check (ReferencedDatabaseConnectionChecker,
CONNECTION_NOT_ASSIGNED)
+ // without carrying its code, so it could not be suppressed or baselined.
Leave the unset
+ // connection to that check.
+ if (!Utils.isEmpty(resolvedConnection)) {
+ try {
+ databaseMeta =
metadataProvider.getSerializer(DatabaseMeta.class).load(resolvedConnection);
+ } catch (HopException e) {
+ cr =
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(
+ PKG, "TableInputMeta.CheckResult.DatabaseMetaError",
resolvedConnection),
+ transformMeta);
+ remarks.add(cr);
+ }
}
if (databaseMeta != null) {
@@ -383,11 +390,20 @@ public class TableInputMeta extends
BaseTransformMeta<TableInput, TableInputData
} finally {
db.close();
}
- } else {
+ } else if (!Utils.isEmpty(resolvedConnection)
+ && StringUtil.containsVariableToken(resolvedConnection)) {
+ // A connection that is not set, or not in the project, is reported by
the pipeline check
+ // (ReferencedDatabaseConnectionChecker) for every transform, so
reporting it here too would
+ // tell the user the same thing twice. The one case that check leaves
alone is a name that
+ // still holds a variable after resolving: it cannot decide such a name
at design time. This
+ // transform can, because it tried to load the connection with the
variables this check ran
+ // with and got nothing back.
cr =
new CheckResult(
ICheckResult.TYPE_RESULT_ERROR,
- "Please select or create a connection to use",
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_RESOLVED,
+ BaseMessages.getString(
+ PKG, "TableInputMeta.CheckResult.ConnectionNotResolved",
resolvedConnection),
transformMeta);
remarks.add(cr);
}
diff --git
a/plugins/transforms/tableinput/src/main/resources/org/apache/hop/pipeline/transforms/tableinput/messages/messages_en_US.properties
b/plugins/transforms/tableinput/src/main/resources/org/apache/hop/pipeline/transforms/tableinput/messages/messages_en_US.properties
index 93d4510ee1..382bca21b6 100644
---
a/plugins/transforms/tableinput/src/main/resources/org/apache/hop/pipeline/transforms/tableinput/messages/messages_en_US.properties
+++
b/plugins/transforms/tableinput/src/main/resources/org/apache/hop/pipeline/transforms/tableinput/messages/messages_en_US.properties
@@ -104,3 +104,4 @@ TableInputMeta.Exception.CouldNotLoadSqlFromFile=Could not
load SQL from file: {
TableInputMeta.Exception.ConnectionNotFound=Unable to find database connection
''{0}''. Check the connection name, it may contain a variable which isn''t set.
TableInputMeta.keyword=sql,query,database,select,jdbc
System.FileType.AllFiles=All files
+TableInputMeta.CheckResult.ConnectionNotResolved=Database connection ''{0}''
could not be resolved: the name still holds a variable that is not set.
diff --git
a/plugins/transforms/tableinput/src/test/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMetaTest.java
b/plugins/transforms/tableinput/src/test/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMetaTest.java
index 34b4e26f9d..f4c3a9a5c2 100644
---
a/plugins/transforms/tableinput/src/test/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMetaTest.java
+++
b/plugins/transforms/tableinput/src/test/java/org/apache/hop/pipeline/transforms/tableinput/TableInputMetaTest.java
@@ -38,6 +38,7 @@ import org.apache.hop.core.xml.XmlHandler;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
import org.apache.hop.metadata.serializer.xml.XmlMetadataUtil;
+import org.apache.hop.metadata.validation.ReferencedDatabaseConnectionChecker;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.pipeline.transform.stream.IStream;
@@ -398,6 +399,133 @@ class TableInputMetaTest {
Assertions.assertTrue(e.getMessage().contains("${connection_name}"),
e.getMessage());
}
+ /**
+ * A connection that is not in the project is reported once, by the pipeline
check, under its
+ * error code. Table input used to add a remark of its own, without a code,
saying the same thing.
+ */
+ @Test
+ void aMissingConnectionIsReportedOnceByPipelineVerify() {
+ TableInputMeta meta = new TableInputMeta();
+ meta.setConnection("doesnotexist");
+ meta.setSql("SELECT 1");
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.addTransform(new TransformMeta("Table input", meta));
+
+ List<ICheckResult> remarks = new ArrayList<>();
+ pipelineMeta.checkTransforms(
+ remarks, false, null, new Variables(), new MemoryMetadataProvider());
+
+ List<ICheckResult> aboutTheConnection =
+ remarks.stream()
+ .filter(r -> r.getType() != ICheckResult.TYPE_RESULT_OK)
+ .filter(r -> r.getText().toLowerCase().contains("connection"))
+ .toList();
+ Assertions.assertEquals(1, aboutTheConnection.size(),
aboutTheConnection.toString());
+ Assertions.assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_DOES_NOT_EXIST,
+ aboutTheConnection.get(0).getErrorCode());
+ }
+
+ /**
+ * The pipeline check stays silent when a connection name still holds a
variable after resolving:
+ * at design time that name cannot be decided. Table input can decide it -
it tried to load the
+ * connection with these variables and got nothing - so it reports, and the
file is not left with
+ * no remark at all.
+ */
+ @Test
+ void anUnresolvedConnectionVariableIsStillReported() {
+ TableInputMeta meta = new TableInputMeta();
+ meta.setConnection("${DB_CONN}");
+ meta.setSql("SELECT 1");
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.addTransform(new TransformMeta("Table input", meta));
+
+ List<ICheckResult> remarks = new ArrayList<>();
+ pipelineMeta.checkTransforms(
+ remarks, false, null, new Variables(), new MemoryMetadataProvider());
+
+ List<ICheckResult> aboutTheConnection =
+ remarks.stream()
+ .filter(r -> r.getType() != ICheckResult.TYPE_RESULT_OK)
+ .filter(r -> r.getText().toLowerCase().contains("connection"))
+ .toList();
+ Assertions.assertEquals(1, aboutTheConnection.size(),
aboutTheConnection.toString());
+ Assertions.assertEquals(
+ ICheckResult.TYPE_RESULT_ERROR, aboutTheConnection.get(0).getType(),
"must stay an error");
+ Assertions.assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_RESOLVED,
+ aboutTheConnection.get(0).getErrorCode());
+ Assertions.assertTrue(
+ aboutTheConnection.get(0).getText().contains("${DB_CONN}"),
+ aboutTheConnection.get(0).getText());
+ }
+
+ /** A connection that resolves and exists is not reported by either check. */
+ @Test
+ void aResolvedConnectionVariableIsNotReported() {
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ Variables variables = new Variables();
+ variables.setVariable("DB_CONN", "doesnotexist");
+
+ TableInputMeta meta = new TableInputMeta();
+ meta.setConnection("${DB_CONN}");
+ meta.setSql("SELECT 1");
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.addTransform(new TransformMeta("Table input", meta));
+
+ List<ICheckResult> remarks = new ArrayList<>();
+ pipelineMeta.checkTransforms(remarks, false, null, variables,
metadataProvider);
+
+ // The variable resolves, so the name can be decided: the pipeline check
owns it again, under
+ // its own code, and table input adds nothing.
+ List<ICheckResult> aboutTheConnection =
+ remarks.stream()
+ .filter(r -> r.getType() != ICheckResult.TYPE_RESULT_OK)
+ .filter(r -> r.getText().toLowerCase().contains("connection"))
+ .toList();
+ Assertions.assertEquals(1, aboutTheConnection.size(),
aboutTheConnection.toString());
+ Assertions.assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_DOES_NOT_EXIST,
+ aboutTheConnection.get(0).getErrorCode());
+ }
+
+ /**
+ * A connection that was never assigned is null, not empty: the field
default on a new transform.
+ * It is reported once, by the pipeline check, under its own code. Table
input used to add an
+ * uncoded error of its own - the load of a null name raised "you need to
specify the name..." -
+ * which no suppression of CONNECTION_NOT_ASSIGNED could clear.
+ */
+ @Test
+ void anUnassignedConnectionIsReportedOnceByPipelineVerify() {
+ assertConnectionRemark(null,
ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED);
+ }
+
+ /** An empty connection name is the same thing said differently, and
reported the same way. */
+ @Test
+ void anEmptyConnectionIsReportedOnceByPipelineVerify() {
+ assertConnectionRemark("",
ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED);
+ }
+
+ private static void assertConnectionRemark(String connection, String
expectedErrorCode) {
+ TableInputMeta meta = new TableInputMeta();
+ meta.setConnection(connection);
+ meta.setSql("SELECT 1");
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.addTransform(new TransformMeta("Table input", meta));
+
+ List<ICheckResult> remarks = new ArrayList<>();
+ pipelineMeta.checkTransforms(
+ remarks, false, null, new Variables(), new MemoryMetadataProvider());
+
+ List<ICheckResult> aboutTheConnection =
+ remarks.stream()
+ .filter(r -> r.getType() != ICheckResult.TYPE_RESULT_OK)
+ .filter(r -> r.getText().toLowerCase().contains("connection"))
+ .toList();
+ Assertions.assertEquals(1, aboutTheConnection.size(),
aboutTheConnection.toString());
+ Assertions.assertEquals(expectedErrorCode,
aboutTheConnection.get(0).getErrorCode());
+ }
+
private static TableInputMeta namedParameterMeta() {
TableInputMeta meta = new TableInputMeta();
meta.setConnection("h2");
