Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* ------------------------------------------------------------------------
*
* Copyright by KNIME AG, Zurich, Switzerland
* Website: http://www.knime.com; Email: [email protected]
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License, Version 3, as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, see <http://www.gnu.org/licenses>.
*
* Additional permission under GNU GPL version 3 section 7:
*
* KNIME interoperates with ECLIPSE solely via ECLIPSE's plug-in APIs.
* Hence, KNIME and ECLIPSE are both independent programs and are not
* derived from each other. Should, however, the interpretation of the
* GNU GPL Version 3 ("License") under any applicable laws result in
* KNIME and ECLIPSE being a combined program, KNIME AG herewith grants
* you the additional permission to use and propagate KNIME together with
* ECLIPSE with only the license terms in place for ECLIPSE applying to
* ECLIPSE and the GNU GPL Version 3 applying for KNIME, provided the
* license terms of ECLIPSE themselves allow for the respective use and
* propagation of ECLIPSE together with KNIME.
* ---------------------------------------------------------------------
*
* History
* Dec 23, 2025 (assistant): created
*/
package org.knime.core.node.workflow;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.util.concurrent.atomic.AtomicInteger;

import org.junit.After;
import org.junit.Test;
import org.mockito.Mockito;

/**
* Unit tests for {@link WorkflowResourceCache}.
*/
@SuppressWarnings("resource") // caches disposed by tests as needed
public class WorkflowResourceCacheTest {

@After
public void tearDown() {
// clean up any lingering context to not leak across tests
while (NodeContext.getContext() != null) {
NodeContext.removeLastContext();
}
}

@Test
public void testComputeIfAbsentCachesPerWorkflow() {
var cache = new WorkflowResourceCache();
var wfm = Mockito.mock(WorkflowManager.class);
Mockito.when(wfm.getWorkflowResourceCache()).thenReturn(cache);

NodeContext.pushContext(wfm);

var calls = new AtomicInteger();
var first = WorkflowResourceCache.computeIfAbsent(TestResource.class, () -> {
calls.incrementAndGet();
return new TestResource();
});
var second = WorkflowResourceCache.computeIfAbsent(TestResource.class, TestResource::new);

assertSame(first, second);
assertEquals(1, calls.get());
}

@Test
public void testPutAndGetFromCache() {
var cache = new WorkflowResourceCache();

assertFalse(cache.getFromCache(TestResource.class).isPresent());

var first = new TestResource();
assertNull(cache.put(TestResource.class, first));
assertSame(first, cache.getFromCache(TestResource.class).orElseThrow());

var second = new TestResource();
assertSame(first, cache.put(TestResource.class, second));
assertSame(second, cache.getFromCache(TestResource.class).orElseThrow());
}

@Test
public void testDisposeClearsEntriesAndDisposesResources() {
var cache = new WorkflowResourceCache();
var resource = new DisposingResource();
cache.put(DisposingResource.class, resource);

cache.dispose();

assertTrue("dispose() should clear cached entries", cache.getFromCache(DisposingResource.class).isEmpty());
assertTrue("dispose() should call WorkflowResource.dispose()", resource.disposed);
}

private static final class TestResource implements WorkflowResource {
@Override
public void dispose() {
// no-op
}
}

private static final class DisposingResource implements WorkflowResource {
boolean disposed;

@Override
public void dispose() {
disposed = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ public enum Type {
/** Workflow is marked as clean (not dirty)
* @since 4.6 */
WORKFLOW_CLEAN,
/**
* The workflow has changed
*
* @since 5.10
*/
WORKFLOW_CHANGED,
/**
* Metadata of the currently open workflow (e.g. project or component) has changed
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@
import org.knime.core.node.workflow.WorkflowPersistor.NodeContainerTemplateLinkUpdateResult;
import org.knime.core.node.workflow.WorkflowPersistor.WorkflowLoadResult;
import org.knime.core.node.workflow.WorkflowPersistor.WorkflowPortTemplate;
import org.knime.core.node.workflow.WorkflowResourceCache;
import org.knime.core.node.workflow.WorkflowResourceCache.WorkflowResource;
import org.knime.core.node.workflow.action.CollapseIntoMetaNodeResult;
import org.knime.core.node.workflow.action.ExpandSubnodeResult;
Expand Down Expand Up @@ -9958,11 +9959,12 @@ void unsetDirty() {
/** {@inheritDoc} */
@Override
public void setDirty() {
boolean sendEvent = !isDirty();
boolean sendWorkflowDirtyEvent = !isDirty();
super.setDirty();
if (sendEvent) {
if (sendWorkflowDirtyEvent) {
notifyWorkflowListeners(new WorkflowEvent(WorkflowEvent.Type.WORKFLOW_DIRTY, getID(), null, null));
}
notifyWorkflowListeners(new WorkflowEvent(WorkflowEvent.Type.WORKFLOW_CHANGED, getID(), null, null));
Comment on lines 9965 to +9967
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WORKFLOW_CHANGED event is fired unconditionally on every setDirty() call, even when the workflow is already dirty. This could result in redundant event notifications for listeners. Consider firing this event only when sendWorkflowDirtyEvent is true, or document why unconditional firing is necessary for the browser sync functionality.

Suggested change
}
notifyWorkflowListeners(new WorkflowEvent(WorkflowEvent.Type.WORKFLOW_CHANGED, getID(), null, null));
notifyWorkflowListeners(new WorkflowEvent(WorkflowEvent.Type.WORKFLOW_CHANGED, getID(), null, null));
}

Copilot uses AI. Check for mistakes.
}

//////////////////////////////////////
Expand Down Expand Up @@ -11352,7 +11354,7 @@ public void setWorkflowContext(final WorkflowContextV2 newWorkflowContext) {
* @return for projects, the non-null {@link WorkflowResourceCache}. For other instances, null.
* @since 5.4
*/
WorkflowResourceCache getWorkflowResourceCache() {
public WorkflowResourceCache getWorkflowResourceCache() {
return m_workflowResourceCache;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,31 @@ public static synchronized <T extends WorkflowResource> T computeIfAbsent(final
});
}

/**
* Returns the resource instance for the given class if it is already present in the cache.
*
* @since 5.10
* @param <T> type of resource
* @param clazz the class used as key for the resource, must not be {@code null}
* @return an {@link Optional} containing the cached instance if present; otherwise {@link Optional#empty()}
*/
public synchronized <T extends WorkflowResource> Optional<T> getFromCache(final Class<T> clazz) {
CheckUtils.checkArgumentNotNull(clazz, "Class must not be null.");
return Optional.ofNullable((T) m_classToInstanceMap.get(clazz));
}

/**
* Registers a new workflow resource with this workflow. The resource is associated with the given class.
* An existing value is returned or {@code null} if the class was previously not associated with a resource.
*
* @since 5.10
* @param <T> Type of resource (extension dependent)
* @param clazz The class of {@code T}
* @param value The resource instance to register
* @return The previous value associated with the class or {@code null} if no such value existed
*/
@SuppressWarnings("unchecked")
synchronized <T extends WorkflowResource> T put(final Class<T> clazz, final T value) {
public synchronized <T extends WorkflowResource> T put(final Class<T> clazz, final T value) {
CheckUtils.checkArgumentNotNull(clazz, "Class must not be null.");
CheckUtils.checkArgumentNotNull(value, "Value must not be null.");
CheckUtils.checkState(!m_isDisposed, "Cache is disposed.");
Expand Down