This is an automated email from the ASF dual-hosted git repository.
hansva 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 99e734fc72 Fix pipelines going to waiting state on critical fail,
fixes #3861 (#8123)
99e734fc72 is described below
commit 99e734fc729c28767b400ac7bbf2d30518d336b3
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Thu Aug 27 15:30:01 2026 +0200
Fix pipelines going to waiting state on critical fail, fixes #3861 (#8123)
---
.../java/org/apache/hop/pipeline/Pipeline.java | 41 +++++++-
.../engines/local/LocalPipelineEngine.java | 2 +
.../pipeline/messages/messages_en_US.properties | 1 +
.../java/org/apache/hop/pipeline/PipelineTest.java | 49 ++++++++++
.../engines/local/LocalPipelineEngineTest.java | 62 +++++++++++++
.../hop/avro/transforms/avrooutput/AvroOutput.java | 32 +++----
.../avro/transforms/avrooutput/AvroOutputMeta.java | 8 +-
.../avrooutput/AvroOutputMetaOutputTypeTest.java | 103 +++++++++++++++++++++
.../transforms/cubeinput/CubeInputMeta.java | 4 +
.../cubeinput/messages/messages_en_US.properties | 1 +
.../transforms/cubeinput/CubeInputMetaTest.java | 20 ++++
.../transforms/jdbcmetadata/JdbcMetadataMeta.java | 10 +-
.../jdbcmetadata/JdbcMetadataMetaTest.java | 27 ++++++
13 files changed, 336 insertions(+), 24 deletions(-)
diff --git a/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
b/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
index 0306757303..cd8ba66612 100644
--- a/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
+++ b/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
@@ -583,6 +583,10 @@ public abstract class Pipeline
try {
prepareExecutionInternal();
prepared = true;
+ } catch (Throwable e) {
+ // Still on the bound namespace: bringing the pipeline down can touch
its files.
+ flagPreparationFailure(e);
+ throw e;
} finally {
HopVfsNamespaces.restoreThread(previous);
if (!prepared) {
@@ -602,6 +606,31 @@ public abstract class Pipeline
}
}
+ /**
+ * A pipeline whose preparation failed never runs, and never reaches a
terminal state by itself:
+ * it is left flagged as preparing or initializing. A Hop server keeps such
an object in its map
+ * forever, because the timer that purges stale objects only collects the
ones that are finished
+ * or stopped. Stop the pipeline and record the error so it is reported as a
failure and can be
+ * cleaned up. See issue #3861.
+ */
+ protected void flagPreparationFailure(Throwable e) {
+ errors.incrementAndGet();
+ // Nothing else logs this: the exception travels up to whoever asked for
the execution, which
+ // on a server is an HTTP reply that the pipeline's own log never sees.
Report it the way a
+ // pipeline reports any other error, so it shows up wherever the log does.
+ if (e != null) {
+ log.logError(
+ BaseMessages.getString(PKG, "Pipeline.Log.ErrorPreparingPipeline",
e.getMessage()), e);
+ }
+ if (isFinished() || isStopped()) {
+ // An inner failure handler already brought the pipeline to a terminal
state.
+ return;
+ }
+ // Stopping is the terminal state this pipeline can still reach through
the normal path: it
+ // releases whatever was already initialized and alerts the execution
stopped listeners.
+ stopAll();
+ }
+
private void prepareExecutionInternal() throws HopException {
setPreparing(true);
executionStartDate = new Date();
@@ -1846,14 +1875,20 @@ public abstract class Pipeline
/** Stops all transforms from running, and alerts any registered listeners.
*/
@Override
public void stopAll() {
- if (transforms == null || isAlreadyStopped.get()) {
+ if (isAlreadyStopped.get()) {
return;
}
- transforms.forEach(combi -> stopTransform(combi, false));
+ // A pipeline that never got as far as allocating its transforms can still
be stopped: it just
+ // has nothing to stop. Bailing out here used to leave it without a
terminal state.
+ if (transforms != null) {
+ transforms.forEach(combi -> stopTransform(combi, false));
+ }
- // if it is stopped it is not paused
+ // if it is stopped it is not paused, nor is it still preparing or
initializing
setPaused(false);
+ setPreparing(false);
+ setInitializing(false);
setStopped(true);
isAlreadyStopped.set(true);
diff --git
a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java
b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java
index 1ebefcc73b..fc24bcb602 100644
---
a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java
+++
b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java
@@ -341,6 +341,8 @@ public class LocalPipelineEngine extends Pipeline
implements IPipelineEngine<Pip
// Flag the pipeline as finished even if an exception was thrown
setFinished(true);
}
+ // Record the failure so the pipeline is reported as failed rather than
as a clean finish.
+ flagPreparationFailure(e);
throw e;
}
}
diff --git
a/engine/src/main/resources/org/apache/hop/pipeline/messages/messages_en_US.properties
b/engine/src/main/resources/org/apache/hop/pipeline/messages/messages_en_US.properties
index 650c1c291f..42fb37bcd0 100644
---
a/engine/src/main/resources/org/apache/hop/pipeline/messages/messages_en_US.properties
+++
b/engine/src/main/resources/org/apache/hop/pipeline/messages/messages_en_US.properties
@@ -52,6 +52,7 @@ Pipeline.Log.AllocatingRowsets=Allocating rowsets...
Pipeline.Log.AllocatingTransformsAndTransformData=Allocating Transforms &
TransformData...
Pipeline.Log.copiesInfo=\ prevcopies \= {0}, nextcopies\={1}
Pipeline.Log.ErrorInitializingTransform=Error initializing transform [{0}]
+Pipeline.Log.ErrorPreparingPipeline=The pipeline could not be prepared for
execution\: {0}
Pipeline.Log.ExecutionStartedForFilename=Execution started for filename [{0}]
Pipeline.Log.ExecutionStartedForPipeline=Execution started for pipeline [{0}]
Pipeline.Log.FailToInitializeAtLeastOneTransform=We failed to initialize at
least one transform. Execution can not begin\!
diff --git a/engine/src/test/java/org/apache/hop/pipeline/PipelineTest.java
b/engine/src/test/java/org/apache/hop/pipeline/PipelineTest.java
index 3b60c41f4e..4027a1fe51 100644
--- a/engine/src/test/java/org/apache/hop/pipeline/PipelineTest.java
+++ b/engine/src/test/java/org/apache/hop/pipeline/PipelineTest.java
@@ -18,7 +18,9 @@
package org.apache.hop.pipeline;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
@@ -339,4 +341,51 @@ class PipelineTest {
"Original value defined at run execution",
pipelineTest.getVariable(Const.INTERNAL_VARIABLE_ENTRY_CURRENT_FOLDER));
}
+
+ /**
+ * Issue #3861: a pipeline that never got as far as allocating its
transforms could not be stopped
+ * at all, so it kept whatever non-terminal state it was in. Nothing ever
cleaned it up.
+ */
+ @Test
+ void stopAllWorksBeforeTheTransformsAreAllocated() {
+ Pipeline pipeline = new LocalPipelineEngine(new PipelineMeta());
+ pipeline.setLogChannel(mock(ILogChannel.class));
+ pipeline.setPreparing(true);
+
+ pipeline.stopAll();
+
+ assertTrue(pipeline.isStopped());
+ assertEquals(Pipeline.STRING_STOPPED, pipeline.getStatusDescription());
+ }
+
+ /**
+ * A stopped pipeline is no longer preparing or initializing. The Hop GUI
reads those flags to
+ * decide whether a pipeline is still on the go, so they may not survive a
stop.
+ */
+ @Test
+ void stopAllClearsTheTransientExecutionFlags() {
+ Pipeline pipeline = new LocalPipelineEngine(new PipelineMeta());
+ pipeline.setLogChannel(mock(ILogChannel.class));
+ pipeline.setPreparing(true);
+ pipeline.setInitializing(true);
+
+ pipeline.stopAll();
+
+ assertFalse(pipeline.isPreparing());
+ assertFalse(pipeline.isInitializing());
+ }
+
+ /** Stopping stays a one-shot operation. */
+ @Test
+ void stopAllOnlyFiresTheStoppedListenersOnce() {
+ Pipeline pipeline = new LocalPipelineEngine(new PipelineMeta());
+ pipeline.setLogChannel(mock(ILogChannel.class));
+ int[] stopped = new int[1];
+ pipeline.addExecutionStoppedListener(p -> stopped[0]++);
+
+ pipeline.stopAll();
+ pipeline.stopAll();
+
+ assertEquals(1, stopped[0]);
+ }
}
diff --git
a/engine/src/test/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngineTest.java
b/engine/src/test/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngineTest.java
index 94e6e77e18..b7b6c4408e 100644
---
a/engine/src/test/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngineTest.java
+++
b/engine/src/test/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngineTest.java
@@ -18,6 +18,7 @@
package org.apache.hop.pipeline.engines.local;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.doThrow;
@@ -27,8 +28,11 @@ import static org.mockito.Mockito.verify;
import org.apache.hop.core.HopEnvironment;
import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.logging.HopLogStore;
import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.variables.IVariables;
import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
+import org.apache.hop.pipeline.Pipeline;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.engine.EngineComponent.ComponentExecutionStatus;
import org.apache.hop.pipeline.transform.BaseTransformData;
@@ -71,6 +75,59 @@ class LocalPipelineEngineTest {
assertTrue(pipeline.isFinished(), "The pipeline should be flagged as
finished");
}
+ /**
+ * Issue #3861: a pipeline that fails after preparation used to be reported
as a clean finish,
+ * with nothing on the status page to say why it never produced a row.
+ */
+ @Test
+ void preparationFailureAfterInitIsReportedAsAnError() {
+ LocalPipelineEngine pipeline =
+ new LocalPipelineEngine(new PipelineMeta()) {
+ @Override
+ public void addTransformExecutionSamplers() throws HopException {
+ throw new HopException("Unable to attach the samplers");
+ }
+ };
+ pipeline.setLogChannel(mock(ILogChannel.class));
+
+ assertThrows(HopException.class, pipeline::prepareExecution);
+
+ assertTrue(pipeline.getErrors() > 0, "The failure should be counted as an
error");
+ assertTrue(
+ pipeline.getStatusDescription().contains("with errors"),
pipeline.getStatusDescription());
+ assertTrue(
+ logOf(pipeline).contains("Unable to attach the samplers"),
+ "The failure should be in the pipeline log: " + logOf(pipeline));
+ }
+
+ /**
+ * Issue #3861: when preparation fails part way through, the pipeline is
left flagged as preparing
+ * and so never reaches a terminal state. A Hop server keeps such an object
in its map forever,
+ * because the timer that purges stale objects only collects finished or
stopped ones.
+ */
+ @Test
+ void preparationFailureLeavesThePipelineInATerminalState() {
+ LocalPipelineEngine pipeline =
+ new LocalPipelineEngine(new PipelineMeta()) {
+ @Override
+ public void activateParameters(IVariables variables) {
+ throw new IllegalStateException("Preparation blew up half way
through");
+ }
+ };
+ pipeline.setLogChannel(mock(ILogChannel.class));
+
+ assertThrows(IllegalStateException.class, pipeline::prepareExecution);
+
+ assertFalse(pipeline.isPreparing(), "A failed preparation may not stay
flagged as preparing");
+ assertFalse(
+ pipeline.isInitializing(), "A failed preparation may not stay flagged
as initializing");
+ assertTrue(pipeline.isStopped(), "A failed preparation should leave a
terminal state");
+ assertEquals(Pipeline.STRING_STOPPED, pipeline.getStatusDescription());
+ assertTrue(
+ logOf(pipeline).contains("Preparation blew up half way through"),
+ "The failure should be in the pipeline log: " + logOf(pipeline));
+ }
+
@Test
void disposeInitializedTransformsDisposesEveryTransform() throws Exception {
LocalPipelineEngine pipeline = new LocalPipelineEngine(new PipelineMeta());
@@ -118,4 +175,9 @@ class LocalPipelineEngineTest {
combi.data = data;
return combi;
}
+
+ /** The pipeline log as a Hop server would serve it on the status page. */
+ private static String logOf(LocalPipelineEngine pipeline) {
+ return HopLogStore.getAppender().getBuffer(pipeline.getLogChannelId(),
false).toString();
+ }
}
diff --git
a/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutput.java
b/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutput.java
index 776aff7572..2a98dc0b82 100644
---
a/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutput.java
+++
b/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutput.java
@@ -342,11 +342,11 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
GenericRecord row = getRecord(r, null, data.avroSchema);
try {
- if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE])) {
+ if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE].equals(
+ meta.getOutputType())) {
data.dataFileWriter.append(row);
- } else if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD])) {
+ } else if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD].equals(
+ meta.getOutputType())) {
data.datumWriter.write(row, data.binaryEncoder);
data.binaryEncoder.flush();
data.byteArrayOutputStream.flush();
@@ -354,8 +354,8 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
r, data.outputRowMeta.size() - 1,
data.byteArrayOutputStream.toByteArray());
data.byteArrayOutputStream.close();
data.byteArrayOutputStream.reset();
- } else if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_JSON_FIELD])) {
+ } else if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_JSON_FIELD].equals(
+ meta.getOutputType())) {
data.datumWriter.write(row, data.jsonEncoder);
data.jsonEncoder.flush();
data.byteArrayOutputStream.flush();
@@ -399,13 +399,13 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
}
data.datumWriter = new GenericDatumWriter<>(data.avroSchema);
- if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD])) {
+ if (AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD].equals(
+ meta.getOutputType())) {
data.encoderFactory = new EncoderFactory();
data.byteArrayOutputStream = new ByteArrayOutputStream();
data.binaryEncoder =
data.encoderFactory.binaryEncoder(data.byteArrayOutputStream, null);
- } else if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE])) {
+ } else if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE].equals(
+ meta.getOutputType())) {
data.dataFileWriter = new DataFileWriter<>(data.datumWriter);
if (!Utils.isEmpty(meta.getCompressionType())
&& !meta.getCompressionType().equalsIgnoreCase("none")) {
@@ -414,8 +414,8 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
openNewFile(meta.getFileName());
data.dataFileWriter.create(data.avroSchema, data.writer);
- } else if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_JSON_FIELD])) {
+ } else if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_JSON_FIELD].equals(
+ meta.getOutputType())) {
data.encoderFactory = new EncoderFactory();
data.byteArrayOutputStream = new ByteArrayOutputStream();
data.jsonEncoder =
@@ -429,8 +429,8 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
}
private void closeOutput() throws HopException {
- if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD])) {
+ if (AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_FIELD].equals(
+ meta.getOutputType())) {
try {
data.binaryEncoder = null;
data.jsonEncoder = null;
@@ -441,8 +441,8 @@ public class AvroOutput extends
BaseTransform<AvroOutputMeta, AvroOutputData> {
} catch (Exception ex) {
throw new HopException("Error cleaning up transform", ex);
}
- } else if (meta.getOutputType()
-
.equals(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE])) {
+ } else if
(AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE].equals(
+ meta.getOutputType())) {
closeFile();
}
data.datumWriter = null;
diff --git
a/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMeta.java
b/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMeta.java
index ac4e97968a..e991b9d1a5 100644
---
a/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMeta.java
+++
b/plugins/tech/avro/src/main/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMeta.java
@@ -163,7 +163,7 @@ public class AvroOutputMeta extends
BaseTransformMeta<AvroOutput, AvroOutputData
@HopMetadataProperty(
key = "output_type",
injectionKeyDescription = "AvroOutput.Injection.OUTPUT_TYPE")
- private String outputType;
+ private String outputType = OUTPUT_TYPES[OUTPUT_TYPE_BINARY_FILE];
@HopMetadataProperty(
key = "output_field_name",
@@ -330,7 +330,7 @@ public class AvroOutputMeta extends
BaseTransformMeta<AvroOutput, AvroOutputData
if (outputTypeId >= 0 && outputTypeId < OUTPUT_TYPES.length) {
this.outputType = OUTPUT_TYPES[outputTypeId];
} else {
- this.outputType = null;
+ this.outputType = OUTPUT_TYPES[OUTPUT_TYPE_BINARY_FILE];
}
}
@@ -561,11 +561,11 @@ public class AvroOutputMeta extends
BaseTransformMeta<AvroOutput, AvroOutputData
throws HopTransformException {
// change the case insensitive flag too
- if (outputType.equalsIgnoreCase(OUTPUT_TYPES[OUTPUT_TYPE_FIELD])) {
+ if (OUTPUT_TYPES[OUTPUT_TYPE_FIELD].equalsIgnoreCase(outputType)) {
IValueMeta v = new ValueMetaBinary(variables.resolve(outputFieldName));
v.setOrigin(origin);
row.addValueMeta(v);
- } else if
(outputType.equalsIgnoreCase(OUTPUT_TYPES[OUTPUT_TYPE_JSON_FIELD])) {
+ } else if
(OUTPUT_TYPES[OUTPUT_TYPE_JSON_FIELD].equalsIgnoreCase(outputType)) {
IValueMeta valueMetaInterface = new
ValueMetaString(variables.resolve(outputFieldName));
valueMetaInterface.setOrigin(origin);
row.addValueMeta(valueMetaInterface);
diff --git
a/plugins/tech/avro/src/test/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMetaOutputTypeTest.java
b/plugins/tech/avro/src/test/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMetaOutputTypeTest.java
new file mode 100644
index 0000000000..a284f923ca
--- /dev/null
+++
b/plugins/tech/avro/src/test/java/org/apache/hop/avro/transforms/avrooutput/AvroOutputMetaOutputTypeTest.java
@@ -0,0 +1,103 @@
+/*
+ * 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.avro.transforms.avrooutput;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.apache.hop.core.row.IRowMeta;
+import org.apache.hop.core.row.RowMeta;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.core.xml.XmlHandler;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.junit.jupiter.api.Test;
+import org.w3c.dom.Node;
+
+/**
+ * Issue #3861: an Avro File Output whose output type was never set took the
whole pipeline down
+ * with a NullPointerException while the pipeline was being prepared, which on
a Hop server left the
+ * pipeline stuck in the server's object list.
+ */
+class AvroOutputMetaOutputTypeTest {
+
+ /**
+ * setDefault() is only called by the GUI when a transform is dropped on the
canvas, so a file
+ * that carries no output_type tag has to fall back to the default on its
own.
+ */
+ @Test
+ void outputTypeDefaultsWithoutSetDefault() {
+ assertEquals(
+ AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE],
+ new AvroOutputMeta().getOutputType());
+ }
+
+ @Test
+ void outputTypeSurvivesLoadingATransformNodeWithoutTheTag() throws Exception
{
+ AvroOutputMeta meta = new AvroOutputMeta();
+ Node node =
+ XmlHandler.getSubNode(
+ XmlHandler.loadXmlString(
+
"<transform><name>avro</name><type>AvroOutput</type></transform>"),
+ "transform");
+ meta.loadXml(node, new MemoryMetadataProvider());
+
+ assertEquals(
+ AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE],
meta.getOutputType());
+ assertDoesNotThrow(() -> getFields(meta));
+ }
+
+ /** An unselected combo hands the dialog -1; that must not null the output
type out. */
+ @Test
+ void unknownOutputTypeIdFallsBackToTheDefault() {
+ AvroOutputMeta meta = new AvroOutputMeta();
+ meta.setOutputTypeById(-1);
+
+ assertEquals(
+ AvroOutputMeta.OUTPUT_TYPES[AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE],
meta.getOutputType());
+ assertEquals(AvroOutputMeta.OUTPUT_TYPE_BINARY_FILE,
meta.getOutputTypeId());
+ assertDoesNotThrow(() -> getFields(meta));
+ }
+
+ /** Even an explicitly nulled output type may not throw a raw
NullPointerException. */
+ @Test
+ void nullOutputTypeDoesNotThrow() {
+ AvroOutputMeta meta = new AvroOutputMeta();
+ meta.setOutputType(null);
+
+ assertDoesNotThrow(() -> getFields(meta));
+ }
+
+ /** The binary-field output type still contributes its field. */
+ @Test
+ void binaryFieldOutputTypeStillAddsTheField() throws Exception {
+ AvroOutputMeta meta = new AvroOutputMeta();
+ meta.setOutputTypeById(AvroOutputMeta.OUTPUT_TYPE_FIELD);
+ meta.setOutputFieldName("avro_record");
+
+ IRowMeta row = new RowMeta();
+ meta.getFields(row, "avro", null, null, new Variables(), new
MemoryMetadataProvider());
+
+ assertEquals(1, row.size());
+ assertEquals("avro_record", row.getValueMeta(0).getName());
+ }
+
+ private static void getFields(AvroOutputMeta meta) throws Exception {
+ meta.getFields(
+ new RowMeta(), "avro", null, null, new Variables(), new
MemoryMetadataProvider());
+ }
+}
diff --git
a/plugins/transforms/cubeinput/src/main/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMeta.java
b/plugins/transforms/cubeinput/src/main/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMeta.java
index b2f8d964f0..ea53a3c3b5 100644
---
a/plugins/transforms/cubeinput/src/main/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMeta.java
+++
b/plugins/transforms/cubeinput/src/main/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMeta.java
@@ -86,6 +86,10 @@ public class CubeInputMeta extends
BaseTransformMeta<CubeInput, CubeInputData> {
throws HopTransformException {
GZIPInputStream fis = null;
DataInputStream dis = null;
+ if (file == null || file.getName() == null) {
+ throw new HopTransformException(
+ BaseMessages.getString(PKG,
"CubeInputMeta.Exception.NoFilenameSpecified"));
+ }
try {
InputStream is =
HopVfs.getInputStream(
diff --git
a/plugins/transforms/cubeinput/src/main/resources/org/apache/hop/pipeline/transforms/cubeinput/messages/messages_en_US.properties
b/plugins/transforms/cubeinput/src/main/resources/org/apache/hop/pipeline/transforms/cubeinput/messages/messages_en_US.properties
index 7791db505c..acb694ea61 100644
---
a/plugins/transforms/cubeinput/src/main/resources/org/apache/hop/pipeline/transforms/cubeinput/messages/messages_en_US.properties
+++
b/plugins/transforms/cubeinput/src/main/resources/org/apache/hop/pipeline/transforms/cubeinput/messages/messages_en_US.properties
@@ -32,6 +32,7 @@ CubeInputDialog.Shell.Title=De-serialize from file
CubeInputDialog.TransformName.Label=Transform name
CubeInputMeta.CheckResult.FileSpecificationsNotChecked=File specifications are
not checked.
CubeInputMeta.Exception.ErrorOpeningOrReadingCubeFile=Error opening/reading
cube file
+CubeInputMeta.Exception.NoFilenameSpecified=No cube file name has been
specified for this transform
CubeInputMeta.Exception.UnableToCloseCubeFile=Unable to close cube file
CubeInputMeta.Exception.UnableToLoadTransformMeta=Unable to load transform
info from XML
CubeInputMeta.Exception.UnableToReadMetaData=Unable to read metadata from cube
file
diff --git
a/plugins/transforms/cubeinput/src/test/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMetaTest.java
b/plugins/transforms/cubeinput/src/test/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMetaTest.java
index 763749382f..361da84d8e 100644
---
a/plugins/transforms/cubeinput/src/test/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMetaTest.java
+++
b/plugins/transforms/cubeinput/src/test/java/org/apache/hop/pipeline/transforms/cubeinput/CubeInputMetaTest.java
@@ -18,7 +18,12 @@
package org.apache.hop.pipeline.transforms.cubeinput;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.apache.hop.core.exception.HopTransformException;
+import org.apache.hop.core.row.RowMeta;
+import org.apache.hop.core.variables.Variables;
import org.apache.hop.pipeline.transform.TransformSerializationTestUtil;
import org.junit.jupiter.api.Test;
@@ -32,4 +37,19 @@ class CubeInputMetaTest {
assertNotNull(meta.getFile());
assertNotNull(meta.getFile().getName());
}
+
+ /**
+ * Issue #3861: with no cube file configured, getFields() dereferenced the
file name and threw a
+ * raw NullPointerException out of prepareExecution instead of reporting the
misconfiguration.
+ */
+ @Test
+ void getFieldsWithoutFilenameReportsTheMisconfiguration() {
+ CubeInputMeta meta = new CubeInputMeta();
+
+ HopTransformException e =
+ assertThrows(
+ HopTransformException.class,
+ () -> meta.getFields(new RowMeta(), "cube input", null, null, new
Variables(), null));
+ assertTrue(e.getMessage().contains("cube file name"), e.getMessage());
+ }
}
diff --git
a/plugins/transforms/jdbc-metadata/src/main/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMeta.java
b/plugins/transforms/jdbc-metadata/src/main/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMeta.java
index 7254b3d817..e20c65088c 100644
---
a/plugins/transforms/jdbc-metadata/src/main/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMeta.java
+++
b/plugins/transforms/jdbc-metadata/src/main/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMeta.java
@@ -553,7 +553,7 @@ public class JdbcMetadataMeta extends
BaseTransformMeta<JdbcMetadata, JdbcMetada
/** Stores the selection of fields that are added to the stream */
@HopMetadataProperty(groupKey = "outputFields", key = "outputField")
- private List<OutputField> outputFields;
+ private List<OutputField> outputFields = new ArrayList<>();
/**
* @return the selection of fields added to the stream
@@ -597,6 +597,14 @@ public class JdbcMetadataMeta extends
BaseTransformMeta<JdbcMetadata, JdbcMetada
int n = outputFields.size();
Object[] methodDescriptor = getMethodDescriptor();
+ if (methodDescriptor == null) {
+ throw new HopTransformException(
+ "Unknown or missing JDBC metadata method '"
+ + getMethodName()
+ + "' in transform '"
+ + origin
+ + "'");
+ }
IValueMeta[] fields = (IValueMeta[]) methodDescriptor[2];
int m = fields.length;
IValueMeta field;
diff --git
a/plugins/transforms/jdbc-metadata/src/test/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMetaTest.java
b/plugins/transforms/jdbc-metadata/src/test/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMetaTest.java
index 3b855a0c24..8c28e58991 100644
---
a/plugins/transforms/jdbc-metadata/src/test/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMetaTest.java
+++
b/plugins/transforms/jdbc-metadata/src/test/java/org/apache/hop/pipeline/transforms/jdbcmetadata/JdbcMetadataMetaTest.java
@@ -19,12 +19,16 @@ package org.apache.hop.pipeline.transforms.jdbcmetadata;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.hop.core.HopClientEnvironment;
+import org.apache.hop.core.exception.HopTransformException;
+import org.apache.hop.core.row.RowMeta;
+import org.apache.hop.core.variables.Variables;
import org.apache.hop.core.xml.XmlHandler;
import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
import org.apache.hop.metadata.serializer.xml.XmlMetadataUtil;
@@ -123,6 +127,29 @@ class JdbcMetadataMetaTest {
assertTrue(copy.getArguments().isEmpty());
}
+ /**
+ * Issue #3861: the method name is only filled in by setDefault(), which the
GUI calls but loading
+ * a pipeline does not. A file without it left the method descriptor null
and getFields() threw a
+ * raw NullPointerException while the pipeline was being prepared.
+ */
+ @Test
+ void getFieldsWithoutMethodNameReportsTheMisconfiguration() {
+ JdbcMetadataMeta meta = new JdbcMetadataMeta();
+
+ HopTransformException e =
+ assertThrows(
+ HopTransformException.class,
+ () ->
+ meta.getFields(new RowMeta(), "jdbc metadata", null, null, new
Variables(), null));
+ assertTrue(e.getMessage().contains("jdbc metadata"), e.getMessage());
+ }
+
+ /** The output field list is allocated up front, so a file without one is
still usable. */
+ @Test
+ void outputFieldsAreNeverNull() {
+ assertNotNull(new JdbcMetadataMeta().getOutputFields());
+ }
+
private static JdbcMetadataMeta serializeAndDeserialize(JdbcMetadataMeta
source)
throws Exception {
String xml =
