Introduce paging to Dynamo DB outbox methods & fix runtime exceptions by dhickie · Pull Request #3180 · BrighterCommand/Brighter

dhickie · 2024-07-02T12:44:43Z

This PR fixes #3108 using the approach agreed in the issue. This has the effect of:

Adhering to page sizes passed to the OutstandingMessages and DispatchedMessages methods in the DynamoDbOutbox
Supporting retrieval of outstanding or dispatched messages from the outbox for all topics, instead of one particular topic

iancooper

A few housekeeping notes around licenses and namespaces, which we should probably document somewhere; a possible option on FakeTimeProvider, but...

... those are niggles and this looks great

iancooper · 2024-07-02T14:25:25Z

@@ -0,0 +1,18 @@
+using System.Collections.Generic;


Can you add the MIT License in a region; you should be able to find this in another file. You can use your own name in the copyright information?

iancooper · 2024-07-02T14:25:31Z

@@ -0,0 +1,23 @@
+using System.Collections.Generic;


Can you add the MIT License in a region; you should be able to find this in another file. You can use your own name in the copyright information?

iancooper · 2024-07-02T14:25:37Z

@@ -0,0 +1,14 @@
+namespace Paramore.Brighter.Outbox.DynamoDB


Can you add the MIT License in a region; you should be able to find this in another file. You can use your own name in the copyright information?

iancooper · 2024-07-02T14:25:49Z

@@ -0,0 +1,20 @@
+using System.Collections.Generic;


Can you add the MIT License in a region; you should be able to find this in another file. You can use your own name in the copyright information?

iancooper · 2024-07-02T14:26:12Z

@@ -0,0 +1,18 @@
+using System.Collections.Generic;
+
+namespace Paramore.Brighter.Outbox.DynamoDB


It's alright to use a file scoped namespace.

iancooper · 2024-07-02T14:26:57Z

@@ -0,0 +1,30 @@
+using System.Collections.Generic;
+
+namespace Paramore.Brighter.Outbox.DynamoDB


It's alright to use a file-scoped namespace

iancooper · 2024-07-02T14:27:04Z

@@ -0,0 +1,16 @@
+namespace Paramore.Brighter.Outbox.DynamoDB


Can you add the MIT License in a region; you should be able to find this in another file. You can use your own name in the copyright information?

iancooper · 2024-07-02T14:27:33Z

@@ -0,0 +1,284 @@
+using System;


Just FYI, we DON'T tend to put the license in test files

iancooper · 2024-07-02T14:30:11Z

        private readonly DynamoDBOperationConfig _dynamoOverwriteTableConfig;
        private readonly Random _random = new Random();

+        // Stores context of the current progress of any paged queries for outstanding or dispatched messages to particular topics


We only tend to document the public interface of a public class, unless it is non-obvious; tl-dr avoid comments on details

Yeah I'm forever trying to decide whether something would or wouldn't be obvious to another developer coming into the codebase. In these instances I thought it would be helpful as Dynamo's paging mechanism (and hence the need for these variables) can be confusing, and it may not be clear why there's a ConcurrentDictionary being used to store what is only ever used as a list. I'm happy to remove the comments if you think that isn't warranted though?

I'd comment more on the why in the code, over the state of the object.

iancooper · 2024-07-02T16:07:56Z

+        await _dynamoDbOutbox.AddAsync(_message, context);
+        await _dynamoDbOutbox.MarkDispatchedAsync(_message.Id, context);
+
+        await Task.Delay(1000);


It's possible we could use FakeTimeProvider to advance a time/date since marked as dispatched and allow us to avoid having to delay here

dhickie · 2024-07-03T14:02:34Z

@iancooper The latest set of changes should address your comments.

iancooper

Thanks @dhickie it looks good.

iancooper · 2024-07-05T16:03:31Z

@dhickie Looks good, do you want to merge?

dhickie · 2024-07-09T15:09:44Z

@iancooper Yep, happy for this to be merged. I've just merged in the latest from master.

…BrighterCommand#3180) * Start of dynamo outbox dispatch query rework * Continuation of dynamo outbox dispatch query rework * Begin adding paging to outstanding messages query * Finish adding support for querying outstanding messages from all topics * Update outstanding message tests * Add tests for dispatched messages * Remove unused code and fix issue with misinterpreted paging tokens * Review comments

dhickie added 8 commits May 30, 2024 16:32

Start of dynamo outbox dispatch query rework

fcd8b8d

Continuation of dynamo outbox dispatch query rework

49920fb

Merge remote-tracking branch 'upstream/master' into issues/3108

f070416

Begin adding paging to outstanding messages query

df1c2e5

Finish adding support for querying outstanding messages from all topics

4adb764

Update outstanding message tests

827f57e

Add tests for dispatched messages

98a44fb

Merge remote-tracking branch 'upstream/master' into issues/3108

c00b2f8

dhickie requested review from DevJonny, holytshirt, iancooper and preardon as code owners July 2, 2024 12:44

Remove unused code and fix issue with misinterpreted paging tokens

7217bd0

iancooper requested changes Jul 2, 2024

View reviewed changes

Review comments

7e9fc97

iancooper approved these changes Jul 3, 2024

View reviewed changes

dhickie mentioned this pull request Jul 4, 2024

Backport support for outstanding and dispatched messages across all topics to v9 #3183

Merged

iancooper added 3 - Done Bug feature request v10 v9 Required for v9 release .NET Pull requests that update .net code labels Jul 5, 2024

Merge remote-tracking branch 'upstream/master' into issues/3108

dd75614

iancooper merged commit d4367bc into BrighterCommand:master Jul 9, 2024

dhickie deleted the issues/3108 branch February 27, 2025 14:54

dependabot Bot mentioned this pull request Oct 13, 2025

Bump Paramore.Brighter from 9.9.12 to 10.0.1 VerifyTests/Verify.Brighter#304

Closed

This was referenced May 7, 2026

Bump Paramore.Brighter from 9.9.13 to 10.4.1 mzet97/ZetAuction#14

Closed

Bump Paramore.Brighter.Extensions.DependencyInjection from 9.9.13 to 10.4.1 mzet97/ZetAuction#15

Closed

Bump Paramore.Brighter and Paramore.Brighter.PostgreSql mzet97/ZetAuction#16

Open

dependabot Bot mentioned this pull request Jun 9, 2026

Bump Paramore.Brighter.Extensions.DependencyInjection from 9.9.13 to 10.5.1 mzet97/ZetAuction#24

Open

		@@ -0,0 +1,18 @@
		using System.Collections.Generic;

		namespace Paramore.Brighter.Outbox.DynamoDB

		@@ -0,0 +1,30 @@
		using System.Collections.Generic;

		namespace Paramore.Brighter.Outbox.DynamoDB

Conversation

dhickie commented Jul 2, 2024

Uh oh!

iancooper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhickie commented Jul 3, 2024

Uh oh!

iancooper left a comment

Choose a reason for hiding this comment

Uh oh!

iancooper commented Jul 5, 2024

Uh oh!

dhickie commented Jul 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants