Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix 4.1.2 #257

Merged
merged 2 commits into from
Nov 19, 2018
Merged

Hotfix 4.1.2 #257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad48af0
Add test to verify that body objects are not deleted
danielmarbach Nov 19, 2018
512b1ff
Remove message body deletion
danielmarbach Nov 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/AcceptanceTests/Sending_messages_larger_than_256kb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Threading.Tasks;
using AcceptanceTesting;
using AcceptanceTesting.Customization;
using AmazonSQS.AcceptanceTests;
using EndpointTemplates;
using Configuration.AdvancedExtensibility;

Expand All @@ -25,7 +26,9 @@ public async Task Should_receive_messages_with_large_payload_correctly()
.Done(c => c.ReceivedPayload != null)
.Run();

var s3Client = SqsTransportExtensions.CreateS3Client();
Assert.AreEqual(payloadToSend, context.ReceivedPayload, "The large payload should be handled correctly using S3");
Assert.DoesNotThrowAsync(async () => await s3Client.GetObjectAsync(SqsTransportExtensions.S3BucketName, $"{SqsTransportExtensions.S3Prefix}/{context.MessageId}"));
}

[Test]
Expand All @@ -39,8 +42,8 @@ public async Task Should_fail_when_no_s3_bucket_is_configured()
// Don't configure an S3 bucket for this endpoint
b.CustomConfig(x =>
{
x.GetSettings().Set("NServiceBus.AmazonSQS.S3BucketForLargeMessages", string.Empty);
x.GetSettings().Set("NServiceBus.AmazonSQS.S3Key", string.Empty);
x.GetSettings().Set(SettingsKeys.S3BucketForLargeMessages, string.Empty);
x.GetSettings().Set(SettingsKeys.S3KeyPrefix, string.Empty);
});

b.When(async (session, c) =>
Expand All @@ -64,8 +67,8 @@ await session.Send(new MyMessageWithLargePayload
// Don't configure an S3 bucket for this endpoint
b.CustomConfig(x =>
{
x.GetSettings().Set("NServiceBus.AmazonSQS.S3BucketForLargeMessages", string.Empty);
x.GetSettings().Set("NServiceBus.AmazonSQS.S3Key", string.Empty);
x.GetSettings().Set(SettingsKeys.S3BucketForLargeMessages, string.Empty);
x.GetSettings().Set(SettingsKeys.S3KeyPrefix, string.Empty);
});
})
.Done(c => c.GotTheException)
Expand All @@ -80,6 +83,8 @@ public class Context : ScenarioContext
{
public byte[] ReceivedPayload { get; set; }

public string MessageId { get; set; }

public bool GotTheException { get; set; }

public Exception Exception { get; set; }
Expand Down Expand Up @@ -110,6 +115,7 @@ public class MyMessageHandler : IHandleMessages<MyMessageWithLargePayload>
public Task Handle(MyMessageWithLargePayload messageWithLargePayload, IMessageHandlerContext context)
{
Context.ReceivedPayload = messageWithLargePayload.Payload;
Context.MessageId = context.MessageId;

return Task.FromResult(0);
}
Expand Down
9 changes: 6 additions & 3 deletions src/AcceptanceTests/SqsTransportExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@ public static TransportExtensions<SqsTransport> ConfigureSqsTransport(this Trans
.QueueNamePrefix(queueNamePrefix)
.PreTruncateQueueNamesForAcceptanceTests();

var s3BucketName = EnvironmentHelper.GetEnvironmentVariable(S3BucketEnvironmentVariableName);
S3BucketName = EnvironmentHelper.GetEnvironmentVariable(S3BucketEnvironmentVariableName);

if (!string.IsNullOrEmpty(s3BucketName))
if (!string.IsNullOrEmpty(S3BucketName))
{
var s3Configuration = transportConfiguration.S3(s3BucketName, "test");
var s3Configuration = transportConfiguration.S3(S3BucketName, S3Prefix);
s3Configuration.ClientFactory(CreateS3Client);
}

return transportConfiguration;
}

public const string S3Prefix = "test";
public static string S3BucketName;

public static IAmazonSQS CreateSQSClient() => new AmazonSQSClient();

public static IAmazonS3 CreateS3Client() => new AmazonS3Client();
Expand Down
31 changes: 2 additions & 29 deletions src/NServiceBus.AmazonSQS/MessagePump.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Threading;
using System.Threading.Tasks;
using Amazon.S3;
using Amazon.S3.Model;
using Amazon.SQS;
using Amazon.SQS.Model;
using AmazonSQS;
Expand Down Expand Up @@ -438,35 +437,9 @@ async Task DeleteMessageAndBodyIfRequired(Message message, TransportMessage tran
return; // if another receiver fetches the data from S3
}

if (transportMessage != null)
if (!string.IsNullOrEmpty(transportMessage?.S3BodyKey))
{
if (!string.IsNullOrEmpty(transportMessage.S3BodyKey))
{
try
{
await s3Client.DeleteObjectAsync(
new DeleteObjectRequest
{
BucketName = configuration.S3BucketForLargeMessages,
Key = transportMessage.S3BodyKey
},
token).ConfigureAwait(false);
}
catch (Exception ex)
{
// If deleting the message body from S3 fails, we don't
// want the exception to make its way through to the _endProcessMessage below,
// as the message has been successfully processed and deleted from the SQS queue
// and effectively doesn't exist anymore.
// It doesn't really matter, as S3 is configured to delete message body data
// automatically after a certain period of time.
Logger.Warn("Couldn't delete message body from S3. Message body data will be aged out by the S3 lifecycle policy when the TTL expires.", ex);
}
}
}
else
{
Logger.Warn("Couldn't delete message body from S3 because the TransportMessage was null. Message body data will be aged out by the S3 lifecycle policy when the TTL expires.");
Logger.Info($"Message body data with key '{transportMessage.S3BodyKey}' will be aged out by the S3 lifecycle policy when the TTL expires.");
}
}

Expand Down