Skip to content

Commit

Permalink
List repository actions should not check branch existence (#1743)
Browse files Browse the repository at this point in the history
  • Loading branch information
nopcoder authored Apr 14, 2021
1 parent e4021e2 commit 88a159e
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 17 deletions.
12 changes: 0 additions & 12 deletions pkg/api/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1164,18 +1164,6 @@ func (c *Controller) ListRepositoryRuns(w http.ResponseWriter, r *http.Request,
return
}

branchID := StringValue(params.Branch)
if branchID != "" {
exists, err := c.Catalog.BranchExists(ctx, repository, branchID)
if handleAPIError(w, err) {
return
}
if !exists {
writeError(w, http.StatusNotFound, catalog.ErrBranchNotFound)
return
}
}

branch := StringValue(params.Branch)
commitID := StringValue(params.Commit)
runsIter, err := c.Actions.ListRunResults(ctx, repository, branch, commitID, paginationAfter(params.After))
Expand Down
99 changes: 98 additions & 1 deletion pkg/api/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import (
"math"
"mime/multipart"
"net/http"
"net/http/httptest"
"path/filepath"
"strconv"
"strings"
"testing"
"text/template"
"time"

"github.com/go-test/deep"
Expand All @@ -26,7 +28,6 @@ import (
)

const (
//timeout = 10 * time.Second
DefaultUserID = "example_user"
)

Expand Down Expand Up @@ -1285,3 +1286,99 @@ func TestController_SetupLakeFSHandler(t *testing.T) {
})
}
}

var listRepositoryRunsActionTemplate = template.Must(template.New("").Parse(`---
name: CommitAction
on:
pre-commit:
branches:
- "*"
hooks:
- id: hook1
type: webhook
properties:
url: {{.URL}}
`))

func TestController_ListRepositoryRuns(t *testing.T) {
clt, _ := setupClientWithAdmin(t, "")
ctx := context.Background()
httpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
defer httpServer.Close()
// create repository
resp, err := clt.CreateRepositoryWithResponse(ctx, &api.CreateRepositoryParams{}, api.CreateRepositoryJSONRequestBody{
DefaultBranch: api.StringPtr("master"),
Name: "repo9",
StorageNamespace: "mem://repo9",
})
verifyResponseOK(t, resp, err)
// upload action for pre-commit
var b bytes.Buffer
testutil.MustDo(t, "execute action template", listRepositoryRunsActionTemplate.Execute(&b, httpServer))
actionContent := b.String()
uploadResp, err := uploadObjectHelper(t, ctx, clt, "_lakefs_actions/pre_commit.yaml", strings.NewReader(actionContent), "repo9", "master")
verifyResponseOK(t, uploadResp, err)
// commit
respCommit, err := clt.CommitWithResponse(ctx, "repo9", "master", api.CommitJSONRequestBody{
Message: "pre-commit action",
})
verifyResponseOK(t, respCommit, err)
// work branch
branchResp, err := clt.CreateBranchWithResponse(ctx, "repo9", api.CreateBranchJSONRequestBody{
Name: "work",
Source: "master",
})
verifyResponseOK(t, branchResp, err)
// upload and commit content on branch
commitIDs := []string{respCommit.JSON201.Id}
const contentCount = 5
for i := 0; i < contentCount; i++ {
content := fmt.Sprintf("content-%d", i)
uploadResp, err := uploadObjectHelper(t, ctx, clt, content, strings.NewReader(content), "repo9", "work")
verifyResponseOK(t, uploadResp, err)
respCommit, err := clt.CommitWithResponse(ctx, "repo9", "work", api.CommitJSONRequestBody{Message: content})
verifyResponseOK(t, respCommit, err)
commitIDs = append(commitIDs, respCommit.JSON201.Id)
}

t.Run("total", func(t *testing.T) {
respList, err := clt.ListRepositoryRunsWithResponse(ctx, "repo9", &api.ListRepositoryRunsParams{
Amount: api.PaginationAmountPtr(100),
})
verifyResponseOK(t, respList, err)
runsCount := len(respList.JSON200.Results)
if runsCount != contentCount+1 {
t.Fatalf("ListRepositoryRuns() got %d results, expected %d+1", runsCount, contentCount)
}
})

t.Run("on branch", func(t *testing.T) {
respList, err := clt.ListRepositoryRunsWithResponse(ctx, "repo9", &api.ListRepositoryRunsParams{
Branch: api.StringPtr("work"),
Amount: api.PaginationAmountPtr(100),
})
verifyResponseOK(t, respList, err)
runsCount := len(respList.JSON200.Results)
if runsCount != contentCount {
t.Fatalf("ListRepositoryRuns() on `work` branch got %d results, expected %d", runsCount, contentCount)
}
})

t.Run("on deleted branch", func(t *testing.T) {
// delete work branch and list them again
delResp, err := clt.DeleteBranchWithResponse(ctx, "repo9", "work")
verifyResponseOK(t, delResp, err)

respList, err := clt.ListRepositoryRunsWithResponse(ctx, "repo9", &api.ListRepositoryRunsParams{
Branch: api.StringPtr("work"),
Amount: api.PaginationAmountPtr(100),
})
verifyResponseOK(t, respList, err)
runsCount := len(respList.JSON200.Results)
if runsCount != contentCount {
t.Fatalf("ListRepositoryRuns() got %d results, expected %d (after delete repository)", runsCount, contentCount)
}
})
}
7 changes: 3 additions & 4 deletions pkg/api/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ func setupHandler(t testing.TB, blockstoreType string, opts ...testutil.GetDBOpt
if blockstoreType == "" {
blockstoreType = mem.BlockstoreType
}
blockAdapter := testutil.NewBlockAdapterByType(t, &block.NoOpTranslator{}, blockstoreType)
viper.Set(config.BlockstoreTypeKey, mem.BlockstoreType)
cfg, err := config.NewConfig()
testutil.MustDo(t, "config", err)
Expand All @@ -90,7 +89,7 @@ func setupHandler(t testing.TB, blockstoreType string, opts ...testutil.GetDBOpt
actionsService := actions.NewService(
conn,
catalog.NewActionsSource(c),
catalog.NewActionsOutputWriter(blockAdapter),
catalog.NewActionsOutputWriter(c.BlockAdapter),
)
c.SetHooksHandler(actionsService)

Expand All @@ -109,7 +108,7 @@ func setupHandler(t testing.TB, blockstoreType string, opts ...testutil.GetDBOpt
handler := api.Serve(
c,
authService,
blockAdapter,
c.BlockAdapter,
meta,
migrator,
collector,
Expand All @@ -120,7 +119,7 @@ func setupHandler(t testing.TB, blockstoreType string, opts ...testutil.GetDBOpt
)

return handler, &dependencies{
blocks: blockAdapter,
blocks: c.BlockAdapter,
authService: authService,
catalog: c,
collector: collector,
Expand Down

0 comments on commit 88a159e

Please sign in to comment.