From 4378b529e3a565d99dfad2314efca1203487fab2 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Thu, 27 Oct 2022 09:19:11 +0200 Subject: [PATCH] Fix issues discovered by UT --- .../deltas/20221027084407_published_appservice.go | 4 ++-- roomserver/storage/postgres/published_table.go | 15 ++++++++------- .../deltas/20221027084407_published_appservice.go | 9 +++++---- roomserver/storage/sqlite3/published_table.go | 14 ++++++++------ roomserver/storage/tables/published_table_test.go | 6 ++++-- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/roomserver/storage/postgres/deltas/20221027084407_published_appservice.go b/roomserver/storage/postgres/deltas/20221027084407_published_appservice.go index 645f6c5d0..be046545a 100644 --- a/roomserver/storage/postgres/deltas/20221027084407_published_appservice.go +++ b/roomserver/storage/postgres/deltas/20221027084407_published_appservice.go @@ -21,11 +21,11 @@ import ( ) func UpPulishedAppservice(ctx context.Context, tx *sql.Tx) error { - _, err := tx.ExecContext(ctx, `ALTER TABLE roomserver_published ADD COLUMN IF NOT EXISTS appservice_id TEXT;`) + _, err := tx.ExecContext(ctx, `ALTER TABLE roomserver_published ADD COLUMN IF NOT EXISTS appservice_id TEXT NOT NULL;`) if err != nil { return fmt.Errorf("failed to execute upgrade: %w", err) } - _, err = tx.ExecContext(ctx, `ALTER TABLE roomserver_published ADD COLUMN IF NOT EXISTS network_id TEXT;`) + _, err = tx.ExecContext(ctx, `ALTER TABLE roomserver_published ADD COLUMN IF NOT EXISTS network_id TEXT NOT NULL;`) if err != nil { return fmt.Errorf("failed to execute upgrade: %w", err) } diff --git a/roomserver/storage/postgres/published_table.go b/roomserver/storage/postgres/published_table.go index f9946da43..7cbdaa8e0 100644 --- a/roomserver/storage/postgres/published_table.go +++ b/roomserver/storage/postgres/published_table.go @@ -28,19 +28,20 @@ const publishedSchema = ` -- Stores which rooms are published in the room directory CREATE TABLE IF NOT EXISTS roomserver_published ( -- The room ID of the room - room_id TEXT NOT NULL PRIMARY KEY, + room_id TEXT NOT NULL, -- The appservice ID of the room - appservice_id TEXT, + appservice_id TEXT NOT NULL, -- The network_id of the room - network_id TEXT, + network_id TEXT NOT NULL, -- Whether it is published or not - published BOOLEAN NOT NULL DEFAULT false + published BOOLEAN NOT NULL DEFAULT false, + PRIMARY KEY (room_id, appservice_id, network_id) ); ` const upsertPublishedSQL = "" + - "INSERT INTO roomserver_published (room_id, published) VALUES ($1, $2) " + - "ON CONFLICT (room_id) DO UPDATE SET published=$2" + "INSERT INTO roomserver_published (room_id, appservice_id, network_id, published) VALUES ($1, $2, $3, $4) " + + "ON CONFLICT (room_id, appservice_id, network_id) DO UPDATE SET published=$4" const selectAllPublishedSQL = "" + "SELECT room_id FROM roomserver_published WHERE published = $1 ORDER BY room_id ASC" @@ -81,7 +82,7 @@ func (s *publishedStatements) UpsertRoomPublished( ctx context.Context, txn *sql.Tx, roomID string, published bool, ) (err error) { stmt := sqlutil.TxStmt(txn, s.upsertPublishedStmt) - _, err = stmt.ExecContext(ctx, roomID, published) + _, err = stmt.ExecContext(ctx, roomID, "", "", published) return } diff --git a/roomserver/storage/sqlite3/deltas/20221027084407_published_appservice.go b/roomserver/storage/sqlite3/deltas/20221027084407_published_appservice.go index a498dfb34..cd923b1c1 100644 --- a/roomserver/storage/sqlite3/deltas/20221027084407_published_appservice.go +++ b/roomserver/storage/sqlite3/deltas/20221027084407_published_appservice.go @@ -23,10 +23,11 @@ import ( func UpPulishedAppservice(ctx context.Context, tx *sql.Tx) error { _, err := tx.ExecContext(ctx, ` ALTER TABLE roomserver_published RENAME TO roomserver_published_tmp; CREATE TABLE IF NOT EXISTS roomserver_published ( - room_id TEXT NOT NULL PRIMARY KEY, - appservice_id TEXT, - network_id TEXT, - published BOOLEAN NOT NULL DEFAULT false + room_id TEXT NOT NULL, + appservice_id TEXT NOT NULL, + network_id TEXT NOT NULL, + published BOOLEAN NOT NULL DEFAULT false, + CONSTRAINT unique_published_idx PRIMARY KEY (room_id, appservice_id, network_id) ); INSERT INTO roomserver_published ( diff --git a/roomserver/storage/sqlite3/published_table.go b/roomserver/storage/sqlite3/published_table.go index 03e7e9653..0724333c3 100644 --- a/roomserver/storage/sqlite3/published_table.go +++ b/roomserver/storage/sqlite3/published_table.go @@ -28,18 +28,20 @@ const publishedSchema = ` -- Stores which rooms are published in the room directory CREATE TABLE IF NOT EXISTS roomserver_published ( -- The room ID of the room - room_id TEXT NOT NULL PRIMARY KEY, + room_id TEXT NOT NULL, -- The appservice ID of the room - appservice_id TEXT, + appservice_id TEXT NOT NULL, -- The network_id of the room - network_id TEXT, + network_id TEXT NOT NULL, -- Whether it is published or not - published BOOLEAN NOT NULL DEFAULT false + published BOOLEAN NOT NULL DEFAULT false, + PRIMARY KEY (room_id, appservice_id, network_id) ); ` const upsertPublishedSQL = "" + - "INSERT OR REPLACE INTO roomserver_published (room_id, published) VALUES ($1, $2)" + "INSERT INTO roomserver_published (room_id, appservice_id, network_id, published) VALUES ($1, $2, $3, $4)" + + " ON CONFLICT (room_id, appservice_id, network_id) DO UPDATE SET published = $4" const selectAllPublishedSQL = "" + "SELECT room_id FROM roomserver_published WHERE published = $1 ORDER BY room_id ASC" @@ -83,7 +85,7 @@ func (s *publishedStatements) UpsertRoomPublished( ctx context.Context, txn *sql.Tx, roomID string, published bool, ) error { stmt := sqlutil.TxStmt(txn, s.upsertPublishedStmt) - _, err := stmt.ExecContext(ctx, roomID, published) + _, err := stmt.ExecContext(ctx, roomID, "", "", published) return err } diff --git a/roomserver/storage/tables/published_table_test.go b/roomserver/storage/tables/published_table_test.go index fff6dc186..62165264e 100644 --- a/roomserver/storage/tables/published_table_test.go +++ b/roomserver/storage/tables/published_table_test.go @@ -2,16 +2,18 @@ package tables_test import ( "context" + "fmt" "sort" "testing" + "github.com/stretchr/testify/assert" + "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver/storage/postgres" "github.com/matrix-org/dendrite/roomserver/storage/sqlite3" "github.com/matrix-org/dendrite/roomserver/storage/tables" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/test" - "github.com/stretchr/testify/assert" ) func mustCreatePublishedTable(t *testing.T, dbType test.DBType) (tab tables.Published, close func()) { @@ -74,6 +76,6 @@ func TestPublishedTable(t *testing.T) { // should now be false, due to the upsert publishedRes, err := tab.SelectPublishedFromRoomID(ctx, nil, room.ID) assert.NoError(t, err) - assert.False(t, publishedRes) + assert.False(t, publishedRes, fmt.Sprintf("expected room %s to be unpublished", room.ID)) }) }