Skip to content

Encapsulate storage database transaction APIs#1115

Draft
mcollina wants to merge 4 commits into
supabase:masterfrom
mcollina:encapsulate-database-transactions
Draft

Encapsulate storage database transaction APIs#1115
mcollina wants to merge 4 commits into
supabase:masterfrom
mcollina:encapsulate-database-transactions

Conversation

@mcollina

@mcollina mcollina commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove generic transaction/permission callback utilities from the public storage Database interface
  • replace them with domain-specific database operations for permission checks, bucket/object mutations, uploads, multipart progress, and TUS lock acquisition
  • keep external side effects such as backend deletes and webhook dispatch outside database transaction helpers
  • narrow the newly added database operation result types instead of returning broad Obj shapes

Validation

  • npx biome check src/storage/database/adapter.ts src/storage/database/knex.ts src/storage/events/objects/object-admin-delete-all-before.ts src/storage/object.ts src/storage/protocols/s3/s3-handler.ts src/storage/protocols/tus/postgres-locker.ts src/storage/storage.ts src/storage/uploader.ts src/test/s3-protocol.test.ts
  • npm run build

@mcollina mcollina force-pushed the encapsulate-database-transactions branch from 09db4fa to 21e9f6d Compare May 20, 2026 09:48
@coveralls

coveralls commented May 20, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 26156167379

Coverage decreased (-0.006%) to 74.868%

Details

  • Coverage decreased (-0.006%) from the base build.
  • Patch coverage: 23 uncovered changes across 5 files (122 of 145 lines covered, 84.14%).
  • 3 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
src/storage/protocols/tus/postgres-locker.ts 28 16 57.14%
src/storage/database/knex.ts 63 58 92.06%
src/storage/storage.ts 10 6 60.0%
src/storage/protocols/s3/s3-handler.ts 3 2 66.67%
src/storage/uploader.ts 10 9 90.0%

Coverage Regressions

3 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/storage/protocols/tus/postgres-locker.ts 3 69.88%

Coverage Stats

Coverage Status
Relevant Lines: 10299
Covered Lines: 8121
Line Coverage: 78.85%
Relevant Branches: 5947
Covered Branches: 4042
Branch Coverage: 67.97%
Branches in Coverage %: Yes
Coverage Strength: 414.24 hits per line

💛 - Coveralls

@mcollina mcollina changed the title Encapsulate storage database transactions Encapsulate storage database transaction APIs May 20, 2026
const [destinationBucket] = await Promise.all([
this.storage.db.asSuperUser().findBucketById(Bucket, 'file_size_limit'),
this.storage.db.asSuperUser().findBucketById(sourceBucketName, 'id'),
])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

add a new method to do this in one go

}
)
.catch(reject)
this.holdLockTransaction(stopSignal, resolve).catch(reject)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

possible blocker, need to verify the S3 locker

return this.withTransaction(async (db) => {
await db.mustLockObject(bucketId, objectName, version)
}, transactionOptions)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is incorrect, the lock is gone after the transaction completes

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.

2 participants