Merge pull request #39 from arachnist/config-refactor

Refactor configuration package.

bot/bot.go

@@ -0,0 +1,18 @@
+package bot
+
+import (
+	"github.com/arachnist/gorepost/config"
+	"sync"
+)
+
+var cfg *config.Config
+var cfgLock sync.Mutex
+
+func Initialize(config *config.Config) {
+	cfg = config
+	cfgLock.Unlock()
+}
+
+func init() {
+	cfgLock.Lock()
+}

bot/channeljoin.go

@@ -7,7 +7,6 @@ package bot
 import (
 	"log"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/dispatcher.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 	"sync"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/helpers.go

@@ -15,7 +15,6 @@ import (
 	"github.com/moovweb/gokogiri"
 	"github.com/moovweb/gokogiri/xpath"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/identify.go

@@ -7,7 +7,6 @@ package bot
 import (
 	"strings"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/jan.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 	"time"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 
@@ -51,6 +50,8 @@ func jan(output func(irc.Message), msg irc.Message) {
 
 func lazyJanInit() {
 	defer janLock.Unlock()
+	cfgLock.Lock()
+	defer cfgLock.Unlock()
 	var err error
 	rand.Seed(time.Now().UnixNano())
 	objects, err = readLines(cfg.LookupString(nil, "DictionaryObjects"))

bot/nickserv.go

@@ -9,7 +9,6 @@ import (
 	"log"
 	"regexp"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/papiez.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 	"time"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 
@@ -34,6 +33,8 @@ func papiez(output func(irc.Message), msg irc.Message) {
 
 func lazyPapiezInit() {
 	defer papiezLock.Unlock()
+	cfgLock.Lock()
+	defer cfgLock.Unlock()
 	var err error
 	rand.Seed(time.Now().UnixNano())
 	adjectives, err = readLines(cfg.LookupString(nil, "DictionaryAdjectives"))

bot/plugins_test.go

@@ -16,7 +16,7 @@ import (
 	"testing"
 	"time"
 
-	cfg "github.com/arachnist/gorepost/config"
+	"github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 
@@ -815,6 +815,9 @@ func configLookupHelper(map[string]string) []string {
 
 func TestMain(m *testing.M) {
 	log.SetOutput(ioutil.Discard)
-	cfg.SetFileListBuilder(configLookupHelper)
 	os.Exit(m.Run())
 }
+
+func init() {
+	Initialize(config.New(configLookupHelper))
+}

bot/seen.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/cloudflare/gokabinet/kt"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

bot/urltitle.go

@@ -12,7 +12,6 @@ import (
 	"strings"
 	"unicode/utf8"
 
-	cfg "github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 

config/config.go

@@ -19,21 +19,24 @@ type cacheEntry struct {
 	contents interface{}
 }
 
-type config struct {
+// A Config is a dynamic configuration key lookup mechanism.
+type Config struct {
 	cache         map[string]cacheEntry
 	buildFileList func(map[string]string) []string
 	l             sync.Mutex
 }
 
-var c config
+// New takes a file list builder and returns a new instance of Config.
-
+//
-func init() {
+// The instance is ready to use.
-	log.Println("Initializing configuration cache")
+func New(f func(map[string]string) []string) *Config {
+	var c Config
 	c.cache = make(map[string]cacheEntry)
-	c.l.Lock() // Lock until we have a proper file list builder
+	c.buildFileList = f
+	return &c
 }
 
-func lookupvar(key, path string) interface{} {
+func (c *Config) lookupvar(key, path string) interface{} {
 	var f interface{}
 	i, err := os.Stat(path)
 	_, ok := c.cache[path]
@@ -52,12 +55,16 @@ func lookupvar(key, path string) interface{} {
 		log.Println("Stale cache for", path)
 		data, err := ioutil.ReadFile(path)
 		if err != nil {
+			log.Println("Purging", path, "from cache:", err)
+			delete(c.cache, path)
 			return nil
 		}
 
 		err = json.Unmarshal(data, &f)
 		if err != nil {
 			log.Println("Cannot parse", path)
+			log.Println("Purging", path, "from cache:", err)
+			delete(c.cache, path)
 			return nil
 		}
 
@@ -74,18 +81,9 @@ func lookupvar(key, path string) interface{} {
 	return c.cache[path].contents.(map[string]interface{})[key]
 }
 
-// SetFileListBuilder registers file list builder function.
-//
-// Registered function takes context (key-value map[string]string) as the only
-// argument and return a slice of strings - file paths.
-func SetFileListBuilder(f func(map[string]string) []string) {
-	c.buildFileList = f
-	c.l.Unlock()
-}
-
 // Lookup searches for requested configuration key in file list built using
 // context.
-func Lookup(context map[string]string, key string) interface{} {
+func (c *Config) Lookup(context map[string]string, key string) interface{} {
 	var value interface{}
 
 	c.l.Lock()
@@ -93,7 +91,7 @@ func Lookup(context map[string]string, key string) interface{} {
 
 	for _, fpath := range c.buildFileList(context) {
 		log.Println("Context:", context, "Looking up", key, "in", fpath)
-		value = lookupvar(key, fpath)
+		value = c.lookupvar(key, fpath)
 		if value != nil {
 			log.Println("Context:", context, "Found", key, value)
 			return value
@@ -105,7 +103,7 @@ func Lookup(context map[string]string, key string) interface{} {
 }
 
 // LookupString is analogous to Lookup(), but does the cast to string.
-func LookupString(context map[string]string, key string) string {
+func (c *Config) LookupString(context map[string]string, key string) string {
 	var value interface{}
 
 	c.l.Lock()
@@ -113,7 +111,7 @@ func LookupString(context map[string]string, key string) string {
 
 	for _, fpath := range c.buildFileList(context) {
 		log.Println("Context:", context, "Looking up", key, "in", fpath)
-		value = lookupvar(key, fpath)
+		value = c.lookupvar(key, fpath)
 		if value != nil {
 			log.Println("Context:", context, "Found", key, value)
 			return value.(string)
@@ -125,7 +123,7 @@ func LookupString(context map[string]string, key string) string {
 }
 
 // LookupInt is analogous to Lookup(), but does the cast to int.
-func LookupInt(context map[string]string, key string) int {
+func (c *Config) LookupInt(context map[string]string, key string) int {
 	var value interface{}
 
 	c.l.Lock()
@@ -133,7 +131,7 @@ func LookupInt(context map[string]string, key string) int {
 
 	for _, fpath := range c.buildFileList(context) {
 		log.Println("Context:", context, "Looking up", key, "in", fpath)
-		value = lookupvar(key, fpath)
+		value = c.lookupvar(key, fpath)
 		if value != nil {
 			log.Println("Context:", context, "Found", key, value)
 			return int(value.(float64))
@@ -145,7 +143,7 @@ func LookupInt(context map[string]string, key string) int {
 }
 
 // LookupStringSlice is analogous to Lookup(), but does the cast to []string
-func LookupStringSlice(context map[string]string, key string) (retval []string) {
+func (c *Config) LookupStringSlice(context map[string]string, key string) (retval []string) {
 	var value interface{}
 
 	c.l.Lock()
@@ -153,7 +151,7 @@ func LookupStringSlice(context map[string]string, key string) (retval []string)
 
 	for _, fpath := range c.buildFileList(context) {
 		log.Println("Context:", context, "Looking up", key, "in", fpath)
-		value = lookupvar(key, fpath)
+		value = c.lookupvar(key, fpath)
 		if value != nil {
 			log.Println("Context:", context, "Found", key, value)
 			for _, s := range value.([]interface{}) {
@@ -170,7 +168,7 @@ func LookupStringSlice(context map[string]string, key string) (retval []string)
 
 // LookupStringMap is analogous to Lookup(), but does the cast to
 // map[string]bool for optimised lookups.
-func LookupStringMap(context map[string]string, key string) (retval map[string]bool) {
+func (c *Config) LookupStringMap(context map[string]string, key string) (retval map[string]bool) {
 	var value interface{}
 	retval = make(map[string]bool)
 
@@ -179,7 +177,7 @@ func LookupStringMap(context map[string]string, key string) (retval map[string]b
 
 	for _, fpath := range c.buildFileList(context) {
 		log.Println("Context:", context, "Looking up", key, "in", fpath)
-		value = lookupvar(key, fpath)
+		value = c.lookupvar(key, fpath)
 		if value != nil {
 			log.Println("Context:", context, "Found", key, value)
 			for _, s := range value.([]interface{}) {

config/config_test.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"reflect"
 	"testing"
+	"time"
 )
 
 var emptyContext map[string]string
@@ -34,7 +35,7 @@ var testsLookup = []struct {
 
 func TestLookup(t *testing.T) {
 	for i, e := range testsLookup {
-		v := Lookup(emptyContext, e.key)
+		v := c.Lookup(emptyContext, e.key)
 		if fmt.Sprintf("%+v", v) != fmt.Sprintf("%+v", e.expectedValue) {
 			t.Log("test#", i, "Lookup key", e.key)
 			t.Logf("expected: %+v\n", e.expectedValue)
@@ -61,7 +62,7 @@ var testsLookupString = []struct {
 
 func TestLookupString(t *testing.T) {
 	for i, e := range testsLookupString {
-		v := LookupString(emptyContext, e.key)
+		v := c.LookupString(emptyContext, e.key)
 		if fmt.Sprintf("%+v", v) != fmt.Sprintf("%+v", e.expectedValue) {
 			t.Log("test#", i+1, "Lookup key", e.key)
 			t.Logf("expected: %+v\n", e.expectedValue)
@@ -88,7 +89,7 @@ var testsLookupInt = []struct {
 
 func TestLookupInt(t *testing.T) {
 	for i, e := range testsLookupInt {
-		v := LookupInt(emptyContext, e.key)
+		v := c.LookupInt(emptyContext, e.key)
 		if fmt.Sprintf("%+v", v) != fmt.Sprintf("%+v", e.expectedValue) {
 			t.Log("test#", i+1, "Lookup key", e.key)
 			t.Logf("expected: %+v\n", e.expectedValue)
@@ -116,7 +117,7 @@ var testsLookupStringSlice = []struct {
 
 func TestLookupStringSlice(t *testing.T) {
 	for i, e := range testsLookupStringSlice {
-		v := LookupStringSlice(emptyContext, e.key)
+		v := c.LookupStringSlice(emptyContext, e.key)
 		if fmt.Sprintf("%+v", v) != fmt.Sprintf("%+v", e.expectedValue) {
 			t.Log("test#", i+1, "Lookup key", e.key)
 			t.Logf("expected: %+v\n", e.expectedValue)
@@ -150,7 +151,7 @@ var testsLookupStringMap = []struct {
 
 func TestLookupStringMap(t *testing.T) {
 	for i, e := range testsLookupStringMap {
-		v := LookupStringMap(emptyContext, e.key)
+		v := c.LookupStringMap(emptyContext, e.key)
 		if !reflect.DeepEqual(v, e.expectedValue) {
 			t.Log("test#", i+1, "Lookup key", e.key)
 			t.Logf("expected: %+v\n", e.expectedValue)
@@ -164,8 +165,82 @@ func configLookupHelper(map[string]string) []string {
 	return []string{"this/thing/does/not/exist.json", ".testconfig.json"}
 }
 
+func complicatedLookupHelper(map[string]string) []string {
+	return []string{
+		".to-be-removed.json",
+		".to-be-replaced-with-gibberish.json",
+		".to-be-chmoded-000.json",
+		".to-be-replaced.json",
+	}
+}
+
+var testEvents = []struct {
+	desc   string
+	action func()
+	ev     int
+}{
+	{
+		desc:   "remove file",
+		action: func() { _ = os.Remove(".to-be-removed.json") },
+		ev:     1,
+	},
+	{
+		desc: "replace with gibberish",
+		action: func() {
+			_ = ioutil.WriteFile(".to-be-replaced-with-gibberish.json", []byte("!@#%^&!%@$&*#@@%*@^#$^&*@&(!"), 0644)
+		},
+		ev: 2,
+	},
+	{
+		desc: "chmod 000",
+		action: func() {
+			_ = os.Chmod(".to-be-chmoded-000.json", 0000)
+			_ = os.Chtimes(".to-be-chmoded-000.json", time.Now().Local(), time.Now().Local())
+		},
+		ev: 3,
+	},
+	{
+		desc:   "replace",
+		action: func() { _ = ioutil.WriteFile(".to-be-replaced.json", []byte("{\"testkey\":99}"), 0644) },
+		ev:     99,
+	},
+}
+
+func TestLookupvarConditions(t *testing.T) {
+	// create files and put their parsed contents in cache
+	for i := len(complicatedLookupHelper(emptyContext)) - 1; i >= 0; i-- {
+		path := complicatedLookupHelper(emptyContext)[i]
+		err := ioutil.WriteFile(path, []byte(fmt.Sprintf("{\"testkey\":%d}", i)), 0644)
+		if err != nil {
+			t.Log("file write failed")
+		}
+
+		if i != c2.LookupInt(emptyContext, "testkey") {
+			t.Log("LookupInt did not return correct value", i)
+			t.Fail()
+		}
+	}
+
+	time.Sleep(time.Second) // make sure we get to update cache
+
+	for _, e := range testEvents {
+		e.action()
+		v := c2.LookupInt(emptyContext, "testkey")
+		if e.ev != v {
+			t.Log("Test failed:", e.desc, "expected:", e.ev, "got:", v)
+			t.Fail()
+		}
+	}
+
+	for _, p := range complicatedLookupHelper(emptyContext) {
+		_ = os.Remove(p)
+	}
+}
+
+var c *Config = New(configLookupHelper)
+var c2 *Config = New(complicatedLookupHelper)
+
 func TestMain(m *testing.M) {
-	SetFileListBuilder(configLookupHelper)
 	log.SetOutput(ioutil.Discard)
 	os.Exit(m.Run())
 }

gorepost.go

@@ -11,7 +11,7 @@ import (
 	"path"
 
 	"github.com/arachnist/gorepost/bot"
-	cfg "github.com/arachnist/gorepost/config"
+	"github.com/arachnist/gorepost/config"
 	"github.com/arachnist/gorepost/irc"
 )
 
@@ -50,7 +50,7 @@ func main() {
 		log.Fatalln("Not a directory:", os.Args[1])
 	}
 
-	cfg.SetFileListBuilder(fileListFuncBuilder(os.Args[1], "common.json"))
+	cfg := config.New(fileListFuncBuilder(os.Args[1], "common.json"))
 
 	logfile, err := os.OpenFile(cfg.LookupString(context, "Logpath"), os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
 	if err != nil {
@@ -62,10 +62,11 @@ func main() {
 
 	log.Println("Configured networks:", len(networks), networks)
 
+	bot.Initialize(cfg)
 	for i, conn := range make([]irc.Connection, len(networks)) {
 		conn := conn
 		log.Println("Setting up", networks[i], "connection")
-		conn.Setup(bot.Dispatcher, networks[i])
+		conn.Setup(bot.Dispatcher, networks[i], cfg)
 	}
 	<-exit
 }

irc/irc.go

@@ -12,7 +12,7 @@ import (
 	"sync"
 	"time"
 
-	cfg "github.com/arachnist/gorepost/config"
+	"github.com/arachnist/gorepost/config"
 )
 
 const delim byte = '\n'
@@ -32,6 +32,7 @@ type Connection struct {
 	quitrecv         chan struct{}
 	quitkeeper       chan struct{}
 	l                sync.Mutex
+	cfg              *config.Config
 }
 
 // Sender sends IRC messages to server and logs their contents.
@@ -150,7 +151,7 @@ func (c *Connection) Keeper() {
 			close(c.quitrecv)
 		}
 		c.quitrecv = make(chan struct{}, 1)
-		servers := cfg.LookupStringSlice(context, "Servers")
+		servers := c.cfg.LookupStringSlice(context, "Servers")
 
 		server := servers[rand.Intn(len(servers))]
 		log.Println(c.network, "connecting to", server)
@@ -162,12 +163,12 @@ func (c *Connection) Keeper() {
 			log.Println(c.network, "Initializing IRC connection")
 			c.Sender(Message{
 				Command:  "NICK",
-				Trailing: cfg.LookupString(context, "Nick"),
+				Trailing: c.cfg.LookupString(context, "Nick"),
 			})
 			c.Sender(Message{
 				Command:  "USER",
-				Params:   []string{cfg.LookupString(context, "User"), "0", "*"},
+				Params:   []string{c.cfg.LookupString(context, "User"), "0", "*"},
-				Trailing: cfg.LookupString(context, "RealName"),
+				Trailing: c.cfg.LookupString(context, "RealName"),
 			})
 		} else {
 			log.Println(c.network, "connection error", err.Error())
@@ -178,7 +179,7 @@ func (c *Connection) Keeper() {
 }
 
 // Setup performs initialization tasks.
-func (c *Connection) Setup(dispatcher func(func(Message), Message), network string) {
+func (c *Connection) Setup(dispatcher func(func(Message), Message), network string, config *config.Config) {
 	rand.Seed(time.Now().UnixNano())
 
 	c.reconnect = make(chan struct{}, 1)
@@ -187,6 +188,7 @@ func (c *Connection) Setup(dispatcher func(func(Message), Message), network stri
 	c.Quit = make(chan struct{}, 1)
 	c.network = network
 	c.dispatcher = dispatcher
+	c.cfg = config
 
 	c.reconnect <- struct{}{}
 	go c.Keeper()

irc/irc_test.go

@@ -17,7 +17,7 @@ import (
 	"testing"
 	"time"
 
-	cfg "github.com/arachnist/gorepost/config"
+	"github.com/arachnist/gorepost/config"
 )
 
 var expectedOutput = []Message{
@@ -105,7 +105,7 @@ func TestSetup(t *testing.T) {
 	go fakeServer(t)
 
 	var conn Connection
-	conn.Setup(fakeDispatcher, "TestNet")
+	conn.Setup(fakeDispatcher, "TestNet", config.New(configLookupHelper))
 
 	time.Sleep(2 * time.Second)
 
@@ -148,7 +148,6 @@ func configLookupHelper(map[string]string) []string {
 }
 
 func TestMain(m *testing.M) {
-	cfg.SetFileListBuilder(configLookupHelper)
 	log.SetOutput(ioutil.Discard)
 	os.Exit(m.Run())
 }