Draft
Conversation
Share test logic in store_test.go with thin per-backend wrappers. Add SetSubscriptionUpdatedAt to both stores, removing the need for raw SQL and the DB() accessor in tests.
- Add SetSubscriptionUpdatedAt to Store interface, remove DB() accessor - Rename store files to store_sqlite.go and store_postgres.go - Use camelCase for test function names - Add tests for upsert field updates and multi-user removal - Use transaction in setupNewPostgresDB - Use lowercase "excluded." in PostgreSQL upsert query
Consolidate SQLite and Postgres store implementations into a single commonStore with database-specific SQL queries passed via configuration. This eliminates ~100 lines of duplicate code while maintaining full functionality for both backends. Key changes: - Move all store methods to commonStore in store.go - Remove sqliteStore and postgresStore wrapper structs - Refactor SQLite to use QueryRow() pattern like Postgres - Pass database-specific queries via storeQueries struct - Make store types unexported, only expose Store interface All tests pass for both SQLite and PostgreSQL backends.
Extract database operations from Manager into a Store interface with SQLite and PostgreSQL implementations using a shared commonStore. Split SQLite migrations into store_sqlite_migrations.go, use shared schema_version table for PostgreSQL, rename user_user/user_tier tables to "user"/tier, and wire up database-url in CLI commands.
Add shared test functions in store_test.go covering all Store interface operations (users, tokens, access control, tiers, billing, stats, etc.) with per-backend wrappers in store_sqlite_test.go and store_postgres_test.go following the webpush test pattern. Fix broken isUniqueConstraintError() which used incorrect interface assertions instead of string matching for SQLite/PostgreSQL errors.
Move message cache from server/message_cache.go into a dedicated message/ package with Store interface, SQLite and PostgreSQL implementations. Extract shared types into model/ package.
binwiederhier
commented
Feb 21, 2026
| @@ -0,0 +1,618 @@ | |||
| package message | |||
Owner
Author
There was a problem hiding this comment.
Reviewed and verified identical to original
| @@ -0,0 +1,145 @@ | |||
| package message | |||
Owner
Author
There was a problem hiding this comment.
Verified that these queries have not changed compared to main
| @@ -0,0 +1,466 @@ | |||
| package message | |||
Owner
Author
There was a problem hiding this comment.
Verified that this schema is identical
binwiederhier
commented
Feb 23, 2026
| @@ -0,0 +1,718 @@ | |||
| package user_test | |||
Owner
Author
There was a problem hiding this comment.
These are all new test functions
| @@ -0,0 +1,187 @@ | |||
| package webpush | |||
| @@ -0,0 +1,252 @@ | |||
| package webpush_test | |||
Owner
Author
There was a problem hiding this comment.
Manually verified the tests are the same
| @@ -0,0 +1,121 @@ | |||
| package webpush | |||
Owner
Author
There was a problem hiding this comment.
Manually compared queries with sqlite impl
| @@ -0,0 +1,142 @@ | |||
| package webpush | |||
Owner
Author
There was a problem hiding this comment.
Manually compared with main sqlite impl and postgres impl
ikopke23
reviewed
Feb 27, 2026
ikopke23
left a comment
ikopke23
left a comment
There was a problem hiding this comment.
I got about half way down the list of files before they finished reviewing the pr
| const ( | ||
| postgresCreateTablesQuery = ` | ||
| CREATE TABLE IF NOT EXISTS message ( | ||
| id BIGSERIAL PRIMARY KEY, |
There was a problem hiding this comment.
BIGSERIAL over generate_uuid?
You have timestamps so the order is not super relevant? Is this a backwards compatibility thing since the previous instance used an Auto Incrementing type?
| sqliteCreateMessagesTableQuery = ` | ||
| BEGIN; | ||
| CREATE TABLE IF NOT EXISTS messages ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| topics := make(map[string]*topic, len(topicIDs)) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.