[BACKLOG-21756] Handling of 'Trans Job Entry > Transformation' field (#…

…4996)
pentaho · Mar 1, 2018 · e444c2e · e444c2e
1 parent ba5f26e
commit e444c2e
Show file tree

Hide file tree

Showing 14 changed files with 173 additions and 73 deletions.
diff --git a/core/pom.xml b/core/pom.xml
@@ -48,6 +48,7 @@
     <woodstox-core-asl.version>4.4.1</woodstox-core-asl.version>
     <httpclient.version>4.5.3</httpclient.version>
     <httpccore.version>4.4.6</httpccore.version>
+    <xmlunit.version>1.5</xmlunit.version>
   </properties>
 
   <dependencies>
@@ -512,6 +513,12 @@
       <version>${spring.framework.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>xmlunit</groupId>
+      <artifactId>xmlunit</artifactId>
+      <version>${xmlunit.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/core/src/main/java/org/pentaho/di/core/logging/LogMessage.java b/core/src/main/java/org/pentaho/di/core/logging/LogMessage.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -22,13 +22,15 @@
 
 package org.pentaho.di.core.logging;
 
+import org.apache.commons.lang.StringUtils;
 import org.pentaho.di.core.Const;
 
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.pentaho.di.core.util.EnvUtil;
+import org.pentaho.di.core.util.StringUtil;
 
 public class LogMessage implements LogMessageInterface {
   private String logChannelId;
@@ -133,14 +135,12 @@ private String formatDetailedSubject( List<String> subjects ) {
   @Override
   @Deprecated
   public String toString() {
-    if ( message == null ) {
+    if ( StringUtils.isBlank( message ) ) {
       return subject;
+    } else if ( StringUtils.isBlank( subject ) ) {
+      return getMessage();
     }
-    if ( arguments != null && arguments.length > 0 ) {
-      return subject + " - " + MessageFormat.format( message, arguments );
-    } else {
-      return subject + " - " + message;
-    }
+    return String.format( "%s - %s", subject, getMessage() );
   }
 
   @Override
@@ -158,11 +158,25 @@ public void setLevel( LogLevel level ) {
    */
   @Override
   public String getMessage() {
-    if ( arguments != null && arguments.length > 0 ) {
-      return MessageFormat.format( message, arguments );
-    } else {
-      return message;
+    String formatted = message;
+    if ( arguments != null ) {
+      // get all "tokens" enclosed by curly brackets within the message
+      final List<String> tokens = new ArrayList<>();
+      StringUtil.getUsedVariables( formatted, "{", "}", tokens, true );
+      // perform MessageFormat.format( ... ) on each token, if we get an exception, we'll know that we have a
+      // segment that isn't parsable by MessageFormat, likely a pdi variable name (${foo}) - in this case, we need to
+      // escape the curly brackets in the message, so that MessageFormat does not complain
+      for ( final String token : tokens ) {
+        try {
+          MessageFormat.format( "{" + token + "}", arguments );
+        } catch ( final IllegalArgumentException iar ) {
+          formatted = formatted.replaceAll( "\\{" + token + "\\}",  "\\'{'" + token + "\\'}'" );
+        }
+      }
+      // now that we have escaped curly brackets in all invalid tokens, we can attempt to format the entire message
+      formatted = MessageFormat.format( formatted, arguments );
     }
+    return formatted;
   }
 
   /**

diff --git a/core/src/main/java/org/pentaho/di/core/util/StringUtil.java b/core/src/main/java/org/pentaho/di/core/util/StringUtil.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -310,7 +310,7 @@ public static String substituteField( String aString, RowMetaInterface rowMeta,
    * @param includeSystemVariables
    *          also check for system variables.
    */
-  private static void getUsedVariables( String aString, String open, String close, List<String> list,
+  public static void getUsedVariables( String aString, String open, String close, List<String> list,
     boolean includeSystemVariables ) {
     if ( aString == null ) {
       return;

diff --git a/core/src/test/java/org/pentaho/di/core/logging/LogMessageTest.java b/core/src/test/java/org/pentaho/di/core/logging/LogMessageTest.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -19,6 +19,7 @@
  * limitations under the License.
  *
  ******************************************************************************/
+
 package org.pentaho.di.core.logging;
 
 import static org.junit.Assert.assertEquals;
@@ -30,6 +31,8 @@
 import org.junit.Test;
 import org.pentaho.di.core.Const;
 
+import java.text.MessageFormat;
+
 /**
  * @author Tatsiana_Kasiankova
  *
@@ -116,6 +119,13 @@ public void testToString_withOneArgument() throws Exception {
     assertEquals( "Subject - Log message for Test", msg.toString( ) );
   }
 
+  @Test
+  public void testGetMessage() {
+    LogMessage msg = new LogMessage( "m {0}, {1}, {2}, {3}, {4,number,#.00}, {5} {foe}", "Channel 01",
+      new Object[] { "Foo", "{abc}", "", null, 123 }, LogLevel.DEBUG );
+    assertEquals( "m Foo, {abc}, , null, 123.00, {5} {foe}", msg.getMessage() );
+  }
+
   private void turnOnLogMarkMapping() {
     System.getProperties().put( Const.KETTLE_LOG_MARK_MAPPINGS, "Y" );
   }
@@ -138,8 +148,7 @@ private static LoggingObjectInterface getTreeLoggingObject() {
   private static LoggingObjectInterface getLoggingObjectWithOneParent() {
     LoggingObjectInterface rootLogObject = new SimpleLoggingObject( "ROOT_SUBJECT", LoggingObjectType.SPOON, null );
     LoggingObjectInterface transLogObject =
-        new SimpleLoggingObject( "TRANS_SUBJECT", LoggingObjectType.TRANS, rootLogObject );
+      new SimpleLoggingObject( "TRANS_SUBJECT", LoggingObjectType.TRANS, rootLogObject );
     return transLogObject;
   }
-
 }
diff --git a/core/src/test/java/org/pentaho/di/core/util/StringUtilTest.java b/core/src/test/java/org/pentaho/di/core/util/StringUtilTest.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -59,6 +59,7 @@ public void testinitCap() {
   Map<String, String> createVariables1( String open, String close ) {
     Map<String, String> map = new HashMap<String, String>();
 
+    map.put( "NULL", null );
     map.put( "EMPTY", "" );
     map.put( "checkcase", "case1" );
     map.put( "CheckCase", "case2" );
@@ -87,6 +88,8 @@ Map<String, String> createVariables1( String open, String close ) {
    */
   public void testSubstituteBasic() {
     Map<String, String> map = createVariables1( "${", "}" );
+    assertEquals( "|${BAD_KEY}|", StringUtil.substitute( "|${BAD_KEY}|", map, "${", "}" ) );
+    assertEquals( "|${NULL}|", StringUtil.substitute( "|${NULL}|", map, "${", "}" ) );
     assertEquals( "||", StringUtil.substitute( "|${EMPTY}|", map, "${", "}" ) );
     assertEquals( "|case1|", StringUtil.substitute( "|${checkcase}|", map, "${", "}" ) );
     assertEquals( "|case2|", StringUtil.substitute( "|${CheckCase}|", map, "${", "}" ) );

diff --git a/core/src/test/java/org/pentaho/di/core/xml/XMLFormatterTest.java b/core/src/test/java/org/pentaho/di/core/xml/XMLFormatterTest.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2016-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2016-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -21,14 +21,23 @@
  ******************************************************************************/
 package org.pentaho.di.core.xml;
 
-import static org.junit.Assert.assertEquals;
+import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
 
 import java.io.InputStream;
 
 import org.apache.commons.io.IOUtils;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
+import org.custommonkey.xmlunit.XMLUnit;
+
 public class XMLFormatterTest {
+
+  @BeforeClass
+  public static void setupClass() {
+    XMLUnit.setIgnoreWhitespace( true );
+  }
+
   @Test
   public void test1() throws Exception {
     String inXml, expectedXml;
@@ -40,7 +49,7 @@ public void test1() throws Exception {
     }
 
     String result = XMLFormatter.format( inXml );
-    assertEquals( expectedXml, result );
+    assertXMLEqual( expectedXml, result );
   }
 
   @Test
@@ -54,7 +63,7 @@ public void test2() throws Exception {
     }
 
     String result = XMLFormatter.format( inXml );
-    assertEquals( expectedXml, result );
+    assertXMLEqual( expectedXml, result );
   }
 
   @Test
@@ -68,7 +77,7 @@ public void test3() throws Exception {
     }
 
     String result = XMLFormatter.format( inXml );
-    assertEquals( expectedXml, result );
+    assertXMLEqual( expectedXml, result );
   }
 
   @Test
@@ -82,6 +91,6 @@ public void test4() throws Exception {
     }
 
     String result = XMLFormatter.format( inXml );
-    assertEquals( expectedXml, result );
+    assertXMLEqual( expectedXml, result );
   }
 }
diff --git a/engine/pom.xml b/engine/pom.xml
@@ -96,6 +96,7 @@
     <oro.version>2.0.8</oro.version>
     <javax.websocket-api.version>1.1</javax.websocket-api.version>
     <rxjava.version>2.0.4</rxjava.version>
+    <xmlunit.version>1.5</xmlunit.version>
   </properties>
 
   <dependencies>
@@ -934,6 +935,12 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <dependency>
+      <groupId>xmlunit</groupId>
+      <artifactId>xmlunit</artifactId>
+      <version>${xmlunit.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/engine/src/main/java/org/pentaho/di/job/JobExecutionConfiguration.java b/engine/src/main/java/org/pentaho/di/job/JobExecutionConfiguration.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -268,7 +268,10 @@ public void getUsedVariables( JobMeta jobMeta ) {
       for ( int i = 0; i < vars.size(); i++ ) {
         String varname = vars.get( i );
         if ( !varname.startsWith( Const.INTERNAL_VARIABLE_PREFIX ) ) {
-          newVariables.put( varname, Const.NVL( variables.get( varname ), sp.getProperty( varname, "" ) ) );
+          // add a variable only if it's defined within this configuration or it is a system property
+          if ( variables.containsKey( varname ) || sp.getProperty( varname ) != null ) {
+            newVariables.put( varname, Const.NVL( variables.get( varname ), sp.getProperty( varname, "" ) ) );
+          }
         }
       }
       // variables.clear();
@@ -705,7 +708,7 @@ public void getUsedArguments( JobMeta jobMeta, String[] commandLineArguments, IM
             }
           }
         } catch ( KettleException ke ) {
-          log.logBasic( ke.getMessage(), ke );
+          // suppress exceptions at this time - we will let the runtime report on any errors
         }
       }
     }

diff --git a/engine/src/main/java/org/pentaho/di/job/entries/trans/JobEntryTrans.java b/engine/src/main/java/org/pentaho/di/job/entries/trans/JobEntryTrans.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2017 by Hitachi Vantara : http://www.pentaho.com
+ * Copyright (C) 2002-2018 by Hitachi Vantara : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.lang.StringUtils;
 import org.pentaho.di.cluster.SlaveServer;
 import org.pentaho.di.core.CheckResultInterface;
 import org.pentaho.di.core.Const;
@@ -700,7 +701,8 @@ public Result execute( Result result, int nr ) throws KettleException {
     try {
       transMeta = getTransMeta( rep, metaStore, this );
     } catch ( KettleException e ) {
-      logError( Const.getStackTracker( e ) );
+      logError( BaseMessages.getString( PKG, "JobTrans.Exception.UnableToRunJob", parentJobMeta.getName(),
+        StringUtils.trim( e.getMessage() ), getName() ), e );
       result.setNrErrors( 1 );
       result.setResult( false );
       return result;
@@ -1367,6 +1369,9 @@ public TransMeta getTransMeta( Repository rep, IMetaStore metaStore, VariableSpa
       }
 
       return transMeta;
+    } catch ( final KettleException ke ) {
+      // if we get a KettleException, simply re-throw it
+      throw ke;
     } catch ( Exception e ) {
       throw new KettleException( BaseMessages.getString( PKG, "JobTrans.Exception.MetaDataLoad" ), e );
     }

diff --git a/...ne/src/main/resources/org/pentaho/di/job/entries/trans/messages/messages_en_US.properties b/...ne/src/main/resources/org/pentaho/di/job/entries/trans/messages/messages_en_US.properties
@@ -79,6 +79,7 @@ JobEntryTransDialog.Exception.UnableToReferenceObjectId.Title=Error
 JobEntryTransDialog.Exception.UnableToReferenceObjectId.Message=Unable to get information about the referenced object with id={0}
 JobTrans.Log.OpeningTransByReference=Opening a transformation by reference with id={0}
 JobTrans.Exception.LogFilenameMissing=Log filename is empty or not specified!
+JobTrans.Exception.UnableToRunJob=Unable to run job [{0}]. {1} [{2}]
 JobTrans.Browse.Label=Browse...
 
 JobTrans.Fileformat.TXT=Text files