fix: batch creation creates correct metadata #1000
Merged
Conversation
This file contains hidden or 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
…atch modification tests This commit modifies the SQL migration to correct the metadata key for read-only keys, ensuring consistency in the handling of 'readonly_key' and 'stream_owner'. Additionally, it introduces a new test to verify that read-only metadata cannot be modified in batch operations, enhancing the robustness of the metadata management system.
This commit refactors the `create_stream` action to delegate the stream creation process to the `create_streams` batch implementation, enhancing consistency and simplifying the code. The previous permission checks and validations have been removed from `create_stream`, streamlining the action's logic.
Bug Report Checklist
@outerlook, please use git blame and specify the link to the commit link that has introduced this bug. |
This commit modifies the error message assertion in the `TestStreamIDValidation` test to check for "duplicate key value violates" instead of "already exists". This change ensures that the error message accurately reflects the underlying database constraint violation when attempting to create a duplicate stream ID for the same owner.
Time Submission Status
|
|
@outerlook what's wrong with already deployed streams? How much impacted? |
|
Every that was deployed using batch create until now. Single and batch create streams diverged and the created metakeys weren't correct. Before this patch, people could just manually alter their own stream owners (not others') and their own This offers no advantage to them; it could only create bad states for themselves if they manually tried. It is highly unlikely that they did. the patch was already executed on server to fix this, which is only: UPDATE metadata
SET metadata_key = 'readonly_key'
WHERE metadata_key IN (
'readonly_key:stream_owner',
'readonly_key:readonly_key'
); |
MicBun
approved these changes
Jun 13, 2025
MicBun
requested changes
Jun 13, 2025
The `create_streams` action was generating duplicate `row_id` values for metadata entries when multiple streams were created in quick succession within the same test context. This was due to the UUID v5 generation relying only on a base UUID derived from `@txid` and a sequential row number, which could lead to collisions across different stream creations if `@txid` was similar and the metadata keys generated in the same order. This commit modifies the `row_id` generation in `create_streams` to include the `stream_id` and `metadata_key` in the "name" portion of the UUID v5, ensuring a unique `row_id` for each distinct metadata entry across all streams created. This resolves the "duplicate key value violates unique constraint `metadata_pkey`" error observed in tests like `TestAUTH04_NestedComposePermissions`.
The `create_streams` action was generating duplicate `row_id` values for metadata entries when multiple streams were created in quick succession within the same test context. This was due to the UUID v5 generation relying only on a base UUID derived from `@txid` and a sequential row number, which could lead to collisions across different stream creations if `@txid` was similar and the metadata keys generated in the same order. This commit modifies the `row_id` generation in `create_streams` to include the `stream_id` and `metadata_key` in the "name" portion of the UUID v5, ensuring a unique `row_id` for each distinct metadata entry across all streams created. This resolves the "duplicate key value violates unique constraint `metadata_pkey`" error observed in tests like `TestAUTH04_NestedComposePermissions`. <!--- Provide a general summary of your changes in the Title above by following our Developer Guidelines --> ## Description <!--- Describe your changes in detail; use bullet points. --> ## Related Problem <!--- If this pull request relates to an existing Problem, please link to it here (https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) --> <!-- example: resolves: #112330 --> resolves: #1000 (review) ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. -->
MicBun
approved these changes
Jun 13, 2025
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
Related Problem
How Has This Been Tested?
note: I'll need to execute a fix on our server directly for already created streams