Skip to content

AP-25387: Replaces all usages of deprecated HubItemVersion with ItemVersion #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
3nol wants to merge 1 commit into
base: master
Choose a base branch
from
Open

AP-25387: Replaces all usages of deprecated HubItemVersion with ItemVersion #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion org.knime.ui.java/src/eclipse/org/knime/ui/java/api/ComponentAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static void openChangeComponentHubItemVersionDialog(final String projectId, fina

final var component = assertIsWritableAndGetComponent(projectId, nodeId);
final var wfKey = getWorkflowKey(projectId, rootWorkflowId);
ManipulateComponents.openChangeComponentHubItemVersionDialog(component, wfKey);
ManipulateComponents.openChangeComponentItemVersionDialog(component, wfKey);
}

/**
Expand Down
32 changes: 24 additions & 8 deletions org.knime.ui.java/src/eclipse/org/knime/ui/java/api/ManipulateComponents.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,18 @@
package org.knime.ui.java.api;

import java.net.URI;
import java.net.URISyntaxException;
import java.time.Instant;
import java.util.Objects;
import java.util.function.BiPredicate;

import org.apache.hc.core5.net.URIBuilder;
import org.knime.core.node.workflow.MetaNodeTemplateInformation.Role;
import org.knime.core.node.workflow.NodeID;
import org.knime.core.node.workflow.SubNodeContainer;
import org.knime.core.ui.util.SWTUtilities;
import org.knime.core.util.hub.HubItemVersion;
import org.knime.core.util.hub.ItemVersion;
import org.knime.core.util.urlresolve.URLResolverUtil;
import org.knime.gateway.api.service.GatewayException;
import org.knime.gateway.api.webui.service.util.ServiceExceptions.OperationNotAllowedException;
import org.knime.gateway.api.webui.service.util.ServiceExceptions.ServiceCallException;
Expand All @@ -84,14 +87,13 @@ private ManipulateComponents() {
* This will not be callable from the FE until NXT-2038 is solved.
* @throws GatewayException
*/
static void openChangeComponentHubItemVersionDialog(final SubNodeContainer component, final WorkflowKey wfKey)
static void openChangeComponentItemVersionDialog(final SubNodeContainer component, final WorkflowKey wfKey)
throws GatewayException {
assertLinkedComponent(component, true);

// WorkflowEntityFactory#isHubItemVersionChangeable disables the action for non-Hub items,
// so we assume that the call is fine


final var shell = SWTUtilities.getActiveShell();
final var wfm = component.getParent();
final var dialog = new ChangeComponentHubVersionDialog(shell, component, wfm);
Expand All @@ -100,19 +102,33 @@ static void openChangeComponentHubItemVersionDialog(final SubNodeContainer compo
}

final var srcUri = component.getTemplateInformation().getSourceURI();
final var currentVersion = HubItemVersion.of(srcUri).orElse(HubItemVersion.currentState());
final var currentVersion = URLResolverUtil.parseVersion(srcUri.getQuery()).orElseGet(ItemVersion::currentState);
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The original code used orElse(HubItemVersion.currentState()) which evaluates the default eagerly, while the new code uses orElseGet(ItemVersion::currentState) which evaluates lazily. If ItemVersion.currentState() has side effects or is computationally expensive, this change in evaluation strategy could alter behavior. Consider whether this semantic change is intentional.

Copilot uses AI. Check for mistakes.
final var targetVersion = dialog.getSelectedVersion();
if (Objects.equals(targetVersion, currentVersion)) {
return;
}

final var newSrcUri = targetVersion.applyTo(srcUri);
final var newSrcUriBuilder = new URIBuilder(srcUri);
final var newQueryParams = URLResolverUtil.applyTo(targetVersion, newSrcUriBuilder.getQueryParams());
newSrcUriBuilder.setParameters(newQueryParams);
Comment on lines +111 to +113
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The query parameters are retrieved, modified, and set back in separate steps. Consider checking if URLResolverUtil.applyTo() could be simplified or if this pattern could be encapsulated in a helper method to reduce the cognitive load of understanding the URI modification flow.

Copilot uses AI. Check for mistakes.
final var workflowMiddleware = DesktopAPI.getDeps(WorkflowMiddleware.class);
final var cmd = workflowMiddleware.getCommands();
cmd.setCommandToExecute(getChangeSubNodeLinkCommand(component, srcUri, newSrcUri, true));
cmd.execute(wfKey, null);

// ChangeComponentHubVersionCommand does not check canExecute of the actual update command
// (1) Change URI to new version.
try {
cmd.setCommandToExecute(getChangeSubNodeLinkCommand(component, srcUri, newSrcUriBuilder.build(), true));
cmd.execute(wfKey, null);
} catch (URISyntaxException e) {
throw ServiceCallException.builder() //
.withTitle("Unable to construct new URI for component") //
.withDetails("The component %s is linked to <%s>, ".formatted(component.getNameWithID(), srcUri) //
+ "from which it is currently impossible to construct a new, versioned URI.") //
.canCopy(true) //
.withCause(e) //
.build();
}

// (2) Perform component update to fetch its info.
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The comment references step '(2)' but there's no corresponding comment for step '(1)' visible in the unchanged code context. While step '(1)' appears at line 117, the numbering should be consistent throughout the method to avoid confusion.

Copilot uses AI. Check for mistakes.
cmd.setCommandToExecute(getUpdateComponentCommand(component));
try {
cmd.execute(wfKey, null);
Expand Down
Loading