Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build resolver for user's UUID #295

Merged
merged 8 commits into from
Jun 20, 2024
Merged

Build resolver for user's UUID #295

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
59ddbcc
create a build resolver based on user id
lizrabuya Jun 20, 2024
5051983
Merge branch '3.x' into sup-2176-resolve-builds-to-user
lizrabuya Jun 20, 2024
3d4c46a
various changes based on PR feedback
lizrabuya Jun 20, 2024
b2fdab4
fix fn call to pass branch name instead of build options
lizrabuya Jun 20, 2024
0877ff7
rename resolver filename for userid
lizrabuya Jun 20, 2024
e53b228
rename fn name for resolver for user id
lizrabuya Jun 20, 2024
67c3073
rename fn name for resolver for user id
lizrabuya Jun 20, 2024
26675e0
Merge branch '3.x' into sup-2176-resolve-builds-to-user
lizrabuya Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions internal/build/resolver/current_user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package resolver

import (
"context"

"github.com/buildkite/cli/v3/internal/build"
pipelineResolver "github.com/buildkite/cli/v3/internal/pipeline/resolver"
"github.com/buildkite/cli/v3/pkg/cmd/factory"
"github.com/buildkite/go-buildkite/v3/buildkite"
)

// ResolveBuildFromCurrentBranch Finds the most recent build for the branch in the current working directory
func ResolveBuildForCurrentUser(branch string, pipelineResolver pipelineResolver.PipelineResolverFn, f *factory.Factory) BuildResolverFn {
lizrabuya marked this conversation as resolved.
Show resolved Hide resolved
lizrabuya marked this conversation as resolved.
Show resolved Hide resolved
return func(ctx context.Context) (*build.Build, error) {
var user *buildkite.User

user, _, err := f.RestAPIClient.User.Get()
if err != nil {
return nil, err
}

opt := &buildkite.BuildsListOptions{
Creator: *user.Email,
Branch: []string{branch},
ListOptions: buildkite.ListOptions{
PerPage: 1,
},
}

return ResolveBuildForUser(ctx, *user.Email, opt, pipelineResolver, f)
}
}
145 changes: 145 additions & 0 deletions internal/build/resolver/current_user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
package resolver_test

import (
"bytes"
"context"
"io"
"net/http"
"os"
"strings"
"testing"

"github.com/buildkite/cli/v3/internal/build/resolver"
"github.com/buildkite/cli/v3/internal/config"
"github.com/buildkite/cli/v3/internal/pipeline"
"github.com/buildkite/cli/v3/pkg/cmd/factory"
"github.com/buildkite/go-buildkite/v3/buildkite"
"github.com/spf13/afero"
)

func TestResolveBuildForCurrentUser(t *testing.T) {
t.Parallel()

pipelineResolver := func(context.Context) (*pipeline.Pipeline, error) {
return &pipeline.Pipeline{
Name: "testing",
Org: "test org",
}, nil
}

t.Run("Errors if user cannot be found", func(t *testing.T) {
t.Parallel()

// mock a failed repsonse
transport := roundTripperFunc(func(r *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: http.StatusNotFound,
}, nil
})
client := &http.Client{Transport: transport}
f := &factory.Factory{
RestAPIClient: buildkite.NewClient(client),
}

r := resolver.ResolveBuildForCurrentUser("main", pipelineResolver, f)
_, err := r(context.Background())

if err == nil {
t.Fatal("Resolver should return error if user not found")
}
})

t.Run("Returns first build found", func(t *testing.T) {
t.Parallel()

in, _ := os.ReadFile("../../../fixtures/build.json")
callIndex := 0
responses := []http.Response{
{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(`{
"id": "abc123-4567-8910-...",
"graphql_id": "VXNlci0tLWU1N2ZiYTBmLWFiMTQtNGNjMC1iYjViLTY5NTc3NGZmYmZiZQ==",
"name": "John Smith",
"email": "john.smith@example.com",
"avatar_url": "https://www.gravatar.com/avatar/abc123...",
"created_at": "2012-03-04T06:07:08.910Z"
}
`)),
},
{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(in)),
},
}
// mock a failed repsonse
transport := roundTripperFunc(func(r *http.Request) (*http.Response, error) {
resp := responses[callIndex]
callIndex++
return &resp, nil
})
client := &http.Client{Transport: transport}
fs := afero.NewMemMapFs()
f := &factory.Factory{
RestAPIClient: buildkite.NewClient(client),
Config: config.New(fs, nil),
}

r := resolver.ResolveBuildForCurrentUser("main", pipelineResolver, f)
build, err := r(context.Background())
if err != nil {
t.Fatal(err)
}

if build.BuildNumber != 584 {
t.Fatalf("Expected build 584, got %d", build.BuildNumber)
}
})

t.Run("Errors if no matching builds found", func(t *testing.T) {
t.Parallel()

callIndex := 0
responses := []http.Response{
{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(`{
"id": "abc123-4567-8910-...",
"graphql_id": "VXNlci0tLWU1N2ZiYTBmLWFiMTQtNGNjMC1iYjViLTY5NTc3NGZmYmZiZQ==",
"name": "John Smith",
"email": "john.smith@example.com",
"avatar_url": "https://www.gravatar.com/avatar/abc123...",
"created_at": "2012-03-04T06:07:08.910Z"
}
`)),
},
{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader("[]")),
},
}
// mock a failed repsonse
transport := roundTripperFunc(func(r *http.Request) (*http.Response, error) {
resp := responses[callIndex]
callIndex++
return &resp, nil
})
client := &http.Client{Transport: transport}
fs := afero.NewMemMapFs()
f := &factory.Factory{
RestAPIClient: buildkite.NewClient(client),
Config: config.New(fs, nil),
}

r := resolver.ResolveBuildForCurrentUser("main", pipelineResolver, f)
build, err := r(context.Background())

if err == nil {
t.Fatal("Should return an error when no build is found")
}

if build != nil {
t.Fatal("Expected no build to be found")
}
})
}
24 changes: 24 additions & 0 deletions internal/build/resolver/current_userid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package resolver

import (
"context"

"github.com/buildkite/cli/v3/internal/build"
pipelineResolver "github.com/buildkite/cli/v3/internal/pipeline/resolver"
"github.com/buildkite/cli/v3/pkg/cmd/factory"
"github.com/buildkite/go-buildkite/v3/buildkite"
)

func ResolveBuildFromCurrentUserId(uuid string, pipelineResolver pipelineResolver.PipelineResolverFn, f *factory.Factory) BuildResolverFn {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessarily the current users ID. It could be any user so we should remove that from the name

return func(ctx context.Context) (*build.Build, error) {

opt := &buildkite.BuildsListOptions{
Creator: uuid,
ListOptions: buildkite.ListOptions{
PerPage: 1,
},
}

return ResolveBuildForUser(ctx, uuid, opt, pipelineResolver, f)
}
}
99 changes: 99 additions & 0 deletions internal/build/resolver/current_userid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package resolver_test

import (
"bytes"
"context"
"io"
"net/http"
"os"
"strings"
"testing"

"github.com/buildkite/cli/v3/internal/build/resolver"
"github.com/buildkite/cli/v3/internal/config"
"github.com/buildkite/cli/v3/internal/pipeline"
"github.com/buildkite/cli/v3/pkg/cmd/factory"
"github.com/buildkite/go-buildkite/v3/buildkite"
"github.com/spf13/afero"
)

func TestResolveBuildFromUserUUID(t *testing.T) {
t.Parallel()

pipelineResolver := func(context.Context) (*pipeline.Pipeline, error) {
return &pipeline.Pipeline{
Name: "testing",
Org: "test org",
}, nil
}

t.Run("Errors when user id is not a member of the organization", func(t *testing.T) {
t.Parallel()
// mock a failed repsonse
transport := roundTripperFunc(func(r *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: http.StatusNotFound,
}, nil
})
client := &http.Client{Transport: transport}
fs := afero.NewMemMapFs()
f := &factory.Factory{
RestAPIClient: buildkite.NewClient(client),
Config: config.New(fs, nil),
}

r := resolver.ResolveBuildFromCurrentUserId("1234", pipelineResolver, f)
_, err := r(context.Background())

if err == nil {
t.Fatal("Resolver should return error if user not found")
}
})

t.Run("Returns first build found", func(t *testing.T) {
t.Parallel()

in, _ := os.ReadFile("../../../fixtures/build.json")
callIndex := 0
responses := []http.Response{
{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(`[{
"id": "abc123-4567-8910-...",
"number": 584,
"creator": {
"id": "0183c4e6-c88c-xxxx-b15e-7801077a9181",
"graphql_id": "VXNlci0tLTAxODNjNGU2LWM4OGxxxxxxxxxiMTVlLTc4MDEwNzdhOTE4MQ=="
}
}]`)),
},
{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader(in)),
},
}
// mock a failed repsonse
transport := roundTripperFunc(func(r *http.Request) (*http.Response, error) {
resp := responses[callIndex]
callIndex++
return &resp, nil
})

client := &http.Client{Transport: transport}
fs := afero.NewMemMapFs()
f := &factory.Factory{
RestAPIClient: buildkite.NewClient(client),
Config: config.New(fs, nil),
}

r := resolver.ResolveBuildFromCurrentUserId("0183c4e6-c88c-xxxx-b15e-7801077a9181", pipelineResolver, f)
build, err := r(context.Background())
if err != nil {
t.Fatal(err)
}

if build.BuildNumber != 584 {
t.Fatalf("Expected build 584, got %d", build.BuildNumber)
}
})
}
82 changes: 34 additions & 48 deletions internal/build/resolver/user_builds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,57 +12,43 @@ import (
"golang.org/x/sync/errgroup"
)

// ResolveBuildFromCurrentBranch Finds the most recent build for the branch in the current working directory
func ResolveBuildForCurrentUser(branch string, pipelineResolver pipelineResolver.PipelineResolverFn, f *factory.Factory) BuildResolverFn {
return func(ctx context.Context) (*build.Build, error) {
var pipeline *pipeline.Pipeline
var user *buildkite.User
// ResolveBuildForUser Finds the most recent build for the user based on the provided options
func ResolveBuildForUser(ctx context.Context, userInfo string, opt *buildkite.BuildsListOptions, pipelineResolver pipelineResolver.PipelineResolverFn, f *factory.Factory) (*build.Build, error) {
lizrabuya marked this conversation as resolved.
Show resolved Hide resolved

// use an errgroup so a few API calls can be done in parallel
// and then we check for any errors that occurred
g, _ := errgroup.WithContext(ctx)
g.Go(func() error {
p, e := pipelineResolver(ctx)
if p != nil {
pipeline = p
}
return e
})
g.Go(func() error {
u, _, e := f.RestAPIClient.User.Get()
if u != nil {
user = u
}
return e
})
err := g.Wait()
if err != nil {
return nil, err
}
if pipeline == nil {
return nil, fmt.Errorf("failed to resolve a pipeline to query builds on.")
}
var pipeline *pipeline.Pipeline

builds, _, err := f.RestAPIClient.Builds.ListByPipeline(f.Config.OrganizationSlug(), pipeline.Name, &buildkite.BuildsListOptions{
Creator: *user.Email,
Branch: []string{branch},
ListOptions: buildkite.ListOptions{
PerPage: 1,
},
})
if err != nil {
return nil, err
}
if len(builds) == 0 {
// we error here because this resolver is explicitly used so any case where it doesn't resolve a build is a
// problem
return nil, fmt.Errorf("failed to find a build for current user (email: %s)", *user.Email)
// use an errgroup so a few API calls can be done in parallel
// and then we check for any errors that occurred
g, _ := errgroup.WithContext(ctx)
g.Go(func() error {
p, e := pipelineResolver(ctx)
if p != nil {
pipeline = p
}
return e
})
err := g.Wait()
if err != nil {
return nil, err
}
lizrabuya marked this conversation as resolved.
Show resolved Hide resolved
if pipeline == nil {
return nil, fmt.Errorf("failed to resolve a pipeline to query builds on")
}

builds, _, err := f.RestAPIClient.Builds.ListByPipeline(f.Config.OrganizationSlug(), pipeline.Name, opt)

return &build.Build{
Organization: f.Config.OrganizationSlug(),
Pipeline: pipeline.Name,
BuildNumber: *builds[0].Number,
}, nil
if err != nil {
return nil, err
}
if len(builds) == 0 {
// we error here because this resolver is explicitly used so any case where it doesn't resolve a build is a
// problem
return nil, fmt.Errorf("failed to find a build for current user (%s)", userInfo)
}
Comment on lines +40 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this applies now with these other resolvers? I suppose if the user doesn't have any builds its not really an error, we could just show a helpful message without it being an error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess thats right, and for consistency too as we do the same for pipelines resolver. I'll update this to not error when no builds are found

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually its probably more nuanced than that. the --user resolver might want to error if that user doesnt exist vs. if the user does exist but doesnt have any builds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jradtilbrook , i will raise a different PR to implement this as it will have to be the same behavior for all build resolvers and ensure that this is handled as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the user does not exist, or is invalid the api returns an error, which is also returned by the resolver. So that should be covered.


return &build.Build{
Organization: f.Config.OrganizationSlug(),
Pipeline: pipeline.Name,
BuildNumber: *builds[0].Number,
}, nil
}
Loading