From efb04516fdd50c771a168939aba0bd0dc3a51c1d Mon Sep 17 00:00:00 2001 From: Joshua Chamberlain Date: Mon, 18 Sep 2023 10:00:27 -0700 Subject: [PATCH] Add tests for config watchers and related functions --- atlas/atlas_test.go | 49 +++++++++ cmd/tegola/cmd/root.go | 38 ++++--- cmd/tegola/cmd/root_test.go | 204 +++++++++++++++++++++++++++++++++++ config/source/file.go | 11 +- config/source/file_test.go | 184 +++++++++++++++++++++++++++++++ config/source/source.go | 8 +- config/source/source_test.go | 95 ++++++++++++++++ 7 files changed, 571 insertions(+), 18 deletions(-) create mode 100644 cmd/tegola/cmd/root_test.go create mode 100644 config/source/file_test.go create mode 100644 config/source/source_test.go diff --git a/atlas/atlas_test.go b/atlas/atlas_test.go index 75d24718a..45b96a923 100644 --- a/atlas/atlas_test.go +++ b/atlas/atlas_test.go @@ -1,6 +1,9 @@ package atlas_test import ( + "strings" + "testing" + "github.com/go-spatial/geom" "github.com/go-spatial/tegola/atlas" "github.com/go-spatial/tegola/internal/env" @@ -51,3 +54,49 @@ var testMap = atlas.Map{ testLayer3, }, } + +func TestAddMaps(t *testing.T) { + a := &atlas.Atlas{} + + // Should initialize from empty + maps := []atlas.Map{ + {Name: "First Map"}, + {Name: "Second Map"}, + } + err := a.AddMaps(maps) + if err != nil { + t.Errorf("Unexpected error when addings maps. %s", err) + } + + m, err := a.Map("Second Map") + if err != nil { + t.Errorf("Failed retrieving map from Atlas. %s", err) + } else if m.Name != "Second Map" { + t.Errorf("Expected map named \"Second Map\". Found %v.", m) + } + + // Should error if duplicate name. + err = a.AddMaps([]atlas.Map{{Name: "First Map"}}) + if err == nil || !strings.Contains(err.Error(), "already exists") { + t.Errorf("Should return error for duplicate map name. err=%s", err) + } +} + +func TestRemoveMaps(t *testing.T) { + a := &atlas.Atlas{} + a.AddMaps([]atlas.Map{ + {Name: "First Map"}, + {Name: "Second Map"}, + }) + + if len(a.AllMaps()) != 2 { + t.Error("Unexpected failure setting up Atlas. No maps added.") + return + } + + a.RemoveMaps([]string{"Second Map"}) + maps := a.AllMaps() + if len(maps) != 1 || maps[0].Name == "Second Map" { + t.Error("Should have deleted \"Second Map\". Didn't.") + } +} diff --git a/cmd/tegola/cmd/root.go b/cmd/tegola/cmd/root.go index 7df6a0481..a6a139e63 100644 --- a/cmd/tegola/cmd/root.go +++ b/cmd/tegola/cmd/root.go @@ -118,14 +118,16 @@ func initConfig(configFile string, cacheRequired bool, logLevel string, logger s return err } + loader := appInitializer{} + // Init providers from the primary config file. - providers, err := initProviders(conf.Providers, conf.Maps, "default") + providers, err := loader.initProviders(conf.Providers, conf.Maps, "default") if err != nil { return err } // Init maps from the primary config file. - if err = initMaps(conf.Maps, providers); err != nil { + if err = loader.initMaps(conf.Maps, providers); err != nil { return err } @@ -158,8 +160,16 @@ func initConfig(configFile string, cacheRequired bool, logLevel string, logger s return nil } +type initializer interface { + initProviders(providersConfig []env.Dict, maps []provider.Map, namespace string) (map[string]provider.TilerUnion, error) + initMaps(maps []provider.Map, providers map[string]provider.TilerUnion) error + unload(app source.App) +} + +type appInitializer struct{} + // initProviders translate provider config from a TOML file into usable Provider objects. -func initProviders(providersConfig []env.Dict, maps []provider.Map, namespace string) (map[string]provider.TilerUnion, error) { +func (l appInitializer) initProviders(providersConfig []env.Dict, maps []provider.Map, namespace string) (map[string]provider.TilerUnion, error) { // first convert []env.Map -> []dict.Dicter provArr := make([]dict.Dicter, len(providersConfig)) for i := range provArr { @@ -175,7 +185,7 @@ func initProviders(providersConfig []env.Dict, maps []provider.Map, namespace st } // initMaps registers maps with Atlas to be ready for service. -func initMaps(maps []provider.Map, providers map[string]provider.TilerUnion) error { +func (l appInitializer) initMaps(maps []provider.Map, providers map[string]provider.TilerUnion) error { if err := register.Maps(nil, maps, providers); err != nil { return fmt.Errorf("could not register maps: %v", err) } @@ -183,6 +193,12 @@ func initMaps(maps []provider.Map, providers map[string]provider.TilerUnion) err return nil } +// unload deregisters the maps and providers of an app. +func (l appInitializer) unload(app source.App) { + register.UnloadMaps(nil, getMapNames(app)) + register.UnloadProviders(getProviderNames(app), app.Key) +} + // initAppConfigSource sets up an additional configuration source for "apps" (groups of providers and maps) to be loaded and unloaded on-the-fly. func initAppConfigSource(ctx context.Context, conf config.Config) error { // Get the config source type. If none, return. @@ -203,13 +219,13 @@ func initAppConfigSource(ctx context.Context, conf config.Config) error { return err } - go watchAppUpdates(ctx, watcher) + go watchAppUpdates(ctx, watcher, appInitializer{}) return nil } // watchAppUpdates will pull from the channels supplied by the given watcher to process new app config. -func watchAppUpdates(ctx context.Context, watcher source.ConfigWatcher) { +func watchAppUpdates(ctx context.Context, watcher source.ConfigWatcher, init initializer) { // Keep a record of what we've loaded so that we can unload when needed. apps := make(map[string]source.App) @@ -229,22 +245,21 @@ func watchAppUpdates(ctx context.Context, watcher source.ConfigWatcher) { // If the new app is named the same as an existing app, first unload the existing one. if old, exists := apps[app.Key]; exists { log.Infof("Unloading app %s...", old.Key) - register.UnloadMaps(nil, getMapNames(old)) - register.UnloadProviders(getProviderNames(old), old.Key) + init.unload(old) delete(apps, old.Key) } log.Infof("Loading app %s...", app.Key) // Init new providers - providers, err := initProviders(app.Providers, app.Maps, app.Key) + providers, err := init.initProviders(app.Providers, app.Maps, app.Key) if err != nil { log.Errorf("Failed initializing providers from %s: %s", app.Key, err) continue } // Init new maps - if err = initMaps(app.Maps, providers); err != nil { + if err = init.initMaps(app.Maps, providers); err != nil { log.Errorf("Failed initializing maps from %s: %s", app.Key, err) continue } @@ -260,8 +275,7 @@ func watchAppUpdates(ctx context.Context, watcher source.ConfigWatcher) { // Unload an app's maps if it was previously loaded. if app, exists := apps[deleted]; exists { log.Infof("Unloading app %s...", app.Key) - register.UnloadMaps(nil, getMapNames(app)) - register.UnloadProviders(getProviderNames(app), app.Key) + init.unload(app) delete(apps, app.Key) } else { log.Infof("Received an unload event for app %s, but couldn't find it.", deleted) diff --git a/cmd/tegola/cmd/root_test.go b/cmd/tegola/cmd/root_test.go new file mode 100644 index 000000000..06f86b657 --- /dev/null +++ b/cmd/tegola/cmd/root_test.go @@ -0,0 +1,204 @@ +package cmd + +import ( + "context" + "reflect" + "strings" + "testing" + "time" + + "github.com/go-spatial/tegola/config" + "github.com/go-spatial/tegola/config/source" + "github.com/go-spatial/tegola/internal/env" + "github.com/go-spatial/tegola/provider" +) + +type initializerMock struct { + initProvidersCalls chan bool + initMapsCalls chan bool + unloadCalls chan bool +} + +func (i initializerMock) initProviders(providersConfig []env.Dict, maps []provider.Map, namespace string) (map[string]provider.TilerUnion, error) { + i.initProvidersCalls <- true + return map[string]provider.TilerUnion{}, nil +} + +func (i initializerMock) initProvidersCalled() bool { + select { + case _, ok := <-i.initProvidersCalls: + return ok + case <-time.After(time.Millisecond): + return false + } +} + +func (i initializerMock) initMaps(maps []provider.Map, providers map[string]provider.TilerUnion) error { + i.initMapsCalls <- true + return nil +} + +func (i initializerMock) initMapsCalled() bool { + select { + case _, ok := <-i.initMapsCalls: + return ok + case <-time.After(time.Millisecond): + return false + } +} + +func (i initializerMock) unload(app source.App) { + i.unloadCalls <- true +} + +func (i initializerMock) unloadCalled() bool { + select { + case _, ok := <-i.unloadCalls: + return ok + case <-time.After(time.Millisecond): + return false + } +} + +func TestInitAppConfigSource(t *testing.T) { + var ( + cfg config.Config + err error + ) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Should return nil if app config source type not specified. + cfg = config.Config{} + err = initAppConfigSource(ctx, cfg) + if err != nil { + t.Errorf("Unexpected error when app config source type is not specified: %s", err) + } + + // Should return error if unable to initialize source. + cfg = config.Config{ + AppConfigSource: env.Dict{ + "type": "something_nonexistent", + }, + } + err = initAppConfigSource(ctx, cfg) + if err == nil { + t.Error("Should return an error if invalid source type provided") + } + + cfg = config.Config{ + AppConfigSource: env.Dict{ + "type": "file", + "dir": "something_nonexistent", + }, + } + err = initAppConfigSource(ctx, cfg) + if err == nil || !strings.Contains(err.Error(), "directory") { + t.Errorf("Should return an error if unable to initialize source; expected an error about the directory, got %v", err) + } +} + +func TestWatchAppUpdates(t *testing.T) { + loader := initializerMock{ + initProvidersCalls: make(chan bool), + initMapsCalls: make(chan bool), + unloadCalls: make(chan bool), + } + // watcher := mock.NewWatcherMock() + watcher := source.ConfigWatcher{ + Updates: make(chan source.App), + Deletions: make(chan string), + } + defer watcher.Close() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go watchAppUpdates(ctx, watcher, loader) + + // Should load new map+provider + app := source.App{ + Providers: []env.Dict{}, + Maps: []provider.Map{}, + Key: "Test1", + } + // watcher.SendUpdate(app) + watcher.Updates <- app + if !loader.initProvidersCalled() { + t.Error("Failed to initialize providers") + } + if !loader.initMapsCalled() { + t.Error("Failed to initialize maps") + } + if loader.unloadCalled() { + t.Error("Unexpected app unload") + } + + // Should load updated map+provider + // watcher.SendUpdate(app) + watcher.Updates <- app + if !loader.unloadCalled() { + t.Error("Failed to unload old app") + } + if !loader.initProvidersCalled() { + t.Error("Failed to initialize providers") + } + if !loader.initMapsCalled() { + t.Error("Failed to initialize maps") + } + + // Should unload map+provider + // watcher.SendDeletion("Test1") + watcher.Deletions <- "Test1" + if !loader.unloadCalled() { + t.Error("Failed to unload old app") + } +} + +func TestGetMapNames(t *testing.T) { + app := source.App{ + Maps: []provider.Map{ + {Name: "First Map"}, + {Name: "Second Map"}, + }, + } + expected := []string{"First Map", "Second Map"} + names := getMapNames(app) + if !reflect.DeepEqual(expected, names) { + t.Errorf("Expected map names %v; found %v", expected, names) + } +} + +func TestGetProviderNames(t *testing.T) { + var ( + app source.App + names []string + expected []string + ) + + // Happy path + app = source.App{ + Providers: []env.Dict{ + {"name": "First Provider"}, + {"name": "Second Provider"}, + }, + } + expected = []string{"First Provider", "Second Provider"} + names = getProviderNames(app) + if !reflect.DeepEqual(expected, names) { + t.Errorf("Expected provider names %v; found %v", expected, names) + } + + // Error + app = source.App{ + Providers: []env.Dict{ + {}, + {"name": "Second Provider"}, + }, + } + expected = []string{"Second Provider"} + names = getProviderNames(app) + if !reflect.DeepEqual(expected, names) { + t.Errorf("Expected provider names %v; found %v", expected, names) + } +} diff --git a/config/source/file.go b/config/source/file.go index 1f01fd406..f2aa2b72b 100644 --- a/config/source/file.go +++ b/config/source/file.go @@ -17,11 +17,7 @@ type FileConfigSource struct { dir string } -func (s *FileConfigSource) Type() string { - return "file" -} - -func (s *FileConfigSource) Init(options env.Dict, baseDir string) error { +func (s *FileConfigSource) init(options env.Dict, baseDir string) error { var err error dir, err := options.String("dir", nil) if err != nil { @@ -37,6 +33,10 @@ func (s *FileConfigSource) Init(options env.Dict, baseDir string) error { return nil } +func (s *FileConfigSource) Type() string { + return "file" +} + // LoadAndWatch will read all the files in the configured directory and then keep watching the directory for changes. func (s *FileConfigSource) LoadAndWatch(ctx context.Context) (ConfigWatcher, error) { appWatcher := ConfigWatcher{ @@ -109,6 +109,7 @@ func (s *FileConfigSource) LoadAndWatch(ctx context.Context) (ConfigWatcher, err case <-ctx.Done(): log.Info("Exiting watcher...") + appWatcher.Close() return } } diff --git a/config/source/file_test.go b/config/source/file_test.go new file mode 100644 index 000000000..b0020d0c0 --- /dev/null +++ b/config/source/file_test.go @@ -0,0 +1,184 @@ +package source + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/BurntSushi/toml" + "github.com/go-spatial/tegola/internal/env" +) + +func TestFileConfigSourceInit(t *testing.T) { + var ( + src FileConfigSource + err error + ) + + src = FileConfigSource{} + err = src.init(env.Dict{}, "") + if err == nil { + t.Error("init() should return an error if no dir provided; no error returned.") + } + + absDir := "/tmp/configs" + src = FileConfigSource{} + src.init(env.Dict{"dir": absDir}, "/opt") + if src.dir != absDir { + t.Errorf("init() should preserve absolute path %s; found %s instead.", absDir, src.dir) + } + + relDir := "configs" + src = FileConfigSource{} + src.init(env.Dict{"dir": relDir}, "/root") + joined := filepath.Join("/root", relDir) + if src.dir != joined { + t.Errorf("init() should place relative path under given basedir; expected %s, found %s.", joined, src.dir) + } +} + +func TestFileConfigSourceLoadAndWatch(t *testing.T) { + var ( + src FileConfigSource + watcher ConfigWatcher + dir string + ctx context.Context + err error + ) + + dir, _ = os.MkdirTemp("", "tegolaapps") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Should error if directory not readable. + src = FileConfigSource{} + src.init(env.Dict{"dir": filepath.Join(dir, "nonexistent")}, "") + watcher, err = src.LoadAndWatch(ctx) + if err == nil { + t.Error("LoadAndWatch() should error if directory doesn't exist; no error returned.") + } + + // Should load files already present in directory. + err = createFile(App{}, filepath.Join(dir, "app1.toml")) + if err != nil { + t.Errorf("Could not create an app config file. %s", err) + return + } + src = FileConfigSource{} + src.init(env.Dict{"dir": dir}, "") + watcher, err = src.LoadAndWatch(ctx) + if err != nil { + t.Errorf("No error expected from LoadAndWatch(): returned %s", err) + return + } + + updates := readAllUpdates(watcher.Updates) + if len(updates) != 1 || updates[0].Key != filepath.Join(dir, "app1.toml") { + t.Errorf("Failed reading preexisting files: len=%d updates=%v", len(updates), updates) + } + + // Should detect new files added to directory. + createFile(App{}, filepath.Join(dir, "app2.toml")) + createFile(App{}, filepath.Join(dir, "app3.toml")) + updates = readAllUpdates(watcher.Updates) + if len(updates) != 2 || updates[0].Key != filepath.Join(dir, "app2.toml") || updates[1].Key != filepath.Join(dir, "app3.toml") { + t.Errorf("Failed reading new files: len=%d updates=%v", len(updates), updates) + } + + // Should detect files removed from directory. + os.Remove(filepath.Join(dir, "app2.toml")) + os.Remove(filepath.Join(dir, "app1.toml")) + deletions := readAllDeletions(watcher.Deletions) + if len(deletions) != 2 || !contains(deletions, filepath.Join(dir, "app2.toml")) || !contains(deletions, filepath.Join(dir, "app1.toml")) { + t.Errorf("Failed detecting deletions: len=%d deletions=%v", len(deletions), deletions) + } +} + +func TestFileConfigSourceLoadAndWatchShouldExitOnDone(t *testing.T) { + dir, _ := os.MkdirTemp("", "tegolaapps") + + src := FileConfigSource{} + src.init(env.Dict{"dir": dir}, "") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + watcher, err := src.LoadAndWatch(ctx) + if err != nil { + t.Errorf("No error expected from LoadAndWatch(): returned %s", err) + return + } + + cancel() + select { + case <-watcher.Updates: + // do nothing + case <-time.After(time.Millisecond): + t.Error("Updates channel should be closed, but is still open.") + } + + select { + case <-watcher.Deletions: + // do nothing + case <-time.After(time.Millisecond): + t.Error("Deletions channel should be closed, but is still open.") + } +} + +func createFile(app App, filename string) error { + f, err := os.Create(filename) + if err != nil { + return err + } + defer f.Close() + + err = toml.NewEncoder(f).Encode(app) + return err +} + +func readAllUpdates(updates chan App) []App { + apps := []App{} + + for { + select { + case app, ok := <-updates: + if !ok { + return apps + } + + apps = append(apps, app) + + case <-time.After(10 * time.Millisecond): + return apps + } + } +} + +func readAllDeletions(deletions chan string) []string { + keys := []string{} + + for { + select { + case key, ok := <-deletions: + if !ok { + return keys + } + + keys = append(keys, key) + + case <-time.After(10 * time.Millisecond): + return keys + } + } +} + +func contains(vals []string, expected string) bool { + for _, str := range vals { + if str == expected { + return true + } + } + + return false +} diff --git a/config/source/source.go b/config/source/source.go index 5e0e5d408..40a974974 100644 --- a/config/source/source.go +++ b/config/source/source.go @@ -22,16 +22,22 @@ type ConfigSource interface { LoadAndWatch(ctx context.Context) (ConfigWatcher, error) } +// ConfigWatcher allows watching for App updates (new and changes) and deletions. type ConfigWatcher struct { Updates chan App Deletions chan string } +func (c ConfigWatcher) Close() { + close(c.Updates) + close(c.Deletions) +} + func InitSource(sourceType string, options env.Dict, baseDir string) (ConfigSource, error) { switch sourceType { case "file": src := FileConfigSource{} - err := src.Init(options, baseDir) + err := src.init(options, baseDir) return &src, err default: diff --git a/config/source/source_test.go b/config/source/source_test.go new file mode 100644 index 000000000..bd43d4e6e --- /dev/null +++ b/config/source/source_test.go @@ -0,0 +1,95 @@ +package source + +import ( + "strings" + "testing" + + "github.com/go-spatial/tegola/internal/env" +) + +func TestInitSource(t *testing.T) { + var ( + src ConfigSource + err error + ) + + _, err = InitSource("invalidtype", env.Dict{}, "") + if err == nil { + t.Error("InitSource should error if invalid source type provided; no error returned.") + } + + _, err = InitSource("file", env.Dict{}, "") + if err == nil { + t.Error("InitSource should return error from underlying source type (file) if no directory provided.") + } + + src, err = InitSource("file", env.Dict{"dir": "config"}, "/tmp") + if err != nil { + t.Errorf("Unexpected error from InitSource: %s", err) + } + + if src.Type() != "file" { + t.Errorf("Expected source type %s, found %s", "file", src.Type()) + } +} + +func TestParseApp(t *testing.T) { + conf := ` + [[providers]] + name = "test_postgis" + type = "mvt_postgis" + uri = "postgres:/username:password@127.0.0.1:5423/some_db" + srid = 3857 + + [[providers.layers]] + name = "dynamic" + sql = "id, ST_AsMVTGeom(wkb_geometry, !BBOX!) as geom FROM some_table WHERE wkb_geometry && !BBOX!" + geometry_type = "polygon" + + [[maps]] + name = "stuff" + + [[maps.layers]] + provider_layer = "test_postgis.dynamic" + min_zoom = 2 + max_zoom = 18 + + [[maps.params]] + name = "param" + token = "!PaRaM!" + ` + + r := strings.NewReader(conf) + + // Should load TOML file. + app, err := parseApp(r, "some_key") + if err != nil { + t.Errorf("Unexpected error from parseApp: %s", err) + return + } + + if app.Key != "some_key" { + t.Errorf("Expected app key \"some_key\", found %s", app.Key) + } + + if len(app.Providers) != 1 { + t.Error("Failed to load providers from TOML") + } else { + name, err := app.Providers[0].String("name", nil) + if err != nil || name != "test_postgis" { + t.Errorf("Expected provider name \"test_postgis\", found %s (err=%s)", name, err) + } + } + + if len(app.Maps) != 1 { + t.Error("Failed to load maps from TOML") + } else if app.Maps[0].Name != "stuff" { + t.Errorf("Expected map name \"stuff\", found %s", app.Maps[0].Name) + } + + // Should normalize map params. + token := "!PARAM!" + if len(app.Maps) == 1 && app.Maps[0].Parameters[0].Token != token { + t.Errorf("Expected map query param with token %s, found %s.", token, app.Maps[0].Parameters[0].Token) + } +}