From ed04eed4411596a9ad83c322b89a7cbb49bf3b0f Mon Sep 17 00:00:00 2001
From: kegsay <kegan@matrix.org>
Date: Wed, 4 Aug 2021 17:08:17 +0100
Subject: [PATCH] Fix sqlite migration issues (#1960)

* Do not store 'null' in the database for empty JSON arrays

This can cause issues, though it should be noted that the majority
of the time this will marshal/unmarshal just fine, see
https://play.golang.org/p/Doe2NZUgv7Q

* bugfix: sqlite migration should handle create events as having no 'before' snapshot

The state snapshot for any given event in the roomserver represents the state _before_
the event. For the create event, this is nothing, so the state snapshot nid should be 0.

In some cases this wasn't happening, resulting in a nice mix of possible options including:
 - A state snapshot without any state blocks `[]` or `null`.
 - A state snapshot with a single state block with a single event, the create event, causing
   a circular loop. This is incorrect as it represents the state before the event, not after.

* Add state key check
---
 .../2021041615092700_state_blocks_refactor.go      | 14 ++++++++++++++
 roomserver/storage/sqlite3/events_table.go         |  3 +++
 roomserver/storage/sqlite3/state_block_table.go    |  2 +-
 roomserver/storage/sqlite3/state_snapshot_table.go |  3 +++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/roomserver/storage/sqlite3/deltas/2021041615092700_state_blocks_refactor.go b/roomserver/storage/sqlite3/deltas/2021041615092700_state_blocks_refactor.go
index 42edbbc6f..8d0331748 100644
--- a/roomserver/storage/sqlite3/deltas/2021041615092700_state_blocks_refactor.go
+++ b/roomserver/storage/sqlite3/deltas/2021041615092700_state_blocks_refactor.go
@@ -93,6 +93,20 @@ func UpStateBlocksRefactor(tx *sql.Tx) error {
 		}
 
 		var newblocks types.StateBlockNIDs
+		if len(blocks) == 0 {
+			// some m.room.create events have a state snapshot but no state blocks at all which makes
+			// sense as there is no state before creation. The correct form should be to give the event
+			// in question a state snapshot NID of 0 to indicate 'no snapshot'.
+			// If we don't do this, we'll fail the assertions later on which try to ensure we didn't forget
+			// any snapshots.
+			_, err = tx.Exec(
+				`UPDATE roomserver_events SET state_snapshot_nid = 0 WHERE event_type_nid = $1 AND event_state_key_nid = $2 AND state_snapshot_nid = $3`,
+				types.MRoomCreateNID, types.EmptyStateKeyNID, snapshot,
+			)
+			if err != nil {
+				return fmt.Errorf("resetting create events snapshots to 0 errored: %s", err)
+			}
+		}
 		for _, block := range blocks {
 			if err = func() error {
 				blockrows, berr := tx.Query(`SELECT event_nid FROM _roomserver_state_block WHERE state_block_nid = $1`, block)
diff --git a/roomserver/storage/sqlite3/events_table.go b/roomserver/storage/sqlite3/events_table.go
index a28d95fa5..b7fe7ee4f 100644
--- a/roomserver/storage/sqlite3/events_table.go
+++ b/roomserver/storage/sqlite3/events_table.go
@@ -571,6 +571,9 @@ func (s *eventStatements) SelectRoomNIDsForEventNIDs(
 }
 
 func eventNIDsAsArray(eventNIDs []types.EventNID) string {
+	if eventNIDs == nil {
+		eventNIDs = []types.EventNID{} // don't store 'null' in the DB
+	}
 	b, _ := json.Marshal(eventNIDs)
 	return string(b)
 }
diff --git a/roomserver/storage/sqlite3/state_block_table.go b/roomserver/storage/sqlite3/state_block_table.go
index a472437a3..58b0b5dc2 100644
--- a/roomserver/storage/sqlite3/state_block_table.go
+++ b/roomserver/storage/sqlite3/state_block_table.go
@@ -86,7 +86,7 @@ func (s *stateBlockStatements) BulkInsertStateData(
 	entries types.StateEntries,
 ) (id types.StateBlockNID, err error) {
 	entries = entries[:util.SortAndUnique(entries)]
-	var nids types.EventNIDs
+	nids := types.EventNIDs{} // zero slice to not store 'null' in the DB
 	for _, e := range entries {
 		nids = append(nids, e.EventNID)
 	}
diff --git a/roomserver/storage/sqlite3/state_snapshot_table.go b/roomserver/storage/sqlite3/state_snapshot_table.go
index ad623d65a..040d99ae6 100644
--- a/roomserver/storage/sqlite3/state_snapshot_table.go
+++ b/roomserver/storage/sqlite3/state_snapshot_table.go
@@ -87,6 +87,9 @@ func prepareStateSnapshotTable(db *sql.DB) (tables.StateSnapshot, error) {
 func (s *stateSnapshotStatements) InsertState(
 	ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, stateBlockNIDs types.StateBlockNIDs,
 ) (stateNID types.StateSnapshotNID, err error) {
+	if stateBlockNIDs == nil {
+		stateBlockNIDs = []types.StateBlockNID{} // zero slice to not store 'null' in the DB
+	}
 	stateBlockNIDs = stateBlockNIDs[:util.SortAndUnique(stateBlockNIDs)]
 	stateBlockNIDsJSON, err := json.Marshal(stateBlockNIDs)
 	if err != nil {