8000 fix: batch creation creates correct metadata by outerlook · Pull Request #1000 · trufnetwork/node · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: batch creation creates correct metadata #1000

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

Merged
merged 5 commits into from
Jun 13, 2025
Merged

Conversation

outerlook
Copy link
Contributor

Description

  • Single create now reroutes to batch create to unify
  • New test ensures both are ok for readonly tests

Related Problem

How Has This Been Tested?

  • all tests pass

note: I'll need to execute a fix on our server directly for already created streams

…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.
@outerlook outerlook self-assigned this Jun 13, 2025
Copy link
pr-time-tracker bot commented Jun 13, 2025

Bug Report Checklist

Status Commit Link Bug Author
❌ Not Submitted

@outerlook, please use git blame and specify the link to the commit link that has introduced this bug.

Send the following message in this PR: `@pr-time-tracker bug commit [link](url) && bug author @name`

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.
@outerlook outerlook requested a review from MicBun June 13, 2025 16:32
Copy link
pr-time-tracker bot commented Jun 13, 2025

Time Submission Status

Member Status Time Action Last Update
@outerlook ❌ Missing - ⚠️ Submit time -
MicBun ✅ Submitted 30min Update time Jun 13, 2025, 11:32 PM

@MicBun
Copy link
Member
MicBun commented Jun 13, 2025

@outerlook what's wrong with already deployed streams? How much impacted?

@outerlook
Copy link
Contributor Author
outerlook commented Jun 13, 2025

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 readonly_key

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'
);

Copy link
Member
@MicBun MicBun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops the changes broke the CI
Will see if I can get a quick fix

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 MicBun merged commit ac47d71 into main Jun 13, 2025
4 of 6 checks passed
@MicBun MicBun deleted the fix/create-metadta branch June 13, 2025 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: creating streams in batch create the wrong metadata keys
2 participants
0