UIEXT-3491 WebUI-Migration Binary Objects to Strings#46
UIEXT-3491 WebUI-Migration Binary Objects to Strings#46tcrundall-tng wants to merge 2 commits intomasterfrom
Conversation
…oicesProvider AP-23709 (Support username and password fields instead of flow variables in OAuth2 Authenticator nodes)
tcrundall-tng
requested review from
Copilot and
knime-ghub-bot
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
This PR advances the WebUI migration effort by moving nodes/settings away from legacy “binary object / flow-variable string” representations toward structured WebUI parameters (including credentials widgets) and programmatic node descriptions.
Changes:
- Migrates Binary Objects to Strings to WebUI
NodeParameters, programmaticNodeDescription, and snapshot-based dialog/settings tests. - Updates Box Authenticator app credentials from a credentials-flow-variable string to a
LegacyCredentialswidget with validation/migration wiring. - Bumps plugin/test bundle versions to 5.12.0 and updates related snapshots/manifests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| org.knime.ext.box.authenticator/src-deprecated/org/knime/ext/box/authenticator/node/BoxAuthenticatorSettings.java | Adds @ValueReference for nested Box app settings to enable validator referencing. |
| org.knime.ext.box.authenticator/src-deprecated/org/knime/ext/box/authenticator/node/BoxAppSettings.java | Replaces flow-variable string with LegacyCredentials + migration + custom validation hook. |
| org.knime.ext.box.authenticator/META-INF/MANIFEST.MF | Updates bundle version to 5.12.0. |
| org.knime.ext.box.authenticator.tests/pom.xml | Updates test artifact revision to 5.12.0. |
| org.knime.ext.box.authenticator.tests/files/test_snapshots/org.knime.ext.box.authenticator.node.BoxAuthenticatorSettingsTest.snap | Updates snapshots for new credentials widget schema/state. |
| org.knime.ext.box.authenticator.tests/META-INF/MANIFEST.MF | Updates fragment version and host range to align with 5.12.0. |
| org.knime.base.filehandling/src/org/knime/base/filehandling/binaryobjectstostrings/BinaryObjectsToStringsNodeParameters.java | Introduces WebUI parameters, persistence, migration annotations, and effect logic. |
| org.knime.base.filehandling/src/org/knime/base/filehandling/binaryobjectstostrings/BinaryObjectsToStringsNodeModel.java | Adds configure-time auto-guessing of the selected binary-object column when unset. |
| org.knime.base.filehandling/src/org/knime/base/filehandling/binaryobjectstostrings/BinaryObjectsToStringsNodeFactory.xml | Removes legacy XML node description in favor of programmatic description. |
| org.knime.base.filehandling/src/org/knime/base/filehandling/binaryobjectstostrings/BinaryObjectsToStringsNodeFactory.java | Implements WebUI dialog factory + KAI interface + programmatic node description. |
| org.knime.base.filehandling.tests/src/org/knime/base/filehandling/binaryobjectstostrings/BinaryObjectsToStringsNodeParametersTest.java | Replaces legacy dialog test with snapshot test for NodeParameters and settings structure. |
| org.knime.base.filehandling.tests/files/test_snapshots/org.knime.base.filehandling.binaryobjectstostrings.BinaryObjectsToStringsNodeParametersTest1.snap | Adds snapshot for loading a concrete settings instance. |
| org.knime.base.filehandling.tests/files/test_snapshots/org.knime.base.filehandling.binaryobjectstostrings.BinaryObjectsToStringsNodeParametersTest0.settings.xml.snap | Adds snapshot for persisted settings XML structure. |
| org.knime.base.filehandling.tests/files/test_snapshots/org.knime.base.filehandling.binaryobjectstostrings.BinaryObjectsToStringsNodeParametersTest.snap | Adds snapshot for default model/schema/UI schema/persist metadata. |
| org.knime.base.filehandling.tests/files/node_settings/BinaryObjectsToStringsNodeParameters.xml | Adds settings fixture used by snapshot tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static final class BoxCredentialValidator extends AbstractCredentialsValidator<BoxAppSettings> { | ||
| BoxCredentialValidator() { | ||
| super(Ref.class); | ||
| } | ||
| } |
There was a problem hiding this comment.
AbstractCredentialsValidator is referenced here but the class is neither defined in this plugin nor imported. This will fail compilation unless a same-package class exists (which is unlikely). Add the correct import (or fully qualify the type) so the validator resolves unambiguously.
| public Decoding load(final NodeSettingsRO settings) throws InvalidSettingsException { | ||
| final String value = settings.getString(CFG_KEY); | ||
| for (final Decoding decoding : Decoding.values()) { | ||
| if (decoding.getName().equals(value)) { | ||
| return decoding; | ||
| } | ||
| } | ||
| throw new InvalidSettingsException("Unknown decoding: \"" + value + "\""); | ||
| } |
There was a problem hiding this comment.
DecodingPersistor.load() calls settings.getString("decoding") without handling the key being absent. If the setting is missing (e.g., older workflows or partially specified settings), this will throw even though the field has a default. Consider using containsKey/getString(key, default) and falling back to Decoding.UTF_8 to make loading robust.
| public OutputMode load(final NodeSettingsRO settings) throws InvalidSettingsException { | ||
| final String value = settings.getString(CFG_KEY); | ||
| if (ReplacePolicy.APPEND.getName().equals(value)) { | ||
| return OutputMode.APPEND; | ||
| } | ||
| return OutputMode.REPLACE; | ||
| } |
There was a problem hiding this comment.
OutputModePersistor.load() reads settings.getString("replace") without a default / presence check. If the key is absent, this will throw even though m_outputMode has a default. Handle missing keys (e.g., containsKey or getString(key, ReplacePolicy.REPLACE.getName())) and return OutputMode.REPLACE for a safe fallback.
UIEXT-3491 (WebUI Migration Binary Objects to Strings)
tcrundall-tng
force-pushed
the
enh/UIEXT-3491-webui-migration-binary-objects-to-strings
branch
from
b78e283 to
8091e7a
Compare
|
No description provided.