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 d29469e959 NullPointerException in hop/registerPipeline Rest API when
log_file flag is enabled, fixes #4677 (#7557)
d29469e959 is described below
commit d29469e959177816b54c54254a4111108cb7a99c
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Jul 17 16:01:18 2026 +0200
NullPointerException in hop/registerPipeline Rest API when log_file flag is
enabled, fixes #4677 (#7557)
---
.../hop/core/logging/SimpleLoggingObject.java | 17 ++-
.../hop/core/logging/SimpleLoggingObjectTest.java | 49 +++++++
.../org/apache/hop/www/AddPipelineServlet.java | 6 +-
.../java/org/apache/hop/www/BaseHttpServlet.java | 28 ++++
.../org/apache/hop/www/BaseWorkflowServlet.java | 16 +--
.../www/RegisterPipelineServletLogFileTest.java | 155 +++++++++++++++++++++
6 files changed, 254 insertions(+), 17 deletions(-)
diff --git
a/core/src/main/java/org/apache/hop/core/logging/SimpleLoggingObject.java
b/core/src/main/java/org/apache/hop/core/logging/SimpleLoggingObject.java
index 0dda734319..22cf5111dd 100644
--- a/core/src/main/java/org/apache/hop/core/logging/SimpleLoggingObject.java
+++ b/core/src/main/java/org/apache/hop/core/logging/SimpleLoggingObject.java
@@ -26,6 +26,7 @@ public class SimpleLoggingObject implements ILoggingObject {
private ILoggingObject parent;
private LogLevel logLevel = DefaultLogLevel.getLogLevel();
private String containerObjectId;
+ private String logChannelId;
private Date registrationDate;
private boolean gatheringMetrics;
private boolean forcingSeparateLogging;
@@ -96,9 +97,23 @@ public class SimpleLoggingObject implements ILoggingObject {
return null;
}
+ /**
+ * @return the log channel this object logs to, null when it does not have
one of its own
+ */
@Override
public String getLogChannelId() {
- return null;
+ return logChannelId;
+ }
+
+ /**
+ * Give this object a log channel of its own. Everything that logs with this
object as its parent
+ * is then found back through it, which is what writing the log of a child
to a file needs. See
+ * issue #4677.
+ *
+ * @param logChannelId the log channel id to set
+ */
+ public void setLogChannelId(String logChannelId) {
+ this.logChannelId = logChannelId;
}
@Override
diff --git
a/core/src/test/java/org/apache/hop/core/logging/SimpleLoggingObjectTest.java
b/core/src/test/java/org/apache/hop/core/logging/SimpleLoggingObjectTest.java
new file mode 100644
index 0000000000..9e981c9b33
--- /dev/null
+++
b/core/src/test/java/org/apache/hop/core/logging/SimpleLoggingObjectTest.java
@@ -0,0 +1,49 @@
+/*
+ * 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.core.logging;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.junit.jupiter.api.Test;
+
+class SimpleLoggingObjectTest {
+
+ /** Not every logging object logs itself, so it does not have to have a log
channel. */
+ @Test
+ void withoutALogChannelOfItsOwnItReportsNone() {
+ SimpleLoggingObject loggingObject =
+ new SimpleLoggingObject("a-servlet", LoggingObjectType.HOP_SERVER,
null);
+
+ assertNull(loggingObject.getLogChannelId());
+ }
+
+ /**
+ * A logging object that is the parent of what a server runs needs a log
channel of its own: it is
+ * what a log file writer is hung on. See issue #4677.
+ */
+ @Test
+ void itReportsTheLogChannelItWasGiven() {
+ SimpleLoggingObject loggingObject =
+ new SimpleLoggingObject("a-servlet", LoggingObjectType.HOP_SERVER,
null);
+
+ loggingObject.setLogChannelId("a-log-channel");
+
+ assertEquals("a-log-channel", loggingObject.getLogChannelId());
+ }
+}
diff --git a/engine/src/main/java/org/apache/hop/www/AddPipelineServlet.java
b/engine/src/main/java/org/apache/hop/www/AddPipelineServlet.java
index 008cc348f4..abdefba115 100644
--- a/engine/src/main/java/org/apache/hop/www/AddPipelineServlet.java
+++ b/engine/src/main/java/org/apache/hop/www/AddPipelineServlet.java
@@ -29,7 +29,6 @@ import org.apache.hop.core.Const;
import org.apache.hop.core.annotations.HopServerServlet;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.logging.LogChannelFileWriter;
-import org.apache.hop.core.logging.LoggingObjectType;
import org.apache.hop.core.logging.SimpleLoggingObject;
import org.apache.hop.core.util.FileUtil;
import org.apache.hop.core.vfs.HopVfs;
@@ -123,9 +122,8 @@ public class AddPipelineServlet extends BaseHttpServlet
implements IHopServerPlu
String serverObjectId = UUID.randomUUID().toString();
SimpleLoggingObject servletLoggingObject =
- new SimpleLoggingObject(CONTEXT_PATH, LoggingObjectType.HOP_SERVER,
null);
- servletLoggingObject.setContainerObjectId(serverObjectId);
-
servletLoggingObject.setLogLevel(pipelineExecutionConfiguration.getLogLevel());
+ getServletLogging(
+ CONTEXT_PATH, serverObjectId,
pipelineExecutionConfiguration.getLogLevel());
IHopMetadataProvider metadataProvider =
new MultiMetadataProvider(
diff --git a/engine/src/main/java/org/apache/hop/www/BaseHttpServlet.java
b/engine/src/main/java/org/apache/hop/www/BaseHttpServlet.java
index 85650eee28..f7f969d501 100644
--- a/engine/src/main/java/org/apache/hop/www/BaseHttpServlet.java
+++ b/engine/src/main/java/org/apache/hop/www/BaseHttpServlet.java
@@ -33,6 +33,10 @@ import org.apache.hc.core5.http.ContentType;
import org.apache.hop.core.Const;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.logging.LogLevel;
+import org.apache.hop.core.logging.LoggingObjectType;
+import org.apache.hop.core.logging.LoggingRegistry;
+import org.apache.hop.core.logging.SimpleLoggingObject;
import org.apache.hop.core.variables.IVariables;
import org.apache.hop.core.variables.Variables;
import org.apache.hop.core.xml.XmlHandler;
@@ -378,4 +382,28 @@ public class BaseHttpServlet extends HttpServlet {
}
return null;
}
+
+ /**
+ * The logging object a servlet hands to the pipeline or workflow it
creates, as its logging
+ * parent.
+ *
+ * <p>It is given a log channel of its own: everything the pipeline or
workflow logs is a child of
+ * it, so writing that log to a file hangs the file writer on this log
channel. Without one there
+ * is nothing to hang it on. See issue #4677.
+ *
+ * @param contextPath the path of the servlet, to name the logging object
after
+ * @param serverObjectId the id this server knows the pipeline or workflow by
+ * @param level the log level that was asked for
+ * @return the logging object to use as a parent
+ */
+ protected SimpleLoggingObject getServletLogging(
+ final String contextPath, final String serverObjectId, final LogLevel
level) {
+ SimpleLoggingObject servletLoggingObject =
+ new SimpleLoggingObject(contextPath, LoggingObjectType.HOP_SERVER,
null);
+ servletLoggingObject.setContainerObjectId(serverObjectId);
+ servletLoggingObject.setLogLevel(level);
+ servletLoggingObject.setLogChannelId(
+
LoggingRegistry.getInstance().registerLoggingSource(servletLoggingObject,
true));
+ return servletLoggingObject;
+ }
}
diff --git a/engine/src/main/java/org/apache/hop/www/BaseWorkflowServlet.java
b/engine/src/main/java/org/apache/hop/www/BaseWorkflowServlet.java
index 603d1a022e..79470adbe0 100644
--- a/engine/src/main/java/org/apache/hop/www/BaseWorkflowServlet.java
+++ b/engine/src/main/java/org/apache/hop/www/BaseWorkflowServlet.java
@@ -23,8 +23,6 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.hop.core.Const;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.logging.LogChannelFileWriter;
-import org.apache.hop.core.logging.LogLevel;
-import org.apache.hop.core.logging.LoggingObjectType;
import org.apache.hop.core.logging.SimpleLoggingObject;
import org.apache.hop.core.parameters.INamedParameters;
import org.apache.hop.core.parameters.UnknownParamException;
@@ -64,7 +62,8 @@ public abstract class BaseWorkflowServlet extends
BodyHttpServlet {
String serverObjectId = UUID.randomUUID().toString();
SimpleLoggingObject servletLoggingObject =
- getServletLogging(serverObjectId,
workflowExecutionConfiguration.getLogLevel());
+ getServletLogging(
+ getContextPath(), serverObjectId,
workflowExecutionConfiguration.getLogLevel());
// Create the workflow and store in the list...
//
@@ -114,7 +113,8 @@ public abstract class BaseWorkflowServlet extends
BodyHttpServlet {
String serverObjectId = UUID.randomUUID().toString();
SimpleLoggingObject servletLoggingObject =
- getServletLogging(serverObjectId,
pipelineExecutionConfiguration.getLogLevel());
+ getServletLogging(
+ getContextPath(), serverObjectId,
pipelineExecutionConfiguration.getLogLevel());
// Create the pipeline and store in the list...
//
@@ -192,12 +192,4 @@ public abstract class BaseWorkflowServlet extends
BodyHttpServlet {
}
workflow.activateParameters(workflow);
}
-
- private SimpleLoggingObject getServletLogging(final String serverObjectId,
final LogLevel level) {
- SimpleLoggingObject servletLoggingObject =
- new SimpleLoggingObject(getContextPath(),
LoggingObjectType.HOP_SERVER, null);
- servletLoggingObject.setContainerObjectId(serverObjectId);
- servletLoggingObject.setLogLevel(level);
- return servletLoggingObject;
- }
}
diff --git
a/engine/src/test/java/org/apache/hop/www/RegisterPipelineServletLogFileTest.java
b/engine/src/test/java/org/apache/hop/www/RegisterPipelineServletLogFileTest.java
new file mode 100644
index 0000000000..7e3a25bc5d
--- /dev/null
+++
b/engine/src/test/java/org/apache/hop/www/RegisterPipelineServletLogFileTest.java
@@ -0,0 +1,155 @@
+/*
+ * 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.www;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.logging.LoggingRegistry;
+import org.apache.hop.core.metadata.SerializableMetadataProvider;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.apache.hop.metadata.serializer.multi.MultiMetadataProvider;
+import org.apache.hop.pipeline.PipelineConfiguration;
+import org.apache.hop.pipeline.PipelineExecutionConfiguration;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.config.PipelineRunConfiguration;
+import org.apache.hop.pipeline.engine.IPipelineEngine;
+import org.apache.hop.pipeline.engines.local.LocalPipelineRunConfiguration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Registering a pipeline with a log file used to fail with a
NullPointerException, because the
+ * logging object of the servlet had no log channel to hang the log file
writer on. See issue #4677.
+ */
+class RegisterPipelineServletLogFileTest {
+
+ private static final String RUN_CONFIGURATION_NAME = "local";
+
+ private Path logFile;
+
+ @BeforeAll
+ static void setUpBeforeClass() throws HopException {
+ HopEnvironment.init();
+ }
+
+ @BeforeEach
+ void setUp() throws Exception {
+ logFile = Files.createTempFile("register-pipeline-servlet", ".log");
+ Files.delete(logFile);
+ }
+
+ @AfterEach
+ void tearDown() throws Exception {
+ Files.deleteIfExists(logFile);
+ }
+
+ @Test
+ void registeringAPipelineWithALogFileWorks() throws Exception {
+ RegisterPipelineServlet servlet = createServlet();
+
+ IPipelineEngine<PipelineMeta> pipeline =
+ assertDoesNotThrow(() ->
servlet.createPipeline(createPipelineConfiguration()));
+
+ assertNotNull(
+ pipeline.getParent().getLogChannelId(),
+ "The pipeline should log to a channel the log file writer can be found
by");
+ }
+
+ /**
+ * Everything the pipeline logs has the logging object of the servlet as its
parent, so the log
+ * file writer registered on it has to be found back from the log channel of
the pipeline. Without
+ * that the log file stays empty.
+ */
+ @Test
+ void theLogFileWriterIsFoundFromTheLogChannelOfThePipeline() throws
Exception {
+ RegisterPipelineServlet servlet = createServlet();
+
+ IPipelineEngine<PipelineMeta> pipeline =
servlet.createPipeline(createPipelineConfiguration());
+
+ // This is what the server does next: it is only then that the pipeline
creates the log channel
+ // it really runs with.
+ pipeline.prepareExecution();
+
+ assertNotNull(
+ LoggingRegistry.getInstance()
+
.getLogChannelFileWriterBuffer(pipeline.getLogChannel().getLogChannelId()),
+ "The pipeline should write its log to the log file that was asked
for");
+ }
+
+ /** Registering a pipeline without a log file keeps working. */
+ @Test
+ void registeringAPipelineWithoutALogFileWorks() throws Exception {
+ RegisterPipelineServlet servlet = createServlet();
+ PipelineConfiguration pipelineConfiguration =
createPipelineConfiguration();
+
pipelineConfiguration.getPipelineExecutionConfiguration().setSetLogfile(false);
+
+ IPipelineEngine<PipelineMeta> pipeline =
+ assertDoesNotThrow(() ->
servlet.createPipeline(pipelineConfiguration));
+
+ assertNotNull(pipeline);
+ }
+
+ private RegisterPipelineServlet createServlet() {
+ HopServerConfig serverConfig = new HopServerConfig();
+ serverConfig.setMetadataProvider(new MultiMetadataProvider(new
Variables()));
+
+ PipelineMap pipelineMap = new PipelineMap();
+ pipelineMap.setHopServerConfig(serverConfig);
+
+ RegisterPipelineServlet servlet = new RegisterPipelineServlet();
+ servlet.setup(pipelineMap, new WorkflowMap());
+ return servlet;
+ }
+
+ private PipelineConfiguration createPipelineConfiguration() throws
HopException {
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.setName("a-pipeline");
+
+ PipelineExecutionConfiguration executionConfiguration = new
PipelineExecutionConfiguration();
+ executionConfiguration.setRunConfiguration(RUN_CONFIGURATION_NAME);
+ executionConfiguration.setSetLogfile(true);
+ executionConfiguration.setLogFileName(logFile.toString());
+
+ MemoryMetadataProvider metadataProvider = new MemoryMetadataProvider();
+ LocalPipelineRunConfiguration engineConfiguration = new
LocalPipelineRunConfiguration();
+ engineConfiguration.setEnginePluginId("Local");
+ metadataProvider
+ .getSerializer(PipelineRunConfiguration.class)
+ .save(
+ new PipelineRunConfiguration(
+ RUN_CONFIGURATION_NAME,
+ "",
+ null,
+ new ArrayList<>(),
+ engineConfiguration,
+ null,
+ false));
+
+ return new PipelineConfiguration(
+ pipelineMeta, executionConfiguration, new
SerializableMetadataProvider(metadataProvider));
+ }
+}
