From efe4155ca351854b98257013e8b808e9dfa03fff Mon Sep 17 00:00:00 2001 From: mattcasters Date: Tue, 29 Sep 2026 11:04:14 +0200 Subject: [PATCH 1/3] issue #8655 - dialogs of db join and write to logs --- .../databasejoin/DatabaseJoinDialog.java | 617 +++++++++--------- .../databasejoin/DatabaseJoinMeta.java | 113 ++++ .../messages/messages_en_US.properties | 22 +- .../databasejoin/DatabaseJoinDialogTest.java | 289 ++++++++ .../writetolog/WriteToLogDialog.java | 418 ++++++------ .../transforms/writetolog/WriteToLogMeta.java | 47 ++ .../messages/messages_en_US.properties | 12 +- .../writetolog/WriteToLogDialogTest.java | 105 +++ 8 files changed, 1132 insertions(+), 491 deletions(-) create mode 100644 plugins/transforms/databasejoin/src/test/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialogTest.java diff --git a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java index c655466284e..eb652e0c904 100644 --- a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java +++ b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java @@ -41,6 +41,9 @@ import org.apache.hop.ui.core.dialog.BaseDialog; import org.apache.hop.ui.core.dialog.ErrorDialog; import org.apache.hop.ui.core.dialog.MessageBox; +import org.apache.hop.ui.core.gui.GuiCompositeWidgets; +import org.apache.hop.ui.core.gui.GuiCompositeWidgetsAdapter; +import org.apache.hop.ui.core.gui.IGuiPluginCompositeButtonsListener; import org.apache.hop.ui.core.widget.ColumnInfo; import org.apache.hop.ui.core.widget.MetaSelectionLine; import org.apache.hop.ui.core.widget.SQLStyledTextComp; @@ -56,14 +59,13 @@ import org.eclipse.swt.events.FocusEvent; import org.eclipse.swt.events.KeyAdapter; import org.eclipse.swt.events.KeyEvent; -import org.eclipse.swt.events.ModifyListener; import org.eclipse.swt.events.MouseAdapter; import org.eclipse.swt.events.MouseEvent; -import org.eclipse.swt.events.SelectionAdapter; -import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.layout.FormAttachment; import org.eclipse.swt.layout.FormData; import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.TableItem; @@ -72,36 +74,22 @@ public class DatabaseJoinDialog extends BaseTransformDialog { private static final Class PKG = DatabaseJoinMeta.class; - private MetaSelectionLine wConnection; - private TextComposite wSql; - private TextVar wSqlFromFile; - - private Text wLimit; - - private Button wOuter; + private Label wlPosition; private TableView wParam; private TableView wResolvedParam; - private Button wUseVars; - private final DatabaseJoinMeta input; - private Label wlPosition; - private Label wlParam; - private ColumnInfo[] ciKey; private ColumnInfo[] ciResolvedParam; private final List inputFields = new ArrayList<>(); private IRowMeta sourceFieldsMeta; - private Button wCache; - - private Label wlCacheSize; - private Text wCacheSize; + private GuiCompositeWidgets widgets; public DatabaseJoinDialog( Shell parent, @@ -118,144 +106,119 @@ public String open() { buildButtonBar().ok(e -> ok()).get(e -> get()).cancel(e -> cancel()).build(); - ModifyListener lsMod = e -> input.setChanged(); backupChanged = input.hasChanged(); - // Connection line - wConnection = addConnectionLine(shell, wSpacer, input.getConnection(), lsMod); - wConnection.addListener( - SWT.Selection, - e -> { - getSqlReservedWords(); - refreshResolvedParametersPanel(); - }); + widgets = + GuiCompositeWidgets.addScrolledComposite( + shell, + variables, + wSpacer, + wOk, + DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID, + input, + w -> { + // Extra-group builders run during createCompositeWidgets, before + // addScrolledComposite returns. Keep the field assigned so they can look up the + // widgets already placed on the same tab. + widgets = w; + w.registerExtraGroup( + BaseMessages.getString(PKG, "DatabaseJoin.Tab.Sql"), "0200", null, this::addSql); + w.registerExtraGroup( + BaseMessages.getString(PKG, "DatabaseJoin.Tab.Parameters"), + "0300", + null, + this::addParameters); + }); + + widgets.setWidgetsListener( + new GuiCompositeWidgetsAdapter() { + @Override + public void widgetModified( + GuiCompositeWidgets compositeWidgets, Control changedWidget, String widgetId) { + if (!loading) { + input.setChanged(); + } + if (DatabaseJoinMeta.WIDGET_CACHED.equals(widgetId)) { + enableFields(); + } else if (DatabaseJoinMeta.WIDGET_CONNECTION.equals(widgetId)) { + onConnectionChanged(); + } else if (DatabaseJoinMeta.WIDGET_SQL_FROM_FILE.equals(widgetId)) { + onSqlFromFileChanged(); + } + } - // ICache? - Label wlCache = new Label(shell, SWT.RIGHT); - wlCache.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Cache.Label")); - PropsUi.setLook(wlCache); - FormData fdlCache = new FormData(); - fdlCache.left = new FormAttachment(0, 0); - fdlCache.right = new FormAttachment(middle, -margin); - fdlCache.top = new FormAttachment(wConnection, margin); - wlCache.setLayoutData(fdlCache); - wCache = new Button(shell, SWT.CHECK); - PropsUi.setLook(wCache); - FormData fdCache = new FormData(); - fdCache.left = new FormAttachment(middle, 0); - fdCache.top = new FormAttachment(wlCache, 0, SWT.CENTER); - wCache.setLayoutData(fdCache); - wCache.addSelectionListener( - new SelectionAdapter() { @Override - public void widgetSelected(SelectionEvent e) { - input.setChanged(); - enableFields(); + public void persistContents(GuiCompositeWidgets compositeWidgets) { + persistSqlEditor(); + persistParameters(); } }); - // ICache size line - wlCacheSize = new Label(shell, SWT.RIGHT); - wlCacheSize.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.CacheSize.Label")); - PropsUi.setLook(wlCacheSize); - wlCacheSize.setEnabled(input.isCached()); - FormData fdlCacheSize = new FormData(); - fdlCacheSize.left = new FormAttachment(0, 0); - fdlCacheSize.right = new FormAttachment(middle, -margin); - fdlCacheSize.top = new FormAttachment(wCache, margin); - wlCacheSize.setLayoutData(fdlCacheSize); - wCacheSize = new Text(shell, SWT.SINGLE | SWT.LEFT | SWT.BORDER); - PropsUi.setLook(wCacheSize); - wCacheSize.setEnabled(input.isCached()); - wCacheSize.addModifyListener(lsMod); - FormData fdCacheSize = new FormData(); - fdCacheSize.left = new FormAttachment(middle, 0); - fdCacheSize.right = new FormAttachment(100, 0); - fdCacheSize.top = new FormAttachment(wCache, margin); - wCacheSize.setLayoutData(fdCacheSize); - - // Load SQL from file - Label wlSqlFromFile = new Label(shell, SWT.RIGHT); - wlSqlFromFile.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.LoadSqlFromFile")); - PropsUi.setLook(wlSqlFromFile); - FormData fdlSqlFromFile = new FormData(); - fdlSqlFromFile.left = new FormAttachment(0, 0); - fdlSqlFromFile.right = new FormAttachment(middle, -margin); - fdlSqlFromFile.top = new FormAttachment(wCacheSize, margin); - wlSqlFromFile.setLayoutData(fdlSqlFromFile); - Button wbSqlFromFile = new Button(shell, SWT.PUSH); - PropsUi.setLook(wbSqlFromFile); - wbSqlFromFile.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Browse")); - FormData fdbSqlFromFile = new FormData(); - fdbSqlFromFile.right = new FormAttachment(100, 0); - fdbSqlFromFile.top = new FormAttachment(wlSqlFromFile, 0, SWT.CENTER); - wbSqlFromFile.setLayoutData(fdbSqlFromFile); - - wSqlFromFile = new TextVar(variables, shell, SWT.SINGLE | SWT.LEFT | SWT.BORDER); - PropsUi.setLook(wSqlFromFile); - wSqlFromFile.addModifyListener(lsMod); - FormData fdSqlFromFile = new FormData(); - fdSqlFromFile.left = new FormAttachment(middle, 0); - fdSqlFromFile.right = new FormAttachment(wbSqlFromFile, -margin); - fdSqlFromFile.top = new FormAttachment(wlSqlFromFile, 0, SWT.CENTER); - wSqlFromFile.setLayoutData(fdSqlFromFile); - wbSqlFromFile.addListener( - SWT.Selection, - e -> { - String path = - BaseDialog.presentFileDialog( - shell, - wSqlFromFile, - variables, - new String[] {"*.sql", "*"}, - new String[] { - BaseMessages.getString(PKG, "DatabaseJoinDialog.SqlFiles"), - BaseMessages.getString(PKG, "System.FileType.AllFiles") - }, - false); - if (path != null) { - loadSqlFromFileAndSetReadOnly(); + // The connection drives SQL syntax highlighting. The Connection tab is laid out before the + // SQL tab, so the selection line already exists by the time the editor is built. + MetaSelectionLine connectionLine = connectionLine(); + if (connectionLine != null) { + connectionLine.addListener(SWT.Selection, e -> onConnectionChanged()); + } + + widgets.setCompositeButtonsListener( + new IGuiPluginCompositeButtonsListener() { + @Override + public void buttonPressed(Object sourceObject) { + // Flush the widgets before the no-op Meta method runs: browseSqlFromFile() reads the + // path from the widget, and the setWidgetsContents that follows a button press would + // otherwise re-read editor state that is about to change. + widgets.getWidgetsContents(input, DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); + } + + @Override + public void afterButtonPressed(Object sourceObject) { + browseSqlFromFile(); } }); - // SQL editor... - Label wlSql = new Label(shell, SWT.NONE); + populateSqlEditor(); + populateParameters(); + enableFields(); + searchPrevTransformFields(); + + input.setChanged(backupChanged); + focusTransformName(); + BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel()); + + return transformName; + } + + /** + * The SQL tab: the styled SQL editor with its line/column readout, plus the read-only table of + * parameters resolved from the SQL. The editor takes all remaining vertical space, which the old + * flat form could not do without squeezing the parameter grid below it. + */ + private void addSql(Composite parent) { + Label wlSql = new Label(parent, SWT.NONE); wlSql.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.SQL.Label")); PropsUi.setLook(wlSql); FormData fdlSql = new FormData(); fdlSql.left = new FormAttachment(0, 0); - fdlSql.top = new FormAttachment(wbSqlFromFile, margin); + fdlSql.right = new FormAttachment(100, 0); + Control lastOnTab = widgets.getWidgetsMap().get(DatabaseJoinMeta.WIDGET_REPLACE_VARIABLES); + fdlSql.top = + lastOnTab == null ? new FormAttachment(0, 0) : new FormAttachment(lastOnTab, margin); wlSql.setLayoutData(fdlSql); wSql = EnvironmentUtils.getInstance().isWeb() ? new StyledTextComp( variables, - shell, + parent, SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL, TextComposite.STYLE_TYPE_SQL) : new SQLStyledTextComp( - variables, shell, SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL); - wSql.addLineStyleListener(getSqlReservedWords()); + variables, parent, SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL); PropsUi.setLook(wSql, Props.WIDGET_STYLE_FIXED); - wSql.addModifyListener(lsMod); + wSql.addLineStyleListener(getSqlReservedWords()); wSql.addModifyListener(e -> refreshResolvedParametersPanel()); - wSqlFromFile.addModifyListener( - e -> { - if (Utils.isEmpty(wSqlFromFile.getText())) { - wSql.setEditable(true); - refreshResolvedParametersPanel(); - } - }); - FormData fdSql = new FormData(); - fdSql.left = new FormAttachment(0, 0); - fdSql.top = new FormAttachment(wlSql, margin); - fdSql.right = new FormAttachment(100, -margin); - fdSql.bottom = new FormAttachment(60, 0); - wSql.setLayoutData(fdSql); - - wSql.addModifyListener(arg0 -> setPosition()); - + wSql.addModifyListener(e -> setPosition()); wSql.addKeyListener( new KeyAdapter() { @Override @@ -297,8 +260,14 @@ public void mouseUp(MouseEvent e) { setPosition(); } }); + FormData fdSql = new FormData(); + fdSql.left = new FormAttachment(0, 0); + fdSql.top = new FormAttachment(wlSql, margin); + fdSql.right = new FormAttachment(100, 0); + fdSql.bottom = new FormAttachment(100, 0); + wSql.setLayoutData(fdSql); - wlPosition = new Label(shell, SWT.NONE); + wlPosition = new Label(parent, SWT.NONE); PropsUi.setLook(wlPosition); FormData fdlPosition = new FormData(); fdlPosition.left = new FormAttachment(0, 0); @@ -306,12 +275,13 @@ public void mouseUp(MouseEvent e) { fdlPosition.right = new FormAttachment(100, 0); wlPosition.setLayoutData(fdlPosition); - Label wlResolvedParam = new Label(shell, SWT.NONE); + Label wlResolvedParam = new Label(parent, SWT.NONE); wlResolvedParam.setText( BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Label")); PropsUi.setLook(wlResolvedParam); FormData fdlResolvedParam = new FormData(); fdlResolvedParam.left = new FormAttachment(0, 0); + fdlResolvedParam.right = new FormAttachment(100, 0); fdlResolvedParam.top = new FormAttachment(wlPosition, margin); wlResolvedParam.setLayoutData(fdlResolvedParam); @@ -335,7 +305,7 @@ public void mouseUp(MouseEvent e) { wResolvedParam = new TableView( variables, - shell, + parent, SWT.BORDER | SWT.FULL_SELECTION | SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL, ciResolvedParam, 1, @@ -352,89 +322,13 @@ public void mouseUp(MouseEvent e) { fdResolvedParam.right = new FormAttachment(100, 0); fdResolvedParam.height = (int) (90 * props.getZoomFactor()); wResolvedParam.setLayoutData(fdResolvedParam); + } - // Limit the number of lines returns - Label wlLimit = new Label(shell, SWT.RIGHT); - wlLimit.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Limit.Label")); - PropsUi.setLook(wlLimit); - FormData fdlLimit = new FormData(); - fdlLimit.left = new FormAttachment(0, 0); - fdlLimit.right = new FormAttachment(middle, -margin); - fdlLimit.top = new FormAttachment(wResolvedParam, margin); - wlLimit.setLayoutData(fdlLimit); - wLimit = new Text(shell, SWT.SINGLE | SWT.LEFT | SWT.BORDER); - PropsUi.setLook(wLimit); - wLimit.addModifyListener(lsMod); - FormData fdLimit = new FormData(); - fdLimit.left = new FormAttachment(middle, 0); - fdLimit.right = new FormAttachment(100, 0); - fdLimit.top = new FormAttachment(wResolvedParam, margin); - wLimit.setLayoutData(fdLimit); - - // Outer join? - Label wlOuter = new Label(shell, SWT.RIGHT); - wlOuter.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Outerjoin.Label")); - wlOuter.setToolTipText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Outerjoin.Tooltip")); - PropsUi.setLook(wlOuter); - FormData fdlOuter = new FormData(); - fdlOuter.left = new FormAttachment(0, 0); - fdlOuter.right = new FormAttachment(middle, -margin); - fdlOuter.top = new FormAttachment(wLimit, margin); - wlOuter.setLayoutData(fdlOuter); - wOuter = new Button(shell, SWT.CHECK); - PropsUi.setLook(wOuter); - wOuter.setToolTipText(wlOuter.getToolTipText()); - FormData fdOuter = new FormData(); - fdOuter.left = new FormAttachment(middle, 0); - fdOuter.top = new FormAttachment(wlOuter, 0, SWT.CENTER); - wOuter.setLayoutData(fdOuter); - wOuter.addSelectionListener( - new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent e) { - input.setChanged(); - } - }); - - // useVars ? - Label wluseVars = new Label(shell, SWT.RIGHT); - wluseVars.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.useVarsjoin.Label")); - wluseVars.setToolTipText(BaseMessages.getString(PKG, "DatabaseJoinDialog.useVarsjoin.Tooltip")); - PropsUi.setLook(wluseVars); - FormData fdluseVars = new FormData(); - fdluseVars.left = new FormAttachment(0, 0); - fdluseVars.right = new FormAttachment(middle, -margin); - fdluseVars.top = new FormAttachment(wOuter, margin); - wluseVars.setLayoutData(fdluseVars); - wUseVars = new Button(shell, SWT.CHECK); - PropsUi.setLook(wUseVars); - wUseVars.setToolTipText(wluseVars.getToolTipText()); - FormData fduseVars = new FormData(); - fduseVars.left = new FormAttachment(middle, 0); - fduseVars.top = new FormAttachment(wluseVars, 0, SWT.CENTER); - wUseVars.setLayoutData(fduseVars); - wUseVars.addSelectionListener( - new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent e) { - input.setChanged(); - } - }); - - // THE BUTTONS - // The parameters - wlParam = new Label(shell, SWT.NONE); - wlParam.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Param.Label")); - PropsUi.setLook(wlParam); - FormData fdlParam = new FormData(); - fdlParam.left = new FormAttachment(0, 0); - fdlParam.top = new FormAttachment(wUseVars, margin); - wlParam.setLayoutData(fdlParam); - - int nrKeyCols = 2; + /** The Parameters tab: the grid that maps positional {@code ?} markers to input fields. */ + private void addParameters(Composite parent) { int nrKeyRows = (input.getParameters() != null ? input.getParameters().size() : 1); - ciKey = new ColumnInfo[nrKeyCols]; + ciKey = new ColumnInfo[2]; ciKey[0] = new ColumnInfo( BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ParameterFieldname"), @@ -447,74 +341,77 @@ public void widgetSelected(SelectionEvent e) { ColumnInfo.COLUMN_TYPE_CCOMBO, ValueMetaFactory.getValueMetaNames()); - ModifyListener lsParamMod = - e -> { - input.setChanged(); - refreshResolvedParametersPanel(); - }; + Label wlParam = new Label(parent, SWT.NONE); + wlParam.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Param.Label")); + PropsUi.setLook(wlParam); + FormData fdlParam = new FormData(); + fdlParam.left = new FormAttachment(0, 0); + fdlParam.right = new FormAttachment(100, 0); + fdlParam.top = new FormAttachment(0, 0); + wlParam.setLayoutData(fdlParam); wParam = new TableView( variables, - shell, + parent, SWT.BORDER | SWT.FULL_SELECTION | SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL, ciKey, nrKeyRows, - lsParamMod, + e -> { + if (!loading) { + input.setChanged(); + } + refreshResolvedParametersPanel(); + }, props); FormData fdParam = new FormData(); fdParam.left = new FormAttachment(0, 0); fdParam.top = new FormAttachment(wlParam, margin); fdParam.right = new FormAttachment(100, 0); - fdParam.bottom = new FormAttachment(wOk, -margin); + fdParam.bottom = new FormAttachment(100, 0); wParam.setLayoutData(fdParam); + } - // - // Search the fields in the background - - final Runnable runnable = - () -> { - TransformMeta transformMeta = pipelineMeta.findTransform(transformName); - if (transformMeta != null) { - try { - IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformMeta); - - // Remember these fields... - sourceFieldsMeta = row; - for (int i = 0; i < row.size(); i++) { - inputFields.add(row.getValueMeta(i).getName()); - } - setComboBoxes(); - if (!shell.isDisposed()) { - shell.getDisplay().asyncExec(() -> refreshResolvedParametersPanel()); - } - } catch (HopException e) { - logError(BaseMessages.getString(PKG, "System.Dialog.GetFieldsFailed.Message")); - } - } - }; - BackgroundThreadFacade.start(runnable); + private MetaSelectionLine connectionLine() { + if (widgets == null) { + return null; + } + Control control = widgets.getWidgetsMap().get(DatabaseJoinMeta.WIDGET_CONNECTION); + return control instanceof MetaSelectionLine line ? line : null; + } - getData(); - focusTransformName(); - BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel()); + private void onConnectionChanged() { + // Only the resolved-parameter table depends on the connection here. The SQL highlighter is + // deliberately not re-seeded: TextComposite has no removeLineStyleListener, so every call to + // addLineStyleListener stacks another listener and the stale keyword sets would keep firing. + // Highlighting therefore uses the keywords of the connection stored on the transform, which is + // what the dialog did before it was split into tabs. + refreshResolvedParametersPanel(); + } - return transformName; + private void onSqlFromFileChanged() { + if (Utils.isEmpty(readWidgetText(DatabaseJoinMeta.WIDGET_SQL_FROM_FILE))) { + wSql.setEditable(true); + refreshResolvedParametersPanel(); + } else { + loadSqlFromFileAndSetReadOnly(); + } } private List getSqlReservedWords() { + String connectionName = readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION); // Do not search keywords when connection is empty - if (Utils.isEmpty(input.getConnection())) { + if (Utils.isEmpty(connectionName)) { return List.of(); } // If connection is a variable that can't be resolved - if (variables.resolve(input.getConnection()).startsWith("${")) { + if (variables.resolve(connectionName).startsWith("${")) { return List.of(); } - DatabaseMeta databaseMeta = pipelineMeta.findDatabase(input.getConnection(), variables); + DatabaseMeta databaseMeta = pipelineMeta.findDatabase(connectionName, variables); if (databaseMeta == null) { return List.of(); } @@ -522,18 +419,44 @@ private List getSqlReservedWords() { } private void enableFields() { - wCacheSize.setEnabled(wCache.getSelection()); - wlCacheSize.setEnabled(wCache.getSelection()); + setEnabled(DatabaseJoinMeta.WIDGET_CACHE_SIZE, isChecked(DatabaseJoinMeta.WIDGET_CACHED)); + } + + private boolean isChecked(String widgetId) { + if (widgets == null) { + return false; + } + Control control = widgets.getWidgetsMap().get(widgetId); + return control instanceof Button button && button.getSelection(); + } + + private void setEnabled(String widgetId, boolean enabled) { + if (widgets == null) { + return; + } + Control label = widgets.getLabelsMap().get(widgetId); + if (label != null && !label.isDisposed()) { + label.setEnabled(enabled); + } + Control widget = widgets.getWidgetsMap().get(widgetId); + if (widget != null && !widget.isDisposed()) { + widget.setEnabled(enabled); + } } - protected void setComboBoxes() { + private void setComboBoxes() { // Something was changed in the row. // - String[] fieldNames = ConstUi.sortFieldNames(inputFields); - ciKey[0].setComboValues(fieldNames); + if (ciKey == null) { + return; + } + ciKey[0].setComboValues(ConstUi.sortFieldNames(inputFields)); } public void setPosition() { + if (wSql == null || wSql.isDisposed() || wlPosition == null || wlPosition.isDisposed()) { + return; + } int lineNumber = wSql.getLineNumber(); int columnNumber = wSql.getColumnNumber(); wlPosition.setText( @@ -542,16 +465,17 @@ public void setPosition() { } private DatabaseJoinMeta.SqlParameterSpec parseCurrentSqlParameterSpec() { - String sourceSql = wSql == null ? null : wSql.getText(); + String sourceSql = wSql == null || wSql.isDisposed() ? null : wSql.getText(); return DatabaseJoinMeta.parseSqlParameterSpec( sourceSql, DatabaseJoinMeta.supportsBracketQuotedIdentifiers( - pipelineMeta.findDatabase(wConnection.getText(), variables))); + pipelineMeta.findDatabase( + readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION), variables))); } private List getDeclaredParametersFromGrid() { List parameters = new ArrayList<>(); - if (wParam == null) { + if (wParam == null || wParam.isDisposed()) { return parameters; } @@ -640,17 +564,16 @@ private void refreshResolvedParametersPanel() { wResolvedParam.setRowNums(); wResolvedParam.optWidth(true); + // The declared grid only maps positional "?" markers. With named "?{field}" placeholders + // there is nothing left to fill in, so the grid is disabled rather than silently ignored. boolean hasPositionalParameters = parameterSpec.getPositionalParameterCount() > 0; if (wParam != null && !wParam.isDisposed()) { wParam.setEnabled(hasPositionalParameters); } - if (wlParam != null && !wlParam.isDisposed()) { - wlParam.setEnabled(hasPositionalParameters); - } } private void loadSqlFromFileAndSetReadOnly() { - String path = variables.resolve(wSqlFromFile.getText()); + String path = variables.resolve(readWidgetText(DatabaseJoinMeta.WIDGET_SQL_FROM_FILE)); if (Utils.isEmpty(path)) { wSql.setEditable(true); return; @@ -662,7 +585,7 @@ private void loadSqlFromFileAndSetReadOnly() { refreshResolvedParametersPanel(); } catch (HopFileException e) { MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_WARNING); - mb.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogTitle")); + mb.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.CouldNotLoadSqlFromFile.Title")); mb.setMessage( BaseMessages.getString(PKG, "DatabaseJoinDialog.CouldNotLoadSqlFromFile", path) + Const.CR @@ -673,41 +596,138 @@ private void loadSqlFromFileAndSetReadOnly() { } } - /** Copy information from the meta-data input to the dialog fields. */ - public void getData() { - logDebug(BaseMessages.getString(PKG, "DatabaseJoinDialog.Log.GettingKeyInfo")); - - wConnection.setText(Const.NVL(input.getConnection(), "")); - - wCache.setSelection(input.isCached()); - wCacheSize.setText("" + input.getCacheSize()); + /** + * Open a file dialog on the transform dialog shell, load the chosen SQL into the editor and make + * the editor read-only. Runs from the composite button listener rather than the annotated Meta + * method: {@code setWidgetsContents} after a Meta mutation re-reads widgets that are still empty + * and would wipe the selection. + */ + private void browseSqlFromFile() { + String path = + BaseDialog.presentFileDialog( + shell, + null, + variables, + new String[] {"*.sql", "*"}, + new String[] { + BaseMessages.getString(PKG, "DatabaseJoinDialog.SqlFiles"), + BaseMessages.getString(PKG, "System.FileType.AllFiles") + }, + false); + if (path == null) { + return; + } + writeWidgetText(DatabaseJoinMeta.WIDGET_SQL_FROM_FILE, path); + input.setSqlFromFile(path); + loadSqlFromFileAndSetReadOnly(); + input.setChanged(); + } + private void populateSqlEditor() { wSql.setText(Const.NVL(input.getSql(), "")); - wSqlFromFile.setText(Const.NVL(input.getSqlFromFile(), "")); - if (!Utils.isEmpty(wSqlFromFile.getText())) { + if (!Utils.isEmpty(input.getSqlFromFile())) { loadSqlFromFileAndSetReadOnly(); } else { wSql.setEditable(true); } - wLimit.setText("" + input.getRowLimit()); - wOuter.setSelection(input.isOuterJoin()); - wUseVars.setSelection(input.isReplaceVariables()); + setPosition(); + } + + private void populateParameters() { + if (wParam == null || wParam.isDisposed()) { + return; + } + wParam.clearAll(); if (input.getParameters() != null) { - int i = 0; for (ParameterField field : input.getParameters()) { - TableItem item = wParam.table.getItem(i++); + TableItem item = new TableItem(wParam.table, SWT.NONE); if (field != null) { - item.setText(1, field.getName()); - item.setText(2, field.getType()); + item.setText(1, Const.NVL(field.getName(), "")); + item.setText(2, Const.NVL(field.getType(), "")); } } } - + if (wParam.table.getItemCount() == 0) { + new TableItem(wParam.table, SWT.NONE); + } wParam.setRowNums(); wParam.optWidth(true); refreshResolvedParametersPanel(); + } - enableFields(); + private void persistSqlEditor() { + if (wSql != null && !wSql.isDisposed()) { + input.setSql(wSql.getText()); + } + } + + private void persistParameters() { + List parameters = getDeclaredParametersFromGrid(); + logDebug( + BaseMessages.getString(PKG, "DatabaseJoinDialog.Log.ParametersFound") + + parameters.size() + + " parameters"); + input.setParameters(parameters); + } + + private String readWidgetText(String widgetId) { + if (widgets == null) { + return ""; + } + Control control = widgets.getWidgetsMap().get(widgetId); + if (control instanceof TextVar textVar) { + return Const.NVL(textVar.getText(), ""); + } + if (control instanceof Text text) { + return Const.NVL(text.getText(), ""); + } + if (control instanceof MetaSelectionLine line) { + return Const.NVL(line.getText(), ""); + } + return ""; + } + + private void writeWidgetText(String widgetId, String value) { + if (widgets == null) { + return; + } + Control control = widgets.getWidgetsMap().get(widgetId); + String text = Const.NVL(value, ""); + if (control instanceof TextVar textVar) { + textVar.setText(text); + } else if (control instanceof Text widget) { + widget.setText(text); + } else if (control instanceof MetaSelectionLine line) { + line.setText(text); + } + } + + private void searchPrevTransformFields() { + // + // Search the fields in the background + // + BackgroundThreadFacade.start( + () -> { + TransformMeta transformMeta = pipelineMeta.findTransform(transformName); + if (transformMeta == null) { + return; + } + try { + IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformMeta); + + // Remember these fields... + sourceFieldsMeta = row; + for (int i = 0; i < row.size(); i++) { + inputFields.add(row.getValueMeta(i).getName()); + } + setComboBoxes(); + if (!shell.isDisposed()) { + shell.getDisplay().asyncExec(this::refreshResolvedParametersPanel); + } + } catch (HopException e) { + logError(BaseMessages.getString(PKG, "System.Dialog.GetFieldsFailed.Message")); + } + }); } private void cancel() { @@ -721,29 +741,22 @@ private void ok() { return; } - input.setConnection(wConnection.getText()); - input.setCached(wCache.getSelection()); - input.setCacheSize(Const.toInt(wCacheSize.getText(), 0)); - input.setRowLimit(Const.toIntExpanded(wLimit.getText(), 0)); - input.setSql(wSql.getText()); - input.setSqlFromFile(wSqlFromFile.getText()); - input.setOuterJoin(wOuter.getSelection()); - input.setReplaceVariables(wUseVars.getSelection()); - List parameters = getDeclaredParametersFromGrid(); - logDebug( - BaseMessages.getString(PKG, "DatabaseJoinDialog.Log.ParametersFound") - + parameters.size() - + " parameters"); - input.setParameters(parameters); + widgets.getWidgetsContents(input, DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); + persistSqlEditor(); + persistParameters(); transformName = wTransformName.getText(); // return value - if (pipelineMeta.findDatabase(wConnection.getText(), variables) == null) { + if (pipelineMeta.findDatabase(readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION), variables) + == null) { MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_ERROR); mb.setMessage( BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogMessage")); mb.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogTitle")); mb.open(); + // Keep the dialog open: disposing here would commit the unusable connection that was just + // reported as invalid, leaving the transform broken with no chance to fix it. + return; } dispose(); diff --git a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java index 7499620a570..21eadac1cb2 100644 --- a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java +++ b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java @@ -34,6 +34,10 @@ import org.apache.hop.core.exception.HopFileException; import org.apache.hop.core.exception.HopPluginException; import org.apache.hop.core.exception.HopTransformException; +import org.apache.hop.core.gui.plugin.GuiElementType; +import org.apache.hop.core.gui.plugin.GuiPlugin; +import org.apache.hop.core.gui.plugin.GuiWidgetElement; +import org.apache.hop.core.gui.plugin.GuiWidgetGroupType; import org.apache.hop.core.row.IRowMeta; import org.apache.hop.core.row.IValueMeta; import org.apache.hop.core.row.RowMeta; @@ -66,20 +70,70 @@ ActionTransformType.LOOKUP, ActionTransformType.JOIN }) +@GuiPlugin public class DatabaseJoinMeta extends BaseTransformMeta { private static final Class PKG = DatabaseJoinMeta.class; + public static final String GUI_PLUGIN_ELEMENT_PARENT_ID = "DATABASE_JOIN_DIALOG_OPTIONS"; + + public static final String GROUP_CONNECTION = "i18n::DatabaseJoin.Tab.Connection"; + public static final String GROUP_CONNECTION_ORDER = "0100"; + public static final String GROUP_SQL = "i18n::DatabaseJoin.Tab.Sql"; + public static final String GROUP_SQL_ORDER = "0200"; + public static final String GROUP_PARAMETERS = "i18n::DatabaseJoin.Tab.Parameters"; + public static final String GROUP_PARAMETERS_ORDER = "0300"; + + public static final String WIDGET_CONNECTION = "connection"; + public static final String WIDGET_CACHED = "cached"; + public static final String WIDGET_CACHE_SIZE = "cacheSize"; + public static final String WIDGET_SQL_FROM_FILE = "sqlFromFile"; + public static final String WIDGET_BROWSE_SQL_FILE = "browseSqlFromFile"; + public static final String WIDGET_ROW_LIMIT = "rowLimit"; + public static final String WIDGET_OUTER_JOIN = "outerJoin"; + public static final String WIDGET_REPLACE_VARIABLES = "replaceVariables"; + + @GuiWidgetElement( + id = WIDGET_CONNECTION, + order = "0100", + type = GuiElementType.METADATA, + metadata = DatabaseMeta.class, + label = "i18n::DatabaseJoinMeta.Connection.Label", + toolTip = "i18n::DatabaseJoinMeta.Connection.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_CONNECTION, + groupOrder = GROUP_CONNECTION_ORDER) @HopMetadataProperty( key = "connection", injectionKeyDescription = "DatabaseJoinMeta.Injection.Connection", hopMetadataPropertyType = HopMetadataPropertyType.RDBMS_CONNECTION) private String connection; + @GuiWidgetElement( + id = WIDGET_CACHED, + order = "0200", + type = GuiElementType.CHECKBOX, + label = "i18n::DatabaseJoinMeta.Cached.Label", + toolTip = "i18n::DatabaseJoinMeta.Cached.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_CONNECTION, + groupOrder = GROUP_CONNECTION_ORDER) @HopMetadataProperty(key = "cache", injectionKeyDescription = "DatabaseJoinMeta.Injection.Cache") private boolean cached; /** Limit the cache size to this! */ + @GuiWidgetElement( + id = WIDGET_CACHE_SIZE, + order = "0300", + type = GuiElementType.TEXT, + label = "i18n::DatabaseJoinMeta.CacheSize.Label", + toolTip = "i18n::DatabaseJoinMeta.CacheSize.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_CONNECTION, + groupOrder = GROUP_CONNECTION_ORDER) @HopMetadataProperty( key = "cache_size", injectionKeyDescription = "DatabaseJoinMeta.Injection.CacheSize") @@ -95,13 +149,52 @@ public class DatabaseJoinMeta extends BaseTransformMeta parameters = new ArrayList<>(); /** false: don't replace variable in script true: replace variable in script */ + @GuiWidgetElement( + id = WIDGET_REPLACE_VARIABLES, + order = "0500", + type = GuiElementType.CHECKBOX, + label = "i18n::DatabaseJoinMeta.ReplaceVariables.Label", + toolTip = "i18n::DatabaseJoinMeta.ReplaceVariables.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_SQL, + groupOrder = GROUP_SQL_ORDER) @HopMetadataProperty( key = "replace_vars", injectionKeyDescription = "DatabaseJoinMeta.Injection.ReplaceVariables") diff --git a/plugins/transforms/databasejoin/src/main/resources/org/apache/hop/pipeline/transforms/databasejoin/messages/messages_en_US.properties b/plugins/transforms/databasejoin/src/main/resources/org/apache/hop/pipeline/transforms/databasejoin/messages/messages_en_US.properties index f0c5b13a7ce..147d9f1b3cb 100644 --- a/plugins/transforms/databasejoin/src/main/resources/org/apache/hop/pipeline/transforms/databasejoin/messages/messages_en_US.properties +++ b/plugins/transforms/databasejoin/src/main/resources/org/apache/hop/pipeline/transforms/databasejoin/messages/messages_en_US.properties @@ -26,6 +26,9 @@ DatabaseJoin.Log.LineNumber=linenr DatabaseJoin.Log.PutoutRow=Put out row\: DatabaseJoin.Log.SQLStatement=Prepare SQL statement \: {0} DatabaseJoin.Name=Database join +DatabaseJoin.Tab.Connection=Connection +DatabaseJoin.Tab.Sql=SQL +DatabaseJoin.Tab.Parameters=Parameters DatabaseJoinDialog.Cache.Label=Enable cache DatabaseJoinDialog.CacheSize.Label=Cache size in rows (0\: cache everything) DatabaseJoinDialog.ColumnInfo.ParameterFieldname=Parameter fieldname @@ -53,10 +56,27 @@ DatabaseJoinDialog.SQL.Label=SQL DatabaseJoinDialog.LoadSqlFromFile=Load SQL from file DatabaseJoinDialog.Browse=Browse... DatabaseJoinDialog.SqlFiles=SQL files -DatabaseJoinDialog.CouldNotLoadSqlFromFile=Could not load SQL from file: {0} +DatabaseJoinDialog.CouldNotLoadSqlFromFile=Could not load SQL from file\: {0} +DatabaseJoinDialog.CouldNotLoadSqlFromFile.Title=Could not load SQL from file DatabaseJoinDialog.TransformName.Label=Transform name DatabaseJoinDialog.useVarsjoin.Label=Replace variables DatabaseJoinDialog.useVarsjoin.Tooltip=Replace variables in SQL script +DatabaseJoinMeta.Connection.Label=Connection +DatabaseJoinMeta.Connection.Tooltip=The database connection used to run the query +DatabaseJoinMeta.Cached.Label=Enable cache +DatabaseJoinMeta.Cached.Tooltip=Cache the query result so repeated lookups do not hit the database again +DatabaseJoinMeta.CacheSize.Label=Cache size in rows (0\: cache everything) +DatabaseJoinMeta.CacheSize.Tooltip=Maximum number of rows to keep in the cache. Zero caches everything. +DatabaseJoinMeta.SqlFromFile.Label=Load SQL from file +DatabaseJoinMeta.SqlFromFile.Tooltip=Optional VFS path to read the query from. When set, the SQL editor becomes read-only. +DatabaseJoinMeta.BrowseSqlFromFile.Label=Browse... +DatabaseJoinMeta.BrowseSqlFromFile.Tooltip=Select the SQL file to read the query from +DatabaseJoinMeta.RowLimit.Label=Number of rows to return +DatabaseJoinMeta.RowLimit.Tooltip=Maximum number of rows to return. Zero returns all matching rows. +DatabaseJoinMeta.OuterJoin.Label=Outer join +DatabaseJoinMeta.OuterJoin.Tooltip=Return at least the input row and add NULLs for the lookup values +DatabaseJoinMeta.ReplaceVariables.Label=Replace variables +DatabaseJoinMeta.ReplaceVariables.Tooltip=Replace variables in the SQL script DatabaseJoinMeta.CheckResult.AllFieldsFound=All fields found in the input stream. DatabaseJoinMeta.CheckResult.CounldNotReadFields=Couldn''t read fields from the previous transform. DatabaseJoinMeta.CheckResult.DatabaseMetaError=Unable to get a reference to databaseMeta for connection: ''{0}'' diff --git a/plugins/transforms/databasejoin/src/test/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialogTest.java b/plugins/transforms/databasejoin/src/test/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialogTest.java new file mode 100644 index 00000000000..936b70ea569 --- /dev/null +++ b/plugins/transforms/databasejoin/src/test/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialogTest.java @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.pipeline.transforms.databasejoin; + +import static org.eclipse.swtbot.swt.finder.matchers.WidgetMatcherFactory.widgetOfType; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; +import org.apache.hop.core.gui.plugin.GuiRegistry; +import org.apache.hop.core.gui.plugin.GuiWidgetElement; +import org.apache.hop.core.plugins.PluginRegistry; +import org.apache.hop.core.plugins.TransformPluginType; +import org.apache.hop.core.variables.Variables; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.pipeline.PipelineMeta; +import org.apache.hop.pipeline.transform.TransformMeta; +import org.apache.hop.ui.testing.SwtBotTestBase; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swtbot.swt.finder.SWTBot; +import org.eclipse.swtbot.swt.finder.exceptions.WidgetNotFoundException; +import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * SWTBot coverage for the tabbed layout of {@link DatabaseJoinDialog} (#8655). + * + *

The dialog used to stack the connection, cache, SQL editor, resolved parameters, query options + * and the parameter grid into one flat form with hardcoded pixel heights, so it could not be + * resized sensibly: the editor and both grids competed for the same vertical space. The three + * sections are now separate tabs. + * + *

Tagged {@code uitest} so it is skipped when there is no display. Wrap Maven with {@code + * tools/with-isolated-display.sh} so the dialog does not steal focus. + */ +@Tag("uitest") +class DatabaseJoinDialogTest extends SwtBotTestBase { + + private static final Class PKG = DatabaseJoinMeta.class; + + private static final String TRANSFORM_NAME = "database join"; + private static final String SHELL_TITLE = "Database join"; + private static final String ERROR_TITLE = + BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogTitle"); + + private static final String TAB_CONNECTION = "Connection"; + private static final String TAB_SQL = "SQL"; + private static final String TAB_PARAMETERS = "Parameters"; + + private static final String CONNECTION = "none"; + private static final String SQL = "select id from lookup where code = ?"; + + /** + * The tabs are built from the annotated widgets on {@link DatabaseJoinMeta}, which the framework + * looks up in the registry by class name. The unit-test JVM does not always run the {@code + * GuiPluginType} scan, so register them the way that scan does. + */ + @BeforeAll + static void registerMetaWidgets() { + GuiRegistry registry = GuiRegistry.getInstance(); + if (registry.findGuiElements( + DatabaseJoinMeta.class.getName(), DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID) + != null) { + return; + } + for (Field field : DatabaseJoinMeta.class.getDeclaredFields()) { + GuiWidgetElement element = field.getAnnotation(GuiWidgetElement.class); + if (element != null) { + registry.addGuiWidgetElement(DatabaseJoinMeta.class.getName(), element, field); + } + } + } + + @Test + void theThreeSectionsAreSeparateTabsInOrder() { + withDialog( + openerFor(new DatabaseJoinMeta()), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + assertEquals( + List.of(TAB_CONNECTION, TAB_SQL, TAB_PARAMETERS), + tabTitles(dialog), + "the dialog should be split into Connection, SQL and Parameters, in that order"); + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + /** The editor and the resolved-parameter table belong on the SQL tab, not a tab each. */ + @Test + void theSqlTabHoldsBothTheEditorAndTheResolvedParameters() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setConnection(CONNECTION); + meta.setSql(SQL); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + selectTab(dialog, TAB_SQL); + assertEquals( + SQL, + sqlEditorText(dialog), + "the stored SQL should be shown in the editor on the tab"); + assertEquals( + 1, + tableCount(dialog), + "the SQL tab holds the resolved-parameter table next to the editor"); + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + /** The declared parameter grid is the only table on the Parameters tab. */ + @Test + void theParametersTabHoldsTheDeclaredParameterGrid() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setConnection(CONNECTION); + meta.setSql(SQL); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + selectTab(dialog, TAB_PARAMETERS); + assertEquals(1, tableCount(dialog), "the Parameters tab holds one grid"); + assertEquals( + null, sqlEditorText(dialog), "the SQL editor belongs on the SQL tab, not here"); + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + @Test + void okStoresTheEditorContents() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setConnection(CONNECTION); + meta.setSql(SQL); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + selectTab(dialog, TAB_SQL); + dialog.button(buttonLabel("System.Button.OK")).click(); + }); + + assertEquals(SQL, meta.getSql(), "OK must store the SQL held in the editor"); + } + + /** + * A transform with no connection cannot run, and the dialog says so. OK used to show that error + * and then dispose anyway, committing the very connection it had just called invalid. + */ + @Test + void okKeepsTheDialogOpenWhenTheConnectionIsInvalid() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setSql(SQL); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + dialog.button(buttonLabel("System.Button.OK")).click(); + + // Dismiss the error box the OK press raises, then check the dialog is still there. + SWTBot error = bot.shell(ERROR_TITLE).activate().bot(); + error.button(buttonLabel("System.Button.OK")).click(); + + assertTrue( + shellIsOpen(dialog), + "the dialog must stay open after reporting an invalid connection"); + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + private boolean shellIsOpen(SWTBot dialog) { + for (SWTBotShell shell : dialog.shells()) { + if (SHELL_TITLE.equals(shell.getText()) && shell.isOpen()) { + return true; + } + } + return false; + } + + private List tabTitles(SWTBot dialog) { + List titles = new ArrayList<>(); + display.syncExec( + () -> { + for (CTabItem item : tabFolder(dialog).getItems()) { + // The look-and-feel pads tab labels with spaces. + titles.add(item.getText().trim()); + } + }); + return titles; + } + + /** + * Brings a tab to the front. Done on the folder rather than through {@code bot.cTabItem(...)} + * because SWTBot's widget finder only walks controls and never sees a dialog's tab items. + */ + private void selectTab(SWTBot dialog, String title) { + display.syncExec( + () -> { + CTabFolder folder = tabFolder(dialog); + for (CTabItem item : folder.getItems()) { + if (item.getText().trim().equals(title)) { + folder.setSelection(item); + folder.layout(); + return; + } + } + }); + assertTrue( + tabTitles(dialog).contains(title), + "expected a '" + title + "' tab, found " + tabTitles(dialog)); + } + + private CTabFolder tabFolder(SWTBot dialog) { + return (CTabFolder) dialog.widget(widgetOfType(CTabFolder.class)); + } + + /** The SQL editor on the selected tab, or null when the tab holds none. */ + private String sqlEditorText(SWTBot dialog) { + try { + return dialog.styledText().getText(); + } catch (WidgetNotFoundException e) { + return null; + } + } + + /** + * How many grids the selected tab shows. Only the selected tab is laid out and visible to SWTBot, + * so this is a per-tab count: the SQL tab has the resolved-parameter table, the Parameters tab + * the declared-parameter grid. + */ + private int tableCount(SWTBot dialog) { + int count = 0; + for (int i = 0; ; i++) { + try { + dialog.table(i); + count++; + } catch (WidgetNotFoundException | IndexOutOfBoundsException e) { + return count; + } + } + } + + private Consumer openerFor(DatabaseJoinMeta meta) { + PipelineMeta pipelineMeta = pipelineWith(meta); + return parent -> new DatabaseJoinDialog(parent, new Variables(), meta, pipelineMeta).open(); + } + + private static PipelineMeta pipelineWith(DatabaseJoinMeta meta) { + String pluginId = PluginRegistry.getInstance().getPluginId(TransformPluginType.class, meta); + assertNotNull(pluginId, "Database join transform must be registered via HopEnvironment.init()"); + PipelineMeta pipelineMeta = new PipelineMeta(); + pipelineMeta.addTransform(new TransformMeta(pluginId, TRANSFORM_NAME, meta)); + return pipelineMeta; + } +} diff --git a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java index 4fbe2c9bc7b..61f43d877a2 100644 --- a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java +++ b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java @@ -33,6 +33,8 @@ import org.apache.hop.ui.core.PropsUi; import org.apache.hop.ui.core.dialog.BaseDialog; import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.gui.GuiCompositeWidgets; +import org.apache.hop.ui.core.gui.GuiCompositeWidgetsAdapter; import org.apache.hop.ui.core.widget.ColumnInfo; import org.apache.hop.ui.core.widget.StyledTextComp; import org.apache.hop.ui.core.widget.TableView; @@ -45,29 +47,34 @@ import org.eclipse.swt.layout.FormAttachment; import org.eclipse.swt.layout.FormData; import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Label; -import org.eclipse.swt.widgets.Listener; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.TableItem; -import org.eclipse.swt.widgets.Text; public class WriteToLogDialog extends BaseTransformDialog { private static final Class PKG = WriteToLogDialog.class; private final WriteToLogMeta input; + /** + * Hand-built on purpose. The combo has to show the translated {@link LogLevel} descriptions and + * map the selection back by position. An annotated {@code GuiElementType.COMBO} derives its items + * from {@code Enum.toString()} and stores the selected label, which is neither translated nor + * round-trippable - see {@link WriteToLogDialogTest} for the case that guards this. + */ private CCombo wLoglevel; - private Button wPrintHeader; + private StyledTextComp wLogMessage; private TableView wFields; - private Button wLimitRows; - private Label wlLimitRowsNumber; - private Text wLimitRowsNumber; private final List inputFields = new ArrayList<>(); private ColumnInfo[] colinf; + private GuiCompositeWidgets widgets; + public WriteToLogDialog( Shell parent, IVariables variables, WriteToLogMeta transformMeta, PipelineMeta pipelineMeta) { super(parent, variables, transformMeta, pipelineMeta); @@ -80,192 +87,286 @@ public String open() { buildButtonBar().ok(e -> ok()).get(e -> get()).cancel(e -> cancel()).build(); - Listener lsModify = event -> input.setChanged(); changed = input.hasChanged(); - // Log Level - Label wlLoglevel = new Label(shell, SWT.RIGHT); + widgets = + GuiCompositeWidgets.addScrolledComposite( + shell, + variables, + wSpacer, + wOk, + WriteToLogMeta.GUI_PLUGIN_ELEMENT_PARENT_ID, + input, + w -> { + // Extra-group builders run during createCompositeWidgets, before + // addScrolledComposite returns. Keep the field assigned so they can look up the + // widgets already placed on the same tab. + widgets = w; + w.registerExtraGroup( + BaseMessages.getString(PKG, "WriteToLog.Tab.Options"), + "0100", + null, + this::addLogLevel); + w.registerExtraGroup( + BaseMessages.getString(PKG, "WriteToLog.Tab.Message"), + "0200", + null, + this::addMessage); + }); + + widgets.setWidgetsListener( + new GuiCompositeWidgetsAdapter() { + @Override + public void widgetModified( + GuiCompositeWidgets compositeWidgets, Control changedWidget, String widgetId) { + if (!loading) { + input.setChanged(); + } + if (WriteToLogMeta.WIDGET_LIMIT_ROWS.equals(widgetId)) { + enableFields(); + } + } + + @Override + public void persistContents(GuiCompositeWidgets compositeWidgets) { + persistLogLevel(); + persistLogMessage(); + persistFields(); + } + }); + + populateLogLevel(); + populateLogMessage(); + populateFields(); + enableFields(); + searchPrevTransformFields(); + + input.setChanged(changed); + focusTransformName(); + BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel()); + + return transformName; + } + + /** + * The log level combo, added to the Options tab. Extra-group contents share the tab composite + * with the annotated fields, so the row has to be hung below the last of them - anchoring it to + * the top of the composite draws it on top of the first annotated row. + */ + private void addLogLevel(Composite parent) { + Label wlLoglevel = new Label(parent, SWT.RIGHT); wlLoglevel.setText(BaseMessages.getString(PKG, "WriteToLogDialog.Loglevel.Label")); PropsUi.setLook(wlLoglevel); FormData fdlLoglevel = new FormData(); fdlLoglevel.left = new FormAttachment(0, 0); fdlLoglevel.right = new FormAttachment(middle, -margin); - fdlLoglevel.top = new FormAttachment(wSpacer, margin); + // WIDGET_LIMIT_ROWS_NUMBER is the last annotated field on this tab (order 0400). + Control lastOnTab = widgets.getWidgetsMap().get(WriteToLogMeta.WIDGET_LIMIT_ROWS_NUMBER); + fdlLoglevel.top = + lastOnTab == null ? new FormAttachment(0, 0) : new FormAttachment(lastOnTab, margin); wlLoglevel.setLayoutData(fdlLoglevel); - wLoglevel = new CCombo(shell, SWT.SINGLE | SWT.READ_ONLY | SWT.BORDER); + + wLoglevel = new CCombo(parent, SWT.SINGLE | SWT.READ_ONLY | SWT.BORDER); wLoglevel.setItems(LogLevel.getLogLevelDescriptions()); PropsUi.setLook(wLoglevel); + wLoglevel.setToolTipText(BaseMessages.getString(PKG, "WriteToLogDialog.Loglevel.Tooltip")); FormData fdLoglevel = new FormData(); fdLoglevel.left = new FormAttachment(middle, 0); - fdLoglevel.top = new FormAttachment(wSpacer, margin); + fdLoglevel.top = new FormAttachment(wlLoglevel, 0, SWT.CENTER); fdLoglevel.right = new FormAttachment(100, 0); wLoglevel.setLayoutData(fdLoglevel); - wLoglevel.addListener(SWT.Selection, lsModify); - - // print header? - Label wlPrintHeader = new Label(shell, SWT.RIGHT); - wlPrintHeader.setText(BaseMessages.getString(PKG, "WriteToLogDialog.PrintHeader.Label")); - PropsUi.setLook(wlPrintHeader); - FormData fdlPrintHeader = new FormData(); - fdlPrintHeader.left = new FormAttachment(0, 0); - fdlPrintHeader.top = new FormAttachment(wLoglevel, margin); - fdlPrintHeader.right = new FormAttachment(middle, -margin); - wlPrintHeader.setLayoutData(fdlPrintHeader); - wPrintHeader = new Button(shell, SWT.CHECK); - wPrintHeader.setToolTipText( - BaseMessages.getString(PKG, "WriteToLogDialog.PrintHeader.Tooltip")); - PropsUi.setLook(wPrintHeader); - FormData fdPrintHeader = new FormData(); - fdPrintHeader.left = new FormAttachment(middle, 0); - fdPrintHeader.top = new FormAttachment(wlPrintHeader, 0, SWT.CENTER); - fdPrintHeader.right = new FormAttachment(100, 0); - wPrintHeader.setLayoutData(fdPrintHeader); - wPrintHeader.addListener(SWT.Selection, lsModify); - - // Limit output? - // ICache? - Label wlLimitRows = new Label(shell, SWT.RIGHT); - wlLimitRows.setText(BaseMessages.getString(PKG, "WriteToLogDialog.LimitRows.Label")); - PropsUi.setLook(wlLimitRows); - FormData fdlLimitRows = new FormData(); - fdlLimitRows.left = new FormAttachment(0, 0); - fdlLimitRows.right = new FormAttachment(middle, -margin); - fdlLimitRows.top = new FormAttachment(wPrintHeader, margin); - wlLimitRows.setLayoutData(fdlLimitRows); - wLimitRows = new Button(shell, SWT.CHECK); - PropsUi.setLook(wLimitRows); - FormData fdLimitRows = new FormData(); - fdLimitRows.left = new FormAttachment(middle, 0); - fdLimitRows.top = new FormAttachment(wlLimitRows, 0, SWT.CENTER); - wLimitRows.setLayoutData(fdLimitRows); - wLimitRows.addListener(SWT.Selection, lsModify); - wLimitRows.addListener(SWT.Selection, e -> enableFields()); - - // LimitRows size line - wlLimitRowsNumber = new Label(shell, SWT.RIGHT); - wlLimitRowsNumber.setText( - BaseMessages.getString(PKG, "WriteToLogDialog.LimitRowsNumber.Label")); - PropsUi.setLook(wlLimitRowsNumber); - wlLimitRowsNumber.setEnabled(input.isLimitRows()); - FormData fdlLimitRowsNumber = new FormData(); - fdlLimitRowsNumber.left = new FormAttachment(0, 0); - fdlLimitRowsNumber.right = new FormAttachment(middle, -margin); - fdlLimitRowsNumber.top = new FormAttachment(wLimitRows, margin); - wlLimitRowsNumber.setLayoutData(fdlLimitRowsNumber); - wLimitRowsNumber = new Text(shell, SWT.SINGLE | SWT.LEFT | SWT.BORDER); - PropsUi.setLook(wLimitRowsNumber); - wLimitRowsNumber.setEnabled(input.isLimitRows()); - wLimitRowsNumber.addListener(SWT.Modify, lsModify); - FormData fdLimitRowsNumber = new FormData(); - fdLimitRowsNumber.left = new FormAttachment(middle, 0); - fdLimitRowsNumber.right = new FormAttachment(100, 0); - fdLimitRowsNumber.top = new FormAttachment(wLimitRows, margin); - wLimitRowsNumber.setLayoutData(fdLimitRowsNumber); - - // Log message to display - Label wlLogMessage = new Label(shell, SWT.RIGHT); - wlLogMessage.setText(BaseMessages.getString(PKG, "WriteToLogDialog.Shell.Title")); + wLoglevel.addListener(SWT.Selection, e -> input.setChanged()); + } + + /** + * The Message tab: the log message template and the fields it can reference. The two belong on + * one tab because picking a field and referencing it in the template is a single task. + */ + private void addMessage(Composite parent) { + Label wlLogMessage = new Label(parent, SWT.NONE); + wlLogMessage.setText(BaseMessages.getString(PKG, "WriteToLogDialog.LogMessage.Label")); PropsUi.setLook(wlLogMessage); FormData fdlLogMessage = new FormData(); fdlLogMessage.left = new FormAttachment(0, 0); - fdlLogMessage.top = new FormAttachment(wLimitRowsNumber, margin); - fdlLogMessage.right = new FormAttachment(middle, -margin); + fdlLogMessage.right = new FormAttachment(100, 0); + fdlLogMessage.top = new FormAttachment(0, 0); wlLogMessage.setLayoutData(fdlLogMessage); wLogMessage = new StyledTextComp( variables, - shell, + parent, SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL, TextComposite.STYLE_TYPE_TEXT); PropsUi.setLook(wLogMessage, Props.WIDGET_STYLE_FIXED); + wLogMessage.addListener(SWT.Modify, e -> input.setChanged()); FormData fdLogMessage = new FormData(); - fdLogMessage.left = new FormAttachment(middle, 0); - fdLogMessage.top = new FormAttachment(wLimitRowsNumber, margin); - fdLogMessage.right = new FormAttachment(100, -margin); - fdLogMessage.height = (int) (125 * props.getZoomFactor()); + fdLogMessage.left = new FormAttachment(0, 0); + fdLogMessage.top = new FormAttachment(wlLogMessage, margin); + fdLogMessage.right = new FormAttachment(100, 0); + // A preferred height, not a fixed one: the field grid below keeps the rest of the tab, so the + // editor grows with the dialog instead of fighting it for space. + fdLogMessage.height = (int) (200 * props.getZoomFactor()); wLogMessage.setLayoutData(fdLogMessage); - wLogMessage.addListener(SWT.Modify, lsModify); - // Table with fields - Label wlFields = new Label(shell, SWT.NONE); + Label wlFields = new Label(parent, SWT.NONE); wlFields.setText(BaseMessages.getString(PKG, "WriteToLogDialog.Fields.Label")); PropsUi.setLook(wlFields); FormData fdlFields = new FormData(); fdlFields.left = new FormAttachment(0, 0); + fdlFields.right = new FormAttachment(100, 0); fdlFields.top = new FormAttachment(wLogMessage, margin); wlFields.setLayoutData(fdlFields); - final int fieldsCols = 1; - final int fieldsRows = input.getLogFields().size(); + colinf = + new ColumnInfo[] { + new ColumnInfo( + BaseMessages.getString(PKG, "WriteToLogDialog.Fieldname.Column"), + ColumnInfo.COLUMN_TYPE_CCOMBO, + new String[] {""}, + false) + }; - colinf = new ColumnInfo[fieldsCols]; - colinf[0] = - new ColumnInfo( - BaseMessages.getString(PKG, "WriteToLogDialog.Fieldname.Column"), - ColumnInfo.COLUMN_TYPE_CCOMBO, - new String[] {""}, - false); wFields = new TableView( variables, - shell, + parent, SWT.BORDER | SWT.FULL_SELECTION | SWT.MULTI, colinf, - fieldsRows, - event -> input.setChanged(), + 1, + e -> input.setChanged(), props); FormData fdFields = new FormData(); fdFields.left = new FormAttachment(0, 0); fdFields.top = new FormAttachment(wlFields, margin); fdFields.right = new FormAttachment(100, 0); - fdFields.bottom = new FormAttachment(100, -50); + fdFields.bottom = new FormAttachment(100, 0); wFields.setLayoutData(fdFields); + } + + private void populateLogLevel() { + LogLevel logLevel = input.getLogLevel(); + if (logLevel == null) { + logLevel = LogLevel.BASIC; + } + wLoglevel.select(logLevel.getLevel()); + } + + private void persistLogLevel() { + // The combo holds the translated descriptions in enum order: map by position, not by label. + int logLevelIndex = wLoglevel.getSelectionIndex(); + if (logLevelIndex < 0 || logLevelIndex >= LogLevel.values().length) { + input.setLogLevel(LogLevel.BASIC); + } else { + input.setLogLevel(LogLevel.values()[logLevelIndex]); + } + } + + private void populateLogMessage() { + wLogMessage.setText(Const.NVL(input.getLogMessage(), "")); + } + + private void persistLogMessage() { + input.setLogMessage(Const.NVL(wLogMessage.getText(), "")); + } + + private void populateFields() { + if (wFields == null || wFields.isDisposed()) { + return; + } + wFields.clearAll(); + if (input.getLogFields() != null) { + for (LogField field : input.getLogFields()) { + TableItem item = new TableItem(wFields.table, SWT.NONE); + if (field != null) { + item.setText(1, Const.NVL(field.getName(), "")); + } + } + } + if (wFields.table.getItemCount() == 0) { + new TableItem(wFields.table, SWT.NONE); + } + wFields.setRowNums(); + wFields.optWidth(true); + } + + private void persistFields() { + if (wFields == null || wFields.isDisposed()) { + return; + } + List fields = new ArrayList<>(); + for (TableItem item : wFields.getNonEmptyItems()) { + LogField field = new LogField(); + field.setName(item.getText(1)); + fields.add(field); + } + input.setLogFields(fields); + } + private void setComboBoxes() { + // Something was changed in the row. + // + if (colinf == null) { + return; + } + colinf[0].setComboValues(ConstUi.sortFieldNames(inputFields)); + } + + private void searchPrevTransformFields() { + // // Search the fields in the background // - final Runnable runnable = + BackgroundThreadFacade.start( () -> { TransformMeta transformMeta = pipelineMeta.findTransform(transformName); - if (transformMeta != null) { - try { - IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformMeta); - - // Remember these fields... - for (int i = 0; i < row.size(); i++) { - inputFields.add(row.getValueMeta(i).getName()); - } - setComboBoxes(); - } catch (HopException e) { - logError(BaseMessages.getString(PKG, "System.Dialog.GetFieldsFailed.Message")); + if (transformMeta == null) { + return; + } + try { + IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformMeta); + for (int i = 0; i < row.size(); i++) { + inputFields.add(row.getValueMeta(i).getName()); } + setComboBoxes(); + } catch (HopException e) { + logError(BaseMessages.getString(PKG, "System.Dialog.GetFieldsFailed.Message")); } - }; - BackgroundThreadFacade.start(runnable); + }); + } - getData(); - input.setChanged(changed); - focusTransformName(); - BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel()); + private void enableFields() { + setEnabled( + WriteToLogMeta.WIDGET_LIMIT_ROWS_NUMBER, isChecked(WriteToLogMeta.WIDGET_LIMIT_ROWS)); + } - return transformName; + private boolean isChecked(String widgetId) { + if (widgets == null) { + return false; + } + Control control = widgets.getWidgetsMap().get(widgetId); + return control instanceof Button button && button.getSelection(); } - protected void setComboBoxes() { - // Something was changed in the row. - // - String[] fieldNames = ConstUi.sortFieldNames(inputFields); - colinf[0].setComboValues(fieldNames); + private void setEnabled(String widgetId, boolean enabled) { + if (widgets == null) { + return; + } + Control label = widgets.getLabelsMap().get(widgetId); + if (label != null && !label.isDisposed()) { + label.setEnabled(enabled); + } + Control widget = widgets.getWidgetsMap().get(widgetId); + if (widget != null && !widget.isDisposed()) { + widget.setEnabled(enabled); + } } private void get() { try { IRowMeta r = pipelineMeta.getPrevTransformFields(variables, transformName); if (r != null) { - ITableItemInsertListener insertListener = (tableItem, v) -> true; - BaseTransformDialog.getFieldsFromPrevious( r, wFields, 1, new int[] {1}, new int[] {}, -1, -1, insertListener); } @@ -278,34 +379,6 @@ private void get() { } } - /** Copy information from the meta-data input to the dialog fields. */ - public void getData() { - - wPrintHeader.setSelection(input.isDisplayHeader()); - wLimitRows.setSelection(input.isLimitRows()); - wLimitRowsNumber.setText("" + input.getLimitRowsNumber()); - - LogLevel logLevel = input.getLogLevel(); - if (logLevel == null) { - logLevel = LogLevel.BASIC; - } - wLoglevel.select(logLevel.getLevel()); - - if (input.getLogMessage() != null) { - wLogMessage.setText(input.getLogMessage()); - } - - for (int i = 0; i < input.getLogFields().size(); i++) { - LogField field = input.getLogFields().get(i); - TableItem ti = wFields.table.getItem(i); - ti.setText(0, "" + (i + 1)); - ti.setText(1, field.getName()); - } - - wFields.setRowNums(); - wFields.optWidth(true); - } - private void cancel() { transformName = null; input.setChanged(changed); @@ -318,40 +391,11 @@ private void ok() { } transformName = wTransformName.getText(); // return value - input.setDisplayHeader(wPrintHeader.getSelection()); - input.setLimitRows(wLimitRows.getSelection()); - input.setLimitRowsNumber(Const.toInt(wLimitRowsNumber.getText(), 0)); - - // The combo holds the translated descriptions in enum order: map by position, not by label. - int logLevelIndex = wLoglevel.getSelectionIndex(); - if (logLevelIndex < 0 || logLevelIndex >= LogLevel.values().length) { - input.setLogLevel(LogLevel.BASIC); - } else { - input.setLogLevel(LogLevel.values()[logLevelIndex]); - } - - if (!Utils.isEmpty(wLogMessage.getText())) { - input.setLogMessage(wLogMessage.getText()); - } else { - input.setLogMessage(""); - } - - int nrFields = wFields.nrNonEmpty(); - List fields = new ArrayList<>(nrFields); - for (int i = 0; i < nrFields; i++) { - TableItem item = wFields.getNonEmpty(i); - LogField field = new LogField(); - field.setName(item.getText(1)); - fields.add(field); - } - input.setLogFields(fields); + widgets.getWidgetsContents(input, WriteToLogMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); + persistLogLevel(); + persistLogMessage(); + persistFields(); dispose(); } - - private void enableFields() { - - wLimitRowsNumber.setEnabled(wLimitRows.getSelection()); - wlLimitRowsNumber.setEnabled(wLimitRows.getSelection()); - } } diff --git a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogMeta.java b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogMeta.java index f0c8e84dc88..7d612cfc302 100644 --- a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogMeta.java +++ b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogMeta.java @@ -26,6 +26,10 @@ import org.apache.hop.core.ICheckResult; import org.apache.hop.core.annotations.Transform; import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.gui.plugin.GuiElementType; +import org.apache.hop.core.gui.plugin.GuiPlugin; +import org.apache.hop.core.gui.plugin.GuiWidgetElement; +import org.apache.hop.core.gui.plugin.GuiWidgetGroupType; import org.apache.hop.core.logging.LogLevel; import org.apache.hop.core.row.IRowMeta; import org.apache.hop.core.variables.IVariables; @@ -46,23 +50,66 @@ categoryDescription = "i18n:org.apache.hop.pipeline.transform:BaseTransform.Category.Utility", keywords = "i18n::WriteToLog.Keyword", documentationUrl = "/pipeline/transforms/writetolog.html") +@GuiPlugin @Getter @Setter public class WriteToLogMeta extends BaseTransformMeta { private static final Class PKG = WriteToLogMeta.class; + public static final String GUI_PLUGIN_ELEMENT_PARENT_ID = "WRITE_TO_LOG_DIALOG_OPTIONS"; + + public static final String GROUP_OPTIONS = "i18n::WriteToLog.Tab.Options"; + public static final String GROUP_OPTIONS_ORDER = "0100"; + public static final String GROUP_MESSAGE = "i18n::WriteToLog.Tab.Message"; + public static final String GROUP_MESSAGE_ORDER = "0200"; + + public static final String WIDGET_LOG_LEVEL = "logLevel"; + public static final String WIDGET_DISPLAY_HEADER = "displayHeader"; + public static final String WIDGET_LIMIT_ROWS = "limitRows"; + public static final String WIDGET_LIMIT_ROWS_NUMBER = "limitRowsNumber"; + + @GuiWidgetElement( + id = WIDGET_DISPLAY_HEADER, + order = "0200", + type = GuiElementType.CHECKBOX, + label = "i18n::WriteToLogMeta.DisplayHeader.Label", + toolTip = "i18n::WriteToLogMeta.DisplayHeader.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_OPTIONS, + groupOrder = GROUP_OPTIONS_ORDER) @HopMetadataProperty( key = "displayHeader", injectionKey = "DISPLAY_HEADER", injectionKeyDescription = "WriteToLogMeta.Injection.DisplayHeader") private boolean displayHeader; + @GuiWidgetElement( + id = WIDGET_LIMIT_ROWS, + order = "0300", + type = GuiElementType.CHECKBOX, + label = "i18n::WriteToLogMeta.LimitRows.Label", + toolTip = "i18n::WriteToLogMeta.LimitRows.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_OPTIONS, + groupOrder = GROUP_OPTIONS_ORDER) @HopMetadataProperty( key = "limitRows", injectionKey = "LIMIT_ROWS", injectionKeyDescription = "WriteToLogMeta.Injection.LimitRows") private boolean limitRows; + @GuiWidgetElement( + id = WIDGET_LIMIT_ROWS_NUMBER, + order = "0400", + type = GuiElementType.TEXT, + label = "i18n::WriteToLogMeta.LimitRowsNumber.Label", + toolTip = "i18n::WriteToLogMeta.LimitRowsNumber.Tooltip", + parentId = GUI_PLUGIN_ELEMENT_PARENT_ID, + groupType = GuiWidgetGroupType.TABS, + group = GROUP_OPTIONS, + groupOrder = GROUP_OPTIONS_ORDER) @HopMetadataProperty( key = "limitRowsNumber", injectionKey = "LIMIT_ROWS_NUMBER", diff --git a/plugins/transforms/writetolog/src/main/resources/org/apache/hop/pipeline/transforms/writetolog/messages/messages_en_US.properties b/plugins/transforms/writetolog/src/main/resources/org/apache/hop/pipeline/transforms/writetolog/messages/messages_en_US.properties index 5adaaa50d8e..a65bdbd4c35 100644 --- a/plugins/transforms/writetolog/src/main/resources/org/apache/hop/pipeline/transforms/writetolog/messages/messages_en_US.properties +++ b/plugins/transforms/writetolog/src/main/resources/org/apache/hop/pipeline/transforms/writetolog/messages/messages_en_US.properties @@ -20,15 +20,25 @@ WriteToLog.Log.CanNotFindField=Can not find field [{0}] in the input stream! WriteToLog.Log.NLigne=Linenr {0} WriteToLog.Keyword=log,debug,print,message,trace WriteToLog.Name=Write to log +WriteToLog.Tab.Options=Options +WriteToLog.Tab.Message=Message WriteToLogDialog.Fieldname.Column=Field WriteToLogDialog.Fields.Label=Fields WriteToLogDialog.LimitRows.Label=Limit rows WriteToLogDialog.LimitRowsNumber.Label=Nr of rows to print +WriteToLogDialog.LogMessage.Label=Log message WriteToLogDialog.Loglevel.Label=Log level +WriteToLogDialog.Loglevel.Tooltip=The severity the message is logged with WriteToLogDialog.PrintHeader.Label=Print header -WriteToLogDialog.PrintHeader.Tooltip=Print header +WriteToLogDialog.PrintHeader.Tooltip=Print the field names above the values WriteToLogDialog.Shell.Title=Write to log WriteToLogDialog.TransformName.Label=Transform name +WriteToLogMeta.DisplayHeader.Label=Print header +WriteToLogMeta.DisplayHeader.Tooltip=Print the field names above the values +WriteToLogMeta.LimitRows.Label=Limit rows +WriteToLogMeta.LimitRows.Tooltip=Stop logging after the given number of rows +WriteToLogMeta.LimitRowsNumber.Label=Nr of rows to print +WriteToLogMeta.LimitRowsNumber.Tooltip=Number of rows to log before stopping. Only used when limit rows is on. WriteToLogMeta.CheckResult.AllFieldsFound=All fields are found in the input stream. WriteToLogMeta.CheckResult.FieldsFound=Fields that were not found in input stream:\n\n{0} WriteToLogMeta.CheckResult.NoFieldsEntered=No fields are entered. diff --git a/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java b/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java index 708477854aa..5fbf7105851 100644 --- a/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java +++ b/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java @@ -19,15 +19,21 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Properties; import java.util.stream.Stream; +import org.apache.hop.core.gui.plugin.GuiRegistry; +import org.apache.hop.core.gui.plugin.GuiWidgetElement; import org.apache.hop.core.logging.LogLevel; import org.apache.hop.core.plugins.PluginRegistry; import org.apache.hop.core.plugins.TransformPluginType; @@ -36,8 +42,15 @@ import org.apache.hop.pipeline.PipelineMeta; import org.apache.hop.pipeline.transform.TransformMeta; import org.apache.hop.ui.testing.SwtBotTestBase; +import org.eclipse.swt.custom.CCombo; +import org.eclipse.swt.graphics.Rectangle; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Shell; import org.eclipse.swtbot.swt.finder.SWTBot; import org.eclipse.swtbot.swt.finder.widgets.SWTBotCCombo; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -66,6 +79,27 @@ static Stream localeCodes() { return Arrays.stream(GlobalMessages.localeCodes); } + /** + * The dialog now builds its tabs from the annotated widgets on {@link WriteToLogMeta}. Those are + * looked up in the registry by class name, and the unit-test JVM does not always run the {@code + * GuiPluginType} scan, so register them the way that scan does. + */ + @BeforeAll + static void registerMetaWidgets() { + GuiRegistry registry = GuiRegistry.getInstance(); + if (registry.findGuiElements( + WriteToLogMeta.class.getName(), WriteToLogMeta.GUI_PLUGIN_ELEMENT_PARENT_ID) + != null) { + return; + } + for (Field field : WriteToLogMeta.class.getDeclaredFields()) { + GuiWidgetElement element = field.getAnnotation(GuiWidgetElement.class); + if (element != null) { + registry.addGuiWidgetElement(WriteToLogMeta.class.getName(), element, field); + } + } + } + @ParameterizedTest(name = "{0}") @MethodSource("localeCodes") void okStoresEverySelectedLogLevel(String localeCode) throws IOException { @@ -126,6 +160,77 @@ void reopeningShowsTheStoredLogLevel() { } } + /** + * The log level combo lives in an extra group on the Options tab, alongside the annotated fields. + * Anchoring it to the top of the tab composite instead of below the last annotated widget drew it + * on top of the Print header row (#8655). Compare the actual control rectangles so the row cannot + * silently land on another one again. + */ + @Test + void theLogLevelRowDoesNotOverlapTheOtherOptionsFields() { + WriteToLogMeta meta = new WriteToLogMeta(); + PipelineMeta pipelineMeta = pipelineWith(meta); + + withDialog( + parent -> new WriteToLogDialog(parent, new Variables(), meta, pipelineMeta).open(), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + Rectangle[] logLevel = new Rectangle[1]; + List checkboxes = new ArrayList<>(); + display.syncExec( + () -> { + for (Rectangle rectangle : controlsOfType(dialogShell(), CCombo.class)) { + logLevel[0] = rectangle; + } + for (Rectangle rectangle : controlsOfType(dialogShell(), Button.class)) { + if (rectangle.width > 0 && rectangle.height > 0) { + checkboxes.add(rectangle); + } + } + }); + + assertNotNull(logLevel[0], "expected the log level combo on the Options tab"); + for (Rectangle checkbox : checkboxes) { + assertFalse( + logLevel[0].intersects(checkbox), + "the log level combo at " + logLevel[0] + " overlaps a checkbox at " + checkbox); + } + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + private static Shell dialogShell() { + for (Shell shell : display.getShells()) { + if (SHELL_TITLE.equals(shell.getText()) && !shell.isDisposed()) { + return shell; + } + } + throw new AssertionError("the '" + SHELL_TITLE + "' dialog is not open"); + } + + /** + * Every visible control of the given type below the dialog shell, including in tab composites. + */ + private static List controlsOfType(Shell shell, Class type) { + List found = new ArrayList<>(); + collect(shell, type, found); + return found; + } + + private static void collect( + Control parent, Class type, List found) { + if (parent instanceof Composite composite) { + for (Control child : composite.getChildren()) { + if (!child.isDisposed() && type.isInstance(child)) { + found.add(child.getBounds()); + } + collect(child, type, found); + } + } + } + /** * The log level labels a Hop GUI running in the given language shows, in enum order. Keys a * translation lacks fall back to en_US, like {@code BaseMessages} does. From b5b18cc3d7993dc1233dbe7129b5b8c6f1111be6 Mon Sep 17 00:00:00 2001 From: mattcasters Date: Sat, 3 Oct 2026 11:37:02 +0200 Subject: [PATCH 2/3] Issue #8655 : Persist numeric fields, fit the SQL tab, and fix OK and browse --- .../databasejoin/DatabaseJoinDialog.java | 128 +++------- .../databasejoin/DatabaseJoinMeta.java | 46 ++-- .../messages/messages_en_US.properties | 2 - .../databasejoin/DatabaseJoinDialogTest.java | 232 ++++++++++++++++++ .../writetolog/WriteToLogDialogTest.java | 26 ++ .../gui/GuiCompositeWidgetsGroupTest.java | 70 ++++++ .../hop/ui/core/gui/GuiCompositeWidgets.java | 15 ++ 7 files changed, 406 insertions(+), 113 deletions(-) diff --git a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java index eb652e0c904..887e36830f0 100644 --- a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java +++ b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java @@ -30,6 +30,7 @@ import org.apache.hop.core.exception.HopFileException; import org.apache.hop.core.row.IRowMeta; import org.apache.hop.core.row.value.ValueMetaFactory; +import org.apache.hop.core.util.StringUtil; import org.apache.hop.core.util.Utils; import org.apache.hop.core.variables.IVariables; import org.apache.hop.core.vfs.HopVfs; @@ -43,7 +44,6 @@ import org.apache.hop.ui.core.dialog.MessageBox; import org.apache.hop.ui.core.gui.GuiCompositeWidgets; import org.apache.hop.ui.core.gui.GuiCompositeWidgetsAdapter; -import org.apache.hop.ui.core.gui.IGuiPluginCompositeButtonsListener; import org.apache.hop.ui.core.widget.ColumnInfo; import org.apache.hop.ui.core.widget.MetaSelectionLine; import org.apache.hop.ui.core.widget.SQLStyledTextComp; @@ -161,22 +161,6 @@ public void persistContents(GuiCompositeWidgets compositeWidgets) { connectionLine.addListener(SWT.Selection, e -> onConnectionChanged()); } - widgets.setCompositeButtonsListener( - new IGuiPluginCompositeButtonsListener() { - @Override - public void buttonPressed(Object sourceObject) { - // Flush the widgets before the no-op Meta method runs: browseSqlFromFile() reads the - // path from the widget, and the setWidgetsContents that follows a button press would - // otherwise re-read editor state that is about to change. - widgets.getWidgetsContents(input, DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); - } - - @Override - public void afterButtonPressed(Object sourceObject) { - browseSqlFromFile(); - } - }); - populateSqlEditor(); populateParameters(); enableFields(); @@ -190,9 +174,9 @@ public void afterButtonPressed(Object sourceObject) { } /** - * The SQL tab: the styled SQL editor with its line/column readout, plus the read-only table of - * parameters resolved from the SQL. The editor takes all remaining vertical space, which the old - * flat form could not do without squeezing the parameter grid below it. + * The SQL tab: the styled SQL editor, its line/column readout, and the read-only table of + * parameters resolved from the SQL. The table is pinned to the bottom of the tab and the readout + * sits above it, so the editor ends at the readout instead of covering both. */ private void addSql(Composite parent) { Label wlSql = new Label(parent, SWT.NONE); @@ -260,30 +244,14 @@ public void mouseUp(MouseEvent e) { setPosition(); } }); - FormData fdSql = new FormData(); - fdSql.left = new FormAttachment(0, 0); - fdSql.top = new FormAttachment(wlSql, margin); - fdSql.right = new FormAttachment(100, 0); - fdSql.bottom = new FormAttachment(100, 0); - wSql.setLayoutData(fdSql); wlPosition = new Label(parent, SWT.NONE); PropsUi.setLook(wlPosition); - FormData fdlPosition = new FormData(); - fdlPosition.left = new FormAttachment(0, 0); - fdlPosition.top = new FormAttachment(wSql, margin); - fdlPosition.right = new FormAttachment(100, 0); - wlPosition.setLayoutData(fdlPosition); Label wlResolvedParam = new Label(parent, SWT.NONE); wlResolvedParam.setText( BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Label")); PropsUi.setLook(wlResolvedParam); - FormData fdlResolvedParam = new FormData(); - fdlResolvedParam.left = new FormAttachment(0, 0); - fdlResolvedParam.right = new FormAttachment(100, 0); - fdlResolvedParam.top = new FormAttachment(wlPosition, margin); - wlResolvedParam.setLayoutData(fdlResolvedParam); ciResolvedParam = new ColumnInfo[3]; ciResolvedParam[0] = @@ -316,12 +284,33 @@ public void mouseUp(MouseEvent e) { null, false, false); + // Pin the table to the bottom of the tab, stack the readout and its label above it, and end + // the editor at the readout. Attaching the editor to the bottom leaves the two below the tab. FormData fdResolvedParam = new FormData(); fdResolvedParam.left = new FormAttachment(0, 0); - fdResolvedParam.top = new FormAttachment(wlResolvedParam, margin); fdResolvedParam.right = new FormAttachment(100, 0); + fdResolvedParam.bottom = new FormAttachment(100, 0); fdResolvedParam.height = (int) (90 * props.getZoomFactor()); wResolvedParam.setLayoutData(fdResolvedParam); + + FormData fdlResolvedParam = new FormData(); + fdlResolvedParam.left = new FormAttachment(0, 0); + fdlResolvedParam.right = new FormAttachment(100, 0); + fdlResolvedParam.bottom = new FormAttachment(wResolvedParam, -margin); + wlResolvedParam.setLayoutData(fdlResolvedParam); + + FormData fdlPosition = new FormData(); + fdlPosition.left = new FormAttachment(0, 0); + fdlPosition.right = new FormAttachment(100, 0); + fdlPosition.bottom = new FormAttachment(wlResolvedParam, -margin); + wlPosition.setLayoutData(fdlPosition); + + FormData fdSql = new FormData(); + fdSql.left = new FormAttachment(0, 0); + fdSql.top = new FormAttachment(wlSql, margin); + fdSql.right = new FormAttachment(100, 0); + fdSql.bottom = new FormAttachment(wlPosition, -margin); + wSql.setLayoutData(fdSql); } /** The Parameters tab: the grid that maps positional {@code ?} markers to input fields. */ @@ -596,33 +585,6 @@ private void loadSqlFromFileAndSetReadOnly() { } } - /** - * Open a file dialog on the transform dialog shell, load the chosen SQL into the editor and make - * the editor read-only. Runs from the composite button listener rather than the annotated Meta - * method: {@code setWidgetsContents} after a Meta mutation re-reads widgets that are still empty - * and would wipe the selection. - */ - private void browseSqlFromFile() { - String path = - BaseDialog.presentFileDialog( - shell, - null, - variables, - new String[] {"*.sql", "*"}, - new String[] { - BaseMessages.getString(PKG, "DatabaseJoinDialog.SqlFiles"), - BaseMessages.getString(PKG, "System.FileType.AllFiles") - }, - false); - if (path == null) { - return; - } - writeWidgetText(DatabaseJoinMeta.WIDGET_SQL_FROM_FILE, path); - input.setSqlFromFile(path); - loadSqlFromFileAndSetReadOnly(); - input.setChanged(); - } - private void populateSqlEditor() { wSql.setText(Const.NVL(input.getSql(), "")); if (!Utils.isEmpty(input.getSqlFromFile())) { @@ -687,21 +649,6 @@ private String readWidgetText(String widgetId) { return ""; } - private void writeWidgetText(String widgetId, String value) { - if (widgets == null) { - return; - } - Control control = widgets.getWidgetsMap().get(widgetId); - String text = Const.NVL(value, ""); - if (control instanceof TextVar textVar) { - textVar.setText(text); - } else if (control instanceof Text widget) { - widget.setText(text); - } else if (control instanceof MetaSelectionLine line) { - line.setText(text); - } - } - private void searchPrevTransformFields() { // // Search the fields in the background @@ -741,24 +688,25 @@ private void ok() { return; } - widgets.getWidgetsContents(input, DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); - persistSqlEditor(); - persistParameters(); - - transformName = wTransformName.getText(); // return value - - if (pipelineMeta.findDatabase(readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION), variables) - == null) { + // Check the connection before copying widgets into the meta. A name that still holds a + // variable cannot be resolved here: warn, then save. Anything else stays open and unsaved. + String connectionName = readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION); + if (pipelineMeta.findDatabase(connectionName, variables) == null) { MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_ERROR); mb.setMessage( BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogMessage")); mb.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.InvalidConnection.DialogTitle")); mb.open(); - // Keep the dialog open: disposing here would commit the unusable connection that was just - // reported as invalid, leaving the transform broken with no chance to fix it. - return; + if (!StringUtil.containsVariableToken(connectionName)) { + return; + } } + widgets.getWidgetsContents(input, DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); + persistSqlEditor(); + persistParameters(); + + transformName = wTransformName.getText(); // return value dispose(); } diff --git a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java index 21eadac1cb2..351e726c42c 100644 --- a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java +++ b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinMeta.java @@ -38,6 +38,7 @@ import org.apache.hop.core.gui.plugin.GuiPlugin; import org.apache.hop.core.gui.plugin.GuiWidgetElement; import org.apache.hop.core.gui.plugin.GuiWidgetGroupType; +import org.apache.hop.core.gui.plugin.ITypeFilename; import org.apache.hop.core.row.IRowMeta; import org.apache.hop.core.row.IValueMeta; import org.apache.hop.core.row.RowMeta; @@ -88,7 +89,6 @@ public class DatabaseJoinMeta extends BaseTransformMeta { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + + textWithLabel(dialog, "DatabaseJoinMeta.CacheSize.Label").setText("15"); + selectTab(dialog, TAB_SQL); + textWithLabel(dialog, "DatabaseJoinMeta.RowLimit.Label").setText("25"); + dialog.button(buttonLabel("System.Button.OK")).click(); + }); + + assertEquals(15, meta.getCacheSize(), "OK must store the cache size typed into the field"); + assertEquals(25, meta.getRowLimit(), "OK must store the row limit typed into the field"); + } + + /** + * The editor used to be attached to the bottom of the tab, so the line/column readout and the + * resolved-parameter table were laid out below the client area. SWTBot still found the table. + */ + @Test + void theSqlTabKeepsTheEditorReadoutAndResolvedParametersInsideTheTab() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setConnection(CONNECTION); + meta.setSql(SQL); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + selectTab(dialog, TAB_SQL); + + Rectangle[] bounds = new Rectangle[4]; + display.syncExec( + () -> { + Shell shell = dialogShell(); + shell.setSize(1000, 900); + shell.layout(true, true); + + CTabItem sqlTab = sqlTabItem(dialog); + Control tabBody = sqlTab.getControl(); + tabBody.getParent().layout(true, true); + shell.layout(true, true); + + Rectangle tabArea = displayBounds(tabBody); + Rectangle editor = null; + Rectangle readout = null; + Rectangle table = null; + for (Control control : controlsUnder(tabBody)) { + if (control instanceof StyledText && editor == null) { + editor = displayBounds(control); + } else if (control instanceof Table && table == null) { + table = displayBounds(control); + } else if (control instanceof Label label && isPositionReadout(label.getText())) { + readout = displayBounds(control); + } + } + bounds[0] = tabArea; + bounds[1] = editor; + bounds[2] = readout; + bounds[3] = table; + }); + + Rectangle tabArea = bounds[0]; + Rectangle editor = bounds[1]; + Rectangle readout = bounds[2]; + Rectangle table = bounds[3]; + assertNotNull(editor, "expected the SQL editor on the SQL tab"); + assertNotNull(readout, "expected the line/column readout on the SQL tab"); + assertNotNull(table, "expected the resolved-parameter table on the SQL tab"); + assertTrue(contains(tabArea, editor), "editor " + editor + " outside tab " + tabArea); + assertTrue(contains(tabArea, readout), "readout " + readout + " outside tab " + tabArea); + assertTrue(contains(tabArea, table), "table " + table + " outside tab " + tabArea); + assertTrue( + editor.y + editor.height <= readout.y, + "the editor must end at the readout, editor " + editor + " readout " + readout); + assertTrue( + readout.y + readout.height <= table.y, + "the readout must sit above the table, readout " + readout + " table " + table); + + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + } + + /** + * A variable connection is only resolved at runtime. OK warns and still saves, and a missing + * connection does not write the widgets before the check, so Cancel drops the edits. + */ + @Test + void okWarnsAndSavesAVariableConnection() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setConnection(CONNECTION); + meta.setSql(SQL); + meta.setRowLimit(7); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + dialog.ccomboBox(0).setText("${DB}"); + selectTab(dialog, TAB_SQL); + textWithLabel(dialog, "DatabaseJoinMeta.RowLimit.Label").setText("11"); + dialog.button(buttonLabel("System.Button.OK")).click(); + + SWTBot error = bot.shell(ERROR_TITLE).activate().bot(); + error.button(buttonLabel("System.Button.OK")).click(); + + assertFalse(shellIsOpen(dialog), "a variable connection must close after the warning"); + }); + + assertEquals( + "${DB}", meta.getConnection(), "OK must keep a connection name that is a variable"); + assertEquals(11, meta.getRowLimit(), "OK must save the other edits along with the variable"); + assertEquals(SQL, meta.getSql()); + } + + @Test + void okDoesNotKeepEditsWhenTheConnectionIsInvalid() { + DatabaseJoinMeta meta = new DatabaseJoinMeta(); + meta.setSql(SQL); + meta.setRowLimit(7); + + withDialog( + openerFor(meta), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + selectTab(dialog, TAB_SQL); + textWithLabel(dialog, "DatabaseJoinMeta.RowLimit.Label").setText("99"); + dialog.button(buttonLabel("System.Button.OK")).click(); + + SWTBot error = bot.shell(ERROR_TITLE).activate().bot(); + error.button(buttonLabel("System.Button.OK")).click(); + + assertTrue(shellIsOpen(dialog), "an unknown connection must leave the dialog open"); + dialog.button(buttonLabel("System.Button.Cancel")).click(); + }); + + assertEquals( + 7, meta.getRowLimit(), "Cancel after a rejected OK must drop the edited row limit"); + assertEquals(SQL, meta.getSql()); + } + + private SWTBotText textWithLabel(SWTBot dialog, String key) { + return dialog.textWithLabel(BaseMessages.getString(PKG, key)); + } + + private boolean isPositionReadout(String text) { + String sample = BaseMessages.getString(PKG, "DatabaseJoinDialog.Position.Label", "1", "0"); + int marker = sample.indexOf('1'); + String prefix = marker > 0 ? sample.substring(0, marker) : "Line "; + return text != null && text.startsWith(prefix); + } + + private CTabItem sqlTabItem(SWTBot dialog) { + CTabFolder folder = tabFolder(dialog); + for (CTabItem item : folder.getItems()) { + if (TAB_SQL.equals(item.getText().trim())) { + return item; + } + } + throw new AssertionError("expected a '" + TAB_SQL + "' tab"); + } + + private static Shell dialogShell() { + for (Shell shell : display.getShells()) { + if (SHELL_TITLE.equals(shell.getText()) && !shell.isDisposed()) { + return shell; + } + } + throw new AssertionError("the '" + SHELL_TITLE + "' dialog is not open"); + } + + private static Rectangle displayBounds(Control control) { + Rectangle bounds = control.getBounds(); + Point origin = control.getParent().toDisplay(bounds.x, bounds.y); + return new Rectangle(origin.x, origin.y, bounds.width, bounds.height); + } + + private static boolean contains(Rectangle outer, Rectangle inner) { + return inner.width > 0 + && inner.height > 0 + && outer.contains(inner.x, inner.y) + && outer.contains(inner.x + inner.width - 1, inner.y + inner.height - 1); + } + + private static List controlsUnder(Control parent) { + List found = new ArrayList<>(); + collectControls(parent, found); + return found; + } + + private static void collectControls(Control parent, List found) { + if (parent instanceof Composite composite) { + for (Control child : composite.getChildren()) { + if (!child.isDisposed()) { + found.add(child); + collectControls(child, found); + } + } + } + } + private boolean shellIsOpen(SWTBot dialog) { for (SWTBotShell shell : dialog.shells()) { if (SHELL_TITLE.equals(shell.getText()) && shell.isOpen()) { @@ -284,6 +507,15 @@ private static PipelineMeta pipelineWith(DatabaseJoinMeta meta) { assertNotNull(pluginId, "Database join transform must be registered via HopEnvironment.init()"); PipelineMeta pipelineMeta = new PipelineMeta(); pipelineMeta.addTransform(new TransformMeta(pluginId, TRANSFORM_NAME, meta)); + MemoryMetadataProvider provider = new MemoryMetadataProvider(); + DatabaseMeta database = new DatabaseMeta(); + database.setName(CONNECTION); + try { + provider.getSerializer(DatabaseMeta.class).save(database); + } catch (HopException e) { + throw new IllegalStateException(e); + } + pipelineMeta.setMetadataProvider(provider); return pipelineMeta; } } diff --git a/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java b/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java index 5fbf7105851..a766c426be9 100644 --- a/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java +++ b/plugins/transforms/writetolog/src/test/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialogTest.java @@ -38,6 +38,7 @@ import org.apache.hop.core.plugins.PluginRegistry; import org.apache.hop.core.plugins.TransformPluginType; import org.apache.hop.core.variables.Variables; +import org.apache.hop.i18n.BaseMessages; import org.apache.hop.i18n.GlobalMessages; import org.apache.hop.pipeline.PipelineMeta; import org.apache.hop.pipeline.transform.TransformMeta; @@ -50,6 +51,7 @@ import org.eclipse.swt.widgets.Shell; import org.eclipse.swtbot.swt.finder.SWTBot; import org.eclipse.swtbot.swt.finder.widgets.SWTBotCCombo; +import org.eclipse.swtbot.swt.finder.widgets.SWTBotText; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -141,6 +143,30 @@ void okStoresEverySelectedLogLevel(String localeCode) throws IOException { } } + /** "Nr of rows to print" is an int behind a text widget. OK must store the edited number. */ + @Test + void okStoresAnEditedRowLimit() { + WriteToLogMeta meta = new WriteToLogMeta(); + meta.setLimitRows(true); + meta.setLimitRowsNumber(5); + PipelineMeta pipelineMeta = pipelineWith(meta); + + withDialog( + parent -> new WriteToLogDialog(parent, new Variables(), meta, pipelineMeta).open(), + bot -> { + SWTBot dialog = bot.shell(SHELL_TITLE).activate().bot(); + SWTBotText rows = + dialog.textWithLabel( + BaseMessages.getString( + WriteToLogMeta.class, "WriteToLogMeta.LimitRowsNumber.Label")); + assertEquals("5", rows.getText()); + rows.setText("42"); + dialog.button(buttonLabel("System.Button.OK")).click(); + }); + + assertEquals(42, meta.getLimitRowsNumber(), "OK must store the edited row limit"); + } + @Test void reopeningShowsTheStoredLogLevel() { for (LogLevel level : LogLevel.values()) { diff --git a/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java b/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java index 0d785d948e4..c0e8b9f9cd8 100644 --- a/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java +++ b/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java @@ -65,6 +65,7 @@ class GuiCompositeWidgetsGroupTest extends SwtBotTestBase { private static final String BOXES_PARENT = "GuiCompositeWidgetsGroupTest-boxes"; private static final String SINGLE_BOX_PARENT = "GuiCompositeWidgetsGroupTest-single-box"; private static final String UNEVEN_PARENT = "GuiCompositeWidgetsGroupTest-uneven"; + private static final String NUMERIC_PARENT = "GuiCompositeWidgetsGroupTest-numeric"; @BeforeAll static void registerSampleWidgets() { @@ -73,6 +74,7 @@ static void registerSampleWidgets() { register(BoxesSample.class); register(SingleBoxSample.class); register(UnevenBoxesSample.class); + register(NumericSample.class); } @Test @@ -99,6 +101,48 @@ void ungroupedWidgetsStayOnAFlatForm() { } } + /** + * A text widget reads back a String. int and long setters must receive the number; String not. + */ + @Test + void textWidgetsRoundTripIntAndLongButNotStringAsNumber() { + Shell shell = new Shell(display); + shell.setLayout(new FormLayout()); + try { + NumericSample source = new NumericSample(); + source.setCount(5); + source.setLimit(9L); + source.setName("kept"); + GuiCompositeWidgets widgets = new GuiCompositeWidgets(new Variables()); + widgets.createCompositeWidgets(source, null, shell, NUMERIC_PARENT, null); + widgets.setWidgetsContents(source, shell, NUMERIC_PARENT); + + TextVar count = (TextVar) widgets.getWidgetsMap().get("count"); + TextVar limit = (TextVar) widgets.getWidgetsMap().get("limit"); + TextVar name = (TextVar) widgets.getWidgetsMap().get("name"); + assertEquals("5", count.getText()); + assertEquals("9", limit.getText()); + + count.setText("42"); + limit.setText("100"); + name.setText("42"); + widgets.getWidgetsContents(source, NUMERIC_PARENT); + assertEquals(42, source.getCount()); + assertEquals(100L, source.getLimit()); + assertEquals( + "42", source.getName(), "a String setter must keep the text, not a parsed number"); + + count.setText("nope"); + limit.setText(""); + widgets.getWidgetsContents(source, NUMERIC_PARENT); + assertEquals(0, source.getCount(), "an unparsable int falls back to 0"); + assertEquals(0L, source.getLimit(), "an empty long falls back to 0"); + assertEquals("42", source.getName()); + } finally { + shell.dispose(); + } + } + @Test void emptyTextWidgetLeavesNullFieldNull() { Shell shell = new Shell(display); @@ -563,6 +607,32 @@ private static Rectangle absoluteBounds(Control control) { return new Rectangle(origin.x, origin.y, bounds.width, bounds.height); } + @GuiPlugin + @Getter + @Setter + public static class NumericSample { + @GuiWidgetElement( + id = "count", + parentId = NUMERIC_PARENT, + type = GuiElementType.TEXT, + label = "Count") + private int count; + + @GuiWidgetElement( + id = "limit", + parentId = NUMERIC_PARENT, + type = GuiElementType.TEXT, + label = "Limit") + private long limit; + + @GuiWidgetElement( + id = "name", + parentId = NUMERIC_PARENT, + type = GuiElementType.TEXT, + label = "Name") + private String name; + } + @GuiPlugin @Getter @Setter diff --git a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java index dbf11f63901..dad4f5dd3d1 100644 --- a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java +++ b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java @@ -1760,6 +1760,21 @@ private void getWidgetsData(Object sourceData, GuiElements guiElements) { return; } + // Text, combo and metadata widgets read back a String. int and long setters reject that + // and the field keeps its old value. String setters are left alone. + // + if (value instanceof String text + && (parameterType == int.class || parameterType == long.class)) { + String trimmed = text.trim(); + // Keep the two assignments separate. A ternary of int and long widens the int to long, + // and reflection then rejects that Long for an int setter. + if (parameterType == int.class) { + value = Const.toInt(trimmed, 0); + } else { + value = Const.toLong(trimmed, 0L); + } + } + if (value != null && !isAssignable(parameterType, value.getClass())) { LogChannel.UI.logError( "Value of type " From b64329cee3f4ba8181795463b9aeabf2f95f5d79 Mon Sep 17 00:00:00 2001 From: mattcasters Date: Mon, 5 Oct 2026 20:31:32 +0200 Subject: [PATCH 3/3] Issue #8655 : Clean up the Database Join and Write to log dialogs The SQL tab layout is expressed as the stack the dialog actually uses, and the caret tracking, parameter refresh, and OK path no longer carry the old dialog's unused listeners. --- .../databasejoin/DatabaseJoinDialog.java | 507 ++++++++---------- .../writetolog/WriteToLogDialog.java | 45 +- 2 files changed, 238 insertions(+), 314 deletions(-) diff --git a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java index 887e36830f0..94df65a6b2b 100644 --- a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java +++ b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoinDialog.java @@ -55,18 +55,13 @@ import org.apache.hop.ui.pipeline.transform.BaseTransformDialog; import org.apache.hop.ui.util.EnvironmentUtils; import org.eclipse.swt.SWT; -import org.eclipse.swt.events.FocusAdapter; -import org.eclipse.swt.events.FocusEvent; -import org.eclipse.swt.events.KeyAdapter; -import org.eclipse.swt.events.KeyEvent; -import org.eclipse.swt.events.MouseAdapter; -import org.eclipse.swt.events.MouseEvent; import org.eclipse.swt.layout.FormAttachment; import org.eclipse.swt.layout.FormData; import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Listener; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.TableItem; import org.eclipse.swt.widgets.Text; @@ -74,17 +69,26 @@ public class DatabaseJoinDialog extends BaseTransformDialog { private static final Class PKG = DatabaseJoinMeta.class; - private TextComposite wSql; + /** Caret movement in the SQL editor. Each of these updates the line/column readout. */ + private static final int[] EDITOR_EVENTS = { + SWT.Modify, + SWT.KeyDown, + SWT.KeyUp, + SWT.FocusIn, + SWT.FocusOut, + SWT.MouseDown, + SWT.MouseUp, + SWT.MouseDoubleClick + }; + private TextComposite wSql; private Label wlPosition; - private TableView wParam; private TableView wResolvedParam; private final DatabaseJoinMeta input; private ColumnInfo[] ciKey; - private ColumnInfo[] ciResolvedParam; private final List inputFields = new ArrayList<>(); private IRowMeta sourceFieldsMeta; @@ -117,9 +121,7 @@ public String open() { DatabaseJoinMeta.GUI_PLUGIN_ELEMENT_PARENT_ID, input, w -> { - // Extra-group builders run during createCompositeWidgets, before - // addScrolledComposite returns. Keep the field assigned so they can look up the - // widgets already placed on the same tab. + // Extra-group builders run inside this call, before the field is assigned. widgets = w; w.registerExtraGroup( BaseMessages.getString(PKG, "DatabaseJoin.Tab.Sql"), "0200", null, this::addSql); @@ -141,26 +143,14 @@ public void widgetModified( if (DatabaseJoinMeta.WIDGET_CACHED.equals(widgetId)) { enableFields(); } else if (DatabaseJoinMeta.WIDGET_CONNECTION.equals(widgetId)) { - onConnectionChanged(); + // Bracket quoting follows the database. The highlighter stays as built in addSql. + refreshResolvedParametersPanel(); } else if (DatabaseJoinMeta.WIDGET_SQL_FROM_FILE.equals(widgetId)) { onSqlFromFileChanged(); } } - - @Override - public void persistContents(GuiCompositeWidgets compositeWidgets) { - persistSqlEditor(); - persistParameters(); - } }); - // The connection drives SQL syntax highlighting. The Connection tab is laid out before the - // SQL tab, so the selection line already exists by the time the editor is built. - MetaSelectionLine connectionLine = connectionLine(); - if (connectionLine != null) { - connectionLine.addListener(SWT.Selection, e -> onConnectionChanged()); - } - populateSqlEditor(); populateParameters(); enableFields(); @@ -174,170 +164,75 @@ public void persistContents(GuiCompositeWidgets compositeWidgets) { } /** - * The SQL tab: the styled SQL editor, its line/column readout, and the read-only table of - * parameters resolved from the SQL. The table is pinned to the bottom of the tab and the readout - * sits above it, so the editor ends at the readout instead of covering both. + * SQL tab. The resolved-parameter table is pinned to the bottom and the line/column readout sits + * above it, so the editor ends at the readout. Ending the editor at the bottom of the tab lays + * the readout and the table outside the client area. */ private void addSql(Composite parent) { - Label wlSql = new Label(parent, SWT.NONE); - wlSql.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.SQL.Label")); - PropsUi.setLook(wlSql); - FormData fdlSql = new FormData(); - fdlSql.left = new FormAttachment(0, 0); - fdlSql.right = new FormAttachment(100, 0); Control lastOnTab = widgets.getWidgetsMap().get(DatabaseJoinMeta.WIDGET_REPLACE_VARIABLES); - fdlSql.top = - lastOnTab == null ? new FormAttachment(0, 0) : new FormAttachment(lastOnTab, margin); - wlSql.setLayoutData(fdlSql); + Label wlSql = + fullWidthLabel(parent, BaseMessages.getString(PKG, "DatabaseJoinDialog.SQL.Label")); + attachTop(wlSql, lastOnTab); - wSql = - EnvironmentUtils.getInstance().isWeb() - ? new StyledTextComp( - variables, - parent, - SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL, - TextComposite.STYLE_TYPE_SQL) - : new SQLStyledTextComp( - variables, parent, SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL); - PropsUi.setLook(wSql, Props.WIDGET_STYLE_FIXED); + wSql = newSqlEditor(parent); + // Keywords of the connection selected while the editor is built. TextComposite cannot remove a + // line-style listener, so a later connection change must not add a second one. wSql.addLineStyleListener(getSqlReservedWords()); - wSql.addModifyListener(e -> refreshResolvedParametersPanel()); - wSql.addModifyListener(e -> setPosition()); - wSql.addKeyListener( - new KeyAdapter() { - @Override - public void keyPressed(KeyEvent e) { - setPosition(); - } + trackEditorCaret(); - @Override - public void keyReleased(KeyEvent e) { - setPosition(); - } - }); - wSql.addFocusListener( - new FocusAdapter() { - @Override - public void focusGained(FocusEvent e) { - setPosition(); - } - - @Override - public void focusLost(FocusEvent e) { - setPosition(); - } - }); - wSql.addMouseListener( - new MouseAdapter() { - @Override - public void mouseDoubleClick(MouseEvent e) { - setPosition(); - } + wlPosition = fullWidthLabel(parent, ""); - @Override - public void mouseDown(MouseEvent e) { - setPosition(); - } - - @Override - public void mouseUp(MouseEvent e) { - setPosition(); - } - }); - - wlPosition = new Label(parent, SWT.NONE); - PropsUi.setLook(wlPosition); - - Label wlResolvedParam = new Label(parent, SWT.NONE); - wlResolvedParam.setText( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Label")); - PropsUi.setLook(wlResolvedParam); - - ciResolvedParam = new ColumnInfo[3]; - ciResolvedParam[0] = - new ColumnInfo( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ResolvedPlaceholder"), - ColumnInfo.COLUMN_TYPE_TEXT, - false); - ciResolvedParam[1] = - new ColumnInfo( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ResolvedInputField"), - ColumnInfo.COLUMN_TYPE_TEXT, - false); - ciResolvedParam[2] = - new ColumnInfo( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ResolvedType"), - ColumnInfo.COLUMN_TYPE_TEXT, - false); + Label wlResolvedParam = + fullWidthLabel( + parent, BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Label")); + ColumnInfo[] resolvedColumns = + new ColumnInfo[] { + textColumn("DatabaseJoinDialog.ColumnInfo.ResolvedPlaceholder"), + textColumn("DatabaseJoinDialog.ColumnInfo.ResolvedInputField"), + textColumn("DatabaseJoinDialog.ColumnInfo.ResolvedType") + }; wResolvedParam = new TableView( variables, parent, SWT.BORDER | SWT.FULL_SELECTION | SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL, - ciResolvedParam, + resolvedColumns, 1, true, - e -> {}, + null, props, true, null, false, false); - // Pin the table to the bottom of the tab, stack the readout and its label above it, and end - // the editor at the readout. Attaching the editor to the bottom leaves the two below the tab. - FormData fdResolvedParam = new FormData(); - fdResolvedParam.left = new FormAttachment(0, 0); - fdResolvedParam.right = new FormAttachment(100, 0); - fdResolvedParam.bottom = new FormAttachment(100, 0); - fdResolvedParam.height = (int) (90 * props.getZoomFactor()); - wResolvedParam.setLayoutData(fdResolvedParam); - - FormData fdlResolvedParam = new FormData(); - fdlResolvedParam.left = new FormAttachment(0, 0); - fdlResolvedParam.right = new FormAttachment(100, 0); - fdlResolvedParam.bottom = new FormAttachment(wResolvedParam, -margin); - wlResolvedParam.setLayoutData(fdlResolvedParam); - - FormData fdlPosition = new FormData(); - fdlPosition.left = new FormAttachment(0, 0); - fdlPosition.right = new FormAttachment(100, 0); - fdlPosition.bottom = new FormAttachment(wlResolvedParam, -margin); - wlPosition.setLayoutData(fdlPosition); - - FormData fdSql = new FormData(); - fdSql.left = new FormAttachment(0, 0); - fdSql.top = new FormAttachment(wlSql, margin); - fdSql.right = new FormAttachment(100, 0); - fdSql.bottom = new FormAttachment(wlPosition, -margin); - wSql.setLayoutData(fdSql); - } - - /** The Parameters tab: the grid that maps positional {@code ?} markers to input fields. */ + + attachBottom(wResolvedParam, (int) (90 * props.getZoomFactor())); + attachAbove(wlResolvedParam, wResolvedParam); + attachAbove(wlPosition, wlResolvedParam); + attachBetween(wSql, wlSql, wlPosition); + } + + /** Parameters tab: the grid that maps positional {@code ?} markers to input fields. */ private void addParameters(Composite parent) { - int nrKeyRows = (input.getParameters() != null ? input.getParameters().size() : 1); - - ciKey = new ColumnInfo[2]; - ciKey[0] = - new ColumnInfo( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ParameterFieldname"), - ColumnInfo.COLUMN_TYPE_CCOMBO, - new String[] {""}, - false); - ciKey[1] = - new ColumnInfo( - BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ParameterType"), - ColumnInfo.COLUMN_TYPE_CCOMBO, - ValueMetaFactory.getValueMetaNames()); - - Label wlParam = new Label(parent, SWT.NONE); - wlParam.setText(BaseMessages.getString(PKG, "DatabaseJoinDialog.Param.Label")); - PropsUi.setLook(wlParam); - FormData fdlParam = new FormData(); - fdlParam.left = new FormAttachment(0, 0); - fdlParam.right = new FormAttachment(100, 0); - fdlParam.top = new FormAttachment(0, 0); - wlParam.setLayoutData(fdlParam); + int nrKeyRows = input.getParameters() != null ? input.getParameters().size() : 1; + + ciKey = + new ColumnInfo[] { + new ColumnInfo( + BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ParameterFieldname"), + ColumnInfo.COLUMN_TYPE_CCOMBO, + new String[] {""}, + false), + new ColumnInfo( + BaseMessages.getString(PKG, "DatabaseJoinDialog.ColumnInfo.ParameterType"), + ColumnInfo.COLUMN_TYPE_CCOMBO, + ValueMetaFactory.getValueMetaNames()) + }; + + Label wlParam = + fullWidthLabel(parent, BaseMessages.getString(PKG, "DatabaseJoinDialog.Param.Label")); + attachTop(wlParam, null); wParam = new TableView( @@ -353,30 +248,81 @@ private void addParameters(Composite parent) { refreshResolvedParametersPanel(); }, props); + attachFill(wParam, wlParam); + } - FormData fdParam = new FormData(); - fdParam.left = new FormAttachment(0, 0); - fdParam.top = new FormAttachment(wlParam, margin); - fdParam.right = new FormAttachment(100, 0); - fdParam.bottom = new FormAttachment(100, 0); - wParam.setLayoutData(fdParam); + private TextComposite newSqlEditor(Composite parent) { + int style = SWT.MULTI | SWT.LEFT | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL; + TextComposite editor = + EnvironmentUtils.getInstance().isWeb() + ? new StyledTextComp(variables, parent, style, TextComposite.STYLE_TYPE_SQL) + : new SQLStyledTextComp(variables, parent, style); + PropsUi.setLook(editor, Props.WIDGET_STYLE_FIXED); + return editor; } - private MetaSelectionLine connectionLine() { - if (widgets == null) { - return null; + private void trackEditorCaret() { + Listener listener = + e -> { + setPosition(); + if (e.type == SWT.Modify) { + refreshResolvedParametersPanel(); + } + }; + for (int event : EDITOR_EVENTS) { + wSql.addListener(event, listener); } - Control control = widgets.getWidgetsMap().get(DatabaseJoinMeta.WIDGET_CONNECTION); - return control instanceof MetaSelectionLine line ? line : null; } - private void onConnectionChanged() { - // Only the resolved-parameter table depends on the connection here. The SQL highlighter is - // deliberately not re-seeded: TextComposite has no removeLineStyleListener, so every call to - // addLineStyleListener stacks another listener and the stale keyword sets would keep firing. - // Highlighting therefore uses the keywords of the connection stored on the transform, which is - // what the dialog did before it was split into tabs. - refreshResolvedParametersPanel(); + private ColumnInfo textColumn(String key) { + return new ColumnInfo(BaseMessages.getString(PKG, key), ColumnInfo.COLUMN_TYPE_TEXT, false); + } + + private Label fullWidthLabel(Composite parent, String text) { + Label label = new Label(parent, SWT.NONE); + label.setText(text); + PropsUi.setLook(label); + return label; + } + + private void attachTop(Control control, Control above) { + FormData data = fullWidth(); + data.top = above == null ? new FormAttachment(0, 0) : new FormAttachment(above, margin); + control.setLayoutData(data); + } + + private void attachBottom(Control control, int height) { + FormData data = fullWidth(); + data.bottom = new FormAttachment(100, 0); + data.height = height; + control.setLayoutData(data); + } + + private void attachAbove(Control control, Control below) { + FormData data = fullWidth(); + data.bottom = new FormAttachment(below, -margin); + control.setLayoutData(data); + } + + private void attachBetween(Control control, Control above, Control below) { + FormData data = fullWidth(); + data.top = new FormAttachment(above, margin); + data.bottom = new FormAttachment(below, -margin); + control.setLayoutData(data); + } + + private void attachFill(Control control, Control above) { + FormData data = fullWidth(); + data.top = new FormAttachment(above, margin); + data.bottom = new FormAttachment(100, 0); + control.setLayoutData(data); + } + + private static FormData fullWidth() { + FormData data = new FormData(); + data.left = new FormAttachment(0, 0); + data.right = new FormAttachment(100, 0); + return data; } private void onSqlFromFileChanged() { @@ -390,12 +336,10 @@ private void onSqlFromFileChanged() { private List getSqlReservedWords() { String connectionName = readWidgetText(DatabaseJoinMeta.WIDGET_CONNECTION); - // Do not search keywords when connection is empty if (Utils.isEmpty(connectionName)) { return List.of(); } - - // If connection is a variable that can't be resolved + // A variable that cannot be resolved here has no keyword list yet. if (variables.resolve(connectionName).startsWith("${")) { return List.of(); } @@ -434,23 +378,22 @@ private void setEnabled(String widgetId, boolean enabled) { } private void setComboBoxes() { - // Something was changed in the row. - // if (ciKey == null) { return; } ciKey[0].setComboValues(ConstUi.sortFieldNames(inputFields)); } - public void setPosition() { + private void setPosition() { if (wSql == null || wSql.isDisposed() || wlPosition == null || wlPosition.isDisposed()) { return; } - int lineNumber = wSql.getLineNumber(); - int columnNumber = wSql.getColumnNumber(); wlPosition.setText( BaseMessages.getString( - PKG, "DatabaseJoinDialog.Position.Label", "" + lineNumber, "" + columnNumber)); + PKG, + "DatabaseJoinDialog.Position.Label", + Integer.toString(wSql.getLineNumber()), + Integer.toString(wSql.getColumnNumber()))); } private DatabaseJoinMeta.SqlParameterSpec parseCurrentSqlParameterSpec() { @@ -467,10 +410,7 @@ private List getDeclaredParametersFromGrid() { if (wParam == null || wParam.isDisposed()) { return parameters; } - - int nrparam = wParam.nrNonEmpty(); - for (int i = 0; i < nrparam; i++) { - TableItem item = wParam.getNonEmpty(i); + for (TableItem item : wParam.getNonEmptyItems()) { ParameterField field = new ParameterField(); field.setName(item.getText(1)); field.setType(ValueMetaFactory.getIdForValueMeta(item.getText(2))); @@ -484,81 +424,99 @@ private void refreshResolvedParametersPanel() { return; } - DatabaseJoinMeta.SqlParameterSpec parameterSpec = parseCurrentSqlParameterSpec(); - List declaredParameters = getDeclaredParametersFromGrid(); - Map declaredTypesByName = new LinkedHashMap<>(); - for (ParameterField parameter : declaredParameters) { - if (!Utils.isEmpty(parameter.getName())) { - declaredTypesByName.put(parameter.getName(), parameter.getType()); - } - } + DatabaseJoinMeta.SqlParameterSpec spec = parseCurrentSqlParameterSpec(); + List declared = getDeclaredParametersFromGrid(); + Map typesByName = typesByName(declared); wResolvedParam.table.removeAll(); int positionalIndex = 0; - for (String parameterReference : parameterSpec.getParameterReferences()) { - boolean positional = parameterReference == null; - String placeholder = positional ? "?" : "?{" + parameterReference + "}"; - - String inputFieldName = parameterReference; - String declaredType = positional ? null : declaredTypesByName.get(parameterReference); - if (positional) { - if (positionalIndex < declaredParameters.size()) { - ParameterField declared = declaredParameters.get(positionalIndex); - inputFieldName = declared.getName(); - declaredType = declared.getType(); - } else { - inputFieldName = null; - } + for (String reference : spec.getParameterReferences()) { + addResolvedRow(reference, positionalIndex, declared, typesByName); + if (reference == null) { positionalIndex++; } + } + + if (wResolvedParam.table.getItemCount() == 0) { + new TableItem(wResolvedParam.table, SWT.NONE); + } + wResolvedParam.setRowNums(); + wResolvedParam.optWidth(true); + + // The declared grid only maps positional "?" markers. Named "?{field}" placeholders leave + // nothing to fill in, so the grid is disabled rather than silently ignored. + if (wParam != null && !wParam.isDisposed()) { + wParam.setEnabled(spec.getPositionalParameterCount() > 0); + } + } - boolean foundInInput = - !Utils.isEmpty(inputFieldName) - && ((sourceFieldsMeta != null && sourceFieldsMeta.indexOfValue(inputFieldName) >= 0) - || inputFields.contains(inputFieldName)); - - String inputFieldDisplay; - if (Utils.isEmpty(inputFieldName)) { - inputFieldDisplay = - BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Unmapped"); - } else if (foundInInput) { - inputFieldDisplay = inputFieldName; + private void addResolvedRow( + String reference, + int positionalIndex, + List declared, + Map typesByName) { + boolean positional = reference == null; + String fieldName = reference; + String declaredType = positional ? null : typesByName.get(reference); + if (positional) { + if (positionalIndex < declared.size()) { + ParameterField field = declared.get(positionalIndex); + fieldName = field.getName(); + declaredType = field.getType(); } else { - inputFieldDisplay = - inputFieldName - + " (" - + BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.NotFound") - + ")"; + fieldName = null; } + } - String typeDisplay = declaredType; - if (foundInInput && sourceFieldsMeta != null) { - int sourceIndex = sourceFieldsMeta.indexOfValue(inputFieldName); - if (sourceIndex >= 0) { - typeDisplay = - ValueMetaFactory.getValueMetaName( - sourceFieldsMeta.getValueMeta(sourceIndex).getType()); - } - } - if (typeDisplay == null) { - typeDisplay = ""; + boolean found = fieldIsInInput(fieldName); + wResolvedParam.add( + positional ? "?" : "?{" + reference + "}", + fieldDisplay(fieldName, found), + typeDisplay(fieldName, found, declaredType)); + } + + private static Map typesByName(List declared) { + Map typesByName = new LinkedHashMap<>(); + for (ParameterField parameter : declared) { + if (!Utils.isEmpty(parameter.getName())) { + typesByName.put(parameter.getName(), parameter.getType()); } + } + return typesByName; + } - wResolvedParam.add(placeholder, inputFieldDisplay, typeDisplay); + private boolean fieldIsInInput(String fieldName) { + if (Utils.isEmpty(fieldName)) { + return false; } + if (sourceFieldsMeta != null && sourceFieldsMeta.indexOfValue(fieldName) >= 0) { + return true; + } + return inputFields.contains(fieldName); + } - if (wResolvedParam.table.getItemCount() == 0) { - new TableItem(wResolvedParam.table, SWT.NONE); + private String fieldDisplay(String fieldName, boolean found) { + if (Utils.isEmpty(fieldName)) { + return BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.Unmapped"); } - wResolvedParam.setRowNums(); - wResolvedParam.optWidth(true); + if (found) { + return fieldName; + } + return fieldName + + " (" + + BaseMessages.getString(PKG, "DatabaseJoinDialog.ResolvedParameters.NotFound") + + ")"; + } - // The declared grid only maps positional "?" markers. With named "?{field}" placeholders - // there is nothing left to fill in, so the grid is disabled rather than silently ignored. - boolean hasPositionalParameters = parameterSpec.getPositionalParameterCount() > 0; - if (wParam != null && !wParam.isDisposed()) { - wParam.setEnabled(hasPositionalParameters); + private String typeDisplay(String fieldName, boolean found, String declaredType) { + if (found && sourceFieldsMeta != null) { + int sourceIndex = sourceFieldsMeta.indexOfValue(fieldName); + if (sourceIndex >= 0) { + return ValueMetaFactory.getValueMetaName( + sourceFieldsMeta.getValueMeta(sourceIndex).getType()); + } } + return declaredType == null ? "" : declaredType; } private void loadSqlFromFileAndSetReadOnly() { @@ -568,8 +526,7 @@ private void loadSqlFromFileAndSetReadOnly() { return; } try { - String content = HopVfs.getTextFileContent(path, StandardCharsets.UTF_8); - wSql.setText(content); + wSql.setText(HopVfs.getTextFileContent(path, StandardCharsets.UTF_8)); wSql.setEditable(false); refreshResolvedParametersPanel(); } catch (HopFileException e) { @@ -624,12 +581,7 @@ private void persistSqlEditor() { } private void persistParameters() { - List parameters = getDeclaredParametersFromGrid(); - logDebug( - BaseMessages.getString(PKG, "DatabaseJoinDialog.Log.ParametersFound") - + parameters.size() - + " parameters"); - input.setParameters(parameters); + input.setParameters(getDeclaredParametersFromGrid()); } private String readWidgetText(String widgetId) { @@ -650,9 +602,6 @@ private String readWidgetText(String widgetId) { } private void searchPrevTransformFields() { - // - // Search the fields in the background - // BackgroundThreadFacade.start( () -> { TransformMeta transformMeta = pipelineMeta.findTransform(transformName); @@ -661,12 +610,8 @@ private void searchPrevTransformFields() { } try { IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformMeta); - - // Remember these fields... sourceFieldsMeta = row; - for (int i = 0; i < row.size(); i++) { - inputFields.add(row.getValueMeta(i).getName()); - } + inputFields.addAll(Arrays.asList(row.getFieldNames())); setComboBoxes(); if (!shell.isDisposed()) { shell.getDisplay().asyncExec(this::refreshResolvedParametersPanel); @@ -706,23 +651,23 @@ private void ok() { persistSqlEditor(); persistParameters(); - transformName = wTransformName.getText(); // return value + transformName = wTransformName.getText(); dispose(); } private void get() { try { - IRowMeta r = pipelineMeta.getPrevTransformFields(variables, transformName); - if (r != null && !r.isEmpty()) { + IRowMeta row = pipelineMeta.getPrevTransformFields(variables, transformName); + if (row != null && !row.isEmpty()) { BaseTransformDialog.getFieldsFromPrevious( - r, wParam, 1, new int[] {1}, new int[] {2}, -1, -1, null); + row, wParam, 1, new int[] {1}, new int[] {2}, -1, -1, null); } - } catch (HopException ke) { + } catch (HopException e) { new ErrorDialog( shell, BaseMessages.getString(PKG, "DatabaseJoinDialog.GetFieldsFailed.DialogTitle"), BaseMessages.getString(PKG, "DatabaseJoinDialog.GetFieldsFailed.DialogMessage"), - ke); + e); } } } diff --git a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java index 61f43d877a2..64a4c20ca5f 100644 --- a/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java +++ b/plugins/transforms/writetolog/src/main/java/org/apache/hop/pipeline/transforms/writetolog/WriteToLogDialog.java @@ -41,7 +41,6 @@ import org.apache.hop.ui.core.widget.TextComposite; import org.apache.hop.ui.hopgui.BackgroundThreadFacade; import org.apache.hop.ui.pipeline.transform.BaseTransformDialog; -import org.apache.hop.ui.pipeline.transform.ITableItemInsertListener; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.CCombo; import org.eclipse.swt.layout.FormAttachment; @@ -59,10 +58,9 @@ public class WriteToLogDialog extends BaseTransformDialog { private final WriteToLogMeta input; /** - * Hand-built on purpose. The combo has to show the translated {@link LogLevel} descriptions and - * map the selection back by position. An annotated {@code GuiElementType.COMBO} derives its items - * from {@code Enum.toString()} and stores the selected label, which is neither translated nor - * round-trippable - see {@link WriteToLogDialogTest} for the case that guards this. + * Hand-built. The combo shows the translated {@link LogLevel} descriptions and maps the selection + * back by position. An annotated combo stores {@code Enum.toString()}, which is neither the + * translated label nor a value {@code Enum.valueOf} can read back. */ private CCombo wLoglevel; @@ -98,9 +96,7 @@ public String open() { WriteToLogMeta.GUI_PLUGIN_ELEMENT_PARENT_ID, input, w -> { - // Extra-group builders run during createCompositeWidgets, before - // addScrolledComposite returns. Keep the field assigned so they can look up the - // widgets already placed on the same tab. + // Extra-group builders run inside this call, before the field is assigned. widgets = w; w.registerExtraGroup( BaseMessages.getString(PKG, "WriteToLog.Tab.Options"), @@ -126,13 +122,6 @@ public void widgetModified( enableFields(); } } - - @Override - public void persistContents(GuiCompositeWidgets compositeWidgets) { - persistLogLevel(); - persistLogMessage(); - persistFields(); - } }); populateLogLevel(); @@ -149,9 +138,9 @@ public void persistContents(GuiCompositeWidgets compositeWidgets) { } /** - * The log level combo, added to the Options tab. Extra-group contents share the tab composite - * with the annotated fields, so the row has to be hung below the last of them - anchoring it to - * the top of the composite draws it on top of the first annotated row. + * Log level combo on the Options tab. It shares that tab with the annotated fields, so the row + * hangs below the last of them. Anchoring it to the top of the composite draws it on top of the + * first annotated row. */ private void addLogLevel(Composite parent) { Label wlLoglevel = new Label(parent, SWT.RIGHT); @@ -178,10 +167,7 @@ private void addLogLevel(Composite parent) { wLoglevel.addListener(SWT.Selection, e -> input.setChanged()); } - /** - * The Message tab: the log message template and the fields it can reference. The two belong on - * one tab because picking a field and referencing it in the template is a single task. - */ + /** Message tab: the log message template and the fields it can reference. */ private void addMessage(Composite parent) { Label wlLogMessage = new Label(parent, SWT.NONE); wlLogMessage.setText(BaseMessages.getString(PKG, "WriteToLogDialog.LogMessage.Label")); @@ -204,8 +190,7 @@ private void addMessage(Composite parent) { fdLogMessage.left = new FormAttachment(0, 0); fdLogMessage.top = new FormAttachment(wlLogMessage, margin); fdLogMessage.right = new FormAttachment(100, 0); - // A preferred height, not a fixed one: the field grid below keeps the rest of the tab, so the - // editor grows with the dialog instead of fighting it for space. + // Preferred height. The field grid below keeps the rest of the tab. fdLogMessage.height = (int) (200 * props.getZoomFactor()); wLogMessage.setLayoutData(fdLogMessage); @@ -254,7 +239,7 @@ private void populateLogLevel() { } private void persistLogLevel() { - // The combo holds the translated descriptions in enum order: map by position, not by label. + // Descriptions are translated, so the stored value is the enum at this position. int logLevelIndex = wLoglevel.getSelectionIndex(); if (logLevelIndex < 0 || logLevelIndex >= LogLevel.values().length) { input.setLogLevel(LogLevel.BASIC); @@ -305,8 +290,6 @@ private void persistFields() { } private void setComboBoxes() { - // Something was changed in the row. - // if (colinf == null) { return; } @@ -314,9 +297,6 @@ private void setComboBoxes() { } private void searchPrevTransformFields() { - // - // Search the fields in the background - // BackgroundThreadFacade.start( () -> { TransformMeta transformMeta = pipelineMeta.findTransform(transformName); @@ -366,9 +346,8 @@ private void get() { try { IRowMeta r = pipelineMeta.getPrevTransformFields(variables, transformName); if (r != null) { - ITableItemInsertListener insertListener = (tableItem, v) -> true; BaseTransformDialog.getFieldsFromPrevious( - r, wFields, 1, new int[] {1}, new int[] {}, -1, -1, insertListener); + r, wFields, 1, new int[] {1}, new int[] {}, -1, -1, null); } } catch (HopException ke) { new ErrorDialog( @@ -389,7 +368,7 @@ private void ok() { if (Utils.isEmpty(wTransformName.getText())) { return; } - transformName = wTransformName.getText(); // return value + transformName = wTransformName.getText(); widgets.getWidgetsContents(input, WriteToLogMeta.GUI_PLUGIN_ELEMENT_PARENT_ID); persistLogLevel();