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 2021/07/26 15:48:02 UTC
[GitHub] [nifi] turcsanyip commented on a change in pull request #5190: NIFI-7947: Add directory deletion functionality in DeleteAzureDataLakeStorage
turcsanyip commented on a change in pull request #5190:
URL: https://github.com/apache/nifi/pull/5190#discussion_r676721143
##########
File path: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/storage/ITDeleteAzureDataLakeStorage.java
##########
@@ -35,15 +35,82 @@
public class ITDeleteAzureDataLakeStorage extends AbstractAzureDataLakeStorageIT {
+ private static final boolean FILE = true;
+ private static final boolean DIRECTORY = false;
+
@Override
protected Class<? extends Processor> getProcessorClass() {
return DeleteAzureDataLakeStorage.class;
}
+ @Test
+ public void testDeleteDirectoryWithFiles() {
+ // GIVEN
+ String directory = "TestDirectory";
+ String filename = "testFile.txt";
+ String fileContent = "AzureFileContent";
+ String inputFlowFileContent = "InputFlowFileContent";
+
+ createDirectoryAndUploadFile(directory, filename, fileContent);
+
+ // WHEN
+ // THEN
+ testSuccessfulDelete(fileSystemName, directory, filename, inputFlowFileContent, inputFlowFileContent, DIRECTORY);
+ }
+
+ @Test
+ public void testDeleteEmptyDirectoryAsFolder() {
Review comment:
It could simply be called `testDeleteEmptyDirectory` or does the `AsFolder` suffix has any extra meaning here?
##########
File path: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/storage/ITDeleteAzureDataLakeStorage.java
##########
@@ -368,10 +438,19 @@ private boolean fileExists(String directory, String filename) {
return fileClient.exists();
}
- private void setRunnerProperties(String fileSystem, String directory, String filename) {
+ private boolean directoryExists(String directory) {
+ DataLakeDirectoryClient directoryClient = fileSystemClient.getDirectoryClient(directory);
+
+ return directoryClient.exists();
+ }
+
+ private void setRunnerProperties(String fileSystem, String directory, String filename, boolean isFile) {
+ runner.setProperty(DeleteAzureDataLakeStorage.FILESYSTEM_OBJECT_TYPE, isFile ? "file" : "folder");
Review comment:
"folder" does not work because the AllowableValue has been renamed to "directory".
Using `AllowableValue.getValue()` would eliminate these renaming issues.
##########
File path: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/storage/ITDeleteAzureDataLakeStorage.java
##########
@@ -35,15 +35,82 @@
public class ITDeleteAzureDataLakeStorage extends AbstractAzureDataLakeStorageIT {
+ private static final boolean FILE = true;
+ private static final boolean DIRECTORY = false;
+
@Override
protected Class<? extends Processor> getProcessorClass() {
return DeleteAzureDataLakeStorage.class;
}
+ @Test
+ public void testDeleteDirectoryWithFiles() {
+ // GIVEN
+ String directory = "TestDirectory";
+ String filename = "testFile.txt";
+ String fileContent = "AzureFileContent";
+ String inputFlowFileContent = "InputFlowFileContent";
+
+ createDirectoryAndUploadFile(directory, filename, fileContent);
+
+ // WHEN
+ // THEN
+ testSuccessfulDelete(fileSystemName, directory, filename, inputFlowFileContent, inputFlowFileContent, DIRECTORY);
+ }
+
+ @Test
+ public void testDeleteEmptyDirectoryAsFolder() {
+ // GIVEN
+ String directory = "TestDirectory";
+ String inputFlowFileContent = "InputFlowFileContent";
+
+ createDirectory(directory);
+
+ // WHEN
+ // THEN
+ testSuccessfulDelete(fileSystemName, directory, null, inputFlowFileContent, inputFlowFileContent, DIRECTORY);
+ }
+
+ @Test
+ public void testDeleteSubdirectory() {
+ // GIVEN
+ String parentDirectory = "TestParentDirectory";
+ String childDirectory = "TestParentDirectory/TestChildDirectory";
+ String filename = "testFile.txt";
+ String fileContent = "AzureFileContent";
Review comment:
`fileContent` is not used. It could be deleted or as a better option, a file could be uploaded in order to test deleting a non-empty subdirectory.
Also for the following test case.
##########
File path: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/processors/azure/storage/ITDeleteAzureDataLakeStorage.java
##########
@@ -35,15 +35,82 @@
public class ITDeleteAzureDataLakeStorage extends AbstractAzureDataLakeStorageIT {
+ private static final boolean FILE = true;
+ private static final boolean DIRECTORY = false;
+
@Override
protected Class<? extends Processor> getProcessorClass() {
return DeleteAzureDataLakeStorage.class;
}
+ @Test
+ public void testDeleteDirectoryWithFiles() {
+ // GIVEN
+ String directory = "TestDirectory";
+ String filename = "testFile.txt";
+ String fileContent = "AzureFileContent";
+ String inputFlowFileContent = "InputFlowFileContent";
+
+ createDirectoryAndUploadFile(directory, filename, fileContent);
+
+ // WHEN
+ // THEN
+ testSuccessfulDelete(fileSystemName, directory, filename, inputFlowFileContent, inputFlowFileContent, DIRECTORY);
Review comment:
Passing the `filename` in is not necessary because it won't be used in case of directory deletion.
I would use `null` here as in case of the following test case.
Furthermore, I would suggest using the filename parameter to control the file/directory deletion mode instead of the boolean property (non-null filename => file, null filename => directory). I know that using null checks is not really nice but the boolean solution is not better either in my opinion.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org