-
Notifications
You must be signed in to change notification settings - Fork 0
Use UUIDs for messages instead of IDs #28
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates message identification from integer IDs to UUIDs throughout the codebase. The change affects message structures, database queries, API endpoints, and test fixtures to use UUID-based identifiers instead of numeric IDs.
- Replaces
MsgID(int64) withMsgUUID(string UUID) across all message-related types - Updates API contracts to use
uuidfield instead ofidfor messages - Modifies database queries to return/query by UUID instead of ID
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/models/msg.go | Defines new MsgUUID type and updates message structs to use UUID instead of ID |
| core/models/msg_test.go | Updates test assertions to check UUIDs instead of IDs |
| core/queue/outboxes.go | Changes item ID generation to use UUID format |
| core/queue/outboxes_test.go | Updates test data and assertions to use UUIDs |
| core/courier/courier.go | Updates courier interface to accept MsgUUID instead of MsgID |
| core/courier/courier_test.go | Updates test expectations for UUID-based courier calls |
| core/courier/events.go | Changes courier events to use MsgUUID |
| service.go | Updates message delivery confirmation to parse and validate UUIDs |
| web/client.go | Changes acknowledgment item ID format to use UUID |
| web/commands/ack_chat.go | Updates acknowledgment command to use MsgUUID |
| web/server.go | Updates send endpoint to accept UUID instead of ID |
| web/server_test.go | Adds UUID generator seeding and updates test fixtures |
| testsuite/courier.go | Updates mock courier to work with UUIDs |
| testsuite/testdata.go | Changes helper functions to return MsgUUID instead of MsgID |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
web/server.go
Outdated
| Secret string `json:"secret" validate:"required"` | ||
| Msg struct { | ||
| ID models.MsgID `json:"id" validate:"required"` | ||
| UUID models.MsgUUID `json:"uuid" validate:"uuid,required"` |
Copilot
AI
Oct 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation tag should be required,uuid to match standard validator library conventions where required comes before format validators.
| UUID models.MsgUUID `json:"uuid" validate:"uuid,required"` | |
| UUID models.MsgUUID `json:"uuid" validate:"required,uuid"` |
service.go
Outdated
| msgUUID := strings.TrimPrefix(string(itemID), "m") | ||
|
|
||
| if !uuids.Is(msgUUID) { | ||
| return fmt.Errorf("error parsing msg uuid: %w", errors.New("invalid uuid")) |
Copilot
AI
Oct 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message wrapping is redundant. Either return errors.New(\"invalid uuid\") directly or use a more descriptive message like fmt.Errorf(\"invalid msg uuid: %s\", msgUUID) without wrapping.
| return fmt.Errorf("error parsing msg uuid: %w", errors.New("invalid uuid")) | |
| return fmt.Errorf("invalid msg uuid: %s", msgUUID) |
Addresses #27