Merge pull request #2029 from mattcasters/master

Fix #2028 : Import success dialog turned into an error
apache · Dec 10, 2022 · 1ac0450 · 1ac0450
2 parents 6fb10c7 + 14918cc
commit 1ac0450
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 38 deletions.
diff --git a/core/src/main/java/org/apache/hop/metadata/api/HopMetadataProperty.java b/core/src/main/java/org/apache/hop/metadata/api/HopMetadataProperty.java
@@ -17,13 +17,12 @@
 
 package org.apache.hop.metadata.api;
 
-import org.apache.hop.core.injection.DefaultInjectionTypeConverter;
-import org.apache.hop.core.injection.InjectionTypeConverter;
-
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import org.apache.hop.core.injection.DefaultInjectionTypeConverter;
+import org.apache.hop.core.injection.InjectionTypeConverter;
 
 /** A field which is painted with this annotation is picked up by the Hop Metadata serializers */
 @Retention(RetentionPolicy.RUNTIME)
@@ -118,14 +117,29 @@ Class<? extends InjectionTypeConverter> injectionConverter() default
    */
   boolean inline() default false;
 
+  /**
+   * Reads old format XML where a list of values is stored inline. XML like the following needs to
+   * be turned into 3 KeyValue pairs:
+   *
+   * <p>{@code <parent> <key>k1</key><value>v1</value> <key>k2</key><value>v2</value>
+   * <key>k3</key><value>v3</value> </parent> }
+   *
+   * <p>In this scenario we would specify the tags "key" and "value" to populate the list correctly.
+   *
+   * @return
+   */
+  String[] inlineListTags() default {};
+
   /**
    * For metadata injection: if you want to convert an integer to a code (and vice-versa)
+   *
    * @return The integer-to-string converter
    */
   Class<? extends IIntCodeConverter> intCodeConverter() default IIntCodeConverter.None.class;
 
   /**
    * For metadata injection: if you want to convert a String (XML, JSON, ...) to an object
+   *
    * @return The string-to-object converter
    */
   Class<? extends IStringObjectConverter> injectionStringObjectConverter() default

diff --git a/core/src/main/java/org/apache/hop/metadata/serializer/xml/XmlMetadataUtil.java b/core/src/main/java/org/apache/hop/metadata/serializer/xml/XmlMetadataUtil.java
@@ -17,6 +17,12 @@
 
 package org.apache.hop.metadata.serializer.xml;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ParameterizedType;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hop.core.Const;
 import org.apache.hop.core.encryption.Encr;
@@ -35,13 +41,6 @@
 import org.apache.hop.metadata.util.ReflectionUtil;
 import org.w3c.dom.Node;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
-import java.lang.reflect.ParameterizedType;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-
 public class XmlMetadataUtil {
   /**
    * This method looks at the fields in the class of the provided object. It then sees which fields
@@ -295,7 +294,7 @@ public static <T> T deSerializeFromXml(
       try {
         // Do not create a new object if the node is null
         //
-        if (node==null) {
+        if (node == null) {
           return null;
         }
 
@@ -362,6 +361,7 @@ public static <T> T deSerializeFromXml(
         boolean storeWithName = property.storeWithName();
         boolean password = property.password();
         boolean storeWithCode = property.storeWithCode();
+        String[] inlineListTags = property.inlineListTags();
 
         Node tagNode;
         if (property.inline()) {
@@ -388,7 +388,8 @@ public static <T> T deSerializeFromXml(
                 metadataProvider,
                 password,
                 storeWithCode,
-                property.intCodeConverter());
+                property.intCodeConverter(),
+                inlineListTags);
 
         try {
           // Only set a value if we have something to set.
@@ -424,7 +425,8 @@ private static Object deSerializeFromXml(
       IHopMetadataProvider metadataProvider,
       boolean password,
       boolean storeWithCode,
-      Class<? extends IIntCodeConverter> intCodeConverterClass)
+      Class<? extends IIntCodeConverter> intCodeConverterClass,
+      String[] inlineListTags)
       throws HopXmlException {
     String elementString = XmlHandler.getNodeValue(elementNode);
 
@@ -536,6 +538,26 @@ private static Object deSerializeFromXml(
       //
       List<Object> list = new ArrayList<>();
       List<Node> itemNodes = XmlHandler.getNodes(groupNode, tag);
+      if (inlineListTags.length > 0) {
+        // Old XML serialization format where everything is just dumped into the same tag.
+        // See also HopMetadataProperty.inlineListTags
+        //
+        Node parentNode = itemNodes.get(0);
+        int listSize = XmlHandler.countNodes(parentNode, inlineListTags[0]);
+        if (listSize > 1) {
+          itemNodes.clear();
+          for (int i = 0; i < listSize; i++) {
+            Node node = parentNode.getOwnerDocument().createElement(tag);
+            for (String inlineTag : inlineListTags) {
+              Node n = XmlHandler.getSubNodeByNr(parentNode, inlineTag, i);
+              if (n != null) {
+                node.appendChild(n);
+              }
+            }
+            itemNodes.add(node);
+          }
+        }
+      }
       for (Node itemNode : itemNodes) {
         // We assume that the constructor of the parent class created the List object
         // so that we can simply add items to the list here.
@@ -556,7 +578,8 @@ private static Object deSerializeFromXml(
                   metadataProvider,
                   password,
                   storeWithCode,
-                  intCodeConverterClass);
+                  intCodeConverterClass,
+                  inlineListTags);
 
           // Add it to the list
           //

diff --git a/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImportDialog.java b/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImportDialog.java
@@ -692,11 +692,11 @@ private void doImport() {
 
       // Show some statistics after the import...
       //
-      MessageBox box = new MessageBox(HopGui.getInstance().getShell(), SWT.ICON_INFORMATION);
+      MessageBox box = new MessageBox(shell, SWT.ICON_INFORMATION|SWT.OK);
       box.setText("Import summary");
       box.setMessage(kettleImport.getImportReport());
       box.open();
-    } catch (Throwable e) {
+    } catch (Exception e) {
       new ErrorDialog(shell, "Error", "Error importing", e);
     }
   }

diff --git a/...rms/excel/src/main/java/org/apache/hop/pipeline/transforms/excelinput/ExcelInputMeta.java b/...rms/excel/src/main/java/org/apache/hop/pipeline/transforms/excelinput/ExcelInputMeta.java
@@ -17,6 +17,9 @@
 
 package org.apache.hop.pipeline.transforms.excelinput;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.vfs2.FileObject;
 import org.apache.hop.core.CheckResult;
@@ -49,10 +52,6 @@
 import org.apache.hop.resource.IResourceNaming;
 import org.apache.hop.resource.ResourceDefinition;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-
 /** Meta data for the Excel transform. */
 @Transform(
     id = "ExcelInput",
@@ -75,7 +74,14 @@ public class ExcelInputMeta extends BaseTransformMeta<ExcelInput, ExcelInputData
   @HopMetadataProperty(
       key = "file",
       injectionGroupKey = "FILENAME_LINES",
-      injectionGroupDescription = "ExcelInput.Injection.FILENAME_LINES")
+      injectionGroupDescription = "ExcelInput.Injection.FILENAME_LINES",
+      inlineListTags = {
+        "name",
+        "filemask",
+        "exclude_filemask",
+        "file_required",
+        "include_subfolders"
+      })
   private List<EIFile> files;
 
   /** The fieldname that holds the name of the file */
@@ -753,8 +759,7 @@ public boolean isAddResultFile() {
    * @return true if all sheets are read.
    */
   public boolean readAllSheets() {
-    return sheets.isEmpty()
-        || (sheets.size() == 1 && StringUtils.isEmpty(sheets.get(0).getName()));
+    return sheets.isEmpty() || (sheets.size() == 1 && StringUtils.isEmpty(sheets.get(0).getName()));
   }
 
   /**

diff --git a/...excel/src/test/java/org/apache/hop/pipeline/transforms/excelinput/ExcelInputMetaTest.java b/...excel/src/test/java/org/apache/hop/pipeline/transforms/excelinput/ExcelInputMetaTest.java
@@ -29,7 +29,9 @@ public void testSerialization() throws Exception {
     ExcelInputMeta meta =
         TransformSerializationTestUtil.testSerialization(
             "/excel-input-transform.xml", ExcelInputMeta.class);
-    assertEquals(1, meta.getFiles().size());
+    assertEquals(2, meta.getFiles().size());
+    assertEquals("file1.xls", meta.getFiles().get(0).getName());
+    assertEquals("file2.xls", meta.getFiles().get(1).getName());
     assertEquals(1, meta.getSheets().size());
     assertEquals(4, meta.getFields().size());
   }

diff --git a/plugins/transforms/excel/src/test/resources/excel-input-transform.xml b/plugins/transforms/excel/src/test/resources/excel-input-transform.xml
@@ -43,11 +43,16 @@
     <accept_field/>
     <accept_transform_name/>
     <file>
-        <name>${PROJECT_HOME}/files/excel/basic.xls</name>
-        <filemask/>
-        <exclude_filemask/>
-        <file_required>N</file_required>
-        <include_subfolders>N</include_subfolders>
+        <name>file1.xls</name>
+        <filemask>mask1</filemask>
+        <exclude_filemask>exclude1</exclude_filemask>
+        <file_required>Y</file_required>
+        <include_subfolders>Y</include_subfolders>
+        <name>file2.xls</name>
+        <filemask>mask2</filemask>
+        <exclude_filemask>exclude2</exclude_filemask>
+        <file_required>Y</file_required>
+        <include_subfolders>Y</include_subfolders>
     </file>
     <fields>
         <field>

diff --git a/ui/src/main/java/org/apache/hop/ui/core/dialog/MessageBox.java b/ui/src/main/java/org/apache/hop/ui/core/dialog/MessageBox.java
@@ -17,6 +17,8 @@
 
 package org.apache.hop.ui.core.dialog;
 
+import java.util.ArrayList;
+import java.util.List;
 import org.apache.hop.core.Const;
 import org.apache.hop.i18n.BaseMessages;
 import org.apache.hop.ui.core.PropsUi;
@@ -33,9 +35,6 @@
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 
-import java.util.ArrayList;
-import java.util.List;
-
 /**
  * A replacement of the system message box dialog to make sure the correct font and colors are used.
  */
@@ -127,12 +126,6 @@ public int open() {
     // Buttons at the bottom
     //
     List<Button> buttons = new ArrayList<>();
-    if ((style & SWT.OK) != 0) {
-      Button wOk = new Button(shell, SWT.PUSH);
-      wOk.setText(BaseMessages.getString(PKG, "System.Button.OK"));
-      wOk.addListener(SWT.Selection, e -> ok());
-      buttons.add(wOk);
-    }
     if ((style & SWT.YES) != 0) {
       Button wYes = new Button(shell, SWT.PUSH);
       wYes.setText(BaseMessages.getString(PKG, "System.Button.Yes"));
@@ -151,6 +144,12 @@ public int open() {
       wCancel.addListener(SWT.Selection, e -> cancel());
       buttons.add(wCancel);
     }
+    if ((style & SWT.OK) != 0 || buttons.isEmpty()) {
+      Button wOk = new Button(shell, SWT.PUSH);
+      wOk.setText(BaseMessages.getString(PKG, "System.Button.OK"));
+      wOk.addListener(SWT.Selection, e -> ok());
+      buttons.add(0, wOk);
+    }
 
     BaseTransformDialog.positionBottomButtons(
         shell, buttons.toArray(new Button[0]), margin, wMessage);