Compression and JustSaying 8.0 Changes #1525

slang25 · 2024-10-03T10:43:24Z

There is still more to go, but I'm pushing this now to get some feedback.

This PR makes the following changes:

Adds message compression
Introduces a "Message Body Serializer/Deserializer" to keep the message format separate
Adds SNS raw delivery support
Fixes missing message attributes for subscribing messages that were directly sent to an SQS queue (i.e. without SNS wrapper)
Removes subscription reliance on Subject
Allows for publishing override for Subject
Removes serialization registry, serializers are flowed through the subscription.
Added LocalSqsSnsMessaging to make integration testing instantaneous.
Made more types internal

Outstanding actions:

Integration tests for subject override
Add subject override to all publishers
Integration tests for more scenarios
Re-introduce LocalStack as a flag for integration tests (have it still run on CI)
Revert samples app
Add missing XML doc comments
Add documentation

martincostello · 2024-10-03T13:25:42Z

src/JustSaying.Extensions.DependencyInjection.Microsoft/IServiceCollectionExtensions.cs

-                var serializerFactory = p.GetRequiredService<IMessageSerializationFactory>();
-                return new MessageSerializationRegister(config.MessageSubjectProvider, serializerFactory);
-            });
+        services.TryAddSingleton((p) => new MessageCompressionRegistry([new GzipMessageBodyCompression()]));


What if we resolved all the compressions from DI and then just registered GZip as the only one?

martincostello · 2024-10-03T13:25:57Z

src/JustSaying.Extensions.DependencyInjection.StructureMap/JustSayingRegistry.cs

-                })
-            .Singleton();
-
+        For<MessageCompressionRegistry>().Use((p) => new MessageCompressionRegistry(new List<IMessageBodyCompression> { new GzipMessageBodyCompression() })).Singleton();


src/JustSaying/AwsTools/MessageHandling/Compression/PublishCompressionOptions.cs

martincostello · 2024-10-03T13:27:27Z

src/JustSaying/AwsTools/MessageHandling/Compression/PublishCompressionOptions.cs

+    /// Gets or sets the compression encoding to be used.
+    /// </summary>
+    /// <remarks>
+    /// This should correspond to a registered compression algorithm in the IMessageCompressionRegistry.


Suggested change

/// This should correspond to a registered compression algorithm in the IMessageCompressionRegistry.

/// This should correspond to a registered compression algorithm in the <see cref="IMessageCompressionRegistry"/>.

martincostello · 2024-10-03T13:27:52Z

src/JustSaying/AwsTools/MessageHandling/Dispatch/MessageDispatcher.cs

@@ -8,21 +8,18 @@

 namespace JustSaying.AwsTools.MessageHandling.Dispatch;

-public class MessageDispatcher : IMessageDispatcher
+internal class MessageDispatcher : IMessageDispatcher


Can we seal it now if it's internal?

martincostello · 2024-10-03T13:55:22Z

src/JustSaying/Messaging/MessageHandling/MessageAttributes.cs

+    /// <summary>
+    /// Gets all available attribute keys.
+    /// </summary>
+    /// <returns>An IReadOnlyCollection of strings representing all attribute keys.</returns>


Suggested change

/// <returns>An IReadOnlyCollection of strings representing all attribute keys.</returns>

/// <returns>An <see cref="IReadOnlyCollection{T}"/> of strings representing all attribute keys.</returns>

martincostello · 2024-10-03T13:57:19Z

src/JustSaying/Messaging/MessageSerialization/NewtonsoftSerializationFactory.cs

-    }
-
-    public IMessageSerializer GetSerializer<T>() where T : Message => _serializer;
+    public IMessageBodySerializer GetSerializer<T>() where T : Message => new NewtonsoftMessageBodySerializer<T>(settings);


Do we want/need to create a new instance every time?

martincostello · 2024-10-03T13:57:54Z

src/JustSaying/Messaging/MessageSerialization/ReceivedMessage.cs

+    }
+}
+
+public sealed class PublishMessage


Move to own file.

src/JustSaying/Messaging/MessageSerialization/SystemTextJsonMessageBodySerializerBase.cs

martincostello · 2024-10-03T14:02:54Z

src/JustSaying/Messaging/MessageSerialization/SystemTextJsonSerializer`1.cs

+    public string Serialize(Message message)
+    {
+        return JsonSerializer.Serialize(message, message.GetType(), _options);
+    }


Can we make this Serialize<T>(T message)? message.GetType() won't play friendly for native AoT if we add support for that in the future.

I can imagine we may not have generic type in all callers. For AoT we can create and store the JTI if available in the constructor (and throw if not available), and use in the serialize and deserialize methods.

hwoodiwiss · 2024-10-04T10:13:26Z

src/JustSaying/Messaging/MessageSerialization/SystemTextJsonSerializer`1.cs

+    /// <summary>
+    /// Initializes a new instance of the <see cref="SystemTextJsonMessageBodySerializer{T}"/> class with default JSON serializer options.
+    /// </summary>
+    public SystemTextJsonMessageBodySerializer()
+    {
+        _options = DefaultJsonSerializerOptions;
+    }


I'd be tempted to say we should drop this constructor in favour of exposing a default options that can be passed to the other constructor. In AoT this one will have to be an instant throw.

src/JustSaying/AwsTools/MessageHandling/IPublishMessageConverter.cs

src/JustSaying/Messaging/MessageSerialization/SystemTextJsonMessageBodySerializer.cs

src/JustSaying/PublicAPI/net461/PublicAPI.Unshipped.txt

martincostello · 2024-10-07T14:52:47Z

src/JustSaying/AwsTools/MessageHandling/ReceivedMessageConverter.cs

@@ -30,12 +30,12 @@ public ReceivedMessage ConvertForReceive(Amazon.SQS.Model.Message message)
            var jsonNode = JsonNode.Parse(body);
            if (jsonNode is JsonObject jsonObject && jsonObject.TryGetPropertyValue("Message", out var messageNode))
            {
-                body = messageNode.ToString();
+                body = messageNode?.ToString();


Should this be ToJsonString()?

martincostello · 2024-10-08T07:36:31Z

...g/Channels/SubscriptionGroupTests/WhenExactlyOnceIsAppliedToHandlerWithoutExplicitTimeout.cs

+        _queue = CreateSuccessfulTestQueue(Guid.NewGuid().ToString(),
+            new TestMessage
+            {
+                Body = $$"""{"Subject":"SimpleMessage", "Message": "{{JsonEncodedText.Encode("""{ "Content": "Hi" }""")}}"}"""


TIL JsonEncodedText.

slang25 added 30 commits May 9, 2024 11:17

Initial subscription compression functionality

a58a62b

WIP - more compression changes

cc7d4d3

Add publisher compression support

2d3c495

Address more feedback

426030d

Further compression development

e9a8a77

Add some extra null handling

ddead74

Tweaked behaviour of CompressMessageIfNeeded

37eaa73

Add more xml doc comments

09413c3

Add null check and remove binary breaking change

ab075b0

Work on public api

d1dd060

Add back API which was made private

fff6506

Fix up API files

aa326ce

Add tests and fixes

c9ec0f9

Move missing encoding check

a4ef78c

Add extra validation

619b199

Add unit tests

ce4b6d4

Rename DefaultCompressionOptions

b3e5222

More work

82a0bca

WIP

cd4f61b

More compression refactoring

7c18a0d

Tidy up

df43e02

Fix CI

8c23f2f

Compression WIP

b0ea0ef

More compression work

4d732a0

Add RawMessageDelivery support

d888b5a

Fix more tests

c593ef6

Add more tests and cleanup code

3657d8d

More tidy up

d644e48

More tidy up

6c04948

More tidy up

cb8e0fd

More tidy up

fe1d325

slang25 requested a review from a team as a code owner October 3, 2024 10:43

slang25 marked this pull request as draft October 3, 2024 10:43

slang25 added 2 commits October 3, 2024 11:45

Merge branch 'main' into refactored-compression

140ba8e

More tidy up

8ca09ad

martincostello reviewed Oct 3, 2024

View reviewed changes

Apply some feedback

d2b2a8f

hwoodiwiss reviewed Oct 4, 2024

View reviewed changes

Apply some feedback

fbe31d9

martincostello reviewed Oct 4, 2024

View reviewed changes

slang25 added 4 commits October 4, 2024 15:57

Apply some feedback

64e0d21

Apply some feedback

ee7febf

Apply some feedback

ef110b8

More changes

8de4441

martincostello reviewed Oct 7, 2024

View reviewed changes

Fix json encoding issues

3872c83

martincostello reviewed Oct 8, 2024

View reviewed changes

slang25 added 2 commits October 8, 2024 10:58

Tidy up a few bits

7e016e6

Apply some feedback

d549a5c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compression and JustSaying 8.0 Changes #1525

Compression and JustSaying 8.0 Changes #1525

slang25 commented Oct 3, 2024 •

edited

Loading

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

martincostello Oct 3, 2024

hwoodiwiss Oct 3, 2024

hwoodiwiss Oct 4, 2024

martincostello Oct 7, 2024

martincostello Oct 8, 2024

	/// This should correspond to a registered compression algorithm in the IMessageCompressionRegistry.
	/// This should correspond to a registered compression algorithm in the <see cref="IMessageCompressionRegistry"/>.

	/// <returns>An IReadOnlyCollection of strings representing all attribute keys.</returns>
	/// <returns>An <see cref="IReadOnlyCollection{T}"/> of strings representing all attribute keys.</returns>

Compression and JustSaying 8.0 Changes #1525

Are you sure you want to change the base?

Compression and JustSaying 8.0 Changes #1525

Conversation

slang25 commented Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slang25 commented Oct 3, 2024 •

edited

Loading