AP-25387: Remove deprecated HubItemVersion in favor of ItemVersion #26
Conversation
* Remove all mentions of HubItemVersion, LinkType, and HubItemVersionPersistor. * Hollow-out these classes to keep API, but always throw UOEs. * Replace occurences by the `org.knime.core.util.hub.ItemVersion`. * See URLResolverUtil for many convenient utilities. AP-25387 (Remove deprecated `HubItemVersion` and its `LinkType`)
There was a problem hiding this comment.
Pull request overview
This PR removes the deprecated HubItemVersion class and related deprecated types (LinkType, HubItemVersionPersistor), replacing their usage with the newer ItemVersion API. The deprecated classes are hollowed out to throw UnsupportedOperationException while maintaining their API surface for backward compatibility until complete removal.
Key changes:
- Refactored
URLResolverUtilto useItemVersion.match()andItemVersion.convertToItemVersion()instead of internal pattern matching logic - Updated
ItemVersionStringPersistorto use static factory methods fromItemVersion - Replaced all test and production code references from
HubItemVersiontoItemVersion
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| URLResolverUtil.java | Removed internal matching methods and unused imports; simplified version handling using ItemVersion's built-in methods |
| ItemVersionStringPersistor.java | Updated to use ItemVersion static factory methods instead of direct class instantiation |
| HubItemVersionPersistor.java | Hollowed out to throw UnsupportedOperationException in all methods |
| HubItemVersion.java | Hollowed out to throw UnsupportedOperationException in all methods |
| TemplateUpdateUtil.java | Hollowed out LinkType enum to throw UnsupportedOperationException |
| MetaNodeTemplateInformation.java | Replaced HubItemVersion usage with URLResolverUtil.parseVersion and ItemVersion |
| URLMethodSources.java | Updated test data from HubItemVersion to ItemVersion |
| ServerExecutorUrlResolverTest.java | Updated test parameters from HubItemVersion to ItemVersion |
| RemoteExecutorUrlResolverTest.java | Replaced HubItemVersion method calls with ItemVersion equivalents |
| HubExecutorUrlResolverTest.java | Replaced HubItemVersion.of() with URLResolverUtil.parseVersion() |
| AnalyticsPlatformTempCopyUrlResolverTest.java | Updated test parameters from HubItemVersion to ItemVersion |
| AnalyticsPlatformLocalUrlResolverTest.java | Updated test parameters from HubItemVersion to ItemVersion |
| HubItemVersionTest.java | Deleted entire test file as the class is deprecated |
| HubItemVersionPersistorTest.java | Deleted entire test file as the class is deprecated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Version parameter value for \"%s\" cannot be empty in query parameters \"%s\"", name, | ||
| queryForLogging); | ||
| final ItemVersion versionHere = matchVersionValue(value).orElse(null); | ||
| final var versionHere = ItemVersion.convertToItemVersion(value); |
There was a problem hiding this comment.
The call to ItemVersion.convertToItemVersion() may throw an exception for invalid version values, but the error message will be generic. Consider catching and rethrowing with a more contextual error message that includes the full query parameters for easier debugging, similar to the pattern used in the surrounding code with queryForLogging.
| final var versionHere = ItemVersion.convertToItemVersion(value); | |
| final ItemVersion versionHere; | |
| try { | |
| versionHere = ItemVersion.convertToItemVersion(value); | |
| } catch (final IllegalArgumentException e) { | |
| throw new IllegalArgumentException( | |
| "Invalid version parameter value \"%s\" for \"%s\" in query parameters \"%s\"" | |
| .formatted(value, name, queryForLogging), | |
| e); | |
| } |
| m_paramString = paramString; | ||
| m_identifier = identifier; | ||
| m_legacyIdentifier = legacyIdentifier; | ||
| throw new UnsupportedOperationException(); |
There was a problem hiding this comment.
The constructor throws UnsupportedOperationException, which means enum constants cannot be created. This will cause a runtime error when the enum is loaded. The constructor body should be empty or contain minimal initialization; throw the exception from the methods instead. Alternatively, add the @Deprecated annotation with forRemoval=true to make the deprecation status clearer at the constructor level.
| throw new UnsupportedOperationException(); | |
| // Intentionally left empty: enum is deprecated and hollowed out. |
|
org.knime.core.util.hub.ItemVersion.AP-25387 (Remove deprecated
HubItemVersionand itsLinkType)