fix(memory-store): Change association structure of files and docs

[creatorrr]

User description

Signed-off-by: Diwank Singh Tomer [email protected]

---

PR Type

Enhancement

---

Description

• Unified the ownership structure for both files and documents by replacing separate tables with single ownership tables (file_owners and doc_owners)
• Added type validation to ensure owner references are valid (either user or agent)
• Implemented triggers and functions to validate owner references on insert and update
• Created optimized indexes for querying by owner type and ID
• Simplified database schema while maintaining data integrity and relationships

---

Changes walkthrough 📝

Relevant files

Enhancement

000005_files.down.sql Restructure file ownership tables cleanup                                memory-store/migrations/000005_files.down.sql Replaced dropping of separate agent_files and user_files tables with dropping of unified file_owners table Added dropping of validation trigger and function for file owners +4/-6      000005_files.up.sql Implement unified file ownership structure                              memory-store/migrations/000005_files.up.sql Replaced separate user_files and agent_files tables with unified file_owners table Added owner type validation with trigger and function Created index on owner fields +37/-19  000006_docs.down.sql Update document ownership cleanup migration                            memory-store/migrations/000006_docs.down.sql Added cleanup for doc_owners table and its dependencies Reorganized drop statements for better clarity Removed drops for deprecated agent_docs and user_docs tables +14/-28  000006_docs.up.sql Implement unified document ownership structure                      memory-store/migrations/000006_docs.up.sql Replaced separate user_docs and agent_docs tables with unified doc_owners table Added owner type validation with trigger and function Created index on owner fields +38/-18

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Data Migration No data migration logic is provided for existing file ownership records from the old tables (user_files, agent_files) to the new file_owners table Data Migration No data migration logic is provided for existing document ownership records from the old tables (user_docs, agent_docs) to the new doc_owners table Validation Gap The owner validation trigger does not check if the owner_type matches the actual type of the referenced ID, allowing a user ID to be inserted with owner_type='agent' and vice versa

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add a unique constraint to prevent duplicate associations between owners and files Add a UNIQUE constraint on (developer_id, owner_type, owner_id, file_id) to prevent the same owner from being associated with the same file multiple times. memory-store/migrations/000005_files.up.sql [60-68] CREATE TABLE IF NOT EXISTS file_owners ( developer_id UUID NOT NULL, file_id UUID NOT NULL, owner_type TEXT NOT NULL, -- 'user' or 'agent' owner_id UUID NOT NULL, CONSTRAINT pk_file_owners PRIMARY KEY (developer_id, file_id), CONSTRAINT fk_file_owners_file FOREIGN KEY (developer_id, file_id) REFERENCES files (developer_id, file_id), - CONSTRAINT ct_file_owners_owner_type CHECK (owner_type IN ('user', 'agent')) + CONSTRAINT ct_file_owners_owner_type CHECK (owner_type IN ('user', 'agent')), + CONSTRAINT uq_file_owners_owner UNIQUE (developer_id, owner_type, owner_id, file_id) ); Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a critical data integrity issue by preventing duplicate owner-file associations, which could lead to data inconsistencies and potential application bugs.	8
	Add a unique constraint to prevent duplicate associations between owners and documents Add a UNIQUE constraint on (developer_id, owner_type, owner_id, doc_id) to prevent the same owner from being associated with the same document multiple times. memory-store/migrations/000006_docs.up.sql [67-75] CREATE TABLE IF NOT EXISTS doc_owners ( developer_id UUID NOT NULL, doc_id UUID NOT NULL, owner_type TEXT NOT NULL, -- 'user' or 'agent' owner_id UUID NOT NULL, CONSTRAINT pk_doc_owners PRIMARY KEY (developer_id, doc_id), CONSTRAINT fk_doc_owners_doc FOREIGN KEY (developer_id, doc_id) REFERENCES docs (developer_id, doc_id), - CONSTRAINT ct_doc_owners_owner_type CHECK (owner_type IN ('user', 'agent')) + CONSTRAINT ct_doc_owners_owner_type CHECK (owner_type IN ('user', 'agent')), + CONSTRAINT uq_doc_owners_owner UNIQUE (developer_id, owner_type, owner_id, doc_id) ); Apply this suggestion Suggestion importance[1-10]: 8 Why: Similar to the files table, this suggestion prevents duplicate owner-document associations, which is crucial for maintaining data integrity and preventing potential application issues.	8
General	Add cascading delete to maintain referential integrity when parent records are removed Add ON DELETE CASCADE to the foreign key constraint to automatically remove owner associations when a file is deleted. memory-store/migrations/000005_files.up.sql [66] -CONSTRAINT fk_file_owners_file FOREIGN KEY (developer_id, file_id) REFERENCES files (developer_id, file_id), +CONSTRAINT fk_file_owners_file FOREIGN KEY (developer_id, file_id) REFERENCES files (developer_id, file_id) ON DELETE CASCADE, Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding ON DELETE CASCADE ensures proper cleanup of related records when a file is deleted, preventing orphaned records and maintaining database consistency.	7

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 30b5763 in 1 minute and 48 seconds

More details

• Looked at 233 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. memory-store/migrations/000005_files.up.sql:65

• Draft comment:
  The primary key constraint pk_file_owners should include owner_type and owner_id to ensure uniqueness for each owner of a file. Consider changing it to:

    CONSTRAINT pk_file_owners PRIMARY KEY (developer_id, file_id, owner_type, owner_id),

• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The current PK means each file can only have one owner total, which seems to be the intended design since we're moving from separate tables to a unified ownership table. If multiple owners were intended, they would have kept separate tables or included owner_type/id in the PK. The trigger validates owner references but doesn't prevent multiple owners because the PK blocks it already.
  I could be wrong about the intended design - maybe files should be able to have both a user owner and an agent owner. The comment might be identifying a real limitation.
  The deliberate schema change from separate tables to a unified table with a restrictive PK strongly suggests single ownership is intended. If multiple owners were needed, the original separate tables design would have been better.
  The comment should be deleted as it appears to misunderstand the intended design of single ownership per file. The current PK correctly enforces this constraint.

2. memory-store/migrations/000006_docs.up.sql:72

• Draft comment:
  The primary key constraint pk_doc_owners should include owner_type and owner_id to ensure uniqueness for each owner of a document. Consider changing it to:

    CONSTRAINT pk_doc_owners PRIMARY KEY (developer_id, doc_id, owner_type, owner_id),

• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The current PK implies a doc can only have one owner, which seems intentional given the table name is doc_owners not doc_owner_relations. The trigger validate_doc_owner ensures referential integrity to users/agents tables. If multiple owners per doc was intended, the comment would be correct, but the table name and structure suggest single ownership is the design goal.
  I could be wrong about the intention - maybe the table name is misleading and it should support multiple owners per document. The index on (developer_id, owner_type, owner_id) suggests querying by owner is important.
  While multiple owners might be useful, the current schema clearly enforces single ownership through the PK. This appears to be an intentional design choice, not an oversight. The index on owner fields is for efficient lookups, not uniqueness.
  The comment should be deleted as it appears to contradict the intentional design of single ownership per document. If multiple owners are needed, that would be a feature request, not a bug fix.

Workflow ID: wflow_CnNWPZx7B1Lr1mGi

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.