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 17d8448b04 Issue #8561 : Do not warn about a missing connection when
Add Sequence uses a counter (#8634)
17d8448b04 is described below
commit 17d8448b04ff5a164ad3d055e8cbe5eb53437697
Author: Matt Casters <[email protected]>
AuthorDate: Sun Sep 27 11:13:31 2026 +0200
Issue #8561 : Do not warn about a missing connection when Add Sequence uses
a counter (#8634)
Add Sequence reported a missing database connection when the sequence was
generated with a counter. The connection is only required when a database
sequence is selected.
---
.../metadata/api/IOptionalDatabaseConnection.java | 34 +++++
.../ReferencedDatabaseConnectionChecker.java | 31 +++-
.../ReferencedDatabaseConnectionCheckerTest.java | 65 ++++++++
.../transforms/addsequence/AddSequenceMeta.java | 164 ++++++++++++---------
.../addsequence/AddSequenceMetaTest.java | 89 +++++++++++
5 files changed, 314 insertions(+), 69 deletions(-)
diff --git
a/core/src/main/java/org/apache/hop/metadata/api/IOptionalDatabaseConnection.java
b/core/src/main/java/org/apache/hop/metadata/api/IOptionalDatabaseConnection.java
new file mode 100644
index 0000000000..9c547aaf29
--- /dev/null
+++
b/core/src/main/java/org/apache/hop/metadata/api/IOptionalDatabaseConnection.java
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.metadata.api;
+
+/**
+ * A metadata object whose relational connection is only used for some
settings.
+ *
+ * <p>Connection checks treat every {@link
HopMetadataPropertyType#RDBMS_CONNECTION} field as
+ * required. Implement this when a connection can be left unset, and return
{@code false} for a
+ * field the current settings do not use. That field is left out of the check.
+ */
+public interface IOptionalDatabaseConnection {
+
+ /**
+ * @param key the serialised key of the connection field, or the field name
when no key is set
+ * @return {@code true} when that connection has to be assigned for the
current settings
+ */
+ boolean isDatabaseConnectionUsed(String key);
+}
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 c9f32d15bd..8ac5c62a8b 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
@@ -31,6 +31,7 @@ import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.metadata.api.HopMetadataPropertyType;
import org.apache.hop.metadata.api.IHopMetadataProvider;
import org.apache.hop.metadata.api.IHopMetadataSerializer;
+import org.apache.hop.metadata.api.IOptionalDatabaseConnection;
import org.apache.hop.metadata.util.HopMetadataPropertyWalker;
import org.apache.hop.metadata.util.HopMetadataPropertyWalker.StringProperty;
import org.apache.hop.pipeline.PipelineMeta;
@@ -44,7 +45,9 @@ import org.apache.hop.workflow.action.ActionMeta;
*
* <p>This is an existence check only. It never opens a JDBC connection. Names
that still contain a
* variable token after resolving the current {@link IVariables} are skipped,
because the name
- * cannot be decided at design time.
+ * cannot be decided at design time. A metadata object that implements {@link
+ * IOptionalDatabaseConnection} can say that a connection field is unused for
the current settings;
+ * that field is left out of this check.
*/
public final class ReferencedDatabaseConnectionChecker {
@@ -159,6 +162,9 @@ public final class ReferencedDatabaseConnectionChecker {
for (StringProperty property :
HopMetadataPropertyWalker.collectStrings(
metadataObject, HopMetadataPropertyType.RDBMS_CONNECTION, true)) {
+ if (!isConnectionUsed(metadataObject, property.key())) {
+ continue;
+ }
ICheckResult remark =
checkConnectionName(
property.value(), ownerKind, ownerName, source, variables,
serializer);
@@ -169,6 +175,29 @@ public final class ReferencedDatabaseConnectionChecker {
return remarks;
}
+ /**
+ * A connection field is required unless the metadata object says the
current settings do not use
+ * it. A failure in that callback is treated as required, so a broken plugin
still reports a
+ * missing connection.
+ */
+ private static boolean isConnectionUsed(Object metadataObject, String key) {
+ if (!(metadataObject instanceof IOptionalDatabaseConnection optional)) {
+ return true;
+ }
+ try {
+ return optional.isDatabaseConnectionUsed(key);
+ } catch (RuntimeException e) {
+ if (HopLogStore.isInitialized()) {
+ LogChannel.GENERAL.logDebug(
+ "Could not decide whether connection '"
+ + key
+ + "' is used, so it is still checked: "
+ + e.getMessage());
+ }
+ return true;
+ }
+ }
+
/**
* Why the lookup failed, in one line fit for a table cell.
*
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 dc370b612b..bac269d681 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
@@ -32,6 +32,7 @@ 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.api.IHopMetadataSerializer;
+import org.apache.hop.metadata.api.IOptionalDatabaseConnection;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.pipeline.transforms.dummy.DummyMeta;
@@ -324,4 +325,68 @@ class ReferencedDatabaseConnectionCheckerTest {
@HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.RDBMS_CONNECTION)
String connection;
}
+
+ /**
+ * Issue #8561. Add Sequence keeps a connection field for the
database-sequence option and leaves
+ * it unset when a counter is used. That field must not be reported as a
missing connection.
+ */
+ static class OptionalConnMeta implements IOptionalDatabaseConnection {
+ @HopMetadataProperty(
+ key = "connection",
+ hopMetadataPropertyType = HopMetadataPropertyType.RDBMS_CONNECTION)
+ String connection;
+
+ boolean used;
+
+ OptionalConnMeta(String connection, boolean used) {
+ this.connection = connection;
+ this.used = used;
+ }
+
+ @Override
+ public boolean isDatabaseConnectionUsed(String key) {
+ return used;
+ }
+ }
+
+ @Test
+ void unusedConnectionIsNotReported() {
+ assertTrue(check(new OptionalConnMeta(null, false), "Add
sequence").isEmpty());
+ assertTrue(check(new OptionalConnMeta("", false), "Add
sequence").isEmpty());
+ assertTrue(check(new OptionalConnMeta("missing-db", false), "Add
sequence").isEmpty());
+ }
+
+ @Test
+ void optionalConnectionIsReportedWhenItIsUsed() {
+ List<ICheckResult> remarks = check(new OptionalConnMeta(null, true), "Add
sequence");
+
+ assertEquals(1, remarks.size());
+ assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED,
remarks.get(0).getErrorCode());
+ }
+
+ @Test
+ void optionalConnectionThatExistsIsSilent() {
+ assertTrue(check(new OptionalConnMeta("sales-db", true), "Add
sequence").isEmpty());
+ }
+
+ /** A usage callback that throws must not hide a missing connection. */
+ static class ThrowingOptionalConnMeta implements IOptionalDatabaseConnection
{
+ @HopMetadataProperty(hopMetadataPropertyType =
HopMetadataPropertyType.RDBMS_CONNECTION)
+ String connection;
+
+ @Override
+ public boolean isDatabaseConnectionUsed(String key) {
+ throw new IllegalStateException("broken");
+ }
+ }
+
+ @Test
+ void aBrokenUsageCallbackStillReportsTheConnection() {
+ List<ICheckResult> remarks = check(new ThrowingOptionalConnMeta(), "Add
sequence");
+
+ assertEquals(1, remarks.size());
+ assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED,
remarks.get(0).getErrorCode());
+ }
}
diff --git
a/plugins/transforms/addsequence/src/main/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMeta.java
b/plugins/transforms/addsequence/src/main/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMeta.java
index 7038ea3873..ade6e663d5 100644
---
a/plugins/transforms/addsequence/src/main/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMeta.java
+++
b/plugins/transforms/addsequence/src/main/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMeta.java
@@ -31,11 +31,13 @@ import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.row.IRowMeta;
import org.apache.hop.core.row.IValueMeta;
import org.apache.hop.core.row.value.ValueMetaInteger;
+import org.apache.hop.core.util.Utils;
import org.apache.hop.core.variables.IVariables;
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.api.IOptionalDatabaseConnection;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.BaseTransformMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
@@ -51,7 +53,8 @@ import org.apache.hop.pipeline.transform.TransformMeta;
keywords = "i18n::AddSequenceMeta.keyword")
@Getter
@Setter
-public class AddSequenceMeta extends BaseTransformMeta<AddSequence,
AddSequenceData> {
+public class AddSequenceMeta extends BaseTransformMeta<AddSequence,
AddSequenceData>
+ implements IOptionalDatabaseConnection {
private static final Class<?> PKG = AddSequenceMeta.class;
@@ -154,6 +157,15 @@ public class AddSequenceMeta extends
BaseTransformMeta<AddSequence, AddSequenceD
row.addValueMeta(v);
}
+ /**
+ * The connection is only used when a database sequence is selected. A
counter leaves it unset,
+ * and that must not be reported as a missing connection.
+ */
+ @Override
+ public boolean isDatabaseConnectionUsed(String key) {
+ return databaseUsed;
+ }
+
@Override
public void check(
List<ICheckResult> remarks,
@@ -165,50 +177,14 @@ public class AddSequenceMeta extends
BaseTransformMeta<AddSequence, AddSequenceD
IRowMeta info,
IVariables variables,
IHopMetadataProvider metadataProvider) {
- CheckResult cr;
- Database db = null;
-
- try {
- DatabaseMeta databaseMeta =
-
metadataProvider.getSerializer(DatabaseMeta.class).load(variables.resolve(connection));
-
- if (databaseUsed) {
- db = new Database(loggingObject, variables, databaseMeta);
- db.connect();
- if (db.checkSequenceExists(
- variables.resolve(schemaName), variables.resolve(sequenceName))) {
- cr =
- new CheckResult(
- ICheckResult.TYPE_RESULT_OK,
- BaseMessages.getString(PKG,
"AddSequenceMeta.CheckResult.SequenceExists.Title"),
- transformMeta);
- } else {
- cr =
- new CheckResult(
- ICheckResult.TYPE_RESULT_ERROR,
- BaseMessages.getString(
- PKG,
-
"AddSequenceMeta.CheckResult.SequenceCouldNotBeFound.Title",
- sequenceName),
- transformMeta);
- }
- remarks.add(cr);
- }
- } catch (HopException e) {
- cr =
- new CheckResult(
- ICheckResult.TYPE_RESULT_ERROR,
- BaseMessages.getString(PKG,
"AddSequenceMeta.CheckResult.UnableToConnectDB.Title")
- + Const.CR
- + e.getMessage(),
- transformMeta);
- remarks.add(cr);
- } finally {
- if (db != null) {
- db.close();
- }
+ // The counter does not open a connection. Loading an unset name here only
raises "you need to
+ // specify the name of the metadata object to load", which the verify
dialog shows as a database
+ // error. Issue #8561.
+ if (databaseUsed) {
+ checkDatabaseSequence(remarks, transformMeta, variables,
metadataProvider);
}
+ CheckResult cr;
if (input.length > 0) {
cr =
new CheckResult(
@@ -226,6 +202,57 @@ public class AddSequenceMeta extends
BaseTransformMeta<AddSequence, AddSequenceD
}
}
+ /**
+ * Verify the database sequence. An unset connection is left to {@code
+ * ReferencedDatabaseConnectionChecker}, which reports it with a code the
linter can baseline.
+ */
+ private void checkDatabaseSequence(
+ List<ICheckResult> remarks,
+ TransformMeta transformMeta,
+ IVariables variables,
+ IHopMetadataProvider metadataProvider) {
+ String resolvedConnection = variables.resolve(connection);
+ if (Utils.isEmpty(resolvedConnection)) {
+ return;
+ }
+
+ Database db = null;
+ try {
+ DatabaseMeta databaseMeta =
+
metadataProvider.getSerializer(DatabaseMeta.class).load(resolvedConnection);
+ db = new Database(loggingObject, variables, databaseMeta);
+ db.connect();
+ CheckResult cr;
+ if (db.checkSequenceExists(variables.resolve(schemaName),
variables.resolve(sequenceName))) {
+ cr =
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_OK,
+ BaseMessages.getString(PKG,
"AddSequenceMeta.CheckResult.SequenceExists.Title"),
+ transformMeta);
+ } else {
+ cr =
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(
+ PKG,
"AddSequenceMeta.CheckResult.SequenceCouldNotBeFound.Title", sequenceName),
+ transformMeta);
+ }
+ remarks.add(cr);
+ } catch (HopException e) {
+ remarks.add(
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG,
"AddSequenceMeta.CheckResult.UnableToConnectDB.Title")
+ + Const.CR
+ + e.getMessage(),
+ transformMeta));
+ } finally {
+ if (db != null) {
+ db.close();
+ }
+ }
+ }
+
@Override
public SqlStatement getSqlStatements(
IVariables variables,
@@ -233,37 +260,38 @@ public class AddSequenceMeta extends
BaseTransformMeta<AddSequence, AddSequenceD
TransformMeta transformMeta,
IRowMeta prev,
IHopMetadataProvider metadataProvider) {
+ SqlStatement retval = new SqlStatement(transformMeta.getName(), null,
null);
+ if (!databaseUsed) {
+ return retval;
+ }
+
Database db = null;
- SqlStatement retval = null;
try {
- DatabaseMeta databaseMeta =
-
metadataProvider.getSerializer(DatabaseMeta.class).load(variables.resolve(connection));
- retval = new SqlStatement(transformMeta.getName(), databaseMeta, null);
- // default: nothing to do!
- if (databaseUsed) {
- // Otherwise, don't bother!
- if (databaseMeta != null) {
- db = new Database(loggingObject, variables, databaseMeta);
- db.connect();
- if (!db.checkSequenceExists(schemaName, sequenceName)) {
- String crTable =
- db.getCreateSequenceStatement(sequenceName, startAt,
incrementBy, maxValue, true);
- retval.setSql(crTable);
- } else {
- retval.setSql(null); // Empty string means: nothing to do: set it
to null...
- }
+ String resolvedConnection = variables.resolve(connection);
+ DatabaseMeta databaseMeta = null;
+ if (!Utils.isEmpty(resolvedConnection)) {
+ databaseMeta =
metadataProvider.getSerializer(DatabaseMeta.class).load(resolvedConnection);
+ }
+ retval.setDatabase(databaseMeta);
+ if (databaseMeta != null) {
+ db = new Database(loggingObject, variables, databaseMeta);
+ db.connect();
+ if (!db.checkSequenceExists(schemaName, sequenceName)) {
+ String crTable =
+ db.getCreateSequenceStatement(sequenceName, startAt,
incrementBy, maxValue, true);
+ retval.setSql(crTable);
} else {
- retval.setError(
- BaseMessages.getString(PKG,
"AddSequenceMeta.ErrorMessage.NoConnectionDefined"));
+ retval.setSql(null); // Empty string means: nothing to do: set it to
null...
}
- }
- } catch (HopException e) {
- if (retval != null) {
+ } else {
retval.setError(
- BaseMessages.getString(PKG,
"AddSequenceMeta.ErrorMessage.UnableToConnectDB")
- + Const.CR
- + e.getMessage());
+ BaseMessages.getString(PKG,
"AddSequenceMeta.ErrorMessage.NoConnectionDefined"));
}
+ } catch (HopException e) {
+ retval.setError(
+ BaseMessages.getString(PKG,
"AddSequenceMeta.ErrorMessage.UnableToConnectDB")
+ + Const.CR
+ + e.getMessage());
} finally {
if (db != null) {
db.close();
diff --git
a/plugins/transforms/addsequence/src/test/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMetaTest.java
b/plugins/transforms/addsequence/src/test/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMetaTest.java
index 4eda296b5d..ff3e87cf19 100644
---
a/plugins/transforms/addsequence/src/test/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMetaTest.java
+++
b/plugins/transforms/addsequence/src/test/java/org/apache/hop/pipeline/transforms/addsequence/AddSequenceMetaTest.java
@@ -22,13 +22,26 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.nullable;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.ICheckResult;
+import org.apache.hop.core.SqlStatement;
+import org.apache.hop.core.database.DatabaseMeta;
import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
+import org.apache.hop.metadata.api.IHopMetadataProvider;
+import org.apache.hop.metadata.api.IHopMetadataSerializer;
+import org.apache.hop.metadata.validation.ReferencedDatabaseConnectionChecker;
+import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.pipeline.transforms.loadsave.LoadSaveTester;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -148,4 +161,80 @@ class AddSequenceMetaTest {
assertEquals(meta.getConnection(), cloned.getConnection());
assertEquals(meta.getStartAt(), cloned.getStartAt());
}
+
+ /**
+ * Issue #8561. The default Add Sequence uses a counter and has no
connection. That must not be
+ * reported as a missing database connection, and verify must not try to
load one.
+ */
+ @Test
+ void counterSequenceDoesNotWarnAboutAMissingConnection() throws Exception {
+ AddSequenceMeta meta = new AddSequenceMeta();
+ meta.setDefault();
+ assertFalse(meta.isDatabaseConnectionUsed("connection"));
+
+ IHopMetadataProvider provider = metadataProviderThatCannotLoad();
+ List<ICheckResult> remarks =
+ ReferencedDatabaseConnectionChecker.checkObject(
+ meta, "Transform", "Add sequence", null, new Variables(),
provider);
+
+ assertTrue(remarks.isEmpty());
+
+ TransformMeta transformMeta = new TransformMeta("Add sequence", meta);
+ List<ICheckResult> checked = new ArrayList<>();
+ meta.check(
+ checked,
+ null,
+ transformMeta,
+ null,
+ new String[] {"Generate rows"},
+ new String[0],
+ null,
+ new Variables(),
+ provider);
+
+ assertEquals(1, checked.size());
+ assertEquals(ICheckResult.TYPE_RESULT_OK, checked.get(0).getType());
+
+ SqlStatement sql = meta.getSqlStatements(new Variables(), null,
transformMeta, null, provider);
+ assertNotNull(sql);
+ assertNull(sql.getSql());
+ assertFalse(sql.hasError());
+ }
+
+ /** Selecting a database sequence with no connection still has to be
reported. */
+ @Test
+ void databaseSequenceWithoutAConnectionIsReported() throws Exception {
+ AddSequenceMeta meta = new AddSequenceMeta();
+ meta.setDefault();
+ meta.setDatabaseUsed(true);
+ meta.setCounterUsed(false);
+ assertTrue(meta.isDatabaseConnectionUsed("connection"));
+
+ IHopMetadataProvider provider = metadataProviderThatCannotLoad();
+ List<ICheckResult> remarks =
+ ReferencedDatabaseConnectionChecker.checkObject(
+ meta, "Transform", "Add sequence", null, new Variables(),
provider);
+
+ assertEquals(1, remarks.size());
+ assertEquals(
+ ReferencedDatabaseConnectionChecker.ERROR_NOT_ASSIGNED,
remarks.get(0).getErrorCode());
+
+ TransformMeta transformMeta = new TransformMeta("Add sequence", meta);
+ SqlStatement sql = meta.getSqlStatements(new Variables(), null,
transformMeta, null, provider);
+ assertTrue(sql.hasError());
+ assertEquals(
+ BaseMessages.getString(
+ AddSequenceMeta.class,
"AddSequenceMeta.ErrorMessage.NoConnectionDefined"),
+ sql.getError());
+ }
+
+ @SuppressWarnings("unchecked")
+ private static IHopMetadataProvider metadataProviderThatCannotLoad() throws
HopException {
+ IHopMetadataProvider provider = mock(IHopMetadataProvider.class);
+ IHopMetadataSerializer<DatabaseMeta> serializer =
mock(IHopMetadataSerializer.class);
+ when(provider.getSerializer(DatabaseMeta.class)).thenReturn(serializer);
+ when(serializer.load(nullable(String.class)))
+ .thenThrow(new HopException("you need to specify the name of the
metadata object to load"));
+ return provider;
+ }
}
