Use reflection and yaml.Unmarshal() for configuration, see #66

cmd/photoprism/photoprism.go

@@ -4,6 +4,7 @@ import (
 	"os"
 
 	"github.com/photoprism/photoprism/internal/commands"
+	"github.com/photoprism/photoprism/internal/context"
 	"github.com/urfave/cli"
 )
 
@@ -14,9 +15,9 @@ func main() {
 	app.Name = "PhotoPrism"
 	app.Usage = "Browse your life in pictures"
 	app.Version = version
-	app.Copyright = "(c) 2018 The PhotoPrism contributors <hello@photoprism.org>"
+	app.Copyright = "(c) 2018-2019 The PhotoPrism contributors <hello@photoprism.org>"
 	app.EnableBashCompletion = true
-	app.Flags = commands.GlobalFlags
+	app.Flags = context.GlobalFlags
 
 	app.Commands = []cli.Command{
 		commands.ConfigCommand,

docker-compose.yml

@@ -19,10 +19,12 @@ services:
       PHOTOPRISM_IMPORT_PATH: "/go/src/github.com/photoprism/photoprism/assets/photos/import"
       PHOTOPRISM_EXPORT_PATH: "/go/src/github.com/photoprism/photoprism/assets/photos/export"
       PHOTOPRISM_ORIGINALS_PATH: "/go/src/github.com/photoprism/photoprism/assets/photos/originals"
+      PHOTOPRISM_DATABASE_DRIVER: "internal"
       PHOTOPRISM_DATABASE_DSN: "root:photoprism@tcp(localhost:4000)/photoprism?parseTime=true"
       PHOTOPRISM_SQL_HOST: "0.0.0.0"
       PHOTOPRISM_SQL_PORT: 4000
       PHOTOPRISM_SQL_PASSWORD: "photoprism"
+      PHOTOPRISM_DARKTABLE_CLI: "/usr/bin/darktable-cli"
 
   database:
     image: mysql:8.0.16

go.mod

@@ -59,4 +59,5 @@ require (
 	gopkg.in/go-playground/assert.v1 v1.2.1 // indirect
 	gopkg.in/go-playground/validator.v8 v8.18.2 // indirect
 	gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce // indirect
+	gopkg.in/yaml.v2 v2.2.2
 )

internal/context/config.go

@@ -1,11 +1,17 @@
 package context
 
 import (
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"reflect"
+
 	_ "github.com/jinzhu/gorm/dialects/mysql"
 	_ "github.com/jinzhu/gorm/dialects/sqlite"
-	"github.com/kylelemons/go-gypsy/yaml"
 	"github.com/photoprism/photoprism/internal/fsutil"
+	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
+	"gopkg.in/yaml.v2"
 )
 
 const (
@@ -25,180 +31,119 @@ type Config struct {
 	Name               string
 	Version            string
 	Copyright          string
-	Debug              bool
+	Debug              bool   `yaml:"debug" flag:"debug"`
-	LogLevel           string
+	LogLevel           string `yaml:"log-level" flag:"log-level"`
 	ConfigFile         string
-	AssetsPath         string
+	AssetsPath         string `yaml:"assets-path" flag:"assets-path"`
-	CachePath          string
+	CachePath          string `yaml:"cache-path" flag:"cache-path"`
-	OriginalsPath      string
+	OriginalsPath      string `yaml:"originals-path" flag:"originals-path"`
-	ImportPath         string
+	ImportPath         string `yaml:"import-path" flag:"import-path"`
-	ExportPath         string
+	ExportPath         string `yaml:"export-path" flag:"export-path"`
-	SqlServerHost      string
+	SqlServerHost      string `yaml:"sql-host" flag:"sql-host"`
-	SqlServerPort      uint
+	SqlServerPort      uint   `yaml:"sql-port" flag:"sql-port"`
-	SqlServerPath      string
+	SqlServerPath      string `yaml:"sql-path" flag:"sql-path"`
-	SqlServerPassword  string
+	SqlServerPassword  string `yaml:"sql-password" flag:"sql-password"`
-	HttpServerHost     string
+	HttpServerHost     string `yaml:"http-host" flag:"http-host"`
-	HttpServerPort     int
+	HttpServerPort     int    `yaml:"http-port" flag:"http-port"`
-	HttpServerMode     string
+	HttpServerMode     string `yaml:"http-mode" flag:"http-mode"`
-	HttpServerPassword string
+	HttpServerPassword string `yaml:"http-password" flag:"http-password"`
-	DarktableCli       string
+	DarktableCli       string `yaml:"darktable-cli" flag:"darktable-cli"`
-	DatabaseDriver     string
+	DatabaseDriver     string `yaml:"database-driver" flag:"database-driver"`
-	DatabaseDsn        string
+	DatabaseDsn        string `yaml:"database-dsn" flag:"database-dsn"`
+}
+
+// NewConfig() creates a new configuration entity by using two methods:
+//
+// 1. SetValuesFromFile: This will initialize values from a yaml config file.
+//
+// 2. SetValuesFromCliContext: Which comes after SetValuesFromFile and overrides
+//    any previous values giving an option two override file configs through the CLI.
+func NewConfig(ctx *cli.Context) *Config {
+	c := &Config{}
+
+	c.Name = ctx.App.Name
+	c.Copyright = ctx.App.Copyright
+	c.Version = ctx.App.Version
+
+	if err := c.SetValuesFromFile(fsutil.ExpandedFilename(ctx.GlobalString("config-file"))); err != nil {
+		log.Debug(err)
+	}
+
+	if err := c.SetValuesFromCliContext(ctx); err != nil {
+		log.Error(err)
+	}
+
+	c.expandFilenames()
+
+	return c
+}
+
+func (c *Config) expandFilenames() {
+	c.AssetsPath = fsutil.ExpandedFilename(c.AssetsPath)
+	c.CachePath = fsutil.ExpandedFilename(c.CachePath)
+	c.OriginalsPath = fsutil.ExpandedFilename(c.OriginalsPath)
+	c.ImportPath = fsutil.ExpandedFilename(c.ImportPath)
+	c.ExportPath = fsutil.ExpandedFilename(c.ExportPath)
+	c.DarktableCli = fsutil.ExpandedFilename(c.DarktableCli)
+	c.SqlServerPath = fsutil.ExpandedFilename(c.SqlServerPath)
 }
 
 // SetValuesFromFile uses a yaml config file to initiate the configuration entity.
 func (c *Config) SetValuesFromFile(fileName string) error {
-	yamlConfig, err := yaml.ReadFile(fileName)
+	if !fsutil.Exists(fileName) {
+		return errors.New(fmt.Sprintf("config file not found: \"%s\"", fileName))
+	}
+
+	yamlConfig, err := ioutil.ReadFile(fileName)
 
 	if err != nil {
 		return err
 	}
 
-	c.ConfigFile = fileName
+	return yaml.Unmarshal(yamlConfig, c)
-	if debug, err := yamlConfig.GetBool("debug"); err == nil {
-		c.Debug = debug
-	}
-
-	if logLevel, err := yamlConfig.Get("log-level"); err == nil {
-		c.LogLevel = logLevel
-	}
-
-	if sqlServerHost, err := yamlConfig.Get("sql-host"); err == nil {
-		c.SqlServerHost = sqlServerHost
-	}
-
-	if sqlServerPort, err := yamlConfig.GetInt("sql-port"); err == nil {
-		c.SqlServerPort = uint(sqlServerPort)
-	}
-
-	if sqlServerPassword, err := yamlConfig.Get("sql-password"); err == nil {
-		c.SqlServerPassword = sqlServerPassword
-	}
-
-	if sqlServerPath, err := yamlConfig.Get("sql-path"); err == nil {
-		c.SqlServerPath = sqlServerPath
-	}
-
-	if httpServerHost, err := yamlConfig.Get("http-host"); err == nil {
-		c.HttpServerHost = httpServerHost
-	}
-
-	if httpServerPort, err := yamlConfig.GetInt("http-port"); err == nil {
-		c.HttpServerPort = int(httpServerPort)
-	}
-
-	if httpServerMode, err := yamlConfig.Get("http-mode"); err == nil {
-		c.HttpServerMode = httpServerMode
-	}
-
-	if httpServerPassword, err := yamlConfig.Get("http-password"); err == nil {
-		c.HttpServerPassword = httpServerPassword
-	}
-
-	if assetsPath, err := yamlConfig.Get("assets-path"); err == nil {
-		c.AssetsPath = fsutil.ExpandedFilename(assetsPath)
-	}
-
-	if cachePath, err := yamlConfig.Get("cache-path"); err == nil {
-		c.CachePath = fsutil.ExpandedFilename(cachePath)
-	}
-
-	if originalsPath, err := yamlConfig.Get("originals-path"); err == nil {
-		c.OriginalsPath = fsutil.ExpandedFilename(originalsPath)
-	}
-
-	if importPath, err := yamlConfig.Get("import-path"); err == nil {
-		c.ImportPath = fsutil.ExpandedFilename(importPath)
-	}
-
-	if exportPath, err := yamlConfig.Get("export-path"); err == nil {
-		c.ExportPath = fsutil.ExpandedFilename(exportPath)
-	}
-
-	if darktableCli, err := yamlConfig.Get("darktable-cli"); err == nil {
-		c.DarktableCli = fsutil.ExpandedFilename(darktableCli)
-	}
-
-	if databaseDriver, err := yamlConfig.Get("database-driver"); err == nil {
-		c.DatabaseDriver = databaseDriver
-	}
-
-	if databaseDsn, err := yamlConfig.Get("database-dsn"); err == nil {
-		c.DatabaseDsn = databaseDsn
-	}
-
-	return nil
 }
 
 // SetValuesFromCliContext uses values from the CLI to setup configuration overrides
 // for the entity.
 func (c *Config) SetValuesFromCliContext(ctx *cli.Context) error {
-	if ctx.GlobalBool("debug") {
+	v := reflect.ValueOf(c).Elem()
-		c.Debug = ctx.GlobalBool("debug")
-	}
 
-	if ctx.GlobalIsSet("log-level") || c.LogLevel == "" {
+	// Iterate through all config fields
-		c.LogLevel = ctx.GlobalString("log-level")
+	for i := 0; i < v.NumField(); i++ {
-	}
+		fieldValue := v.Field(i)
 
-	if ctx.GlobalIsSet("assets-path") || c.AssetsPath == "" {
+		tagValue := v.Type().Field(i).Tag.Get("flag")
-		c.AssetsPath = fsutil.ExpandedFilename(ctx.GlobalString("assets-path"))
-	}
 
-	if ctx.GlobalIsSet("cache-path") || c.CachePath == "" {
+		// Automatically assign values to fields with "flag" tag
-		c.CachePath = fsutil.ExpandedFilename(ctx.GlobalString("cache-path"))
+		if tagValue != "" {
+			switch t := fieldValue.Interface().(type) {
+			case int, int64:
+				// Only if explicitly set or current value is empty (use default)
+				if ctx.GlobalIsSet(tagValue) || fieldValue.Int() == 0 {
+					f := ctx.GlobalInt64(tagValue)
+					fieldValue.SetInt(f)
 				}
-
+			case uint, uint64:
-	if ctx.GlobalIsSet("originals-path") || c.OriginalsPath == "" {
+				// Only if explicitly set or current value is empty (use default)
-		c.OriginalsPath = fsutil.ExpandedFilename(ctx.GlobalString("originals-path"))
+				if ctx.GlobalIsSet(tagValue) || fieldValue.Uint() == 0 {
+					f := ctx.GlobalUint64(tagValue)
+					fieldValue.SetUint(f)
 				}
-
+			case string:
-	if ctx.GlobalIsSet("import-path") || c.ImportPath == "" {
+				// Only if explicitly set or current value is empty (use default)
-		c.ImportPath = fsutil.ExpandedFilename(ctx.GlobalString("import-path"))
+				if ctx.GlobalIsSet(tagValue) || fieldValue.String() == "" {
+					f := ctx.GlobalString(tagValue)
+					fieldValue.SetString(f)
 				}
-
+			case bool:
-	if ctx.GlobalIsSet("export-path") || c.ExportPath == "" {
+				if ctx.GlobalIsSet(tagValue) {
-		c.ExportPath = fsutil.ExpandedFilename(ctx.GlobalString("export-path"))
+					f := ctx.GlobalBool(tagValue)
+					fieldValue.SetBool(f)
 				}
-
+			default:
-	if ctx.GlobalIsSet("darktable-cli") || c.DarktableCli == "" {
+				log.Warnf("can't assign value of type %s from cli flag %s", t, tagValue)
-		c.DarktableCli = fsutil.ExpandedFilename(ctx.GlobalString("darktable-cli"))
 			}
-
-	if ctx.GlobalIsSet("database-driver") || c.DatabaseDriver == "" {
-		c.DatabaseDriver = ctx.GlobalString("database-driver")
 		}
-
-	if ctx.GlobalIsSet("database-dsn") || c.DatabaseDsn == "" {
-		c.DatabaseDsn = ctx.GlobalString("database-dsn")
-	}
-
-	if ctx.GlobalIsSet("sql-host") || c.SqlServerHost == "" {
-		c.SqlServerHost = ctx.GlobalString("sql-host")
-	}
-
-	if ctx.GlobalIsSet("sql-port") || c.SqlServerPort == 0 {
-		c.SqlServerPort = ctx.GlobalUint("sql-port")
-	}
-
-	if ctx.GlobalIsSet("sql-password") || c.SqlServerPassword == "" {
-		c.SqlServerPassword = ctx.GlobalString("sql-password")
-	}
-
-	if ctx.GlobalIsSet("sql-path") || c.SqlServerPath == "" {
-		c.SqlServerPath = ctx.GlobalString("sql-path")
-	}
-
-	if ctx.GlobalIsSet("http-host") || c.HttpServerHost == "" {
-		c.HttpServerHost = ctx.GlobalString("http-host")
-	}
-
-	if ctx.GlobalIsSet("http-port") || c.HttpServerPort == 0 {
-		c.HttpServerPort = ctx.GlobalInt("http-port")
-	}
-
-	if ctx.GlobalIsSet("http-mode") || c.HttpServerMode == "" {
-		c.HttpServerMode = ctx.GlobalString("http-mode")
 	}
 
 	return nil

internal/context/config_test.go

@@ -7,13 +7,28 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestNewConfig(t *testing.T) {
+	ctx := CliTestContext()
+
+	assert.True(t, ctx.IsSet("assets-path"))
+	assert.False(t, ctx.Bool("debug"))
+
+	c := NewConfig(ctx)
+
+	assert.IsType(t, new(Config), c)
+
+	assert.Equal(t, fsutil.ExpandedFilename("../../assets"), c.AssetsPath)
+	assert.False(t, c.Debug)
+}
+
 func TestConfig_SetValuesFromFile(t *testing.T) {
 	c := NewConfig(CliTestContext())
 
-	err := c.SetValuesFromFile(fsutil.ExpandedFilename("../../configs/photoprism.yml"))
+	err := c.SetValuesFromFile("testdata/config.yml")
 
 	assert.Nil(t, err)
 
+	assert.False(t, c.Debug)
 	assert.Equal(t, "/srv/photoprism", c.AssetsPath)
 	assert.Equal(t, "/srv/photoprism/cache", c.CachePath)
 	assert.Equal(t, "/srv/photoprism/photos/originals", c.OriginalsPath)
@@ -21,4 +36,5 @@ func TestConfig_SetValuesFromFile(t *testing.T) {
 	assert.Equal(t, "/srv/photoprism/photos/export", c.ExportPath)
 	assert.Equal(t, "internal", c.DatabaseDriver)
 	assert.Equal(t, "root:photoprism@tcp(localhost:4000)/photoprism?parseTime=true", c.DatabaseDsn)
+	assert.Equal(t, 81, c.HttpServerPort)
 }

internal/context/context.go

@@ -1,6 +1,7 @@
 package context
 
 import (
+	"errors"
 	"os"
 	"time"
 
@@ -19,41 +20,21 @@ type Context struct {
 	config *Config
 }
 
-// NewConfig() creates a new configuration entity by using two methods:
+func initLogger(debug bool)  {
-//
-// 1. SetValuesFromFile: This will initialize values from a yaml config file.
-//
-// 2. SetValuesFromCliContext: Which comes after SetValuesFromFile and overrides
-//    any previous values giving an option two override file configs through the CLI.
-func NewConfig(ctx *cli.Context) *Config {
-	c := &Config{}
-
-	c.Name = ctx.App.Name
-	c.Copyright = ctx.App.Copyright
-	c.Version = ctx.App.Version
-
 	log.SetFormatter(&log.TextFormatter{
 		DisableColors: false,
 		FullTimestamp: true,
 	})
 
-	if err := c.SetValuesFromFile(fsutil.ExpandedFilename(ctx.GlobalString("config-file"))); err != nil {
+	if debug {
-		log.Debug(err)
+		log.SetLevel(log.DebugLevel)
+	} else {
+		log.SetLevel(log.InfoLevel)
 	}
-
-	if err := c.SetValuesFromCliContext(ctx); err != nil {
-		log.Error(err)
-	}
-
-	return c
 }
 
 func NewContext(ctx *cli.Context) *Context {
-	if ctx.GlobalBool("debug") {
+	initLogger(ctx.GlobalBool("debug"))
-		log.SetLevel(log.DebugLevel)
-	} else {
-		log.SetLevel(log.ErrorLevel)
-	}
 
 	c := &Context{config: NewConfig(ctx)}
 
@@ -62,7 +43,6 @@ func NewContext(ctx *cli.Context) *Context {
 	return c
 }
 
-
 // CreateDirectories creates all the folders that photoprism needs. These are:
 // OriginalsPath
 // ThumbnailsPath
@@ -107,6 +87,14 @@ func (c *Context) connectToDatabase() error {
 	dbDriver := c.DatabaseDriver()
 	dbDsn := c.DatabaseDsn()
 
+	if dbDriver == "" {
+		return errors.New("can't connect: database driver not specified")
+	}
+
+	if dbDsn == "" {
+		return errors.New("can't connect: database DSN not specified")
+	}
+
 	isTiDB := false
 	initSuccess := false
 
@@ -257,16 +245,27 @@ func (c *Context) ExportPath() string {
 
 // DarktableCli returns the darktable-cli binary file name.
 func (c *Context) DarktableCli() string {
+	if c.config.DarktableCli == "" {
+		return "/usr/bin/darktable-cli"
+	}
 	return c.config.DarktableCli
 }
 
 // DatabaseDriver returns the database driver name.
 func (c *Context) DatabaseDriver() string {
+	if c.config.DatabaseDriver == "" {
+		return DbTiDB
+	}
+
 	return c.config.DatabaseDriver
 }
 
 // DatabaseDsn returns the database data source name (DSN).
 func (c *Context) DatabaseDsn() string {
+	if c.config.DatabaseDsn == "" {
+		return "root:photoprism@tcp(localhost:4000)/photoprism?parseTime=true"
+	}
+
 	return c.config.DatabaseDsn
 }
 
@@ -318,7 +317,9 @@ func (c *Context) HttpPublicBuildPath() string {
 // Db returns the db connection.
 func (c *Context) Db() *gorm.DB {
 	if c.db == nil {
-		c.connectToDatabase()
+		if err := c.connectToDatabase(); err != nil {
+			log.Fatal(err)
+		}
 	}
 
 	return c.db

internal/context/context_test.go

@@ -7,16 +7,16 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestNewAppConfig(t *testing.T) {
+func TestNewContext(t *testing.T) {
 	ctx := CliTestContext()
 
 	assert.True(t, ctx.IsSet("assets-path"))
 	assert.False(t, ctx.Bool("debug"))
 
-	c := NewConfig(ctx)
+	c := NewContext(ctx)
 
-	assert.IsType(t, new(Config), c)
+	assert.IsType(t, new(Context), c)
 
-	assert.Equal(t, fsutil.ExpandedFilename("../../assets"), c.AssetsPath)
+	assert.Equal(t, fsutil.ExpandedFilename("../../assets"), c.AssetsPath())
-	assert.False(t, c.Debug)
+	assert.False(t, c.Debug())
 }

internal/context/flags.go

@@ -1,4 +1,4 @@
-package commands
+package context
 
 import "github.com/urfave/cli"
 

internal/context/test_context.go

@@ -28,10 +28,10 @@ func testDataPath(assetsPath string) string {
 
 func NewTestConfig() *Config {
 	assetsPath := fsutil.ExpandedFilename("../../assets")
+
 	testDataPath := testDataPath(assetsPath)
 
 	c := &Config{
-		ConfigFile:     "../../configs/photoprism.yml",
 		DarktableCli:   "/usr/bin/darktable-cli",
 		AssetsPath:     assetsPath,
 		CachePath:      testDataPath + "/cache",
@@ -54,7 +54,7 @@ func TestContext() *Context {
 }
 
 func NewTestContext() *Context {
-	log.SetLevel(log.FatalLevel)
+	log.SetLevel(log.DebugLevel)
 
 	c := &Context{config: NewTestConfig()}
 
@@ -99,12 +99,12 @@ func (c *Context) DownloadTestData(t *testing.T) {
 
 		if hash != TestDataHash {
 			os.Remove(TestDataZip)
-			t.Logf("Removed outdated test data zip file (fingerprint %s)\n", hash)
+			t.Logf("removed outdated test data zip file (fingerprint %s)\n", hash)
 		}
 	}
 
 	if !fsutil.Exists(TestDataZip) {
-		fmt.Printf("Downloading latest test data zip file from %s\n", TestDataURL)
+		fmt.Printf("downloading latest test data zip file from %s\n", TestDataURL)
 
 		if err := fsutil.Download(TestDataZip, TestDataURL); err != nil {
 			fmt.Printf("Download failed: %s\n", err.Error())
@@ -114,12 +114,12 @@ func (c *Context) DownloadTestData(t *testing.T) {
 
 func (c *Context) UnzipTestData(t *testing.T) {
 	if _, err := fsutil.Unzip(TestDataZip, testDataPath(c.AssetsPath())); err != nil {
-		t.Logf("Could not unzip test data: %s\n", err.Error())
+		t.Logf("could not unzip test data: %s\n", err.Error())
 	}
 }
 
 func (c *Context) InitializeTestData(t *testing.T) {
-	t.Log("Initializing test data")
+	t.Log("initializing test data")
 
 	c.RemoveTestData(t)
 

internal/context/testdata/config.yml

@@ -0,0 +1,16 @@
+debug: false
+darktable-cli: /usr/bin/darktable-cli
+assets-path: /srv/photoprism
+cache-path: /srv/photoprism/cache
+originals-path: /srv/photoprism/photos/originals
+import-path: /srv/photoprism/photos/import
+export-path: /srv/photoprism/photos/export
+sql-host: localhost
+sql-port: 4000
+sql-password: photoprism
+http-host:
+http-mode: release
+http-port: 81
+http-password:
+database-driver: internal
+database-dsn: root:photoprism@tcp(localhost:4000)/photoprism?parseTime=true

internal/fsutil/file.go

@@ -9,6 +9,8 @@ import (
 	"os/user"
 	"path/filepath"
 	"strings"
+
+	log "github.com/sirupsen/logrus"
 )
 
 // Returns true if file exists
@@ -21,7 +23,8 @@ func Exists(filename string) bool {
 // Returns full path; ~ replaced with actual home directory
 func ExpandedFilename(filename string) string {
 	if filename == "" {
-		panic("filename was empty")
+		log.Debug("check configuration: empty file or directory name")
+		return ""
 	}
 
 	if len(filename) > 2 && filename[:2] == "~/" {

internal/fsutil/file_test.go

@@ -7,12 +7,12 @@ import (
 )
 
 func TestExists(t *testing.T) {
-	assert.True(t, Exists("./_fixtures/test.jpg"))
+	assert.True(t, Exists("./testdata/test.jpg"))
 	assert.False(t, Exists("./foo.jpg"))
 }
 
 func TestExpandedFilename(t *testing.T) {
-	filename := ExpandedFilename("./_fixtures/test.jpg")
+	filename := ExpandedFilename("./testdata/test.jpg")
 
 	assert.IsType(t, "", filename)
 

internal/fsutil/hash_test.go

@@ -7,6 +7,6 @@ import (
 )
 
 func TestHash(t *testing.T) {
-	hash := Hash("_fixtures/test.jpg")
+	hash := Hash("testdata/test.jpg")
 	assert.Equal(t, "516cb1fefbfd9fa66f1db50b94503a480cee30db", hash)
 }