From ba4bbd1d478c740897ce384021dce0300541e220 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 21 Oct 2017 22:50:12 +0200 Subject: [PATCH] datasource as cfg: refactor tests to use yaml files --- conf/datasources.yaml | 34 +++---- pkg/setting/datasources/datasources.go | 46 +++++----- pkg/setting/datasources/datasources_test.go | 90 ++++++------------- .../double-default-datasources.yaml | 12 +++ .../two-datasources-purge-others.yaml | 10 +++ .../test-configs/two-datasources.yaml | 10 +++ .../test-configs/zero-datasources.yaml | 2 + pkg/setting/datasources/types.go | 40 +++++---- 8 files changed, 120 insertions(+), 124 deletions(-) create mode 100644 pkg/setting/datasources/test-configs/double-default-datasources.yaml create mode 100644 pkg/setting/datasources/test-configs/two-datasources-purge-others.yaml create mode 100644 pkg/setting/datasources/test-configs/two-datasources.yaml create mode 100644 pkg/setting/datasources/test-configs/zero-datasources.yaml diff --git a/conf/datasources.yaml b/conf/datasources.yaml index 11f55318188..8d40d49f83c 100644 --- a/conf/datasources.yaml +++ b/conf/datasources.yaml @@ -1,30 +1,20 @@ -purgeOtherDatasources: false +purge_other_datasources: false datasources: - - name: Graphite202 + - name: Graphite type: graphite access: proxy url: http://localhost:8080 - password: - user: - database: - basicAuth: - basicAuthUser: - basicAuthPassword: - withCredentials: - isDefault: true - jsonData: {} - secureJsonFields: {} + password: #string + user: #string + database: #string + basic_auth: #bool + basic_authUser: #string + basic_auth_password: #string + with_credentials: #bool + is_default: true #bool + json_data: '{"graphiteVersion":"0.9"}' # string json + secure_json_fields: '' #string json - name: Prometheus type: prometheus access: proxy url: http://localhost:9090 - password: - user: - database: - basicAuth: - basicAuthUser: - basicAuthPassword: - withCredentials: - isDefault: true - jsonData: {} - secureJsonFields: {} diff --git a/pkg/setting/datasources/datasources.go b/pkg/setting/datasources/datasources.go index a14d9326253..25eeba426f3 100644 --- a/pkg/setting/datasources/datasources.go +++ b/pkg/setting/datasources/datasources.go @@ -1,6 +1,7 @@ package datasources import ( + "errors" "io/ioutil" "path/filepath" @@ -11,10 +12,9 @@ import ( yaml "gopkg.in/yaml.v2" ) -type DatasourcesAsConfig struct { - PurgeOtherDatasources bool - Datasources []DataSourceFromConfig -} +var ( + ErrInvalidConfigToManyDefault = errors.New("datasource.yaml config is invalid. Only one datasource can be marked as default") +) func Init(configPath string) error { dc := NewDatasourceConfiguration() @@ -28,14 +28,14 @@ type DatasourceConfigurator struct { } func NewDatasourceConfiguration() DatasourceConfigurator { - return newDatasourceConfiguration(log.New("setting.datasource"), diskConfigReader{}, sqlDatasourceRepository{}) + return newDatasourceConfiguration(log.New("setting.datasource"), sqlDatasourceRepository{}) } -func newDatasourceConfiguration(log log.Logger, cfgProvider configProvider, repo datasourceRepository) DatasourceConfigurator { +func newDatasourceConfiguration(log log.Logger, repo datasourceRepository) DatasourceConfigurator { return DatasourceConfigurator{ - log: log.New("setting.datasource"), + log: log, repository: repo, - cfgProvider: cfgProvider, + cfgProvider: configProvider{}, } } @@ -45,15 +45,23 @@ func (dc *DatasourceConfigurator) applyChanges(configPath string) error { return err } - allDatasources, err := dc.repository.loadAllDatasources() - if err != nil { - return err - } - + defaultCount := 0 for i := range cfg.Datasources { if cfg.Datasources[i].OrgId == 0 { cfg.Datasources[i].OrgId = 1 } + + if cfg.Datasources[i].IsDefault { + defaultCount++ + if defaultCount > 1 { + return ErrInvalidConfigToManyDefault + } + } + } + + allDatasources, err := dc.repository.loadAllDatasources() + if err != nil { + return err } if err := dc.deleteDatasourcesNotInConfiguration(cfg, allDatasources); err != nil { @@ -73,11 +81,11 @@ func (dc *DatasourceConfigurator) applyChanges(configPath string) error { dc.log.Info("inserting datasource from configuration ", "name", ds.Name) insertCmd := createInsertCommand(ds) err := dc.repository.insert(insertCmd) - if err != nil && err != models.ErrDataSourceNameExists { + if err != nil { return err } } else { - dc.log.Info("updating datasource from configuration", "name", ds.Name) + dc.log.Debug("updating datasource from configuration", "name", ds.Name) updateCmd := createUpdateCommand(ds, dbDatasource.Id) if err := dc.repository.update(updateCmd); err != nil { return err @@ -119,14 +127,10 @@ type datasourceRepository interface { loadAllDatasources() ([]*models.DataSource, error) } -type configProvider interface { - readConfig(string) (*DatasourcesAsConfig, error) -} - type sqlDatasourceRepository struct{} -type diskConfigReader struct{} +type configProvider struct{} -func (diskConfigReader) readConfig(path string) (*DatasourcesAsConfig, error) { +func (configProvider) readConfig(path string) (*DatasourcesAsConfig, error) { filename, _ := filepath.Abs(path) yamlFile, err := ioutil.ReadFile(filename) diff --git a/pkg/setting/datasources/datasources_test.go b/pkg/setting/datasources/datasources_test.go index 3bee56d74af..b39b20e6558 100644 --- a/pkg/setting/datasources/datasources_test.go +++ b/pkg/setting/datasources/datasources_test.go @@ -9,79 +9,59 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -var logger log.Logger = log.New("fake.logger") +var ( + logger log.Logger = log.New("fake.logger") + oneDatasourcesConfig string = "" + twoDatasourcesConfig string = "./test-configs/two-datasources.yaml" + twoDatasourcesConfigPurgeOthers string = "./test-configs/two-datasources-purge-others.yaml" + doubleDatasourcesConfig string = "./test-configs/double-default-datasources.yaml" +) func TestDatasourceAsConfig(t *testing.T) { Convey("Testing datasource as configuration", t, func() { - fakeCfg := &fakeConfig{} fakeRepo := &fakeRepository{} Convey("One configured datasource", func() { - fakeCfg.cfg = &DatasourcesAsConfig{ - PurgeOtherDatasources: false, - Datasources: []DataSourceFromConfig{ - {Name: "graphite", OrgId: 1}, - }, - } - Convey("no datasource in database", func() { - dc := newDatasourceConfiguration(logger, fakeCfg, fakeRepo) - err := dc.applyChanges("mock/config.yaml") + dc := newDatasourceConfiguration(logger, fakeRepo) + err := dc.applyChanges(twoDatasourcesConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 1) + So(len(fakeRepo.inserted), ShouldEqual, 2) So(len(fakeRepo.updated), ShouldEqual, 0) }) Convey("One datasource in database with same name", func() { fakeRepo.loadAll = []*models.DataSource{ - {Name: "graphite", OrgId: 1, Id: 1}, + {Name: "Graphite", OrgId: 1, Id: 1}, } Convey("should update one datasource", func() { - dc := newDatasourceConfiguration(logger, fakeCfg, fakeRepo) - err := dc.applyChanges("mock/config.yaml") - if err != nil { - t.Fatalf("applyChanges return an error %v", err) - } - - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 0) - So(len(fakeRepo.updated), ShouldEqual, 1) - }) - }) - - Convey("One datasource in database with new name", func() { - fakeRepo.loadAll = []*models.DataSource{ - {Name: "old-graphite", OrgId: 1, Id: 1}, - } - - Convey("should update one datasource", func() { - dc := newDatasourceConfiguration(logger, fakeCfg, fakeRepo) - err := dc.applyChanges("mock/config.yaml") + dc := newDatasourceConfiguration(logger, fakeRepo) + err := dc.applyChanges(twoDatasourcesConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } So(len(fakeRepo.deleted), ShouldEqual, 0) So(len(fakeRepo.inserted), ShouldEqual, 1) - So(len(fakeRepo.updated), ShouldEqual, 0) + So(len(fakeRepo.updated), ShouldEqual, 1) + }) + }) + + Convey("Two datasources with is_default", func() { + dc := newDatasourceConfiguration(logger, fakeRepo) + err := dc.applyChanges(doubleDatasourcesConfig) + Convey("should raise error", func() { + So(err, ShouldEqual, ErrInvalidConfigToManyDefault) }) }) }) Convey("Two configured datasource and purge others ", func() { - fakeCfg.cfg = &DatasourcesAsConfig{ - PurgeOtherDatasources: true, - Datasources: []DataSourceFromConfig{ - {Name: "graphite", OrgId: 1}, - {Name: "prometheus", OrgId: 1}, - }, - } - Convey("two other datasources in database", func() { fakeRepo.loadAll = []*models.DataSource{ {Name: "old-graphite", OrgId: 1, Id: 1}, @@ -89,8 +69,8 @@ func TestDatasourceAsConfig(t *testing.T) { } Convey("should have two new datasources", func() { - dc := newDatasourceConfiguration(logger, fakeCfg, fakeRepo) - err := dc.applyChanges("mock/config.yaml") + dc := newDatasourceConfiguration(logger, fakeRepo) + err := dc.applyChanges(twoDatasourcesConfigPurgeOthers) if err != nil { t.Fatalf("applyChanges return an error %v", err) } @@ -103,23 +83,15 @@ func TestDatasourceAsConfig(t *testing.T) { }) Convey("Two configured datasource and purge others = false", func() { - fakeCfg.cfg = &DatasourcesAsConfig{ - PurgeOtherDatasources: false, - Datasources: []DataSourceFromConfig{ - {Name: "graphite", OrgId: 1}, - {Name: "prometheus", OrgId: 1}, - }, - } - Convey("two other datasources in database", func() { fakeRepo.loadAll = []*models.DataSource{ - {Name: "graphite", OrgId: 1, Id: 1}, + {Name: "Graphite", OrgId: 1, Id: 1}, {Name: "old-graphite2", OrgId: 1, Id: 2}, } Convey("should have two new datasources", func() { - dc := newDatasourceConfiguration(logger, fakeCfg, fakeRepo) - err := dc.applyChanges("mock/config.yaml") + dc := newDatasourceConfiguration(logger, fakeRepo) + err := dc.applyChanges(twoDatasourcesConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } @@ -141,14 +113,6 @@ type fakeRepository struct { loadAll []*models.DataSource } -type fakeConfig struct { - cfg *DatasourcesAsConfig -} - -func (fc *fakeConfig) readConfig(path string) (*DatasourcesAsConfig, error) { - return fc.cfg, nil -} - func (fc *fakeRepository) delete(cmd *models.DeleteDataSourceByIdCommand) error { fc.deleted = append(fc.deleted, cmd) return nil diff --git a/pkg/setting/datasources/test-configs/double-default-datasources.yaml b/pkg/setting/datasources/test-configs/double-default-datasources.yaml new file mode 100644 index 00000000000..07f69809fea --- /dev/null +++ b/pkg/setting/datasources/test-configs/double-default-datasources.yaml @@ -0,0 +1,12 @@ +purge_other_datasources: false +datasources: + - name: Graphite + type: graphite + access: proxy + url: http://localhost:8080 + is_default: true + - name: Prometheus + type: prometheus + access: proxy + url: http://localhost:9090 + is_default: true diff --git a/pkg/setting/datasources/test-configs/two-datasources-purge-others.yaml b/pkg/setting/datasources/test-configs/two-datasources-purge-others.yaml new file mode 100644 index 00000000000..e884b09c97f --- /dev/null +++ b/pkg/setting/datasources/test-configs/two-datasources-purge-others.yaml @@ -0,0 +1,10 @@ +purge_other_datasources: true +datasources: + - name: Graphite + type: graphite + access: proxy + url: http://localhost:8080 + - name: Prometheus + type: prometheus + access: proxy + url: http://localhost:9090 diff --git a/pkg/setting/datasources/test-configs/two-datasources.yaml b/pkg/setting/datasources/test-configs/two-datasources.yaml new file mode 100644 index 00000000000..9105f1ba354 --- /dev/null +++ b/pkg/setting/datasources/test-configs/two-datasources.yaml @@ -0,0 +1,10 @@ +purge_other_datasources: false +datasources: + - name: Graphite + type: graphite + access: proxy + url: http://localhost:8080 + - name: Prometheus + type: prometheus + access: proxy + url: http://localhost:9090 diff --git a/pkg/setting/datasources/test-configs/zero-datasources.yaml b/pkg/setting/datasources/test-configs/zero-datasources.yaml new file mode 100644 index 00000000000..47d185e5f1e --- /dev/null +++ b/pkg/setting/datasources/test-configs/zero-datasources.yaml @@ -0,0 +1,2 @@ +purge_other_datasources: false +datasources: diff --git a/pkg/setting/datasources/types.go b/pkg/setting/datasources/types.go index 577fc49469f..c708c23b551 100644 --- a/pkg/setting/datasources/types.go +++ b/pkg/setting/datasources/types.go @@ -3,25 +3,29 @@ package datasources import "github.com/grafana/grafana/pkg/models" import "github.com/grafana/grafana/pkg/components/simplejson" -type DataSourceFromConfig struct { - Id int64 - OrgId int64 - Version int +type DatasourcesAsConfig struct { + PurgeOtherDatasources bool `json:"purge_other_datasources" yaml:"purge_other_datasources"` + Datasources []DataSourceFromConfig `json:"datasources" yaml:"datasources"` +} - Name string - Type string - Access string - Url string - Password string - User string - Database string - BasicAuth bool - BasicAuthUser string - BasicAuthPassword string - WithCredentials bool - IsDefault bool - JsonData string - SecureJsonData map[string]string +type DataSourceFromConfig struct { + OrgId int64 `json:"org_id" yaml:"org_id"` + Version int `json:"version" yaml:"version"` + + Name string `json:"name" yaml:"name"` + Type string `json:"type" yaml:"type"` + Access string `json:"access" yaml:"access"` + Url string `json:"url" yaml:"url"` + Password string `json:"password" yaml:"password"` + User string `json:"user" yaml:"user"` + Database string `json:"database" yaml:"database"` + BasicAuth bool `json:"basic_auth" yaml:"basic_auth"` + BasicAuthUser string `json:"basic_auth_user" yaml:"basic_auth_user"` + BasicAuthPassword string `json:"basic_auth_password" yaml:"basic_auth_password"` + WithCredentials bool `json:"with_credentials" yaml:"with_credentials"` + IsDefault bool `json:"is_default" yaml:"is_default"` + JsonData string `json:"json_data" yaml:"json_data"` + SecureJsonData map[string]string `json:"secure_json_data" yaml:"secure_json_data"` } func createInsertCommand(ds DataSourceFromConfig) *models.AddDataSourceCommand {