From c4f490018810bada7740b005d0e56099f4557dac Mon Sep 17 00:00:00 2001 From: ecrupper Date: Thu, 6 Apr 2023 15:25:35 -0500 Subject: [PATCH 1/2] enhance(repo): add repo topics to repo object + build environment --- constants/limit.go | 3 ++ database/repo.go | 22 +++++++++++++ database/repo_test.go | 23 +++++++++++++ library/repo.go | 77 ++++++++++++++++++++++++++++++------------- library/repo_test.go | 13 ++++++++ 5 files changed, 115 insertions(+), 23 deletions(-) diff --git a/constants/limit.go b/constants/limit.go index 7cb3ece6..90f1d003 100644 --- a/constants/limit.go +++ b/constants/limit.go @@ -26,4 +26,7 @@ const ( // FavoritesMaxSize defines the maximum size in characters for user favorites. FavoritesMaxSize = 5000 + + // TopicsMaxSize defines the maximum size in characters for repo topics. Ex: GitHub has a 20-topic, 50-char limit. + TopicsMaxSize = 1020 ) diff --git a/database/repo.go b/database/repo.go index df8fd48a..cac284ad 100644 --- a/database/repo.go +++ b/database/repo.go @@ -9,7 +9,9 @@ import ( "encoding/base64" "errors" + "github.com/go-vela/types/constants" "github.com/go-vela/types/library" + "github.com/lib/pq" ) var ( @@ -36,6 +38,10 @@ var ( // ErrEmptyRepoVisibility defines the error type when a // Repo type has an empty Visibility field provided. ErrEmptyRepoVisibility = errors.New("empty repo visibility provided") + + // ErrExceededTopicsLimit defines the error type when a + // User type has Topics field provided that exceeds the database limit. + ErrExceededTopicsLimit = errors.New("exceeded topics limit") ) // Repo is the database representation of a repo. @@ -49,6 +55,7 @@ type Repo struct { Link sql.NullString `sql:"link"` Clone sql.NullString `sql:"clone"` Branch sql.NullString `sql:"branch"` + Topics pq.StringArray `sql:"topics" gorm:"type:varchar(1020)"` BuildLimit sql.NullInt64 `sql:"build_limit"` Timeout sql.NullInt64 `sql:"timeout"` Counter sql.NullInt32 `sql:"counter"` @@ -210,6 +217,7 @@ func (r *Repo) ToLibrary() *library.Repo { repo.SetLink(r.Link.String) repo.SetClone(r.Clone.String) repo.SetBranch(r.Branch.String) + repo.SetTopics(r.Topics) repo.SetBuildLimit(r.BuildLimit.Int64) repo.SetTimeout(r.Timeout.Int64) repo.SetCounter(int(r.Counter.Int32)) @@ -261,6 +269,19 @@ func (r *Repo) Validate() error { return ErrEmptyRepoVisibility } + // calculate total size of favorites while sanitizing entries + total := 0 + + for i, t := range r.Topics { + r.Topics[i] = sanitize(t) + total += len(t) + } + + // verify the Favorites field is within the database constraints + if total > constants.TopicsMaxSize { + return ErrExceededTopicsLimit + } + // ensure that all Repo string fields // that can be returned as JSON are sanitized // to avoid unsafe HTML content @@ -289,6 +310,7 @@ func RepoFromLibrary(r *library.Repo) *Repo { Link: sql.NullString{String: r.GetLink(), Valid: true}, Clone: sql.NullString{String: r.GetClone(), Valid: true}, Branch: sql.NullString{String: r.GetBranch(), Valid: true}, + Topics: pq.StringArray(r.GetTopics()), BuildLimit: sql.NullInt64{Int64: r.GetBuildLimit(), Valid: true}, Timeout: sql.NullInt64{Int64: r.GetTimeout(), Valid: true}, Counter: sql.NullInt32{Int32: int32(r.GetCounter()), Valid: true}, diff --git a/database/repo_test.go b/database/repo_test.go index 5d2cd638..053b0516 100644 --- a/database/repo_test.go +++ b/database/repo_test.go @@ -164,6 +164,7 @@ func TestDatabase_Repo_ToLibrary(t *testing.T) { want.SetLink("https://github.com/github/octocat") want.SetClone("https://github.com/github/octocat.git") want.SetBranch("master") + want.SetTopics([]string{"cloud", "security"}) want.SetBuildLimit(10) want.SetTimeout(30) want.SetCounter(0) @@ -188,6 +189,14 @@ func TestDatabase_Repo_ToLibrary(t *testing.T) { } func TestDatabase_Repo_Validate(t *testing.T) { + // setup types + topics := []string{} + longTopic := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" + + for len(topics) < 21 { + topics = append(topics, longTopic) + } + // setup tests tests := []struct { failure bool @@ -263,6 +272,18 @@ func TestDatabase_Repo_Validate(t *testing.T) { FullName: sql.NullString{String: "github/octocat", Valid: true}, }, }, + { // topics exceed max size + failure: true, + repo: &Repo{ + ID: sql.NullInt64{Int64: 1, Valid: true}, + UserID: sql.NullInt64{Int64: 1, Valid: true}, + Hash: sql.NullString{String: "superSecretHash", Valid: true}, + Org: sql.NullString{String: "github", Valid: true}, + Name: sql.NullString{String: "octocat", Valid: true}, + FullName: sql.NullString{String: "github/octocat", Valid: true}, + Topics: topics, + }, + }, } // run tests @@ -296,6 +317,7 @@ func TestDatabase_RepoFromLibrary(t *testing.T) { r.SetLink("https://github.com/github/octocat") r.SetClone("https://github.com/github/octocat.git") r.SetBranch("master") + r.SetTopics([]string{"cloud", "security"}) r.SetBuildLimit(10) r.SetTimeout(30) r.SetCounter(0) @@ -334,6 +356,7 @@ func testRepo() *Repo { Link: sql.NullString{String: "https://github.com/github/octocat", Valid: true}, Clone: sql.NullString{String: "https://github.com/github/octocat.git", Valid: true}, Branch: sql.NullString{String: "master", Valid: true}, + Topics: []string{"cloud", "security"}, BuildLimit: sql.NullInt64{Int64: 10, Valid: true}, Timeout: sql.NullInt64{Int64: 30, Valid: true}, Counter: sql.NullInt32{Int32: 0, Valid: true}, diff --git a/library/repo.go b/library/repo.go index 3ac2752f..3526b2b7 100644 --- a/library/repo.go +++ b/library/repo.go @@ -6,35 +6,37 @@ package library import ( "fmt" + "strings" ) // Repo is the library representation of a repo. // // swagger:model Repo type Repo struct { - ID *int64 `json:"id,omitempty"` - UserID *int64 `json:"user_id,omitempty"` - Hash *string `json:"-"` - Org *string `json:"org,omitempty"` - Name *string `json:"name,omitempty"` - FullName *string `json:"full_name,omitempty"` - Link *string `json:"link,omitempty"` - Clone *string `json:"clone,omitempty"` - Branch *string `json:"branch,omitempty"` - BuildLimit *int64 `json:"build_limit,omitempty"` - Timeout *int64 `json:"timeout,omitempty"` - Counter *int `json:"counter,omitempty"` - Visibility *string `json:"visibility,omitempty"` - Private *bool `json:"private,omitempty"` - Trusted *bool `json:"trusted,omitempty"` - Active *bool `json:"active,omitempty"` - AllowPull *bool `json:"allow_pull,omitempty"` - AllowPush *bool `json:"allow_push,omitempty"` - AllowDeploy *bool `json:"allow_deploy,omitempty"` - AllowTag *bool `json:"allow_tag,omitempty"` - AllowComment *bool `json:"allow_comment,omitempty"` - PipelineType *string `json:"pipeline_type,omitempty"` - PreviousName *string `json:"previous_name,omitempty"` + ID *int64 `json:"id,omitempty"` + UserID *int64 `json:"user_id,omitempty"` + Hash *string `json:"-"` + Org *string `json:"org,omitempty"` + Name *string `json:"name,omitempty"` + FullName *string `json:"full_name,omitempty"` + Link *string `json:"link,omitempty"` + Clone *string `json:"clone,omitempty"` + Branch *string `json:"branch,omitempty"` + Topics *[]string `json:"topics,omitempty"` + BuildLimit *int64 `json:"build_limit,omitempty"` + Timeout *int64 `json:"timeout,omitempty"` + Counter *int `json:"counter,omitempty"` + Visibility *string `json:"visibility,omitempty"` + Private *bool `json:"private,omitempty"` + Trusted *bool `json:"trusted,omitempty"` + Active *bool `json:"active,omitempty"` + AllowPull *bool `json:"allow_pull,omitempty"` + AllowPush *bool `json:"allow_push,omitempty"` + AllowDeploy *bool `json:"allow_deploy,omitempty"` + AllowTag *bool `json:"allow_tag,omitempty"` + AllowComment *bool `json:"allow_comment,omitempty"` + PipelineType *string `json:"pipeline_type,omitempty"` + PreviousName *string `json:"previous_name,omitempty"` } // Environment returns a list of environment variables @@ -48,6 +50,7 @@ func (r *Repo) Environment() map[string]string { "VELA_REPO_ALLOW_PUSH": ToString(r.GetAllowPush()), "VELA_REPO_ALLOW_TAG": ToString(r.GetAllowTag()), "VELA_REPO_BRANCH": ToString(r.GetBranch()), + "VELA_REPO_TOPICS": strings.Join(r.GetTopics()[:], ","), "VELA_REPO_BUILD_LIMIT": ToString(r.GetBuildLimit()), "VELA_REPO_CLONE": ToString(r.GetClone()), "VELA_REPO_FULL_NAME": ToString(r.GetFullName()), @@ -197,6 +200,19 @@ func (r *Repo) GetBranch() string { return *r.Branch } +// GetTopics returns the Topics field. +// +// When the provided Repo type is nil, or the field within +// the type is nil, it returns the zero value for the field. +func (r *Repo) GetTopics() []string { + // return zero value if Repo type or Topics field is nil + if r == nil || r.Topics == nil { + return []string{} + } + + return *r.Topics +} + // GetBuildLimit returns the BuildLimit field. // // When the provided Repo type is nil, or the field within @@ -496,6 +512,19 @@ func (r *Repo) SetBranch(v string) { r.Branch = &v } +// SetTopics sets the Topics field. +// +// When the provided Repo type is nil, it +// will set nothing and immediately return. +func (r *Repo) SetTopics(v []string) { + // return if Repo type is nil + if r == nil { + return + } + + r.Topics = &v +} + // SetBuildLimit sets the BuildLimit field. // // When the provided Repo type is nil, it @@ -700,6 +729,7 @@ func (r *Repo) String() string { PreviousName: %s, Private: %t, Timeout: %d, + Topics: %s, Trusted: %t, UserID: %d Visibility: %s, @@ -723,6 +753,7 @@ func (r *Repo) String() string { r.GetPreviousName(), r.GetPrivate(), r.GetTimeout(), + r.GetTopics(), r.GetTrusted(), r.GetUserID(), r.GetVisibility(), diff --git a/library/repo_test.go b/library/repo_test.go index 25c10de9..a23a4274 100644 --- a/library/repo_test.go +++ b/library/repo_test.go @@ -20,6 +20,7 @@ func TestLibrary_Repo_Environment(t *testing.T) { "VELA_REPO_ALLOW_PUSH": "true", "VELA_REPO_ALLOW_TAG": "false", "VELA_REPO_BRANCH": "master", + "VELA_REPO_TOPICS": "cloud,security", "VELA_REPO_BUILD_LIMIT": "10", "VELA_REPO_CLONE": "https://github.com/github/octocat.git", "VELA_REPO_FULL_NAME": "github/octocat", @@ -111,6 +112,10 @@ func TestLibrary_Repo_Getters(t *testing.T) { t.Errorf("GetBranch is %v, want %v", test.repo.GetBranch(), test.want.GetBranch()) } + if !reflect.DeepEqual(test.repo.GetTopics(), test.want.GetTopics()) { + t.Errorf("GetTopics is %v, want %v", test.repo.GetTopics(), test.want.GetTopics()) + } + if test.repo.GetBuildLimit() != test.want.GetBuildLimit() { t.Errorf("GetBuildLimit is %v, want %v", test.repo.GetBuildLimit(), test.want.GetBuildLimit()) } @@ -195,6 +200,7 @@ func TestLibrary_Repo_Setters(t *testing.T) { test.repo.SetLink(test.want.GetLink()) test.repo.SetClone(test.want.GetClone()) test.repo.SetBranch(test.want.GetBranch()) + test.repo.SetTopics(test.want.GetTopics()) test.repo.SetBuildLimit(test.want.GetBuildLimit()) test.repo.SetTimeout(test.want.GetTimeout()) test.repo.SetCounter(test.want.GetCounter()) @@ -246,6 +252,10 @@ func TestLibrary_Repo_Setters(t *testing.T) { t.Errorf("SetBranch is %v, want %v", test.repo.GetBranch(), test.want.GetBranch()) } + if !reflect.DeepEqual(test.repo.GetTopics(), test.want.GetTopics()) { + t.Errorf("SetTopics is %v, want %v", test.repo.GetTopics(), test.want.GetTopics()) + } + if test.repo.GetBuildLimit() != test.want.GetBuildLimit() { t.Errorf("SetBuildLimit is %v, want %v", test.repo.GetBuildLimit(), test.want.GetBuildLimit()) } @@ -324,6 +334,7 @@ func TestLibrary_Repo_String(t *testing.T) { PreviousName: %s, Private: %t, Timeout: %d, + Topics: %s, Trusted: %t, UserID: %d Visibility: %s, @@ -347,6 +358,7 @@ func TestLibrary_Repo_String(t *testing.T) { r.GetPreviousName(), r.GetPrivate(), r.GetTimeout(), + r.GetTopics(), r.GetTrusted(), r.GetUserID(), r.GetVisibility(), @@ -372,6 +384,7 @@ func testRepo() *Repo { r.SetLink("https://github.com/github/octocat") r.SetClone("https://github.com/github/octocat.git") r.SetBranch("master") + r.SetTopics([]string{"cloud", "security"}) r.SetBuildLimit(10) r.SetTimeout(30) r.SetCounter(0) From 1a5e81f30a6ee551aa04f277d6a79227f181c0d6 Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Thu, 6 Apr 2023 17:04:01 -0600 Subject: [PATCH 2/2] Update database/repo.go Co-authored-by: David May <49894298+wass3rw3rk@users.noreply.github.com> --- database/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/repo.go b/database/repo.go index cac284ad..9935ccec 100644 --- a/database/repo.go +++ b/database/repo.go @@ -40,7 +40,7 @@ var ( ErrEmptyRepoVisibility = errors.New("empty repo visibility provided") // ErrExceededTopicsLimit defines the error type when a - // User type has Topics field provided that exceeds the database limit. + // Repo type has Topics field provided that exceeds the database limit. ErrExceededTopicsLimit = errors.New("exceeded topics limit") )