You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/08 18:58:02 UTC

[GitHub] [nifi] NissimShiman commented on a change in pull request #4620: NIFI-6242 PutFileTransfer generation incorrect provenance event

NissimShiman commented on a change in pull request #4620:
URL: https://github.com/apache/nifi/pull/4620#discussion_r538713027



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestPutSFTP.java
##########
@@ -243,9 +244,58 @@ public void testPutSFTPBatching() throws IOException {
         putSFTPRunner.clearTransferState();
     }
 
+    @Test
+    public void testPutSFTPProvenanceTransitUri() throws IOException {
+        emptyTestDirectory();
+
+        putSFTPRunner.setProperty(SFTPTransfer.REJECT_ZERO_BYTE, "false");
+        Map<String,String> attributes = new HashMap<>();
+        attributes.put("filename", "testfile.txt");
+        attributes.put("transfer-host","localhost");
+
+        putSFTPRunner.enqueue(Paths.get(testFile), attributes);
+
+        attributes = new HashMap<>();
+        attributes.put("filename", "testfile1.txt");
+        attributes.put("transfer-host","127.0.0.1");
+
+        putSFTPRunner.enqueue(Paths.get(testFile), attributes);
+        putSFTPRunner.run();
+
+        putSFTPRunner.assertTransferCount(PutSFTP.REL_SUCCESS, 2);
+        putSFTPRunner.getProvenanceEvents().forEach(k->{
+            assert(k.toString().contains("sftp://localhost"));
+        });
+        //Two files in batch, should have 2 transferred to success, 0 to failure
+        putSFTPRunner.assertTransferCount(PutSFTP.REL_SUCCESS, 2);
+        putSFTPRunner.assertTransferCount(PutSFTP.REL_REJECT, 0);
+
+        MockFlowFile flowFile1 = putSFTPRunner.getFlowFilesForRelationship(PutFileTransfer.REL_SUCCESS).get(0);
+        MockFlowFile flowFile2 = putSFTPRunner.getFlowFilesForRelationship(PutFileTransfer.REL_SUCCESS).get(1);
+        putSFTPRunner.clearProvenanceEvents();
+        putSFTPRunner.clearTransferState();
+
+        //Test different destinations on flow file attributes
+        putSFTPRunner.setProperty(SFTPTransfer.HOSTNAME,"${transfer-host}"); //set to derive hostname
+
+        putSFTPRunner.enqueue(flowFile1);
+        putSFTPRunner.run();

Review comment:
       remove putSFTPRunner.run();
   
   test case should pass without it as well

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutFileTransfer.java
##########
@@ -95,12 +96,19 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
         }
 
         final ComponentLog logger = getLogger();
-        final String hostname = context.getProperty(FileTransfer.HOSTNAME).evaluateAttributeExpressions(flowFile).getValue();
+
+        String hostname = context.getProperty(FileTransfer.HOSTNAME).evaluateAttributeExpressions(flowFile).getValue();
+        //Check for constant attribute
+        final boolean staticHostname = hostname!=null && !hostname.isEmpty() && Objects.equals(hostname, context.getProperty(FileTransfer.HOSTNAME)
+                                 .getValue());
 
         final int maxNumberOfFiles = context.getProperty(FileTransfer.BATCH_SIZE).asInteger();
         int fileCount = 0;
         try (final T transfer = getFileTransfer(context)) {
             do {
+                if(!staticHostname) {

Review comment:
       use:
   if (context.getProperty(FileTransfer.HOSTNAME).isExpressionLanguagePresent())
   instead
   

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTP.java
##########
@@ -144,6 +146,61 @@ public void basicFileUpload() throws IOException {
         // Check file was uploaded
         Assert.assertTrue(results.exists("c:\\data\\randombytes-1"));
     }
+    @Test
+    public void basicProvenanceEventTest() throws IOException {
+        TestRunner runner = TestRunners.newTestRunner(PutFTP.class);
+        runner.setValidateExpressionUsage(false);

Review comment:
       Assuming you agree with comments above, this can be removed (as well as from lines 137/138 as well)

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTP.java
##########
@@ -144,6 +146,61 @@ public void basicFileUpload() throws IOException {
         // Check file was uploaded
         Assert.assertTrue(results.exists("c:\\data\\randombytes-1"));
     }
+    @Test

Review comment:
       Put a blank line between tests

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTP.java
##########
@@ -144,6 +146,61 @@ public void basicFileUpload() throws IOException {
         // Check file was uploaded
         Assert.assertTrue(results.exists("c:\\data\\randombytes-1"));
     }
+    @Test
+    public void basicProvenanceEventTest() throws IOException {
+        TestRunner runner = TestRunners.newTestRunner(PutFTP.class);
+        runner.setValidateExpressionUsage(false);
+
+        runner.setProperty(FTPTransfer.HOSTNAME, "localhost");
+        runner.setProperty(FTPTransfer.USERNAME, username);
+        runner.setProperty(FTPTransfer.PASSWORD, password);
+        runner.setProperty(FTPTransfer.PORT, Integer.toString(ftpPort));
+
+        // Get two flowfiles to test by running data
+        try (FileInputStream fis = new FileInputStream("src/test/resources/randombytes-1")) {
+            Map<String, String> attributes = new HashMap<String, String>();
+            attributes.put(CoreAttributes.FILENAME.key(), "randombytes-1");
+            attributes.put("transfer-host", "localhost");
+            runner.enqueue(fis, attributes);
+            runner.run();
+        }
+        try (FileInputStream fis = new FileInputStream("src/test/resources/hello.txt")) {
+            Map<String, String> attributes = new HashMap<String, String>();
+            attributes.put(CoreAttributes.FILENAME.key(), "hello.txt");
+
+            attributes.put("transfer-host", "127.0.0.1");
+            runner.enqueue(fis, attributes);
+            runner.run();
+        }
+        runner.assertQueueEmpty();
+        runner.assertTransferCount(PutFTP.REL_SUCCESS, 2);
+
+        MockFlowFile flowFile1 = runner.getFlowFilesForRelationship(PutFileTransfer.REL_SUCCESS).get(0);
+
+        MockFlowFile flowFile2 = runner.getFlowFilesForRelationship(PutFileTransfer.REL_SUCCESS).get(1);
+
+        runner.clearProvenanceEvents();
+        runner.clearTransferState();
+        HashMap<String, String> map1 = new HashMap<>();
+        HashMap<String, String> map2 = new HashMap<>();
+        map1.put(CoreAttributes.FILENAME.key(), "randombytes-xx");
+        map2.put(CoreAttributes.FILENAME.key(), "randombytes-yy");
+
+        flowFile1.putAttributes(map1);
+        flowFile2.putAttributes(map2);
+        //set to derive hostname
+        runner.setProperty(FTPTransfer.HOSTNAME, "${transfer-host}");
+
+        runner.enqueue(flowFile1);
+        runner.run();

Review comment:
       Remove runner.run()




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org