hansva commented on issue #5953: URL: https://github.com/apache/hop/issues/5953#issuecomment-5817614206
# Transform dialogs lose configured values when incoming fields can't be loaded Follow-up to [#5953](https://github.com/apache/hop/issues/5953). In the JSON Input dialog, the "Select field" value is wiped when the previous transform's fields can't be loaded, for example when an upstream Table Input has broken SQL. Pressing OK then saves an empty field name. A sweep of all 198 GUI classes that load previous-transform fields found the same bug in many other dialogs. This document lists every affected place. - Line numbers refer to [`dd96261d0d`](https://github.com/apache/hop/tree/dd96261d0dc8a16dd5e8516c01cccfc2915d39be). - No workflow action dialog loads previous-transform fields, so actions are not affected. - No dialog clears its fields grid by itself when it opens. Every grid clear found is behind a user-clicked button. ## Root cause Most of these dialogs use one of two patterns: - **Save, clear, fetch, restore:** the dialog saves the combo text, calls `removeAll()` (which clears the text), then calls `getPrevTransformFields(...)`. It restores the text only after the fetch succeeds. When the fetch throws, the value stays blank and OK saves it empty. - **Clear, refill:** the dialog clears the combo and adds items back, but never restores the text. The value is lost even when the fetch works. SWT behaviour (checked in the SWT 3.134 sources): | Widget | `removeAll()` | `setItems(...)` | |---|---|---| | `org.eclipse.swt.widgets.Combo` (Windows, GTK, macOS) | clears text | clears text | | `org.eclipse.swt.custom.CCombo` | clears text | clears text only if not editable | | `ComboVar` / `LabelCombo` (wrap `CCombo`, pass their style flags through) | clears text | clears text only if built `READ_ONLY` without `setEditable(true)` | `ColumnInfo.setComboValues(...)` never touches the cell values, so `TableView` columns are safe. ## 1. Value lost when the fetch fails (30 dialogs, 36 combos) "Read-only" means the combo is not editable, so the user cannot retype the value either. | Transform | Combo(s) | When it runs | |---|---|---| | [JSON Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsoninput/JsonInputDialog.java#L1112) | `wFieldValue` | dialog open (#5953) | | [JSON Normalize](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonnormalize/JsonNormalizeInputDialog.java#L1226) | `wFieldValue` | dialog open | | [Binary File Output](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/binaryfileoutput/src/main/java/org/apache/hop/pipeline/transforms/binaryfileoutput/BinaryFileOutputDialog.java#L258) | `wBinaryField`, `wFilenameField` | focus (read-only) | | [Change File Encoding](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/changefileencoding/src/main/java/org/apache/hop/pipeline/transforms/changefileencoding/ChangeFileEncodingDialog.java#L348) | `wFileName`, `wTargetFileName` | focus | | [Clone Row](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/clonerow/src/main/java/org/apache/hop/pipeline/transforms/clonerow/CloneRowDialog.java#L303) | `wNrCloneField` | focus | | [Column Exists](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/columnexists/src/main/java/org/apache/hop/pipeline/transforms/columnexists/ColumnExistsDialog.java#L401) | `wTableName`, `wColumnName` | focus | | [Credit Card Validator](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/creditcardvalidator/src/main/java/org/apache/hop/pipeline/transforms/creditcardvalidator/CreditCardValidatorDialog.java#L290) | `wFieldName` | focus | | [Detect Language](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/detectlanguage/src/main/java/org/apache/hop/pipeline/transforms/language/DetectLanguageDialog.java#L175) | `wCorpusFieldName` | focus (read-only) | | [Dynamic SQL Row](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/dynamicsqlrow/src/main/java/org/apache/hop/pipeline/transforms/dynamicsqlrow/DynamicSqlRowDialog.java#L445) | `wSqlFieldName` | focus | | [Execute Row SQL Script](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/execsqlrow/src/main/java/org/apache/hop/pipeline/transforms/execsqlrow/ExecSqlRowDialog.java#L406) | `wSqlFieldName` | focus | | [Execute a Process](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/execprocess/src/main/java/org/apache/hop/pipeline/transforms/execprocess/ExecProcessDialog.java#L400) | `wProcess` | focus | | [File Exists](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/fileexists/src/main/java/org/apache/hop/pipeline/transforms/fileexists/FileExistsDialog.java#L280) | `wFileName` | focus (read-only) | | [Check if File is Locked](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/filelocked/src/main/java/org/apache/hop/pipeline/transforms/filelocked/FileLockedDialog.java#L190) | `wFileName` | focus | | [Get Table Names](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/gettablenames/src/main/java/org/apache/hop/pipeline/transforms/gettablenames/GetTableNamesDialog.java#L581) | `wSchemaField` | focus | | [HTML to Text](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/html2text/src/main/java/org/apache/hop/pipeline/transforms/html2text/Html2TextDialog.java#L335) | `wHtmlFieldName` | focus (read-only) | | [HTTP Post](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/httppost/src/main/java/org/apache/hop/pipeline/transforms/httppost/HttpPostDialog.java#L1240) | `wUrlField` | focus; `setItems(null)` throws after `removeAll()` | | [Email Messages Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/misc/mail/src/main/java/org/apache/hop/mail/pipeline/transforms/mailinput/MailInputDialog.java#L1513) | `wFolderField` | focus | | [Language Model Chat](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/languagemodelchat/src/main/java/org/apache/hop/pipeline/transforms/languagemodelchat/internals/ui/GeneralSettingsComposite.java#L291) | `inputFieldInput` | focus (read-only) | | [LDAP Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/ldap/src/main/java/org/apache/hop/pipeline/transforms/ldapinput/LdapInputDialog.java#L1522) | `wSearchBaseField` | focus | | [Load File Content In Memory](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/loadfileinput/src/main/java/org/apache/hop/pipeline/transforms/loadfileinput/LoadFileInputDialog.java#L1026) | `wFilenameField` | focus | | [PGP Decrypt Stream](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/pgp/src/main/java/org/apache/hop/pipeline/transforms/pgpdecryptstream/PGPDecryptStreamDialog.java#L337) | `wStreamFieldName`, `wPassPhraseFieldName` | focus (read-only) | | [PGP Encrypt Stream](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/pgp/src/main/java/org/apache/hop/pipeline/transforms/pgpencryptstream/PGPEncryptStreamDialog.java#L339) | `wStreamFieldName`, `wKeyNameFieldName` | focus (read-only) | | [Process Files](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/processfiles/src/main/java/org/apache/hop/pipeline/transforms/processfiles/ProcessFilesDialog.java#L409) | `wSourceFileNameField`, `wTargetFileNameField` | focus; also lost when no fields come back | | [Property Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/propertyinput/src/main/java/org/apache/hop/pipeline/transforms/propertyinput/PropertyInputDialog.java#L1060) | `wFilenameField` | every focus | | [Regex Evaluation](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/regexeval/src/main/java/org/apache/hop/pipeline/transforms/regexeval/RegexEvalDialog.java#L633) | `wFieldEvaluate` | every focus | | [Run SSH Commands](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/ssh/src/main/java/org/apache/hop/pipeline/transforms/ssh/SshDialog.java#L700) | `wCommandField` | focus; also lost when no fields come back | | [Stanford Simple NLP](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/stanfordnlp/src/main/java/org/apache/hop/pipeline/transforms/stanford/nlp/simple/StanfordSimpleNlpDialog.java#L230) | `wCorpusFieldName` | focus (read-only) | | [Tika](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/tika/src/main/java/org/apache/hop/pipeline/transforms/tika/TikaDialog.java#L713) | `wFilenameField` | focus | | [Value Mapper](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/valuemapper/src/main/java/org/apache/hop/pipeline/transforms/valuemapper/ValueMapperDialog.java#L257) | `wFieldName` | focus (read-only, never retried) | | [Check if Webservice is Available](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/webserviceavailable/src/main/java/org/apache/hop/pipeline/transforms/webserviceavailable/WebServiceAvailableDialog.java#L260) | `wURL` | focus | | [XSL Transformation](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/xml/src/main/java/org/apache/hop/pipeline/transforms/xml/xslt/XsltDialog.java#L587) | `wField`, `wXSLField` | every focus | ## 2. Value lost even when the fetch succeeds (5 dialogs) | Transform | Combo | Problem | |---|---|---| | [Get Data from XML](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/xml/src/main/java/org/apache/hop/pipeline/transforms/xml/getxmldata/GetXmlDataDialog.java#L1291) | `wXMLField` | Cleared on every focus. When the fetch fails, it [writes the literal `<EMPTY>`](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/xml/src/main/java/org/apache/hop/pipeline/transforms/xml/getxmldata/GetXmlDataDialog.java#L1308), which OK saves as the field name. | | [YAML Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/yamlinput/src/main/java/org/apache/hop/pipeline/transforms/yamlinput/YamlInputDialog.java#L919) | `wYAMLLField` | Cleared on every focus. | | [Table Exists](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/tableexists/src/main/java/org/apache/hop/pipeline/transforms/tableexists/TableExistsDialog.java#L266) | `wTableName` | Cleared on every focus (read-only). | | [File Metadata](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/filemetadata/src/main/java/org/apache/hop/pipeline/transforms/filemetadata/FileMetadataDialog.java#L399) | `wFilenameField` | Cleared on first focus. | | [Get Files Rows Count](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/getfilesrowcount/src/main/java/org/apache/hop/pipeline/transforms/getfilesrowcount/GetFilesRowsCountDialog.java#L828) | `wFilenameField` | Cleared on every focus. | ## 3. Other ways settings get lost or corrupted (7 dialogs) - **[LDAP Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/ldap/src/main/java/org/apache/hop/pipeline/transforms/ldapinput/LdapInputDialog.java#L1533):** a copy-paste bug, `wFilterField.setText(basefield)`, overwrites the dynamic filter field with the search-base value on every successful fetch. - **[Property Output](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/propertyoutput/src/main/java/org/apache/hop/pipeline/transforms/propertyoutput/PropertyOutputDialog.java#L162):** when the fetch fails, `fieldNames` stays `null` and `setItems(null)` throws while the dialog is being built, so the dialog cannot be opened. - **[Delay Row](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/delay/src/main/java/org/apache/hop/pipeline/transforms/delay/DelayDialog.java#L207):** when the fetch fails, OK saves the configured timeout field name as a literal timeout. At runtime this silently becomes a 0 ms delay. - **[Microsoft Excel Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/excel/src/main/java/org/apache/hop/pipeline/transforms/excelinput/ExcelInputDialog.java#L1117):** the "accept filenames from field" value is set with `select(indexOf(value))`, so it shows empty when that field is not in the upstream list, and OK saves it empty. - **[Regex Evaluation](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/regexeval/src/main/java/org/apache/hop/pipeline/transforms/regexeval/RegexEvalDialog.java#L646):** the value is restored with `select(indexOf(...))`, so a renamed field or a `${VAR}` value is cleared. An empty value is also silently set to the first upstream field just because the combo got focus. - **[Kafka Producer](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/kafka/src/main/java/org/apache/hop/pipeline/transforms/kafka/producer/KafkaProducerOutputDialog.java#L464):** when the fetch fails, OK is blocked, so the only way out is Cancel and all edits are lost. `ok()` has already written some values into the meta before it stops. - **[Table Compare](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/tablecompare/src/main/java/org/apache/hop/pipeline/transforms/tablecompare/TableCompareDialog.java#L546) (not confirmed at runtime):** when the fetch fails, a `NullPointerException` inside an `asyncExec` escapes the dialog's `open()`. Edits made afterwards may not be saved. ## 4. Same pattern after a failed database or API lookup (5 dialogs) - **[Salesforce Delete](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/salesforce/src/main/java/org/apache/hop/pipeline/transforms/salesforcedelete/SalesforceDeleteDialog.java#L538), [Insert](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/salesforce/src/main/java/org/apache/hop/pipeline/transforms/salesforceinsert/SalesforceInsertDialog.java#L1061), [Update](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/salesforce/src/main/java/org/apache/hop/pipeline/transforms/salesforceupdate/SalesforceUpdateDialog.java#L921) and [Upsert](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/salesforce/src/main/java/org/apache/hop/pipeline/transforms/salesforceupsert/SalesforceUpsertDialog.java#L1098):** `wModule` is lost when the connection or module fetch fails. Upsert also loses [`wUpsertField`](https://github.com/apache/hop/blob/dd 96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/salesforce/src/main/java/org/apache/hop/pipeline/transforms/salesforceupsert/SalesforceUpsertDialog.java#L704). - **[Snowflake Bulk Loader](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/databases/snowflake/src/main/java/org/apache/hop/pipeline/transforms/snowflake/bulkloader/SnowflakeBulkLoaderDialog.java#L514):** `wStageName` is lost when the `show stages` query fails. - **[MongoDB Delete](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/mongodb/src/main/java/org/apache/hop/pipeline/transforms/mongodbdelete/MongoDbDeleteDialog.java#L648):** `wCollection` is lost when the connection cannot be loaded. - **[Cassandra Output](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/cassandra/src/main/java/org/apache/hop/pipeline/transforms/cassandraoutput/CassandraOutputDialog.java#L596):** "Get tables" clears the typed table name and never restores it. - **[Google Sheets Input](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/tech/google/src/main/java/org/apache/hop/pipeline/transforms/googlesheets/GoogleSheetsInputDialog.java#L856):** "Get fields" clears the whole fields grid, with no prompt, before calling the Sheets API. Any failure leaves the grid empty. ## 5. Shared code - **[`GuiCompositeWidgets.setComboValues`](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java#L1844):** calls `setItems` on a native `Combo`, which clears the text. Text Chunker, Embed Text and Git Input each work around this by hand. The fix belongs in this method. - **[`BaseTransformDialog.getFieldsFromPrevious(IVariables, ComboVar, ...)`](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/ui/src/main/java/org/apache/hop/ui/pipeline/transform/BaseTransformDialog.java#L1228):** keeps the value correctly, but shows the error dialog again on every focus. The EDI to XML, HTTP Client and Get Subfolder Names dialogs use it. ## 6. Error-dialog noise (no data lost) - **On every focus:** about 15 dialogs show the "failed to get fields" error again each time the combo gets focus, because the "already fetched" flag is only set on success. Examples: Mail, Detect Language, PGP Encrypt/Decrypt, Stanford NLP, Delay Row, Property Input, Regex Evaluation. When focus returns to the combo after the error dialog closes, this may loop (not confirmed at runtime). - **On dialog open:** about 12 dialogs show the error as soon as they open. Examples: CSV File Input, Switch / Case, Data Validator, Filter Rows, JMS Producer, Join Rows, Avro File Input, Mod Partitioner. XML Input Stream shows two. ## Secondary: explicit "Edit mapping" / "Map fields" dialogs These only happen through a user-clicked mapping dialog, never when a transform dialog opens. - **[ETL Metadata Injection](https://github.com/apache/hop/blob/dd96261d0dc8a16dd5e8516c01cccfc2915d39be/plugins/transforms/metainject/src/main/java/org/apache/hop/pipeline/transforms/metainject/MetaInjectDialog.java#L1388):** the auto-map dialog drops existing mappings whose source transform failed to load, without telling the user. - **Graph Output, Salesforce transforms, Database Value Validation:** pressing OK in the mapping dialog while there are no incoming fields clears the existing mappings. ## Proposed fix Add a shared helper in `BaseTransformDialog` or a small `ui` utility: - **`getPreviousFieldNames(...)`:** fetches the previous fields once per dialog and caches them. Returns an empty array on failure and shows at most one error dialog. This removes the noise in section 6. - **`setItemsKeepingText(control, items)`:** works for `Combo`, `CCombo`, `ComboVar` and `LabelCombo`. Never calls `removeAll()`, always restores the current text, and never uses `select(indexOf(...))`. `GuiCompositeWidgets.setComboValues` and `BaseTransformDialog.getFieldsFromPrevious(ComboVar)` would delegate to the helper, and the dialogs in sections 1, 2 and 4 would switch to it. The bugs in section 3 each need their own small fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
