Skip to content

Commit

Permalink
condense item
Browse files Browse the repository at this point in the history
  • Loading branch information
ecrupper committed Mar 27, 2024
1 parent f35c68a commit f1dd103
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 49 deletions.
2 changes: 1 addition & 1 deletion api/build/approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func ApproveBuild(c *gin.Context) {
ctx,
queue.FromGinContext(c),
database.FromContext(c),
internal.ToItem(b, r, r.GetOwner()),
internal.ToItem(b, r),
b.GetHost(),
)

Expand Down
5 changes: 1 addition & 4 deletions api/build/compile_publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ func CompileAndPublish(
b := cfg.Build
baseErr := cfg.BaseErr

// send API call to capture repo owner
logrus.Debugf("capturing owner of repository %s", cfg.Repo.GetFullName())

// confirm current repo owner has at least write access to repo (needed for status update later)
_, err := scm.RepoAccess(c, u.GetName(), u.GetToken(), r.GetOrg(), r.GetName())
if err != nil {
Expand Down Expand Up @@ -440,7 +437,7 @@ func CompileAndPublish(
return false, nil, nil, retErr
}

return true, p, internal.ToItem(b, repo, u), nil
return true, p, internal.ToItem(b, repo), nil
}

// getPRNumberFromBuild is a helper function to
Expand Down
1 change: 0 additions & 1 deletion api/build/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ func UpdateBuild(c *gin.Context) {
b.GetStatus() == constants.StatusCanceled ||
b.GetStatus() == constants.StatusKilled ||
b.GetStatus() == constants.StatusError) && b.GetEvent() != constants.EventSchedule {

// send API call to set the status on the commit
err = scm.FromContext(c).Status(ctx, r.GetOwner(), b, r.GetOrg(), r.GetName())
if err != nil {
Expand Down
18 changes: 9 additions & 9 deletions api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func PostWebhook(c *gin.Context) {
)

// capture the build, repo, and user from the items
b, repo, u := item.Build, item.Repo, item.User
b, repo = item.Build, item.Repo

// set hook build_id to the generated build id
h.SetBuildID(b.GetID())
Expand Down Expand Up @@ -396,17 +396,17 @@ func PostWebhook(c *gin.Context) {

switch repo.GetApproveBuild() {
case constants.ApproveForkAlways:
err = gatekeepBuild(c, b, repo, u)
err = gatekeepBuild(c, b, repo)
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)
}

return
case constants.ApproveForkNoWrite:
// determine if build sender has write access to parent repo. If not, this call will result in an error
_, err = scm.FromContext(c).RepoAccess(ctx, b.GetSender(), u.GetToken(), r.GetOrg(), r.GetName())
_, err = scm.FromContext(c).RepoAccess(ctx, b.GetSender(), r.GetOwner().GetToken(), r.GetOrg(), r.GetName())
if err != nil {
err = gatekeepBuild(c, b, repo, u)
err = gatekeepBuild(c, b, repo)
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)
}
Expand All @@ -420,13 +420,13 @@ func PostWebhook(c *gin.Context) {
//
// NOTE: this call is cumbersome for repos with lots of contributors. Potential TODO: improve this if
// GitHub adds a single-contributor API endpoint.
contributor, err := scm.FromContext(c).RepoContributor(ctx, u, b.GetSender(), r.GetOrg(), r.GetName())
contributor, err := scm.FromContext(c).RepoContributor(ctx, r.GetOwner(), b.GetSender(), r.GetOrg(), r.GetName())
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)
}

if !contributor {
err = gatekeepBuild(c, b, repo, u)
err = gatekeepBuild(c, b, repo)
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)
}
Expand All @@ -443,7 +443,7 @@ func PostWebhook(c *gin.Context) {
}

// send API call to set the status on the commit
err = scm.FromContext(c).Status(ctx, u, b, repo.GetOrg(), repo.GetName())
err = scm.FromContext(c).Status(ctx, repo.GetOwner(), b, repo.GetOrg(), repo.GetName())
if err != nil {
logrus.Errorf("unable to set commit status for %s/%d: %v", repo.GetFullName(), b.GetNumber(), err)
}
Expand Down Expand Up @@ -677,7 +677,7 @@ func RenameRepository(ctx context.Context, h *library.Hook, r *types.Repo, c *gi

// gatekeepBuild is a helper function that will set the status of a build to 'pending approval' and
// send a status update to the SCM.
func gatekeepBuild(c *gin.Context, b *library.Build, r *types.Repo, u *library.User) error {
func gatekeepBuild(c *gin.Context, b *library.Build, r *types.Repo) error {
logrus.Debugf("fork PR build %s/%d waiting for approval", r.GetFullName(), b.GetNumber())
b.SetStatus(constants.StatusPendingApproval)

Expand All @@ -693,7 +693,7 @@ func gatekeepBuild(c *gin.Context, b *library.Build, r *types.Repo, u *library.U
}

// send API call to set the status on the commit
err = scm.FromContext(c).Status(c, u, b, r.GetOrg(), r.GetName())
err = scm.FromContext(c).Status(c, r.GetOwner(), b, r.GetOrg(), r.GetName())
if err != nil {
logrus.Errorf("unable to set commit status for %s/%d: %v", r.GetFullName(), b.GetNumber(), err)
}
Expand Down
10 changes: 7 additions & 3 deletions compiler/native/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1816,7 +1816,9 @@ func TestNative_Compile_Pipeline_Type(t *testing.T) {
}

compiler.WithMetadata(m)
compiler.WithRepo(&api.Repo{PipelineType: &tt.args.pipelineType})

pipelineType := tt.args.pipelineType
compiler.WithRepo(&api.Repo{PipelineType: &pipelineType})

got, _, err := compiler.Compile(yaml)
if err != nil {
Expand Down Expand Up @@ -3127,7 +3129,8 @@ func Test_Compile_Inline(t *testing.T) {
compiler.WithMetadata(m)

if tt.args.pipelineType != "" {
compiler.WithRepo(&api.Repo{PipelineType: &tt.args.pipelineType})
pipelineType := tt.args.pipelineType
compiler.WithRepo(&api.Repo{PipelineType: &pipelineType})
}

got, _, err := compiler.Compile(yaml)
Expand Down Expand Up @@ -3475,7 +3478,8 @@ func Test_CompileLite(t *testing.T) {

compiler.WithMetadata(m)
if tt.args.pipelineType != "" {
compiler.WithRepo(&api.Repo{PipelineType: &tt.args.pipelineType})
pipelineType := tt.args.pipelineType
compiler.WithRepo(&api.Repo{PipelineType: &pipelineType})
}

yaml, err := os.ReadFile(tt.args.file)
Expand Down
6 changes: 2 additions & 4 deletions internal/item.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,21 @@ import (
// upgrade or downgrade, so it can handle such stale data gracefully.
// For example, the worker could fail a stale build or ask the server to recompile it.
// This is not a public API and is unrelated to the version key in pipeline yaml.
const ItemVersion uint64 = 2
const ItemVersion uint64 = 3

// Item is the queue representation of an item to publish to the queue.
type Item struct {
Build *library.Build `json:"build"`
Repo *api.Repo `json:"repo"`
User *library.User `json:"user"`
// The 0-value is the implicit ItemVersion for queued Items that pre-date adding the field.
ItemVersion uint64 `json:"item_version"`
}

// ToItem creates a queue item from a build, repo and user.
func ToItem(b *library.Build, r *api.Repo, u *library.User) *Item {
func ToItem(b *library.Build, r *api.Repo) *Item {
return &Item{
Build: b,
Repo: r,
User: u,
ItemVersion: ItemVersion,
}
}
16 changes: 1 addition & 15 deletions internal/item_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,6 @@ func TestTypes_ToItem(t *testing.T) {
AllowTag: &booL,
AllowEvents: e,
}
u := &library.User{
ID: &num64,
Name: &str,
Token: &str,
Active: &booL,
Admin: &booL,
}
want := &Item{
Build: &library.Build{
ID: &num64,
Expand Down Expand Up @@ -126,18 +119,11 @@ func TestTypes_ToItem(t *testing.T) {
AllowTag: &booL,
AllowEvents: e,
},
User: &library.User{
ID: &num64,
Name: &str,
Token: &str,
Active: &booL,
Admin: &booL,
},
ItemVersion: ItemVersion,
}

// run test
got := ToItem(b, r, u)
got := ToItem(b, r)

if !reflect.DeepEqual(got, want) {
t.Errorf("ToItem is %v, want %v", got, want)
Expand Down
1 change: 0 additions & 1 deletion queue/redis/length_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func TestRedis_Length(t *testing.T) {
_item := &internal.Item{
Build: _build,
Repo: _repo,
User: _user,
}

// setup queue item
Expand Down
1 change: 0 additions & 1 deletion queue/redis/pop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ func TestRedis_Pop(t *testing.T) {
_item := &internal.Item{
Build: _build,
Repo: _repo,
User: _user,
}

var signed []byte
Expand Down
1 change: 0 additions & 1 deletion queue/redis/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ func TestRedis_Push(t *testing.T) {
_item := &internal.Item{
Build: _build,
Repo: _repo,
User: _user,
}

// setup queue item
Expand Down
9 changes: 0 additions & 9 deletions queue/redis/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,6 @@ var (
AllowDeploy: Bool(false),
AllowTag: Bool(false),
}

_user = &library.User{
ID: Int64(1),
Name: String("octocat"),
Token: nil,
Hash: nil,
Active: Bool(true),
Admin: Bool(false),
}
)

func TestRedis_New(t *testing.T) {
Expand Down

0 comments on commit f1dd103

Please sign in to comment.