sentriz
Adding support for PostgreSQL as database
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.
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.
thanks! at first glace this looks very nice. I appreciate the minimal diff. will hopefully take a closer look during the week
Anything I can help to move this along?
sorry for the delay! just had a look :+1: :+1:
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
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)
@02strichhttps://github.com/02strich still interested in moving this along?
— Reply to this email directly, view it on GitHubhttps://github.com/sentriz/gonic/pull/260#issuecomment-2022609255, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAK2X4ZKEGKJP7MEDFFR3LY2KR5FAVCNFSM6AAAAAASGIOUPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRSGYYDSMRVGU. You are receiving this because you were mentioned.Message ID: @.***>
thanks for rebase :+1: 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) {
@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.
@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)
@sentriz unfortunately I do not and am confused about it. Are you seeing anything in the changes that would explain it?
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
Something like in https://github.com/sentriz/gonic/actions/runs/8770050330/job/24066203709