Adding support for PostgreSQL as database

[02strich]

This adds support for a second database backend: PostgreSQL (in addition to sqlite3). This allows externalizing the database used by gonic.

There is likely more testing required (as I am still working through tests), but wanted to put this up first to get some thoughts/comments if something like this is of interest.

[02strich]

Sorry that it took so long to get back on this.

Did a bit more testing and now validated both SQLite and PostgreSQL in the following scenarios:

• Scan with music and podcasts
• Query using Subsonic interface for artists, genres, albums, folders and so on
• Create and manage playlists using Subsonic interface

Happy to test more if you can provide hints on what would be good.

[sentriz]

thanks! at first glace this looks very nice. I appreciate the minimal diff. will hopefully take a closer look during the week

[02strich]

Anything I can help to move this along?

[sentriz]

i think i would prefer we just have two new options like

-db-driver
-db-dsn

then we can pass those directly to gorm.Open()

what do you think? it would give us all the flexibility and future compatibility

for that to work we just need

• a bit of logic in main to check that "db-path" is present, print a deprecation message, and just set driver to sqlite and dsn to the file name
• db.New() to have a special case for sqlite to add the DefaultOptions() and SetMaxOpenConns to the dsn url (the file:/// thing)
• any thing else?

[02strich]

That would mean our users would need to understand the GORM DSNs, which is not ideal but also not the end of the world. The bigger concern I have is that the SQL used is slightly different across the DBs (as there are already adjustments between SQLite and PostgreSQL), so saying all will work seems bold.

If you feel that is ok and the testing limits are more a documentation thing, that works for me though.

[sentriz]

could you explain a bit what's changed here?

[02strich]

SQLite is able to deal with non-ASCII strings in its string type, while other DBMS do not. This means the session_key needs to be encoded to make it ASCII-safe (in this case with base64)

[sentriz]

the COLLATE NOCASE IIRC was there to case insensitively sort albums. is there a way we could do it that works for both postgres and sqlite? not a big deal if not i suppose

[02strich]

I have been trying to find a way - and it seems there is no good overlap in the SQL dialects around handling collation. Happy to dig further, but not optimistic.

[sentriz]

Anything I can help to move this along?

sorry for the delay! just had a look 👍 👍

[sentriz]

hey would you mind rebasing? and sorry apparently I've been force pushing to master . should be fine to rebase though since you only have 1 commit

[uumas]

@02strich still interested in moving this along?

[02strich]

Yeah, still interested - sorry for the long radio silence. I should have something be end of next week.

…

________________________________ From: Uumas ***@***.***> Sent: Wednesday, March 27, 2024 5:06:58 AM To: sentriz/gonic Cc: Stefan Richter; Mention Subject: Re: [sentriz/gonic] Adding support for PostgreSQL as database (PR #260) @02strich<https://github.com/02strich> still interested in moving this along? — Reply to this email directly, view it on GitHub<#260 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAAK2X4ZKEGKJP7MEDFFR3LY2KR5FAVCNFSM6AAAAAASGIOUPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRSGYYDSMRVGU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[02strich]

Rebased!

[sentriz]

thanks for rebase 👍 what do you think about these changes? just making a global db-uri config option, and a shim for users still using db-path

diff --git a/cmd/gonic/gonic.go b/cmd/gonic/gonic.go
index 76fef21..2c9a0c6 100644
--- a/cmd/gonic/gonic.go
+++ b/cmd/gonic/gonic.go
@@ -68,12 +68,7 @@ func main() {
 
 	confPlaylistsPath := flag.String("playlists-path", "", "path to your list of new or existing m3u playlists that gonic can manage")
 
-	confSqlitePath := flag.String("db-path", "gonic.db", "path to database (optional, default: gonic.db)")
-	confPostgresHost := flag.String("postgres-host", "", "name of the PostgreSQL gonicServer (optional)")
-	confPostgresPort := flag.Int("postgres-port", 5432, "port to use for PostgreSQL connection (optional, default: 5432)")
-	confPostgresName := flag.String("postgres-db", "gonic", "name of the PostgreSQL database (optional, default: gonic)")
-	confPostgresUser := flag.String("postgres-user", "gonic", "name of the PostgreSQL user (optional, default: gonic)")
-	confPostgresSslModel := flag.String("postgres-ssl-mode", "verify-full", "the ssl mode used for connecting to the PostreSQL instance (optional, default: verify-full)")
+	confDBURI := flag.String("db-uri", "", "db URI")
 
 	confScanIntervalMins := flag.Uint("scan-interval", 0, "interval (in minutes) to automatically scan music (optional)")
 	confScanAtStart := flag.Bool("scan-at-start-enabled", false, "whether to perform an initial scan at startup (optional)")
@@ -99,6 +94,7 @@ func main() {
 	confExpvar := flag.Bool("expvar", false, "enable the /debug/vars endpoint (optional)")
 
 	deprecatedConfGenreSplit := flag.String("genre-split", "", "(deprecated, see multi-value settings)")
+	deprecatedConfDBPath := flag.String("db-path", "gonic.db", "(deprecated, see db-uri)")
 
 	flag.Parse()
 	flagconf.ParseEnv()
@@ -143,12 +139,11 @@ func main() {
 		log.Fatalf("couldn't create covers cache path: %v\n", err)
 	}
 
-	var dbc *db.DB
-	if len(*confPostgresHost) > 0 {
-		dbc, err = db.NewPostgres(*confPostgresHost, *confPostgresPort, *confPostgresName, *confPostgresUser, os.Getenv("GONIC_POSTGRES_PW"), *confPostgresSslModel)
-	} else {
-		dbc, err = db.NewSqlite3(*confSqlitePath, db.DefaultOptions())
+	if *deprecatedConfDBPath != "" {
+		*confDBURI = "sqlite3://" + *deprecatedConfDBPath
 	}
+
+	dbc, err := db.New(*confDBURI)
 	if err != nil {
 		log.Fatalf("error opening database: %v\n", err)
 	}
@@ -156,7 +151,6 @@ func main() {
 
 	err = dbc.Migrate(db.MigrationContext{
 		Production:        true,
-		DBPath:            *confSqlitePath,
 		OriginalMusicPath: confMusicPaths[0].path,
 		PlaylistsPath:     *confPlaylistsPath,
 		PodcastsPath:      *confPodcastPath,
diff --git a/db/db.go b/db/db.go
index ef642d8..063fff6 100644
--- a/db/db.go
+++ b/db/db.go
@@ -1,7 +1,6 @@
 package db
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"log"
@@ -13,68 +12,48 @@ import (
 	"time"
 
 	"github.com/jinzhu/gorm"
-	"github.com/mattn/go-sqlite3"
 
 	// TODO: remove this dep
 	"go.senan.xyz/gonic/server/ctrlsubsonic/specid"
 )
 
-func DefaultOptions() url.Values {
-	return url.Values{
-		// with this, multiple connections share a single data and schema cache.
-		// see https://www.sqlite.org/sharedcache.html
-		"cache": {"shared"},
-		// with this, the db sleeps for a little while when locked. can prevent
-		// a SQLITE_BUSY. see https://www.sqlite.org/c3ref/busy_timeout.html
-		"_busy_timeout": {"30000"},
-		"_journal_mode": {"WAL"},
-		"_foreign_keys": {"true"},
-	}
-}
-
-func mockOptions() url.Values {
-	return url.Values{
-		"_foreign_keys": {"true"},
-	}
-}
-
 type DB struct {
 	*gorm.DB
 }
 
-func NewSqlite3(path string, options url.Values) (*DB, error) {
-	// https://github.com/mattn/go-sqlite3#connection-string
-	url := url.URL{
-		Scheme: "file",
-		Opaque: path,
+func New(uri string) (*DB, error) {
+	if uri == "" {
+		return nil, fmt.Errorf("empty db uri")
 	}
-	url.RawQuery = options.Encode()
-	db, err := gorm.Open("sqlite3", url.String())
+	url, err := url.Parse(uri)
 	if err != nil {
-		return nil, fmt.Errorf("with gorm: %w", err)
+		return nil, fmt.Errorf("parse uri: %w", err)
 	}
-	return newDB(db)
-}
 
-func NewPostgres(host string, port int, databaseName string, username string, password string, sslmode string) (*DB, error) {
-	pathAndArgs := fmt.Sprintf("host=%s port=%d user=%s dbname=%s password=%s sslmode=%s", host, port, username, databaseName, password, sslmode)
-	db, err := gorm.Open("postgres", pathAndArgs)
+	//nolint:goconst
+	switch url.Scheme {
+	case "sqlite3":
+		q := url.Query()
+		q.Set("cache", "shared")
+		q.Set("_busy_timeout", "30000")
+		q.Set("_journal_mode", "WAL")
+		q.Set("_foreign_keys", "true")
+		url.RawQuery = q.Encode()
+	case "postgres":
+	default:
+		return nil, fmt.Errorf("unknown db scheme")
+	}
+
+	db, err := gorm.Open(url.Scheme, strings.TrimPrefix(url.String(), url.Scheme+"://"))
 	if err != nil {
 		return nil, fmt.Errorf("with gorm: %w", err)
 	}
-	return newDB(db)
-}
 
-func newDB(db *gorm.DB) (*DB, error) {
 	db.SetLogger(log.New(os.Stdout, "gorm ", 0))
 	db.DB().SetMaxOpenConns(1)
 	return &DB{DB: db}, nil
 }
 
-func NewMock() (*DB, error) {
-	return NewSqlite3(":memory:", mockOptions())
-}
-
 func (db *DB) InsertBulkLeftMany(table string, head []string, left int, col []int) error {
 	if len(col) == 0 {
 		return nil
@@ -625,45 +604,3 @@ func join[T fmt.Stringer](in []T, sep string) string {
 	}
 	return strings.Join(strs, sep)
 }
-
-func DumpToSqlite3(ctx context.Context, db *gorm.DB, to string) error {
-	dest, err := NewSqlite3(to, url.Values{})
-	if err != nil {
-		return fmt.Errorf("create dest db: %w", err)
-	}
-	defer dest.Close()
-
-	connSrc, err := db.DB().Conn(ctx)
-	if err != nil {
-		return fmt.Errorf("getting src raw conn: %w", err)
-	}
-	defer connSrc.Close()
-
-	connDest, err := dest.DB.DB().Conn(ctx)
-	if err != nil {
-		return fmt.Errorf("getting dest raw conn: %w", err)
-	}
-	defer connDest.Close()
-
-	err = connDest.Raw(func(connDest interface{}) error {
-		return connSrc.Raw(func(connSrc interface{}) error {
-			connDestq := connDest.(*sqlite3.SQLiteConn)
-			connSrcq := connSrc.(*sqlite3.SQLiteConn)
-			bk, err := connDestq.Backup("main", connSrcq, "main")
-			if err != nil {
-				return fmt.Errorf("create backup db: %w", err)
-			}
-			for done, _ := bk.Step(-1); !done; { //nolint: revive
-			}
-			if err := bk.Finish(); err != nil {
-				return fmt.Errorf("finishing dump: %w", err)
-			}
-			return nil
-		})
-	})
-	if err != nil {
-		return fmt.Errorf("backing up: %w", err)
-	}
-
-	return nil
-}
diff --git a/db/db_test.go b/db/db_test.go
index 0fb2bf1..a5fef75 100644
--- a/db/db_test.go
+++ b/db/db_test.go
@@ -22,7 +22,7 @@ func TestGetSetting(t *testing.T) {
 	key := SettingKey(randKey())
 	value := "howdy"
 
-	testDB, err := NewMock()
+	testDB, err := New("sqlite3://:memory:")
 	if err != nil {
 		t.Fatalf("error creating db: %v", err)
 	}
diff --git a/db/migrations.go b/db/migrations.go
index 90e1c74..3fd2840 100644
--- a/db/migrations.go
+++ b/db/migrations.go
@@ -2,7 +2,6 @@
 package db
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"log"
@@ -20,7 +19,6 @@ import (
 
 type MigrationContext struct {
 	Production        bool
-	DBPath            string
 	OriginalMusicPath string
 	PlaylistsPath     string
 	PodcastsPath      string
@@ -59,7 +57,6 @@ func (db *DB) Migrate(ctx MigrationContext) error {
 		construct(ctx, "202206101425", migrateUser),
 		construct(ctx, "202207251148", migrateStarRating),
 		construct(ctx, "202211111057", migratePlaylistsQueuesToFullID),
-		constructNoTx(ctx, "202212272312", backupDBPre016),
 		construct(ctx, "202304221528", migratePlaylistsToM3U),
 		construct(ctx, "202305301718", migratePlayCountToLength),
 		construct(ctx, "202307281628", migrateAlbumArtistsMany2Many),
@@ -741,13 +738,6 @@ func migratePlaylistsPaths(tx *gorm.DB, ctx MigrationContext) error {
 	return nil
 }
 
-func backupDBPre016(tx *gorm.DB, ctx MigrationContext) error {
-	if ctx.Production {
-		return nil
-	}
-	return DumpToSqlite3(context.Background(), tx, fmt.Sprintf("%s.%d.bak", ctx.DBPath, time.Now().Unix()))
-}
-
 func migrateAlbumTagArtistString(tx *gorm.DB, _ MigrationContext) error {
 	return tx.AutoMigrate(Album{}).Error
 }
diff --git a/mockfs/mockfs.go b/mockfs/mockfs.go
index 53bf944..80929a7 100644
--- a/mockfs/mockfs.go
+++ b/mockfs/mockfs.go
@@ -2,7 +2,6 @@
 package mockfs
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"os"
@@ -11,6 +10,8 @@ import (
 	"testing"
 	"time"
 
+	_ "github.com/jinzhu/gorm/dialects/sqlite"
+
 	"go.senan.xyz/gonic/db"
 	"go.senan.xyz/gonic/scanner"
 	"go.senan.xyz/gonic/tags/tagcommon"
@@ -35,7 +36,7 @@ func NewWithExcludePattern(tb testing.TB, excludePattern string) *MockFS {
 func newMockFS(tb testing.TB, dirs []string, excludePattern string) *MockFS {
 	tb.Helper()
 
-	dbc, err := db.NewMock()
+	dbc, err := db.New("sqlite3://:memory:")
 	if err != nil {
 		tb.Fatalf("create db: %v", err)
 	}
@@ -299,23 +300,6 @@ func (m *MockFS) SetTags(path string, cb func(*TagInfo)) {
 	cb(m.tagReader.paths[absPath])
 }
 
-func (m *MockFS) DumpDB(suffix ...string) {
-	var p []string
-	p = append(p,
-		"gonic", "dump",
-		strings.ReplaceAll(m.t.Name(), string(filepath.Separator), "-"),
-		fmt.Sprint(time.Now().UnixNano()),
-	)
-	p = append(p, suffix...)
-
-	destPath := filepath.Join(os.TempDir(), strings.Join(p, "-"))
-	if err := db.DumpToSqlite3(context.Background(), m.db.DB, destPath); err != nil {
-		m.t.Fatalf("dumping db: %v", err)
-	}
-
-	m.t.Error(destPath)
-}
-
 type tagReader struct {
 	paths map[string]*TagInfo
 }
diff --git a/scanner/scanner_fuzz_test.go b/scanner/scanner_fuzz_test.go
index b4fc28b..c48faed 100644
--- a/scanner/scanner_fuzz_test.go
+++ b/scanner/scanner_fuzz_test.go
@@ -9,7 +9,6 @@ import (
 	"reflect"
 	"testing"
 
-	_ "github.com/jinzhu/gorm/dialects/sqlite"
 	"github.com/stretchr/testify/assert"
 	"go.senan.xyz/gonic/mockfs"
 )
diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go
index c49b6f8..a5bd38d 100644
--- a/scanner/scanner_test.go
+++ b/scanner/scanner_test.go
@@ -10,7 +10,6 @@ import (
 	"testing"
 
 	"github.com/jinzhu/gorm"
-	_ "github.com/jinzhu/gorm/dialects/sqlite"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
diff --git a/server/ctrlsubsonic/handlers_by_folder_test.go b/server/ctrlsubsonic/handlers_by_folder_test.go
index 0efc4e2..6c0c4dd 100644
--- a/server/ctrlsubsonic/handlers_by_folder_test.go
+++ b/server/ctrlsubsonic/handlers_by_folder_test.go
@@ -3,8 +3,6 @@ package ctrlsubsonic
 import (
 	"net/url"
 	"testing"
-
-	_ "github.com/jinzhu/gorm/dialects/sqlite"
 )
 
 func TestGetIndexes(t *testing.T) {

[02strich]

@sentriz that works for me - I made two adjustments: centralized the dialect import to be in db/db.go and had to adjust the database URI for the postgres driver (as it expects the full URI)

Independent of the postgres changes, I had some issues with the cascade delete not working in the tests (which use sqlite3 still), so made some changes to have the cascade for the scanner at least in the go code.

[sentriz]

@02strich what was the issue with cascading deletes? i would like to keep that working if possible. it's used all over the place with gonic stuff (stars and ratings from users, podcast episodes from podcasts, tracks from albums, etc)

[02strich]

@sentriz unfortunately I do not and am confused about it. Are you seeing anything in the changes that would explain it?

[sentriz]

no I mean what is the issue that you faced before you implemented the manual cascade? if you revert the manual cascading in scanner , what happens

[02strich]

Something like in https://github.com/sentriz/gonic/actions/runs/8770050330/job/24066203709