Add custom collection names #407
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add custom collection name for Snapshots
jefflinse
reviewed
May 29, 2023
| @@ -140,6 +125,62 @@ func TestWithCollectionNamesIntegration(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestWithSnapshotCollectionNamesIntegration(t *testing.T) { | |||
There was a problem hiding this comment.
Table-based tests would be useful here.
There was a problem hiding this comment.
Yep, feel free to do it, or it can be done as part of a larger test overhaul in the future (nothing planned though).
|
|
||
| // providing empty snapshot collection names should result in an error | ||
| _, err = NewEventStore(url, db, | ||
| WithSnapshotCollectionName(""), |
There was a problem hiding this comment.
What happens if " " or similar is passed in? This could happen if the caller is dynamically forming the collection name and doesn't sufficiently check that it's not just pure whitespace.
There was a problem hiding this comment.
Good point, relates to your comment about trimming above. Feel free to add a test case for it, but I'll accept the PR without it too.
maxekman
approved these changes
May 30, 2023
|
@maxekman I added an additional check for spaces. |
|
@maxekman fill free to merge |
maxekman
approved these changes
Jun 17, 2023
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Let use a custom name for Outbox and Snapshots collections.
Affected Components
Related Issues
None
Solution and Design
This is a replication of the available functionality to set event and streams collection names.
Added an Option to set the custom name:
The WithSnapshotCollectionName is added to kepp compatibility with exitent WithtCollectionName (and because I want to review if Snapshot should be part of this EventStore or should be another store).
Steps to test and verify
Create an event store with custom name:
NewEventStore(url, db, WithSnapshotCollectionName("foo"))Check in database the collection created is
fooCreate an outbox with custom name:
NewOutbox(url, db, WithCollectionName("foo-outbox"))Check in database the collection created is
foo-outbox