Skip to content

Commit bbd10e3

Browse files
ucswiftucswift
committed
RE1-T115 PR#358 fixes
1 parent b52de41 commit bbd10e3

2 files changed

Lines changed: 247 additions & 24 deletions

File tree

‎Tests/Resgrid.Tests/Web/Tts/S3StorageServiceTests.cs‎

Lines changed: 165 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
using System.IO;
44
using System.Net;
55
using System.Net.Http;
6+
using System.Reflection;
67
using System.Threading;
78
using System.Threading.Tasks;
9+
using Amazon.Runtime;
810
using Amazon.S3;
911
using Amazon.S3.Model;
1012
using FluentAssertions;
@@ -56,32 +58,81 @@ public async Task upload_async_should_buffer_non_seekable_stream_for_retries()
5658
}
5759

5860
[Test]
59-
public async Task exists_async_should_treat_malformed_metadata_response_as_existing_object()
61+
public async Task exists_async_should_verify_with_presigned_head_when_metadata_unmarshalling_fails()
6062
{
6163
var s3Client = new Mock<IAmazonS3>(MockBehavior.Strict);
6264
s3Client
6365
.Setup(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()))
64-
.ThrowsAsync(new FormatException("bad metadata expiration header"));
66+
.ThrowsAsync(CreateMetadataUnmarshallingException("bad metadata expiration header"));
67+
s3Client
68+
.Setup(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()))
69+
.Returns<GetPreSignedUrlRequest>(request =>
70+
{
71+
request.BucketName.Should().Be("tts-bucket");
72+
request.Key.Should().Be("tts/audio.wav");
73+
request.Verb.Should().Be(HttpVerb.HEAD);
74+
request.Protocol.Should().Be(Protocol.HTTP);
75+
return "http://download.example.com/tts/audio.wav?signature=head";
76+
});
6577

66-
var handler = new RecordingHttpMessageHandler((_, _) =>
67-
Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)));
78+
var handler = new RecordingHttpMessageHandler((request, _) =>
79+
{
80+
request.Method.Should().Be(HttpMethod.Head);
81+
request.RequestUri.Should().Be(new Uri("http://download.example.com/tts/audio.wav?signature=head"));
82+
return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK));
83+
});
6884
var service = CreateService(s3Client.Object, handler, useSsl: false);
6985

7086
var exists = await service.ExistsAsync("tts/audio.wav", CancellationToken.None);
7187

7288
exists.Should().BeTrue();
73-
handler.Requests.Should().BeEmpty();
89+
handler.Requests.Should().HaveCount(1);
7490
s3Client.Verify(x => x.GetObjectMetadataAsync(It.Is<GetObjectMetadataRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav"), It.IsAny<CancellationToken>()), Times.Once);
75-
s3Client.Verify(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()), Times.Never);
91+
s3Client.Verify(x => x.GetPreSignedURL(It.Is<GetPreSignedUrlRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav" && request.Verb == HttpVerb.HEAD && request.Protocol == Protocol.HTTP)), Times.Once);
92+
}
93+
94+
[Test]
95+
public async Task exists_async_should_return_false_when_presigned_head_reports_missing_after_metadata_unmarshalling_failure()
96+
{
97+
var s3Client = new Mock<IAmazonS3>(MockBehavior.Strict);
98+
s3Client
99+
.Setup(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()))
100+
.ThrowsAsync(CreateMetadataUnmarshallingException("bad metadata expiration header"));
101+
s3Client
102+
.Setup(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()))
103+
.Returns<GetPreSignedUrlRequest>(request =>
104+
{
105+
request.BucketName.Should().Be("tts-bucket");
106+
request.Key.Should().Be("tts/audio.wav");
107+
request.Verb.Should().Be(HttpVerb.HEAD);
108+
request.Protocol.Should().Be(Protocol.HTTP);
109+
return "http://download.example.com/tts/audio.wav?signature=head";
110+
});
111+
112+
var handler = new RecordingHttpMessageHandler((request, _) =>
113+
{
114+
request.Method.Should().Be(HttpMethod.Head);
115+
return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound));
116+
});
117+
var service = CreateService(s3Client.Object, handler, useSsl: false);
118+
119+
var exists = await service.ExistsAsync("tts/audio.wav", CancellationToken.None);
120+
121+
exists.Should().BeFalse();
122+
handler.Requests.Should().HaveCount(1);
123+
s3Client.Verify(x => x.GetPreSignedURL(It.Is<GetPreSignedUrlRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav" && request.Verb == HttpVerb.HEAD && request.Protocol == Protocol.HTTP)), Times.Once);
76124
}
77125

78126
[Test]
79-
public async Task upload_async_should_treat_malformed_put_response_as_success_without_using_presigned_uploads()
127+
public async Task upload_async_should_treat_malformed_put_response_as_success_when_the_object_is_verified()
80128
{
81129
var s3Client = new Mock<IAmazonS3>(MockBehavior.Strict);
82130
s3Client
83131
.Setup(x => x.PutObjectAsync(It.IsAny<PutObjectRequest>(), It.IsAny<CancellationToken>()))
84132
.ThrowsAsync(new FormatException("bad expiration header"));
133+
s3Client
134+
.Setup(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()))
135+
.ReturnsAsync(new GetObjectMetadataResponse());
85136

86137
var handler = new RecordingHttpMessageHandler((_, _) =>
87138
Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)));
@@ -93,15 +144,16 @@ public async Task upload_async_should_treat_malformed_put_response_as_success_wi
93144

94145
handler.Requests.Should().BeEmpty();
95146
s3Client.Verify(x => x.PutObjectAsync(It.Is<PutObjectRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav" && request.ContentType == "audio/wav"), It.IsAny<CancellationToken>()), Times.Once);
96-
s3Client.Verify(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()), Times.Never);
147+
s3Client.Verify(x => x.GetObjectMetadataAsync(It.Is<GetObjectMetadataRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav"), It.IsAny<CancellationToken>()), Times.Once);
97148
s3Client.Verify(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()), Times.Never);
98149
}
99150

100151
[Test]
101-
public async Task upload_async_should_treat_malformed_put_response_as_success_even_if_sdk_disposes_input_stream()
152+
public async Task upload_async_should_retry_with_a_presigned_put_when_verification_reports_the_object_missing()
102153
{
103154
var s3Client = new Mock<IAmazonS3>(MockBehavior.Strict);
104155
byte[] capturedPayload = null;
156+
byte[] presignedPayload = null;
105157
s3Client
106158
.Setup(x => x.PutObjectAsync(It.IsAny<PutObjectRequest>(), It.IsAny<CancellationToken>()))
107159
.Returns<PutObjectRequest, CancellationToken>(async (request, _) =>
@@ -112,19 +164,40 @@ public async Task upload_async_should_treat_malformed_put_response_as_success_ev
112164
request.InputStream.Dispose();
113165
throw new FormatException("bad expiration header");
114166
});
167+
s3Client
168+
.Setup(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()))
169+
.ThrowsAsync(CreateNotFoundS3Exception());
170+
s3Client
171+
.Setup(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()))
172+
.Returns<GetPreSignedUrlRequest>(request =>
173+
{
174+
request.BucketName.Should().Be("tts-bucket");
175+
request.Key.Should().Be("tts/audio.wav");
176+
request.Verb.Should().Be(HttpVerb.PUT);
177+
request.Protocol.Should().Be(Protocol.HTTP);
178+
request.ContentType.Should().Be("audio/wav");
179+
return "http://upload.example.com/tts/audio.wav?signature=put";
180+
});
115181

116-
var handler = new RecordingHttpMessageHandler((_, _) =>
117-
Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)));
118-
var service = CreateService(s3Client.Object, handler);
182+
var handler = new RecordingHttpMessageHandler(async (request, _) =>
183+
{
184+
request.Method.Should().Be(HttpMethod.Put);
185+
request.RequestUri.Should().Be(new Uri("http://upload.example.com/tts/audio.wav?signature=put"));
186+
request.Content.Headers.ContentType?.MediaType.Should().Be("audio/wav");
187+
presignedPayload = await request.Content.ReadAsByteArrayAsync();
188+
return new HttpResponseMessage(HttpStatusCode.OK);
189+
});
190+
var service = CreateService(s3Client.Object, handler, useSsl: false);
119191

120192
await using var content = new MemoryStream(new byte[] { 7, 5, 3, 1 }, writable: false);
121193

122194
await service.UploadAsync("tts/audio.wav", content, "audio/wav", CancellationToken.None);
123195

124196
capturedPayload.Should().Equal(7, 5, 3, 1);
125-
handler.Requests.Should().BeEmpty();
126-
s3Client.Verify(x => x.GetObjectMetadataAsync(It.IsAny<GetObjectMetadataRequest>(), It.IsAny<CancellationToken>()), Times.Never);
127-
s3Client.Verify(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()), Times.Never);
197+
presignedPayload.Should().Equal(7, 5, 3, 1);
198+
handler.Requests.Should().HaveCount(1);
199+
s3Client.Verify(x => x.GetObjectMetadataAsync(It.Is<GetObjectMetadataRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav"), It.IsAny<CancellationToken>()), Times.Once);
200+
s3Client.Verify(x => x.GetPreSignedURL(It.Is<GetPreSignedUrlRequest>(request => request.BucketName == "tts-bucket" && request.Key == "tts/audio.wav" && request.Verb == HttpVerb.PUT && request.Protocol == Protocol.HTTP && request.ContentType == "audio/wav")), Times.Once);
128201
}
129202

130203
[Test]
@@ -172,6 +245,83 @@ public async Task get_object_url_async_should_prefer_absolute_endpoint_scheme_ov
172245
s3Client.Verify(x => x.GetPreSignedURL(It.IsAny<GetPreSignedUrlRequest>()), Times.Once);
173246
}
174247

248+
private static AmazonS3Exception CreateNotFoundS3Exception()
249+
{
250+
return new AmazonS3Exception(
251+
"Object was not found.",
252+
ErrorType.Unknown,
253+
"NoSuchKey",
254+
"request-id",
255+
HttpStatusCode.NotFound);
256+
}
257+
258+
private static AmazonUnmarshallingException CreateMetadataUnmarshallingException(string message)
259+
{
260+
var innerException = new FormatException(message);
261+
262+
foreach (var constructor in typeof(AmazonUnmarshallingException).GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
263+
{
264+
var parameters = constructor.GetParameters();
265+
var arguments = new object[parameters.Length];
266+
var usedInnerException = false;
267+
var usedMessage = false;
268+
var supported = true;
269+
270+
for (var index = 0; index < parameters.Length; index++)
271+
{
272+
var parameterType = parameters[index].ParameterType;
273+
274+
if (!usedInnerException && typeof(Exception).IsAssignableFrom(parameterType))
275+
{
276+
arguments[index] = innerException;
277+
usedInnerException = true;
278+
continue;
279+
}
280+
281+
if (parameterType == typeof(string))
282+
{
283+
arguments[index] = usedMessage ? "/HeadObjectResult" : message;
284+
usedMessage = true;
285+
continue;
286+
}
287+
288+
if (parameterType == typeof(bool))
289+
{
290+
arguments[index] = false;
291+
continue;
292+
}
293+
294+
if (parameterType == typeof(int))
295+
{
296+
arguments[index] = 0;
297+
continue;
298+
}
299+
300+
supported = false;
301+
break;
302+
}
303+
304+
if (!supported || !usedInnerException)
305+
{
306+
continue;
307+
}
308+
309+
try
310+
{
311+
if (constructor.Invoke(arguments) is AmazonUnmarshallingException exception
312+
&& exception.InnerException is FormatException)
313+
{
314+
return exception;
315+
}
316+
}
317+
catch
318+
{
319+
}
320+
}
321+
322+
throw new InvalidOperationException("Unable to construct AmazonUnmarshallingException for the test.");
323+
}
324+
175325
private static S3StorageService CreateService(IAmazonS3 s3Client, RecordingHttpMessageHandler handler = null, bool useSsl = true, string endpoint = null)
176326
{
177327
handler ??= new RecordingHttpMessageHandler((_, _) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)));

‎Web/Resgrid.Web.Tts/Services/S3StorageService.cs‎

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,75 @@ await ExecuteWithRetryAsync(
5252
{
5353
return false;
5454
}
55-
catch (FormatException ex)
55+
catch (AmazonUnmarshallingException ex) when (ex.InnerException is FormatException formatException)
5656
{
57+
return await HandleMalformedMetadataResponseAsync(objectKey, ex, formatException, cancellationToken);
58+
}
59+
}
60+
61+
private async Task<bool> HandleMalformedMetadataResponseAsync(
62+
string objectKey,
63+
AmazonUnmarshallingException exception,
64+
FormatException formatException,
65+
CancellationToken cancellationToken)
66+
{
67+
_logger.LogWarning(
68+
exception,
69+
"The S3 client could not parse the metadata response for {ObjectKey}. Verifying existence with a presigned HEAD request. Inner format error: {InnerFormatErrorMessage}",
70+
objectKey,
71+
formatException.Message);
72+
73+
_logger.LogDebug(
74+
formatException,
75+
"Inner FormatException while parsing the metadata response for {ObjectKey}. Last known location: {LastKnownLocation}.",
76+
objectKey,
77+
exception.LastKnownLocation ?? "unknown");
78+
79+
try
80+
{
81+
var exists = await ExistsWithPresignedHeadAsync(objectKey, cancellationToken);
82+
5783
_logger.LogDebug(
58-
ex,
59-
"The S3 client could not parse the metadata response for {ObjectKey}. Treating the object as existing because the metadata request completed before the response parsing failure.",
60-
objectKey);
84+
"Presigned HEAD verification after the metadata parsing failure reported that {ObjectKey} {ExistenceState}.",
85+
objectKey,
86+
exists ? "exists" : "does not exist");
6187

62-
return true;
88+
return exists;
6389
}
90+
catch (AmazonServiceException verificationException)
91+
{
92+
_logger.LogWarning(
93+
verificationException,
94+
"Unable to verify whether {ObjectKey} exists after the metadata parsing failure. Assuming the object exists because S3 returned a response before the unmarshalling error.",
95+
objectKey);
96+
}
97+
catch (HttpRequestException verificationException)
98+
{
99+
_logger.LogWarning(
100+
verificationException,
101+
"Unable to verify whether {ObjectKey} exists after the metadata parsing failure due to connectivity. Assuming the object exists because S3 returned a response before the unmarshalling error.",
102+
objectKey);
103+
}
104+
catch (TaskCanceledException verificationException) when (!cancellationToken.IsCancellationRequested)
105+
{
106+
_logger.LogWarning(
107+
verificationException,
108+
"Unable to verify whether {ObjectKey} exists after the metadata parsing failure due to timeout. Assuming the object exists because S3 returned a response before the unmarshalling error.",
109+
objectKey);
110+
}
111+
catch (IOException verificationException)
112+
{
113+
_logger.LogWarning(
114+
verificationException,
115+
"Unable to verify whether {ObjectKey} exists after the metadata parsing failure due to IO. Assuming the object exists because S3 returned a response before the unmarshalling error.",
116+
objectKey);
117+
}
118+
119+
// The metadata request reached S3 and only failed while the SDK parsed the response.
120+
// If the explicit presigned HEAD verification also fails, preserve the best-effort
121+
// behavior and assume the object exists so callers such as CacheService understand
122+
// this path can still return an optimistic result.
123+
return true;
64124
}
65125

66126
public async Task UploadAsync(string objectKey, Stream content, string contentType, CancellationToken cancellationToken)
@@ -80,19 +140,32 @@ await ExecuteWithRetryAsync(
80140
}
81141
}
82142

83-
private Task HandleMalformedPutResponseAsync(
143+
private async Task HandleMalformedPutResponseAsync(
84144
string objectKey,
85145
byte[] payload,
86146
string contentType,
87147
FormatException exception,
88148
CancellationToken cancellationToken)
89149
{
90-
_logger.LogDebug(
150+
_logger.LogWarning(
91151
exception,
92-
"The S3 client could not parse the PUT response for {ObjectKey}. Treating the upload as successful because the object upload completed before the response parsing failure.",
152+
"The S3 client could not parse the PUT response for {ObjectKey}. Verifying whether the upload persisted before falling back to a presigned PUT upload.",
153+
objectKey);
154+
155+
if (await WasUploadPersistedAsync(objectKey, cancellationToken))
156+
{
157+
_logger.LogInformation(
158+
"The upload for {ObjectKey} was verified after the PUT response parsing failure. Treating the upload as successful.",
159+
objectKey);
160+
161+
return;
162+
}
163+
164+
_logger.LogWarning(
165+
"The upload for {ObjectKey} could not be verified after the PUT response parsing failure. Retrying with a presigned PUT upload.",
93166
objectKey);
94167

95-
return Task.CompletedTask;
168+
await UploadWithPresignedUrlAsync(objectKey, payload, contentType, cancellationToken);
96169
}
97170

98171
private async Task<bool> WasUploadPersistedAsync(string objectKey, CancellationToken cancellationToken)

0 commit comments

Comments
 (0)