Simplify config package

kytrinyx · kytrinyx · commit 7f5be848cc65 · 2015-01-19T06:47:55.000-08:00
We only ever do three things from the outside:

1. Load an existing config, falling back to defaults if it doesn't exist.
2. Update values on a config.
3. Write the config to disc.

Everything else is details.

In order to be able to set a fake homeDir on this, the New function
delegates to a separate load function. This also allows us to avoid
potential race conditions in the tests by not setting ENV variables.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * Stop supporting legacy config files (~/.exercism.go)
 * Deleted deprecated login/logout commands
 * Deleted deprecated key names in config
+* [#153](https://github.com/exercism/cli/pull/153): Refactored configuration package - [@kytrinyx](https://github.com/kytrinyx)
 * Your contribution here
 
 ## v1.9.2 (Jan 11, 2015)
diff --git a/cmd/configure.go b/cmd/configure.go
@@ -13,7 +13,7 @@ import (
 // If a setting is not passed as an argument, default
 // values are used.
 func Configure(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/debug.go b/cmd/debug.go
@@ -24,7 +24,7 @@ func Debug(ctx *cli.Context) {
 	}
 	fmt.Printf("Home Dir: %s\n", dir)
 
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/demo.go b/cmd/demo.go
@@ -12,7 +12,7 @@ import (
 
 // Demo returns one problem for each active track.
 func Demo(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/download.go b/cmd/download.go
@@ -14,7 +14,7 @@ import (
 
 // Download returns specified submissions and related problem.
 func Download(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/fetch.go b/cmd/fetch.go
@@ -12,7 +12,7 @@ import (
 
 // Fetch returns exercism problems.
 func Fetch(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/restore.go b/cmd/restore.go
@@ -12,7 +12,7 @@ import (
 
 // Restore returns a user's solved problems.
 func Restore(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/submit.go b/cmd/submit.go
@@ -18,7 +18,7 @@ func Submit(ctx *cli.Context) {
 		log.Fatal("Please enter a file name")
 	}
 
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/tracks.go b/cmd/tracks.go
@@ -12,7 +12,7 @@ import (
 
 // Tracks lists available tracks.
 func Tracks(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/cmd/unsubmit.go b/cmd/unsubmit.go
@@ -13,7 +13,7 @@ import (
 // If no iteration is specified, the most recent iteration
 // is deleted.
 func Unsubmit(ctx *cli.Context) {
-	c, err := config.Read(ctx.GlobalString("config"))
+	c, err := config.New(ctx.GlobalString("config"))
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/config/config.go b/config/config.go
@@ -3,7 +3,6 @@ package config
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -65,99 +64,116 @@ func Home() (string, error) {
 	return dir, nil
 }
 
-// Read loads the config from the stored JSON file.
-func Read(file string) (*Config, error) {
+func New(path string) (*Config, error) {
 	c := &Config{}
-	err := c.Read(file)
+	err := c.load(path, os.Getenv(fileEnvKey))
 	return c, err
 }
 
 // Update sets new values where given.
 func (c *Config) Update(key, host, dir, xapi string) {
+	key = strings.TrimSpace(key)
 	if key != "" {
 		c.APIKey = key
 	}
 
+	host = strings.TrimSpace(host)
 	if host != "" {
 		c.API = host
 	}
 
+	dir = strings.TrimSpace(dir)
 	if dir != "" {
 		c.Dir = dir
 	}
 
+	xapi = strings.TrimSpace(xapi)
 	if xapi != "" {
 		c.XAPI = xapi
 	}
-
-	c.configure()
 }
 
-// Expand takes inputs for a config file location and builds an absolute path.
-func Expand(path, env, home string) string {
-	if path == "" {
-		path = env
-	}
-
-	if path != "" && path[0] == '~' {
-		path = strings.Replace(path, "~/", fmt.Sprintf("%s/", home), 1)
-	}
-
-	if path == "" {
-		path = filepath.Join(home, File)
+// Write saves the config as JSON.
+func (c *Config) Write() error {
+	// truncates existing file if it exists
+	f, err := os.Create(c.File)
+	if err != nil {
+		return err
 	}
+	defer f.Close()
 
-	return path
+	e := json.NewEncoder(f)
+	return e.Encode(c)
 }
 
-// Read loads the config from the stored JSON file.
-func (c *Config) Read(file string) error {
-	home, err := c.homeDir()
+func (c *Config) load(argPath, envPath string) error {
+	path, err := c.resolvePath(argPath, envPath)
 	if err != nil {
 		return err
 	}
+	c.File = path
 
-	c.File = Expand(file, os.Getenv(fileEnvKey), home)
+	if err := c.read(); err != nil {
+		return err
+	}
 
+	// in case people manually update the config file
+	// with weird formatting
+	c.APIKey = strings.TrimSpace(c.APIKey)
+	c.Dir = strings.TrimSpace(c.Dir)
+	c.API = strings.TrimSpace(c.API)
+	c.XAPI = strings.TrimSpace(c.XAPI)
+
+	return c.setDefaults()
+}
+
+func (c *Config) read() error {
 	if _, err := os.Stat(c.File); err != nil {
 		if os.IsNotExist(err) {
-			c.configure()
 			return nil
 		}
 		return err
 	}
-
 	f, err := os.Open(c.File)
 	if err != nil {
 		return err
 	}
 	defer f.Close()
 
 	d := json.NewDecoder(f)
-	err = d.Decode(&c)
-	if err != nil {
-		return err
+	return d.Decode(&c)
+}
+
+// IsAuthenticated returns true if the config contains an API key.
+// This does not check whether or not that key is valid.
+func (c *Config) IsAuthenticated() bool {
+	return c.APIKey != ""
+}
+
+// homeDir caches the lookup of the user's home directory.
+func (c *Config) homeDir() (string, error) {
+	if c.home != "" {
+		return c.home, nil
 	}
-	c.configure()
-	return nil
+	return Home()
 }
 
-// Write() saves the config as JSON.
-func (c *Config) Write() error {
-	// truncates existing file if it exists
-	f, err := os.Create(c.File)
+func (c *Config) resolvePath(argPath, envPath string) (string, error) {
+	path := argPath
+	if path == "" {
+		path = envPath
+	}
+	if path == "" {
+		path = filepath.Join("~", File)
+	}
+	h, err := c.homeDir()
 	if err != nil {
-		return err
+		return "", err
 	}
-	defer f.Close()
-
-	e := json.NewEncoder(f)
-	return e.Encode(c)
+	return strings.Replace(path, "~", h, 1), nil
 }
 
-func (c *Config) configure() (*Config, error) {
-	c.sanitize()
-
+func (c *Config) setDefaults() error {
 	if c.API == "" {
 		c.API = hostAPI
 	}
@@ -166,44 +182,15 @@ func (c *Config) configure() (*Config, error) {
 		c.XAPI = hostXAPI
 	}
 
-	homeDir, err := c.homeDir()
+	h, err := c.homeDir()
 	if err != nil {
-		return c, err
+		return err
 	}
 
 	if c.Dir == "" {
-		// fall back to default value
-		c.Dir = filepath.Join(homeDir, DirExercises)
-	} else {
-		// replace '~' with user's home
-		c.Dir = strings.Replace(c.Dir, "~/", fmt.Sprintf("%s/", homeDir), 1)
-	}
-
-	if c.File == "" {
-		c.File = filepath.Join(homeDir, File)
-	}
-
-	return c, nil
-}
-
-// IsAuthenticated returns true if the config contains an API key.
-// This does not check whether or not that key is valid.
-func (c *Config) IsAuthenticated() bool {
-	return c.APIKey != ""
-}
-
-// See: http://stackoverflow.com/questions/7922270/obtain-users-home-directory
-// we can't cross compile using cgo and use user.Current()
-func (c *Config) homeDir() (string, error) {
-	if c.home != "" {
-		return c.home, nil
+		c.Dir = filepath.Join(h, DirExercises)
 	}
-	return Home()
-}
+	c.Dir = strings.Replace(c.Dir, "~", h, 1)
 
-func (c *Config) sanitize() {
-	c.APIKey = strings.TrimSpace(c.APIKey)
-	c.Dir = strings.TrimSpace(c.Dir)
-	c.API = strings.TrimSpace(c.API)
-	c.XAPI = strings.TrimSpace(c.XAPI)
+  return nil
 }
diff --git a/config/config_test.go b/config/config_test.go
diff --git a/fixtures/config.json b/fixtures/config.json
diff --git a/fixtures/dirty.json b/fixtures/dirty.json
diff --git a/fixtures/home/.exercism.json b/fixtures/home/.exercism.json
diff --git a/fixtures/home/custom.json b/fixtures/home/custom.json

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ import (`
`13`	`13`	`// If a setting is not passed as an argument, default`
`14`	`14`	`// values are used.`
`15`	`15`	`func Configure(ctx *cli.Context) {`
`16`		`- c, err := config.Read(ctx.GlobalString("config"))`
	`16`	`+ c, err := config.New(ctx.GlobalString("config"))`
`17`	`17`	`if err != nil {`
`18`	`18`	`log.Fatal(err)`
`19`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ func Debug(ctx *cli.Context) {`
`24`	`24`	`}`
`25`	`25`	`fmt.Printf("Home Dir: %s\n", dir)`
`26`	`26`
`27`		`- c, err := config.Read(ctx.GlobalString("config"))`
	`27`	`+ c, err := config.New(ctx.GlobalString("config"))`
`28`	`28`	`if err != nil {`
`29`	`29`	`log.Fatal(err)`
`30`	`30`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ import (`
`12`	`12`
`13`	`13`	`// Demo returns one problem for each active track.`
`14`	`14`	`func Demo(ctx *cli.Context) {`
`15`		`- c, err := config.Read(ctx.GlobalString("config"))`
	`15`	`+ c, err := config.New(ctx.GlobalString("config"))`
`16`	`16`	`if err != nil {`
`17`	`17`	`log.Fatal(err)`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ import (`
`14`	`14`
`15`	`15`	`// Download returns specified submissions and related problem.`
`16`	`16`	`func Download(ctx *cli.Context) {`
`17`		`- c, err := config.Read(ctx.GlobalString("config"))`
	`17`	`+ c, err := config.New(ctx.GlobalString("config"))`
`18`	`18`	`if err != nil {`
`19`	`19`	`log.Fatal(err)`
`20`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ func Submit(ctx *cli.Context) {`
`18`	`18`	`log.Fatal("Please enter a file name")`
`19`	`19`	`}`
`20`	`20`
`21`		`- c, err := config.Read(ctx.GlobalString("config"))`
	`21`	`+ c, err := config.New(ctx.GlobalString("config"))`
`22`	`22`	`if err != nil {`
`23`	`23`	`log.Fatal(err)`
`24`	`24`	`}`