[PDI-15181] Delete Files job entry will delete all files under the in…

…stall directory when file location parameter is blank (#2477) - Before processing file, first checking whether it is a kettle variable, extracting it, and then checking if it is an empty value. - Small refactoring of filed names in processFile method
pentaho · May 10, 2016 · ad5e8e9 · ad5e8e9
1 parent 53c6bab
commit ad5e8e9
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 26 deletions.
diff --git a/engine/src/org/pentaho/di/job/entries/deletefiles/JobEntryDeleteFiles.java b/engine/src/org/pentaho/di/job/entries/deletefiles/JobEntryDeleteFiles.java
@@ -207,15 +207,15 @@ public Result execute( Result result, int nr ) throws KettleException {
     Multimap<String, String> pathToMaskMap = populateDataForJobExecution( resultRows );
 
     for ( Map.Entry<String, String> pathToMask : pathToMaskMap.entries() ) {
-      final String filePath = pathToMask.getKey();
+      final String filePath = environmentSubstitute( pathToMask.getKey() );
       if ( filePath.trim().isEmpty() ) {
         // Relative paths are permitted, and providing an empty path means deleting all files inside a root pdi-folder.
         // It is much more likely to be a mistake than a desirable action, so we don't delete anything (see PDI-15181)
         if ( log.isDetailed() ) {
           logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.NoPathProvided" ) );
         }
       } else {
-        final String fileMask = pathToMask.getValue();
+        final String fileMask = environmentSubstitute( pathToMask.getValue() );
 
         if ( parentJob.isStopped() ) {
           break;
@@ -282,54 +282,50 @@ private Multimap<String, String> populateDataForJobExecution( List<RowMetaAndDat
     return pathToMaskMap;
   }
 
-  boolean processFile( String inputFileName, String inputWildcard, Job parentJob ) {
+  boolean processFile( String path, String wildcard, Job parentJob ) {
-    boolean rcode = false;
+    boolean isDeleted = false;
     FileObject fileFolder = null;
-    String realFileFolderName = environmentSubstitute( inputFileName );
-    String realWildCard = environmentSubstitute( inputWildcard );
 
     try {
-      fileFolder = KettleVFS.getFileObject( realFileFolderName, this );
+      fileFolder = KettleVFS.getFileObject( path, this );
 
       if ( fileFolder.exists() ) {
-        // the file or folder exists
         if ( fileFolder.getType() == FileType.FOLDER ) {
-          // It's a folder
+
           if ( log.isDetailed() ) {
-            logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.ProcessingFolder", realFileFolderName ) );
+            logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.ProcessingFolder", path ) );
-            // Delete Files
           }
 
-          int Nr = fileFolder.delete( new TextFileSelector( fileFolder.toString(), realWildCard, parentJob ) );
+          int totalDeleted = fileFolder.delete( new TextFileSelector( fileFolder.toString(), wildcard, parentJob ) );
 
           if ( log.isDetailed() ) {
-            logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.TotalDeleted", String.valueOf( Nr ) ) );
+            logDetailed(
+              BaseMessages.getString( PKG, "JobEntryDeleteFiles.TotalDeleted", String.valueOf( totalDeleted ) ) );
           }
-          rcode = true;
+          isDeleted = true;
         } else {
-          // It's a file
+
           if ( log.isDetailed() ) {
-            logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.ProcessingFile", realFileFolderName ) );
+            logDetailed( BaseMessages.getString( PKG, "JobEntryDeleteFiles.ProcessingFile", path ) );
           }
-          boolean deleted = fileFolder.delete();
+          isDeleted = fileFolder.delete();
-          if ( !deleted ) {
+          if ( !isDeleted ) {
-            logError( BaseMessages.getString( PKG, "JobEntryDeleteFiles.CouldNotDeleteFile", realFileFolderName ) );
+            logError( BaseMessages.getString( PKG, "JobEntryDeleteFiles.CouldNotDeleteFile", path ) );
           } else {
             if ( log.isBasic() ) {
-              logBasic( BaseMessages.getString( PKG, "JobEntryDeleteFiles.FileDeleted", inputFileName ) );
+              logBasic( BaseMessages.getString( PKG, "JobEntryDeleteFiles.FileDeleted", path ) );
             }
-            rcode = true;
           }
         }
       } else {
         // File already deleted, no reason to try to delete it
         if ( log.isBasic() ) {
-          logBasic( BaseMessages.getString( PKG, "JobEntryDeleteFiles.FileAlreadyDeleted", realFileFolderName ) );
+          logBasic( BaseMessages.getString( PKG, "JobEntryDeleteFiles.FileAlreadyDeleted", path ) );
         }
-        rcode = true;
+        isDeleted = true;
       }
     } catch ( Exception e ) {
-      logError( BaseMessages.getString( PKG, "JobEntryDeleteFiles.CouldNotProcess", realFileFolderName, e
+      logError( BaseMessages.getString( PKG, "JobEntryDeleteFiles.CouldNotProcess", path, e
         .getMessage() ), e );
     } finally {
       if ( fileFolder != null ) {
@@ -341,7 +337,7 @@ boolean processFile( String inputFileName, String inputWildcard, Job parentJob )
       }
     }
 
-    return rcode;
+    return isDeleted;
   }
 
   private class TextFileSelector implements FileSelector {
@@ -411,7 +407,7 @@ public boolean traverseDescendents( FileSelectInfo info ) {
    *
    * @param selectedfile
    * @param wildcard
-   * @return True if the selectedfile matches the inputWildcard
+   * @return True if the selectedfile matches the wildcard
    **********************************************************/
   private boolean GetFileWildcard( String selectedfile, String wildcard ) {
     boolean getIt = true;

diff --git a/engine/test-src/org/pentaho/di/job/entries/deletefiles/JobEntryDeleteFilesTest.java b/engine/test-src/org/pentaho/di/job/entries/deletefiles/JobEntryDeleteFilesTest.java
@@ -108,6 +108,23 @@ public void filesPath_AreProcessed_ArgsOfPreviousMeta() throws Exception {
     verify( jobEntry, times( metaAndDataList.size() ) ).processFile( anyString(), anyString(), any( Job.class ) );
   }
 
+  @Test
+  public void filesPathVariables_AreProcessed_OnlyIfValueIsNotBlank() throws Exception {
+    final String pathToFileBlankValue = "pathToFileBlankValue";
+    final String pathToFileValidValue = "pathToFileValidValue";
+
+    jobEntry.setVariable( pathToFileBlankValue, Const.EMPTY_STRING );
+    jobEntry.setVariable( pathToFileValidValue, PATH_TO_FILE );
+
+    jobEntry.setArguments( new String[] { asVariable( pathToFileBlankValue ), asVariable( pathToFileValidValue ) } );
+    jobEntry.setFilemasks( new String[] { null, null } );
+    jobEntry.setArgFromPrevious( false );
+
+    jobEntry.execute( new Result(), 0 );
+
+    verify( jobEntry ).processFile( eq( PATH_TO_FILE ), anyString(), any( Job.class ) );
+  }
+
   @Test
   public void specifyingTheSamePath_WithDifferentWildcards() throws Exception {
     final String fileExtensionTxt = ".txt";
@@ -132,4 +149,7 @@ private RowMetaAndData constructRowMetaAndData( Object... data ) {
     return new RowMetaAndData( meta, data );
   }
 
+  private String asVariable( String variable ) {
+    return "${" + variable + "}";
+  }
 }