From 048cffa1d1d42ae8f76eba41cbb13c4aa4d44492 Mon Sep 17 00:00:00 2001
From: Enkidu93 <lowryec17@gcc.edu>
Date: Thu, 7 Sep 2023 17:24:18 -0400
Subject: [PATCH] Fixes as per PR review --ECL

---
 .../Services/S3FileStorage.cs                 | 17 +++++++++----
 .../Services/S3WriteStream.cs                 | 24 +++++++++++++------
 .../Services/SharedFileService.cs             | 12 +++++++---
 3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/src/SIL.Machine.AspNetCore/Services/S3FileStorage.cs b/src/SIL.Machine.AspNetCore/Services/S3FileStorage.cs
index 413860de7..9058da211 100644
--- a/src/SIL.Machine.AspNetCore/Services/S3FileStorage.cs
+++ b/src/SIL.Machine.AspNetCore/Services/S3FileStorage.cs
@@ -5,15 +5,23 @@ public class S3FileStorage : FileStorage
     private readonly AmazonS3Client _client;
     private readonly string _bucketName;
     private readonly string _basePath;
+    private readonly ILoggerFactory _loggerFactory;
 
-    public S3FileStorage(string bucketName, string basePath, string accessKeyId, string secretAccessKey, string region)
+    public S3FileStorage(
+        string bucketName,
+        string basePath,
+        string accessKeyId,
+        string secretAccessKey,
+        string region,
+        ILoggerFactory loggerFactory
+    )
     {
         _client = new AmazonS3Client(
             accessKeyId,
             secretAccessKey,
             new AmazonS3Config
             {
-                RetryMode = Amazon.Runtime.RequestRetryMode.Standard,
+                RetryMode = RequestRetryMode.Standard,
                 MaxErrorRetry = 3,
                 RegionEndpoint = RegionEndpoint.GetBySystemName(region)
             }
@@ -23,6 +31,7 @@ public S3FileStorage(string bucketName, string basePath, string accessKeyId, str
         //Ultimately, object keys can neither begin nor end with slashes; this is what broke the earlier low-level implementation
         _basePath = basePath.EndsWith("/") ? basePath.Remove(basePath.Length - 1, 1) : basePath;
         _basePath = _basePath.StartsWith("/") ? _basePath.Remove(0, 1) : _basePath;
+        _loggerFactory = loggerFactory;
     }
 
     public override void Dispose() { }
@@ -78,8 +87,8 @@ public override async Task<Stream> OpenWrite(string path, CancellationToken canc
         InitiateMultipartUploadRequest request = new() { BucketName = _bucketName, Key = objectId };
         InitiateMultipartUploadResponse response = await _client.InitiateMultipartUploadAsync(request);
         return new BufferedStream(
-            new S3WriteStream(_client, objectId, _bucketName, response.UploadId),
-            1024 * 1024 * 5
+            new S3WriteStream(_client, objectId, _bucketName, response.UploadId, _loggerFactory),
+            S3WriteStream.FiveMB
         );
     }
 
diff --git a/src/SIL.Machine.AspNetCore/Services/S3WriteStream.cs b/src/SIL.Machine.AspNetCore/Services/S3WriteStream.cs
index 584a0e09c..339ca3a26 100644
--- a/src/SIL.Machine.AspNetCore/Services/S3WriteStream.cs
+++ b/src/SIL.Machine.AspNetCore/Services/S3WriteStream.cs
@@ -9,13 +9,21 @@ public class S3WriteStream : Stream
     private readonly List<UploadPartResponse> _uploadResponses;
     private readonly ILogger<S3WriteStream> _logger;
 
-    public S3WriteStream(AmazonS3Client client, string key, string bucketName, string uploadId)
+    public const int FiveMB = 5 * 1024 * 1024;
+
+    public S3WriteStream(
+        AmazonS3Client client,
+        string key,
+        string bucketName,
+        string uploadId,
+        ILoggerFactory loggerFactory
+    )
     {
         _client = client;
         _key = key;
         _bucketName = bucketName;
         _uploadId = uploadId;
-        _logger = new LoggerFactory().CreateLogger<S3WriteStream>();
+        _logger = loggerFactory.CreateLogger<S3WriteStream>();
         _uploadResponses = new List<UploadPartResponse>();
     }
 
@@ -59,7 +67,7 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
                     UploadId = _uploadId,
                     PartNumber = partNumber,
                     InputStream = ms,
-                    PartSize = 5 * 1024 * 1024
+                    PartSize = FiveMB
                 };
             request.StreamTransferProgress += new EventHandler<StreamTransferProgressArgs>(
                 (_, e) =>
@@ -76,7 +84,8 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
         }
         catch (Exception e)
         {
-            await Abort(e);
+            await AbortAsync(e);
+            throw;
         }
     }
 
@@ -106,7 +115,8 @@ protected override void Dispose(bool disposing)
             }
             catch (Exception e)
             {
-                Abort(e).WaitAndUnwrapException();
+                AbortAsync(e).WaitAndUnwrapException();
+                throw;
             }
         }
         base.Dispose(disposing);
@@ -134,11 +144,11 @@ public async override ValueTask DisposeAsync()
         }
         catch (Exception e)
         {
-            await Abort(e);
+            await AbortAsync(e);
         }
     }
 
-    private async Task Abort(Exception e)
+    private async Task AbortAsync(Exception e)
     {
         _logger.LogError(e, $"Aborted upload {_uploadId} to {_bucketName}/{_key}");
         AbortMultipartUploadRequest abortMPURequest =
diff --git a/src/SIL.Machine.AspNetCore/Services/SharedFileService.cs b/src/SIL.Machine.AspNetCore/Services/SharedFileService.cs
index 526cf95fd..5804584c1 100644
--- a/src/SIL.Machine.AspNetCore/Services/SharedFileService.cs
+++ b/src/SIL.Machine.AspNetCore/Services/SharedFileService.cs
@@ -1,13 +1,18 @@
-namespace SIL.Machine.AspNetCore.Services;
+using SIL.Reporting;
+
+namespace SIL.Machine.AspNetCore.Services;
 
 public class SharedFileService : ISharedFileService
 {
     private readonly Uri? _baseUri;
     private readonly FileStorage _fileStorage;
     private readonly bool _supportFolderDelete = true;
+    private readonly ILoggerFactory _loggerFactory;
 
-    public SharedFileService(IOptions<SharedFileOptions>? options = null)
+    public SharedFileService(ILoggerFactory loggerFactory, IOptions<SharedFileOptions>? options = null)
     {
+        _loggerFactory = loggerFactory;
+
         if (options?.Value.Uri is null)
         {
             _fileStorage = new InMemoryStorage();
@@ -30,7 +35,8 @@ public SharedFileService(IOptions<SharedFileOptions>? options = null)
                         _baseUri.AbsolutePath,
                         options.Value.S3AccessKeyId,
                         options.Value.S3SecretAccessKey,
-                        options.Value.S3Region
+                        options.Value.S3Region,
+                        _loggerFactory
                     );
                     _supportFolderDelete = false;
                     break;