From e7f44e318bf6993edd85f631a7e88403b7c075be Mon Sep 17 00:00:00 2001 From: Dwaipayan Mukherjee Date: Fri, 29 Jun 2018 17:39:59 -0700 Subject: [PATCH] address PR comments --- .../TestAzureBlockBlobFileSystemE2E.cs | 60 +++++++++++-------- .../TestAzureDataLakeFileSystemE2E.cs | 18 ++++-- .../TestHadoopFileSystem.cs | 28 +++++++-- .../AzureBlob/AzureBlockBlobFileSystem.cs | 12 +++- .../AzureBlob/AzureCloudBlobClient.cs | 8 +-- .../AzureBlob/AzureCloudBlobContainer.cs | 3 +- .../AzureBlob/AzureCloudBlobDirectory.cs | 9 ++- .../AzureBlob/AzureCloudBlockBlob.cs | 11 ++-- .../FileSystem/AzureBlob/ICloudBlobClient.cs | 2 +- .../AzureDataLake/AzureDataLakeFileSystem.cs | 5 +- 10 files changed, 99 insertions(+), 57 deletions(-) diff --git a/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureBlockBlobFileSystemE2E.cs b/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureBlockBlobFileSystemE2E.cs index 80f390a48f..e798ecd6e8 100644 --- a/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureBlockBlobFileSystemE2E.cs +++ b/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureBlockBlobFileSystemE2E.cs @@ -69,26 +69,22 @@ public void Dispose() private bool CheckBlobExists(ICloudBlob blob) { - var task = blob.ExistsAsync(); - return task.Result; + return blob.ExistsAsync().GetAwaiter().GetResult(); } private bool CheckContainerExists(CloudBlobContainer container) { - var task = container.ExistsAsync(); - return task.Result; + return container.ExistsAsync().GetAwaiter().GetResult(); } private ICloudBlob GetBlobReferenceFromServer(CloudBlobContainer container, string blobName) { - var task = container.GetBlobReferenceFromServerAsync(blobName); - return task.Result; + return container.GetBlobReferenceFromServerAsync(blobName).GetAwaiter().GetResult(); } private string DownloadText(CloudBlockBlob blob) { - var task = blob.DownloadTextAsync(); - return task.Result; + return blob.DownloadTextAsync().GetAwaiter().GetResult(); } [Fact(Skip = SkipMessage)] @@ -268,18 +264,34 @@ public void TestCopyFromLocalE2E() } [Fact(Skip = SkipMessage)] - public void TestIsDirectory() + public void TestIsDirectoryValidDirectoryLevel1E2E() { - const string Directory1 = "dir1"; - const string Directory2 = "dir1/dir2"; - const string Directory3 = "dir3"; - var blockBlobs1 = CreateTempBlobs(Directory1); - CreateTempBlobs(Directory2); + const string Directory = "dir"; + CreateTempBlobs(Directory); + Assert.True(_fileSystem.IsDirectory(PathToFile(Directory))); + } - Assert.True(_fileSystem.IsDirectory(PathToFile(Directory1))); - Assert.True(_fileSystem.IsDirectory(PathToFile(Directory2))); - Assert.False(_fileSystem.IsDirectory(PathToFile(Directory3))); - Assert.False(_fileSystem.IsDirectory(blockBlobs1.First().Uri)); + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryValidDirectoryLevel2E2E() + { + const string Directory = "dir1/dir2"; + CreateTempBlobs(Directory); + Assert.True(_fileSystem.IsDirectory(PathToFile(Directory))); + } + + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFakeDirectoryE2E() + { + const string Directory = "dir"; + Assert.False(_fileSystem.IsDirectory(PathToFile(Directory))); + } + + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFileE2E() + { + const string Directory = "dir"; + var blockBlobs = CreateTempBlobs(Directory); + Assert.False(_fileSystem.IsDirectory(blockBlobs.First().Uri)); } [Fact(Skip = SkipMessage)] @@ -328,18 +340,16 @@ public void TestDeleteDirectorySecondLevelE2E() Assert.True(CheckContainerExists(_container)); } - private List CreateTempBlobs(string directory, int fileCount = 3) + private IEnumerable CreateTempBlobs(string directory, int fileCount = 3) { - var blockBlobs = new List(); - for (var i = 0; i < fileCount; i++) + return Enumerable.Range(0, fileCount).Select(i => { var filePath = directory + '/' + i; var blockBlob = _container.GetBlockBlobReference(filePath); UploadFromString(blockBlob, "hello"); - Assert.True(CheckBlobExists(blockBlob)); - blockBlobs.Add(blockBlob); - } - return blockBlobs; + Assert.True(CheckBlobExists(blockBlob), "Blob does not exist: " + filePath); + return blockBlob; + }); } private static void UploadFromString(ICloudBlob blob, string str) diff --git a/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureDataLakeFileSystemE2E.cs b/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureDataLakeFileSystemE2E.cs index f139e3df2d..6d4841b699 100644 --- a/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureDataLakeFileSystemE2E.cs +++ b/lang/cs/Org.Apache.REEF.IO.Tests/TestAzureDataLakeFileSystemE2E.cs @@ -195,16 +195,24 @@ public void TestCreateDirectoryE2E() } [Fact(Skip = SkipMessage)] - public void TestIsDirectoryE2E() + public void TestIsDirectoryValidDirectoryE2E() { string dirName = $"/{_defaultFolderName}"; - string fakeDirName = $"/fakeDir"; - _adlsClient.CreateDirectory(dirName); - string fileName = UploadFromString(ContentsText); + Assert.True(_fileSystem.IsDirectory(PathToFile(dirName))); + } - Assert.True(_fileSystem.IsDirectory(PathToFile(dirName))); + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFakeDirectoryE2E() + { + string fakeDirName = $"/fakeDir"; Assert.False(_fileSystem.IsDirectory(PathToFile(fakeDirName))); + } + + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFileE2E() + { + string fileName = UploadFromString(ContentsText); Assert.False(_fileSystem.IsDirectory(PathToFile(fileName))); } diff --git a/lang/cs/Org.Apache.REEF.IO.Tests/TestHadoopFileSystem.cs b/lang/cs/Org.Apache.REEF.IO.Tests/TestHadoopFileSystem.cs index 0de6dd7c9e..7872bc8286 100644 --- a/lang/cs/Org.Apache.REEF.IO.Tests/TestHadoopFileSystem.cs +++ b/lang/cs/Org.Apache.REEF.IO.Tests/TestHadoopFileSystem.cs @@ -165,29 +165,47 @@ public void CreateUriForPathWithPrefix() } /// - /// Tests whether .IsDirectory() works. + /// Tests whether .IsDirectory() works for valid directory. /// [Fact(Skip = SkipMessage)] - public void TestIsDirectory() + public void TestIsDirectoryValidDirectory() { // Create directory var remoteDirUri = GetTempUri(); _fileSystem.CreateDirectory(remoteDirUri); + Assert.True(_fileSystem.IsDirectory(remoteDirUri)); + + // Clean up + _fileSystem.DeleteDirectory(remoteDirUri); + } + + /// + /// Tests whether .IsDirectory() works for fake directory. + /// + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFakeDirectory() + { // Create fake directory uri var remoteFakeuri = GetTempUri(); + Assert.False(_fileSystem.IsDirectory(remoteFakeuri)); + } + + /// + /// Tests whether .IsDirectory() works for file. + /// + [Fact(Skip = SkipMessage)] + public void TestIsDirectoryFile() + { // Create file var remoteFileUri = GetTempUri(); var localFile = FileSystemTestUtilities.MakeLocalTempFile(); _fileSystem.CopyFromLocal(localFile, remoteFileUri); - Assert.True(_fileSystem.IsDirectory(remoteDirUri)); - Assert.False(_fileSystem.IsDirectory(remoteFakeuri)); Assert.False(_fileSystem.IsDirectory(remoteFileUri)); // Clean up - _fileSystem.DeleteDirectory(remoteDirUri); _fileSystem.Delete(remoteFileUri); File.Delete(localFile); } diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureBlockBlobFileSystem.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureBlockBlobFileSystem.cs index 6f1f7a1c6b..6b63465d2d 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureBlockBlobFileSystem.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureBlockBlobFileSystem.cs @@ -133,9 +133,15 @@ public void CreateDirectory(Uri directoryUri) public bool IsDirectory(Uri uri) { var path = uri.AbsolutePath.TrimStart('/'); - const int maxBlobResults = 1; - var blobItems = _client.ListBlobsSegmented(path, false, BlobListingDetails.Metadata, maxBlobResults, null, null, null).Results; - return blobItems.Any() && blobItems.First() is CloudBlobDirectory; + var blobItems = _client.ListBlobsSegmented( + path, + useFlatListing: false, + BlobListingDetails.Metadata, + maxResults: 1, + continuationToken: null, + blobRequestOptions: null, + operationContext: null).Results; + return blobItems.OfType().Any(); } /// diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobClient.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobClient.cs index 8a72e22b0e..72ed015b84 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobClient.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobClient.cs @@ -55,8 +55,7 @@ public Uri BaseUri public ICloudBlob GetBlobReferenceFromServer(Uri blobUri) { - var task = _client.GetBlobReferenceFromServerAsync(blobUri); - return task.Result; + return _client.GetBlobReferenceFromServerAsync(blobUri).GetAwaiter().GetResult(); } public ICloudBlobContainer GetContainerReference(string containerName) @@ -78,15 +77,14 @@ public BlobResultSegment ListBlobsSegmented( BlobRequestOptions blobRequestOptions, OperationContext operationContext) { - var task = _client.ListBlobsSegmentedAsync( + return _client.ListBlobsSegmentedAsync( prefix, useFlatListing, blobListingDetails, maxResults, continuationToken, blobRequestOptions, - operationContext); - return task.Result; + operationContext).GetAwaiter().GetResult(); } public BlobResultSegment ListDirectoryBlobsSegmented( diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobContainer.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobContainer.cs index e2fbd6161d..53ae2aca14 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobContainer.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobContainer.cs @@ -35,8 +35,7 @@ public AzureCloudBlobContainer(CloudBlobContainer container, BlobRequestOptions public bool CreateIfNotExists() { - var task = _container.CreateIfNotExistsAsync(_requestOptions, null); - return task.Result; + return _container.CreateIfNotExistsAsync(_requestOptions, null).GetAwaiter().GetResult(); } public void DeleteIfExists() diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobDirectory.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobDirectory.cs index 4d3295e637..43963084fd 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobDirectory.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlobDirectory.cs @@ -39,8 +39,13 @@ public ICloudBlobDirectory GetDirectoryReference(string directoryName) public IEnumerable ListBlobs(bool useFlatListing = false) { - var task = _directory.ListBlobsSegmentedAsync(useFlatListing, BlobListingDetails.All, null, null, null, null); - return task.Result.Results; + return _directory.ListBlobsSegmentedAsync( + useFlatBlobListing: useFlatListing, + BlobListingDetails.All, + maxResults: null, + null, + null, + null).GetAwaiter().GetResult().Results; } } } diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlockBlob.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlockBlob.cs index c8835687d1..26e9ec9fdf 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlockBlob.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/AzureCloudBlockBlob.cs @@ -67,7 +67,7 @@ public Stream Open() { #if REEF_DOTNET_BUILD var task = _blob.OpenReadAsync(null, _requestOptions, null); - return task.Result; + return task.GetAwaiter().GetResult(); #else return _blob.OpenRead(null, _requestOptions, null); #endif @@ -76,8 +76,7 @@ public Stream Open() public Stream Create() { #if REEF_DOTNET_BUILD - var task = _blob.OpenWriteAsync(null, _requestOptions, null); - return task.Result; + return _blob.OpenWriteAsync(null, _requestOptions, null).GetAwaiter().GetResult(); #else return _blob.OpenWrite(null, _requestOptions, null); #endif @@ -85,8 +84,7 @@ public Stream Create() public bool Exists() { - var task = _blob.ExistsAsync(_requestOptions, null); - return task.Result; + return _blob.ExistsAsync(_requestOptions, null).GetAwaiter().GetResult(); } public void Delete() @@ -101,8 +99,7 @@ public void DeleteIfExists() public string StartCopy(Uri source) { - var task = _blob.StartCopyAsync(source, null, null, _requestOptions, null); - return task.Result; + return _blob.StartCopyAsync(source, null, null, _requestOptions, null).GetAwaiter().GetResult(); } public void DownloadToFile(string path, FileMode mode) diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/ICloudBlobClient.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/ICloudBlobClient.cs index 24492d036f..d4ce481177 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/ICloudBlobClient.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureBlob/ICloudBlobClient.cs @@ -59,7 +59,7 @@ internal interface ICloudBlobClient /// Paginates a blob listing with prefix. /// BlobResultSegment ListBlobsSegmented( - string path, + string prefix, bool useFlatListing, BlobListingDetails blobListingDetails, int? maxResults, diff --git a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureDataLake/AzureDataLakeFileSystem.cs b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureDataLake/AzureDataLakeFileSystem.cs index 7e14e5ae74..b0f250c0d8 100644 --- a/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureDataLake/AzureDataLakeFileSystem.cs +++ b/lang/cs/Org.Apache.REEF.IO/FileSystem/AzureDataLake/AzureDataLakeFileSystem.cs @@ -17,6 +17,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Net; @@ -249,10 +250,10 @@ public Uri CreateUriForPath(string path) } catch (UriFormatException) { - resultUri = new Uri(new Uri(this.UriPrefix), path); + resultUri = new Uri(new Uri(UriPrefix), path); } - if (!resultUri.AbsoluteUri.StartsWith(this.UriPrefix)) + if (!resultUri.AbsoluteUri.StartsWith(UriPrefix, true, CultureInfo.InvariantCulture)) { throw new ArgumentException($"Given URI must begin with valid prefix ({this.UriPrefix})", nameof(path)); }