mattcasters commented on code in PR #8424:
URL: https://github.com/apache/hop/pull/8424#discussion_r4028637545
##########
ui/src/main/java/org/apache/hop/ui/core/database/DatabaseMetaEditor.java:
##########
@@ -528,6 +529,41 @@ private Optional<IGuiPluginCompositeWidgetsListener>
pluginWidgetsListener() {
: Optional.empty();
}
+ // The port survives a connection type change, so fetching the new type's
default has to be
+ // something the user asks for. The button sits at the right of the
generated port field and is
+ // only there for types that declare a default port.
+ private void addDefaultPortButton() {
+ Control portControl =
guiCompositeWidgets.getWidgetsMap().get(BaseDatabaseMeta.ELEMENT_ID_PORT);
+ if (portControl == null
+ || portControl.isDisposed()
+ || !(portControl.getLayoutData() instanceof FormData fdPort)
+ || getMetadata().getIDatabase().getDefaultDatabasePort() <= 0) {
+ return;
+ }
+
+ Button wbDefaultPort = new Button(wDatabaseSpecificComp, SWT.PUSH);
Review Comment:
**[bug]** The Default port button is created as a sibling of the generated
port field but is never registered in
`GuiCompositeWidgets.getActionWidgetsMap()`. `setWidgetsHidden()` only hides
the label, the widget, and that action control. Oracle hides hostname/port for
TNS alias and descriptor connections, so this button stays visible, keeps its
preferred height, and remains clickable on a collapsed row. Putting it in
`widgetsMap` would break `getWidgetsContents` for the port field.
**Suggestion:** Parent the button to `portControl.getParent()` and
`put(ELEMENT_ID_PORT, wbDefaultPort)` on `actionWidgetsMap`, matching
FILENAME/FOLDER browse buttons, so hide/show and row collapse include it.
##########
ui/src/main/java/org/apache/hop/ui/core/database/DatabaseMetaEditor.java:
##########
@@ -612,6 +658,29 @@ private void changeConnectionType() {
busyChangingConnectionType.set(false);
}
+ // Carry the connection details over to another database type. A port that
is filled in comes
+ // along: a switch within a database family (MySQL, MariaDB) points at the
same server, and a
+ // port the user typed or a variable reference is not the old type's to
discard. An empty port
+ // falls back to the new type's default rather than to a cached value, so a
port that was
+ // cleared does not come back from the type cache.
+ private void copyEnteredFields(IDatabase entered, IDatabase target) {
+ target.setAccessType(entered.getAccessType());
+ target.setHostname(entered.getHostname());
+ target.setDatabaseName(entered.getDatabaseName());
+ target.setUsername(entered.getUsername());
+ target.setPassword(entered.getPassword());
+ target.setServername(entered.getServername());
+ target.setDataTablespace(entered.getDataTablespace());
+ target.setIndexTablespace(entered.getIndexTablespace());
+ if (StringUtils.isNotEmpty(entered.getPort())) {
Review Comment:
**[bug]** `copyEnteredFields` always writes a non-empty entered port onto
the cached target. That is correct when the target shows a port widget.
Databricks annotates `port` as `ignored = true` and builds
`jdbc:databricks://host:port;...` from `DatabaseMeta.getPort()`. Switching
MySQL (3306) → Databricks previously kept 443 from `populateMetaMap()`; this
now silently stores 3306 with no field to correct it.
**Suggestion:** Do not copy the port when the target type ignores/hides that
GUI element (or after recreating widgets, if `ELEMENT_ID_PORT` is absent). Fall
back to `target.getDefaultDatabasePort()` in that case, same as the empty-port
branch.
##########
ui/src/main/java/org/apache/hop/ui/core/database/DatabaseMetaEditor.java:
##########
@@ -528,6 +529,41 @@ private Optional<IGuiPluginCompositeWidgetsListener>
pluginWidgetsListener() {
: Optional.empty();
}
+ // The port survives a connection type change, so fetching the new type's
default has to be
+ // something the user asks for. The button sits at the right of the
generated port field and is
+ // only there for types that declare a default port.
+ private void addDefaultPortButton() {
+ Control portControl =
guiCompositeWidgets.getWidgetsMap().get(BaseDatabaseMeta.ELEMENT_ID_PORT);
+ if (portControl == null
+ || portControl.isDisposed()
+ || !(portControl.getLayoutData() instanceof FormData fdPort)
+ || getMetadata().getIDatabase().getDefaultDatabasePort() <= 0) {
+ return;
+ }
+
+ Button wbDefaultPort = new Button(wDatabaseSpecificComp, SWT.PUSH);
+ wbDefaultPort.setText(BaseMessages.getString(PKG,
"DatabaseDialog.button.DefaultPort"));
+ PropsUi.setLook(wbDefaultPort);
+ FormData fdDefaultPort = new FormData();
+ fdDefaultPort.right = new FormAttachment(100, 0);
+ fdDefaultPort.top = new FormAttachment(portControl, 0, SWT.CENTER);
+ wbDefaultPort.setLayoutData(fdDefaultPort);
+ fdPort.right = new FormAttachment(wbDefaultPort, -PropsUi.getMargin());
+
+ wbDefaultPort.addListener(SWT.Selection, event -> setDefaultPort());
Review Comment:
**[suggestion]** `enableFields()` disables the port `TextVar` when a manual
URL is set. This button is not in `widgetsMap` and is never disabled.
`TextVar.setText` still applies to a disabled control, so Default port mutates
a field the rest of the dialog treats as inactive.
**Suggestion:** Keep a field (or the `actionWidgetsMap` entry) and disable
the button with the port widget in `enableFields()`, or extend `enableWidgets`
to cover action controls.
##########
ui/src/main/java/org/apache/hop/ui/core/database/DatabaseMetaEditor.java:
##########
@@ -528,6 +529,41 @@ private Optional<IGuiPluginCompositeWidgetsListener>
pluginWidgetsListener() {
: Optional.empty();
}
+ // The port survives a connection type change, so fetching the new type's
default has to be
Review Comment:
**[suggestion]** The comments on `addDefaultPortButton` (here), the
`metaMap` replacement, and `copyEnteredFields` narrate the bug history and
restate what the code does (button placement, map-vs-entered split,
MySQL/MariaDB family rationale). Hop comments should be short and explain a
non-obvious constraint.
**Suggestion:** Drop those three blocks. If anything remains, one line on
the empty-port vs cached-port choice is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
