From 5ef83a7ba09e02717aa888082f69bdade737f36d Mon Sep 17 00:00:00 2001
From: Philipp Heckel <pheckel@datto.com>
Date: Fri, 10 Dec 2021 20:28:56 -0500
Subject: [PATCH] Test DB migration

---
 server/cache_sqlite.go      |  5 +-
 server/cache_sqlite_test.go | 91 ++++++++++++++++++++++++++++++-------
 2 files changed, 77 insertions(+), 19 deletions(-)

diff --git a/server/cache_sqlite.go b/server/cache_sqlite.go
index 19eddee5..c6813d5c 100644
--- a/server/cache_sqlite.go
+++ b/server/cache_sqlite.go
@@ -76,9 +76,7 @@ const (
 
 	// 1 -> 2
 	migrate1To2AlterMessagesTableQuery = `
-		BEGIN;
 		ALTER TABLE messages ADD COLUMN published INT NOT NULL DEFAULT(1);
-		COMMIT;
 	`
 )
 
@@ -220,7 +218,7 @@ func setupDB(db *sql.DB) error {
 	if err != nil {
 		return setupNewDB(db)
 	}
-	defer rowsMC.Close()
+	rowsMC.Close()
 
 	// If 'messages' table exists, check 'schemaVersion' table
 	schemaVersion := 0
@@ -233,6 +231,7 @@ func setupDB(db *sql.DB) error {
 		if err := rowsSV.Scan(&schemaVersion); err != nil {
 			return err
 		}
+		rowsSV.Close()
 	}
 
 	// Do migrations
diff --git a/server/cache_sqlite_test.go b/server/cache_sqlite_test.go
index eb7b64a6..384da256 100644
--- a/server/cache_sqlite_test.go
+++ b/server/cache_sqlite_test.go
@@ -3,7 +3,7 @@ package server
 import (
 	"database/sql"
 	"fmt"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"path/filepath"
 	"testing"
 	"time"
@@ -32,7 +32,7 @@ func TestSqliteCache_Prune(t *testing.T) {
 func TestSqliteCache_Migration_From0(t *testing.T) {
 	filename := newSqliteTestCacheFile(t)
 	db, err := sql.Open("sqlite3", filename)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	// Create "version 0" schema
 	_, err = db.Exec(`
@@ -46,32 +46,91 @@ func TestSqliteCache_Migration_From0(t *testing.T) {
 		CREATE INDEX IF NOT EXISTS idx_topic ON messages (topic);
 		COMMIT;
 	`)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	// Insert a bunch of messages
 	for i := 0; i < 10; i++ {
 		_, err = db.Exec(`INSERT INTO messages (id, time, topic, message) VALUES (?, ?, ?, ?)`,
 			fmt.Sprintf("abcd%d", i), time.Now().Unix(), "mytopic", fmt.Sprintf("some message %d", i))
-		assert.Nil(t, err)
+		require.Nil(t, err)
 	}
+	require.Nil(t, db.Close())
 
 	// Create cache to trigger migration
 	c := newSqliteTestCacheFromFile(t, filename)
-	messages, err := c.Messages("mytopic", sinceAllMessages, false)
-	assert.Nil(t, err)
-	assert.Equal(t, 10, len(messages))
-	assert.Equal(t, "some message 5", messages[5].Message)
-	assert.Equal(t, "", messages[5].Title)
-	assert.Nil(t, messages[5].Tags)
-	assert.Equal(t, 0, messages[5].Priority)
+	checkSchemaVersion(t, c.db)
 
-	rows, err := c.db.Query(`SELECT version  FROM schemaVersion`)
-	assert.Nil(t, err)
-	assert.True(t, rows.Next())
+	messages, err := c.Messages("mytopic", sinceAllMessages, false)
+	require.Nil(t, err)
+	require.Equal(t, 10, len(messages))
+	require.Equal(t, "some message 5", messages[5].Message)
+	require.Equal(t, "", messages[5].Title)
+	require.Nil(t, messages[5].Tags)
+	require.Equal(t, 0, messages[5].Priority)
+}
+
+func TestSqliteCache_Migration_From1(t *testing.T) {
+	filename := newSqliteTestCacheFile(t)
+	db, err := sql.Open("sqlite3", filename)
+	require.Nil(t, err)
+
+	// Create "version 1" schema
+	_, err = db.Exec(`
+		CREATE TABLE IF NOT EXISTS messages (
+			id VARCHAR(20) PRIMARY KEY,
+			time INT NOT NULL,
+			topic VARCHAR(64) NOT NULL,
+			message VARCHAR(512) NOT NULL,
+			title VARCHAR(256) NOT NULL,
+			priority INT NOT NULL,
+			tags VARCHAR(256) NOT NULL
+		);
+		CREATE INDEX IF NOT EXISTS idx_topic ON messages (topic);
+		CREATE TABLE IF NOT EXISTS schemaVersion (
+			id INT PRIMARY KEY,
+			version INT NOT NULL
+		);		
+		INSERT INTO schemaVersion (id, version) VALUES (1, 1);
+	`)
+	require.Nil(t, err)
+
+	// Insert a bunch of messages
+	for i := 0; i < 10; i++ {
+		_, err = db.Exec(`INSERT INTO messages (id, time, topic, message, title, priority, tags) VALUES (?, ?, ?, ?, ?, ?, ?)`,
+			fmt.Sprintf("abcd%d", i), time.Now().Unix(), "mytopic", fmt.Sprintf("some message %d", i), "", 0, "")
+		require.Nil(t, err)
+	}
+	require.Nil(t, db.Close())
+
+	// Create cache to trigger migration
+	c := newSqliteTestCacheFromFile(t, filename)
+	checkSchemaVersion(t, c.db)
+
+	// Add delayed message
+	delayedMessage := newDefaultMessage("mytopic", "some delayed message")
+	delayedMessage.Time = time.Now().Add(time.Minute).Unix()
+	require.Nil(t, c.AddMessage(delayedMessage))
+
+	// 10, not 11!
+	messages, err := c.Messages("mytopic", sinceAllMessages, false)
+	require.Nil(t, err)
+	require.Equal(t, 10, len(messages))
+
+	// 11!
+	messages, err = c.Messages("mytopic", sinceAllMessages, true)
+	require.Nil(t, err)
+	require.Equal(t, 11, len(messages))
+}
+
+func checkSchemaVersion(t *testing.T, db *sql.DB) {
+	rows, err := db.Query(`SELECT version FROM schemaVersion`)
+	require.Nil(t, err)
+	require.True(t, rows.Next())
 
 	var schemaVersion int
-	assert.Nil(t, rows.Scan(&schemaVersion))
-	assert.Equal(t, 2, schemaVersion)
+	require.Nil(t, rows.Scan(&schemaVersion))
+	require.Equal(t, currentSchemaVersion, schemaVersion)
+	require.Nil(t, rows.Close())
 }
 
 func newSqliteTestCache(t *testing.T) *sqliteCache {