diff --git a/model/config.go b/model/config.go index 5707d605d..05aa587ac 100644 --- a/model/config.go +++ b/model/config.go @@ -16,18 +16,24 @@ package model // ConfigStore persists pipeline configuration to storage. type ConfigStore interface { - ConfigLoad(buildID int64) ([]*Config, error) - ConfigFind(repo *Repo, sha string) (*Config, error) + ConfigsForBuild(buildID int64) ([]*Config, error) + ConfigFindIdentical(repoID int64, sha string) (*Config, error) ConfigFindApproved(*Config) (bool, error) ConfigCreate(*Config) error + BuildConfigCreate(*BuildConfig) error } // Config represents a pipeline configuration. type Config struct { - ID int64 `json:"-" meddler:"config_id,pk"` - RepoID int64 `json:"-" meddler:"config_repo_id"` - BuildID int64 `json:"-" meddler:"config_build_id"` - Data string `json:"data" meddler:"config_data"` - Hash string `json:"hash" meddler:"config_hash"` - Name string `json:"name" meddler:"config_name"` + ID int64 `json:"-" meddler:"config_id,pk"` + RepoID int64 `json:"-" meddler:"config_repo_id"` + Data string `json:"data" meddler:"config_data"` + Hash string `json:"hash" meddler:"config_hash"` + Name string `json:"name" meddler:"config_name"` +} + +// BuildConfig is the n:n relation between Build and Config +type BuildConfig struct { + ConfigID int64 `json:"-" meddler:"config_id"` + BuildID int64 `json:"-" meddler:"build_id"` } diff --git a/server/build.go b/server/build.go index 6e5082d11..f45f41740 100644 --- a/server/build.go +++ b/server/build.go @@ -268,7 +268,7 @@ func PostApproval(c *gin.Context) { build.Reviewer = user.Login // fetch the build file from the database - configs, err := Config.Storage.Config.ConfigLoad(build.ID) + configs, err := Config.Storage.Config.ConfigsForBuild(build.ID) if err != nil { logrus.Errorf("failure to get build config for %s. %s", repo.FullName, err) c.AbortWithError(404, err) @@ -440,7 +440,7 @@ func PostBuild(c *gin.Context) { } // fetch the .drone.yml file from the database - configs, err := Config.Storage.Config.ConfigLoad(build.ID) + configs, err := Config.Storage.Config.ConfigsForBuild(build.ID) if err != nil { logrus.Errorf("failure to get build config for %s. %s", repo.FullName, err) c.AbortWithError(404, err) diff --git a/server/hook.go b/server/hook.go index a15f4a3c5..5b64ce844 100644 --- a/server/hook.go +++ b/server/hook.go @@ -149,14 +149,6 @@ func PostHook(c *gin.Context) { c.AbortWithError(404, err) return } - // persist the build config for historical correctness, restarts, etc - // conf, err := findOrPersistPipelineConfig(repo, remoteYamlConfig) - // if err != nil { - // logrus.Errorf("failure to find or persist build config for %s. %s", repo.FullName, err) - // c.AbortWithError(500, err) - // return - // } - // build.ConfigID = conf.ID // verify that pipeline can be built at all // parsedPipelineConfig, err := yaml.ParseString(conf.Data) @@ -186,6 +178,17 @@ func PostHook(c *gin.Context) { return } + // persist the build config for historical correctness, restarts, etc + for _, remoteYamlConfig := range remoteYamlConfigs { + conf, err := findOrPersistPipelineConfig(build, remoteYamlConfig.Data) + fmt.Println(conf) + if err != nil { + logrus.Errorf("failure to find or persist build config for %s. %s", repo.FullName, err) + c.AbortWithError(500, err) + return + } + } + c.JSON(200, build) if build.Status == model.StatusBlocked { @@ -262,25 +265,34 @@ func PostHook(c *gin.Context) { queueBuild(build, repo, buildItems) } -func findOrPersistPipelineConfig(repo *model.Repo, remoteYamlConfig []byte) (*model.Config, error) { +func findOrPersistPipelineConfig(build *model.Build, remoteYamlConfig []byte) (*model.Config, error) { sha := shasum(remoteYamlConfig) - conf, err := Config.Storage.Config.ConfigFind(repo, sha) + conf, err := Config.Storage.Config.ConfigFindIdentical(build.RepoID, sha) if err != nil { conf = &model.Config{ - RepoID: repo.ID, + RepoID: build.RepoID, Data: string(remoteYamlConfig), Hash: sha, } err = Config.Storage.Config.ConfigCreate(conf) if err != nil { // retry in case we receive two hooks at the same time - conf, err = Config.Storage.Config.ConfigFind(repo, sha) + conf, err = Config.Storage.Config.ConfigFindIdentical(build.RepoID, sha) if err != nil { return nil, err } } } + buildConfig := &model.BuildConfig{ + ConfigID: conf.ID, + BuildID: build.ID, + } + err = Config.Storage.Config.BuildConfigCreate(buildConfig) + if err != nil { + return nil, err + } + return conf, nil } diff --git a/store/datastore/config.go b/store/datastore/config.go index aea0a13e7..1be819bff 100644 --- a/store/datastore/config.go +++ b/store/datastore/config.go @@ -22,17 +22,17 @@ import ( "github.com/russross/meddler" ) -func (db *datastore) ConfigLoad(buildID int64) ([]*model.Config, error) { +func (db *datastore) ConfigsForBuild(buildID int64) ([]*model.Config, error) { stmt := sql.Lookup(db.driver, "config-find-id") var configs = []*model.Config{} err := meddler.QueryAll(db, &configs, stmt, buildID) return configs, err } -func (db *datastore) ConfigFind(repo *model.Repo, hash string) (*model.Config, error) { +func (db *datastore) ConfigFindIdentical(repoID int64, hash string) (*model.Config, error) { stmt := sql.Lookup(db.driver, "config-find-repo-hash") conf := new(model.Config) - err := meddler.QueryRow(db, conf, stmt, repo.ID, hash) + err := meddler.QueryRow(db, conf, stmt, repoID, hash) return conf, err } @@ -51,3 +51,7 @@ func (db *datastore) ConfigFindApproved(config *model.Config) (bool, error) { func (db *datastore) ConfigCreate(config *model.Config) error { return meddler.Insert(db, "config", config) } + +func (db *datastore) BuildConfigCreate(buildConfig *model.BuildConfig) error { + return meddler.Insert(db, "build_config", buildConfig) +} diff --git a/store/datastore/config_test.go b/store/datastore/config_test.go index bc09a6293..52ec10570 100644 --- a/store/datastore/config_test.go +++ b/store/datastore/config_test.go @@ -33,20 +33,28 @@ func TestConfig(t *testing.T) { buildID = int64(1) ) - if err := s.ConfigCreate( - &model.Config{ - RepoID: 2, - BuildID: 1, - Data: data, - Hash: hash, - Name: "default", + config := &model.Config{ + RepoID: 2, + Data: data, + Hash: hash, + Name: "default", + } + if err := s.ConfigCreate(config); err != nil { + t.Errorf("Unexpected error: insert config: %s", err) + return + } + + if err := s.BuildConfigCreate( + &model.BuildConfig{ + ConfigID: config.ID, + BuildID: buildID, }, ); err != nil { t.Errorf("Unexpected error: insert config: %s", err) return } - config, err := s.ConfigFind(&model.Repo{ID: 2}, hash) + config, err := s.ConfigFindIdentical(int64(2), hash) if err != nil { t.Error(err) return @@ -57,9 +65,6 @@ func TestConfig(t *testing.T) { if got, want := config.RepoID, int64(2); got != want { t.Errorf("Want config repo id %d, got %d", want, got) } - if got, want := config.BuildID, buildID; got != want { - t.Errorf("Want config build id %d, got %d", want, got) - } if got, want := config.Data, data; got != want { t.Errorf("Want config data %s, got %s", want, got) } @@ -70,7 +75,7 @@ func TestConfig(t *testing.T) { t.Errorf("Want config name %s, got %s", want, got) } - loaded, err := s.ConfigLoad(buildID) + loaded, err := s.ConfigsForBuild(buildID) if err != nil { t.Errorf("Want config by id, got error %q", err) return @@ -120,13 +125,17 @@ func TestConfigApproved(t *testing.T) { s.CreateBuild(buildBlocked) s.CreateBuild(buildPending) conf := &model.Config{ - RepoID: repo.ID, - BuildID: buildBlocked.ID, - Data: data, - Hash: hash, + ID: int64(8), + RepoID: repo.ID, + Data: data, + Hash: hash, } - if err := s.ConfigCreate(conf); err != nil { - t.Errorf("Unexpected error: insert config: %s", err) + buildConfig := &model.BuildConfig{ + ConfigID: int64(8), + BuildID: buildBlocked.ID, + } + if err := s.BuildConfigCreate(buildConfig); err != nil { + t.Errorf("Unexpected error: insert build_config: %s", err) return } @@ -137,12 +146,16 @@ func TestConfigApproved(t *testing.T) { s.CreateBuild(buildRunning) conf2 := &model.Config{ - RepoID: repo.ID, - BuildID: buildRunning.ID, - Data: data, - Hash: "xxx", + ID: int64(9), + RepoID: repo.ID, + Data: data, + Hash: "xxx", } - if err := s.ConfigCreate(conf2); err != nil { + buildConfig2 := &model.BuildConfig{ + ConfigID: int64(9), + BuildID: buildRunning.ID, + } + if err := s.BuildConfigCreate(buildConfig2); err != nil { t.Errorf("Unexpected error: insert config: %s", err) return } diff --git a/store/datastore/ddl/sqlite/ddl_gen.go b/store/datastore/ddl/sqlite/ddl_gen.go index 136dd6d62..34cb2a6ac 100644 --- a/store/datastore/ddl/sqlite/ddl_gen.go +++ b/store/datastore/ddl/sqlite/ddl_gen.go @@ -161,12 +161,8 @@ var migrations = []struct { stmt: alterTableUpdateFileMeta, }, { - name: "alter-table-add-config-build-id", - stmt: alterTableAddConfigBuildId, - }, - { - name: "update-table-set-config-config-id", - stmt: updateTableSetConfigConfigId, + name: "create-table-build-config", + stmt: createTableBuildConfig, }, { name: "alter-table-add-config-name", @@ -641,15 +637,17 @@ UPDATE files SET ` // -// 019_add_column_config_build_id.sql +// 019_create_table_build_config.sql // -var alterTableAddConfigBuildId = ` -ALTER TABLE config ADD COLUMN config_build_id INTEGER -` - -var updateTableSetConfigConfigId = ` -UPDATE config SET config_build_id = (SELECT builds.build_id FROM builds WHERE builds.build_config_id = config.config_id) +var createTableBuildConfig = ` +CREATE TABLE IF NOT EXISTS build_config ( + config_id INTEGER NOT NULL +,build_id INTEGER NOT NULL +,PRIMARY KEY (config_id, build_id) +,FOREIGN KEY (config_id) REFERENCES config (config_id) +,FOREIGN KEY (build_id) REFERENCES builds (build_id) +); ` // diff --git a/store/datastore/ddl/sqlite/files/019_add_column_config_build_id.sql b/store/datastore/ddl/sqlite/files/019_add_column_config_build_id.sql deleted file mode 100644 index 469ea8677..000000000 --- a/store/datastore/ddl/sqlite/files/019_add_column_config_build_id.sql +++ /dev/null @@ -1,7 +0,0 @@ --- name: alter-table-add-config-build-id - -ALTER TABLE config ADD COLUMN config_build_id INTEGER - --- name: update-table-set-config-config-id - -UPDATE config SET config_build_id = (SELECT builds.build_id FROM builds WHERE builds.build_config_id = config.config_id) diff --git a/store/datastore/ddl/sqlite/files/019_create_table_build_config.sql b/store/datastore/ddl/sqlite/files/019_create_table_build_config.sql new file mode 100644 index 000000000..be5d58718 --- /dev/null +++ b/store/datastore/ddl/sqlite/files/019_create_table_build_config.sql @@ -0,0 +1,9 @@ +-- name: create-table-build-config + +CREATE TABLE IF NOT EXISTS build_config ( + config_id INTEGER NOT NULL +,build_id INTEGER NOT NULL +,PRIMARY KEY (config_id, build_id) +,FOREIGN KEY (config_id) REFERENCES config (config_id) +,FOREIGN KEY (build_id) REFERENCES builds (build_id) +); diff --git a/store/datastore/sql/sqlite/files/config.sql b/store/datastore/sql/sqlite/files/config.sql index 8a358300a..64f63e7a0 100644 --- a/store/datastore/sql/sqlite/files/config.sql +++ b/store/datastore/sql/sqlite/files/config.sql @@ -1,21 +1,20 @@ -- name: config-find-id SELECT - config_id + config.config_id ,config_repo_id -,config_build_id ,config_hash ,config_data ,config_name FROM config -WHERE config_build_id = ? +LEFT JOIN build_config ON config.config_id = build_config.config_id +WHERE build_config.build_id = ? -- name: config-find-repo-hash SELECT config_id ,config_repo_id -,config_build_id ,config_hash ,config_data ,config_name @@ -27,6 +26,10 @@ WHERE config_repo_id = ? SELECT build_id FROM builds WHERE build_repo_id = ? -AND build_id in (SELECT config_build_id FROM config WHERE config.config_id = ?) +AND build_id in ( + SELECT build_id + FROM build_config + WHERE build_config.config_id = ? + ) AND build_status NOT IN ('blocked', 'pending') LIMIT 1 diff --git a/store/datastore/sql/sqlite/sql_gen.go b/store/datastore/sql/sqlite/sql_gen.go index c0418612d..4d2798f9d 100644 --- a/store/datastore/sql/sqlite/sql_gen.go +++ b/store/datastore/sql/sqlite/sql_gen.go @@ -55,21 +55,20 @@ var index = map[string]string{ var configFindId = ` SELECT - config_id + config.config_id ,config_repo_id -,config_build_id ,config_hash ,config_data ,config_name FROM config -WHERE config_build_id = ? +LEFT JOIN build_config ON config.config_id = build_config.config_id +WHERE build_config.build_id = ? ` var configFindRepoHash = ` SELECT config_id ,config_repo_id -,config_build_id ,config_hash ,config_data ,config_name @@ -81,7 +80,11 @@ WHERE config_repo_id = ? var configFindApproved = ` SELECT build_id FROM builds WHERE build_repo_id = ? -AND build_id in (SELECT config_build_id FROM config WHERE config.config_id = ?) +AND build_id in ( + SELECT build_id + FROM build_config + WHERE build_config.config_id = ? + ) AND build_status NOT IN ('blocked', 'pending') LIMIT 1 ` diff --git a/store/store.go b/store/store.go index e1839cb4d..f0849fa2c 100644 --- a/store/store.go +++ b/store/store.go @@ -111,10 +111,11 @@ type Store interface { PermDelete(perm *model.Perm) error PermFlush(user *model.User, before int64) error - ConfigLoad(int64) ([]*model.Config, error) - ConfigFind(*model.Repo, string) (*model.Config, error) + ConfigsForBuild(buildID int64) ([]*model.Config, error) + ConfigFindIdentical(repoID int64, sha string) (*model.Config, error) ConfigFindApproved(*model.Config) (bool, error) ConfigCreate(*model.Config) error + BuildConfigCreate(*model.BuildConfig) error SenderFind(*model.Repo, string) (*model.Sender, error) SenderList(*model.Repo) ([]*model.Sender, error)