Skip to content

Commit

Permalink
feat: paginate tag list
Browse files Browse the repository at this point in the history
  • Loading branch information
jvallesm committed Mar 26, 2024
1 parent de1a185 commit 98087fe
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 25 deletions.
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ go-gen: ## Generate codes
.PHONY: unit-test
unit-test: ## Run unit test
@go test -v -race -coverpkg=./... -coverprofile=coverage.out ./...
@go tool cover -func=coverage.out
@go tool cover -html=coverage.out
@rm coverage.out
@cat coverage.out | grep -v "mock" > coverage.final.out
@go tool cover -func=coverage.final.out
@go tool cover -html=coverage.final.out
@rm coverage.out coverage.final.out

.PHONY: integration-test
integration-test: ## Run integration test
Expand Down
6 changes: 0 additions & 6 deletions pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,6 @@ import (
"gorm.io/gorm"
)

// DefaultPageSize is the default pagination page size when page size is not assigned
const DefaultPageSize = 10

// MaxPageSize is the maximum pagination page size if the assigned value is over this number
const MaxPageSize = 100

// Repository interface
type Repository interface {
}
Expand Down
42 changes: 40 additions & 2 deletions pkg/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import (
pb "github.com/instill-ai/protogen-go/artifact/artifact/v1alpha"
)

const (
defaultPageSize = 10
maxPageSize = 100
)

// Service implements the Artifact domain use cases.
type Service interface {
ListRepositoryTags(context.Context, *pb.ListRepositoryTagsRequest) (*pb.ListRepositoryTagsResponse, error)
Expand All @@ -33,6 +38,10 @@ func NewService(registryClient RegistryClient) Service {
// ListRepositoryTags fetches and paginates the tags of a repository in a
// remote distribution registry.
func (s *service) ListRepositoryTags(ctx context.Context, req *pb.ListRepositoryTagsRequest) (*pb.ListRepositoryTagsResponse, error) {
pageSize := s.pageSizeInRange(req.GetPageSize())
page := s.pageInRange(req.GetPage())
idx0, idx1 := page*pageSize, (page+1)*pageSize

_, repository, ok := strings.Cut(req.GetParent(), "repositories/")
if !ok {
return nil, fmt.Errorf("namespace error")
Expand All @@ -43,8 +52,17 @@ func (s *service) ListRepositoryTags(ctx context.Context, req *pb.ListRepository
return nil, err
}

tags := make([]*pb.RepositoryTag, 0, len(tagIDs))
for _, tagID := range tagIDs {
var paginatedIDs []string
switch {
case idx0 >= len(tagIDs):
case idx1 > len(tagIDs):
paginatedIDs = tagIDs[idx0:]
default:
paginatedIDs = tagIDs[idx0:idx1]
}

tags := make([]*pb.RepositoryTag, 0, len(paginatedIDs))
for _, tagID := range paginatedIDs {
tags = append(tags, &pb.RepositoryTag{
Id: tagID,
Name: tagName(repository, tagID),
Expand All @@ -57,3 +75,23 @@ func (s *service) ListRepositoryTags(ctx context.Context, req *pb.ListRepository
func tagName(repo, id string) string {
return fmt.Sprintf("repositories/%s/tags/%s", repo, id)
}

func (s *service) pageSizeInRange(pageSize int32) int {
if pageSize <= 0 {
return defaultPageSize
}

if pageSize > maxPageSize {
return maxPageSize
}

Check warning on line 86 in pkg/service/service.go

View check run for this annotation

Codecov / codecov/patch

pkg/service/service.go#L85-L86

Added lines #L85 - L86 were not covered by tests

return int(pageSize)
}

func (s *service) pageInRange(page int32) int {
if page <= 0 {
return 0
}

return int(page)
}
69 changes: 57 additions & 12 deletions pkg/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,25 @@ func TestService_ListRepositoryTags(t *testing.T) {
c := qt.New(t)
ctx := context.Background()

var pageSize int32 = 2
newReq := func(reqMod func(*artifactPB.ListRepositoryTagsRequest)) *artifactPB.ListRepositoryTagsRequest {
req := &artifactPB.ListRepositoryTagsRequest{
Parent: "repositories/" + repo,
Parent: "repositories/" + repo,
PageSize: &pageSize,
}
if reqMod != nil {
reqMod(req)
}
return req
}

tagIDs := []string{"v1", "v2"}
want := []*artifactPB.RepositoryTag{
{
Name: "repositories/krule-wombat/llava-34b/tags/v1",
Id: "v1",
},
{
Name: "repositories/krule-wombat/llava-34b/tags/v2",
Id: "v2",
},
tagIDs := []string{"1.0.0", "1.0.1", "1.1.0-beta", "1.1.0", "latest"}
want := make([]*artifactPB.RepositoryTag, len(tagIDs))
for i, tagID := range tagIDs {
want[i] = &artifactPB.RepositoryTag{
Name: "repositories/krule-wombat/llava-34b/tags/" + tagID,
Id: tagID,
}
}

testcases := []struct {
Expand All @@ -70,10 +69,56 @@ func TestService_ListRepositoryTags(t *testing.T) {
wantErr: "foo",
},
{
name: "ok",
name: "ok - no pagination",
in: func(req *artifactPB.ListRepositoryTagsRequest) { req.PageSize = nil },
registryTags: tagIDs,
want: want,
},
{
name: "ok - page -1",
in: func(req *artifactPB.ListRepositoryTagsRequest) {
var page int32 = -1
req.Page = &page
},

registryTags: tagIDs,
want: want[:2],
},
{
name: "ok - page 0",
registryTags: tagIDs,
want: want[:2],
},
{
name: "ok - page 1",
in: func(req *artifactPB.ListRepositoryTagsRequest) {
var page int32 = 1
req.Page = &page
},

registryTags: tagIDs,
want: want[2:4],
},
{
name: "ok - page 2",
in: func(req *artifactPB.ListRepositoryTagsRequest) {
var page int32 = 2
req.Page = &page
},

registryTags: tagIDs,
want: want[4:],
},
{
name: "ok - page 3",
in: func(req *artifactPB.ListRepositoryTagsRequest) {
var page int32 = 3
req.Page = &page
},

registryTags: tagIDs,
want: want[0:0],
},
}

for _, tc := range testcases {
Expand Down
6 changes: 4 additions & 2 deletions pkg/usage/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type usage struct {
version string
}

const maxPageSize = 100

// NewUsage initiates a usage instance
func NewUsage(ctx context.Context, r repository.Repository, mu mgmtPB.MgmtPrivateServiceClient, rc *redis.Client, usc usagePB.UsageServiceClient) Usage {
logger, _ := logger.GetZapLogger(ctx)
Expand Down Expand Up @@ -77,7 +79,7 @@ func (u *usage) RetrieveArtifactUsageData() interface{} {

// Roll over all users and update the metrics with the cached uuid
userPageToken := ""
userPageSizeMax := int32(repository.MaxPageSize)
userPageSizeMax := int32(maxPageSize)
for {
userResp, err := u.mgmtPrivateServiceClient.ListUsersAdmin(ctx, &mgmtPB.ListUsersAdminRequest{
PageSize: &userPageSizeMax,
Expand All @@ -102,7 +104,7 @@ func (u *usage) RetrieveArtifactUsageData() interface{} {

// Roll over all orgs and update the metrics with the cached uuid
orgPageToken := ""
orgPageSizeMax := int32(repository.MaxPageSize)
orgPageSizeMax := int32(maxPageSize)
for {
orgResp, err := u.mgmtPrivateServiceClient.ListOrganizationsAdmin(ctx, &mgmtPB.ListOrganizationsAdminRequest{
PageSize: &orgPageSizeMax,
Expand Down

0 comments on commit 98087fe

Please sign in to comment.