Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions database/mock/store.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions database/query/entities.sql
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ WHERE entity_instances.entity_type = $1
ORDER BY entity_instances.id
LIMIT sqlc.arg('limit')::bigint;

-- ListEntitiesByTypePaginated retrieves entities of a given type for a project/provider
-- with cursor-based pagination. Pass a null (all-zeros) UUID as cursor for the first page.
-- The query over-fetches one row to determine if there are more results.

-- name: ListEntitiesByTypePaginated :many
SELECT * FROM entity_instances
WHERE entity_type = sqlc.arg(entity_type)
AND provider_id = sqlc.arg(provider_id)
AND project_id = ANY(sqlc.arg(projects)::uuid[])
AND id > sqlc.arg(cursor)::uuid
ORDER BY id
LIMIT (sqlc.arg(page_limit)::bigint + 1);

-- EntityExistsAfterID checks if any entity of a given type exists after a cursor ID.

-- name: EntityExistsAfterID :one
Expand Down
25 changes: 17 additions & 8 deletions internal/controlplane/handlers_repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,11 @@ func (s *Server) repoCreateInfoFromUpstreamEntityRef(

// ListRepositories returns a list of repositories for a given project
// This function will typically be called by the client to get a list of
// repositories that are registered present in the minder database
// repositories that are registered present in the minder database.
// Pagination is cursor-based: pass an empty cursor for the first page,
// then use the returned cursor for subsequent pages.
func (s *Server) ListRepositories(ctx context.Context,
_ *pb.ListRepositoriesRequest) (*pb.ListRepositoriesResponse, error) {
in *pb.ListRepositoriesRequest) (*pb.ListRepositoriesResponse, error) {
entityCtx := engcontext.EntityFromContext(ctx)
projectID := entityCtx.Project.ID
providerName := entityCtx.Provider.Name
Expand All @@ -154,8 +156,18 @@ func (s *Server) ListRepositories(ctx context.Context,
return nil, status.Errorf(codes.Internal, "cannot get provider: %v", err)
}

// Fetch repositories using the service
repoEntities, err := s.repos.ListRepositories(ctx, projectID, provider.ID)
// Parse cursor from request (base64-encoded UUID)
cursorStr := in.GetCursor()
Comment on lines +159 to +160

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This says it is a base64-encoded UUID, but there's no parsing being done here. In fact, it doesn't seem like we need the temporary cursorStr at all -- it's only used once and we could just put it inline there.


// Use request limit if provided, otherwise default
limit := in.GetLimit()
if limit <= 0 {
limit = 100
}

// Fetch repositories using the paginated service method
repoEntities, nextCursor, err := s.repos.ListRepositoriesPaginated(
ctx, projectID, provider.ID, cursorStr, limit)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -191,11 +203,8 @@ func (s *Server) ListRepositories(ctx context.Context,
results = append(results, pbRepo)
}

// TODO: Implement cursor-based pagination using entity IDs
// For now, return all results without pagination

resp.Results = results
resp.Cursor = ""
resp.Cursor = nextCursor

return &resp, nil
}
Expand Down
13 changes: 8 additions & 5 deletions internal/controlplane/handlers_repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,12 @@ func TestServer_ListRepositories(t *testing.T) {
{
Name: "List repositories succeeds with multiple results",
RepoServiceSetup: rf.NewRepoService(
rf.WithSuccessfulListRepositories(
simpleDbRepository(repoName, remoteRepoId),
simpleDbRepository(repoName2, remoteRepoId2),
rf.WithSuccessfulListRepositoriesPaginated(
[]*models.EntityWithProperties{
simpleDbRepository(repoName, remoteRepoId),
simpleDbRepository(repoName2, remoteRepoId2),
},
"",
),
),
ProviderSetup: func(ctrl *gomock.Controller, mgr *mockmanager.MockProviderManager) {
Expand Down Expand Up @@ -246,14 +249,14 @@ func TestServer_ListRepositories(t *testing.T) {
{
Name: "List repositories succeeds with empty results",
RepoServiceSetup: rf.NewRepoService(
rf.WithSuccessfulListRepositories(),
rf.WithSuccessfulListRepositoriesPaginated(nil, ""),
),
ExpectedResults: nil,
},
{
Name: "List repositories fails when repo service returns error",
RepoServiceSetup: rf.NewRepoService(
rf.WithFailedListRepositories(errDefault),
rf.WithFailedListRepositoriesPaginated(errDefault),
),
ExpectedError: errDefault.Error(),
},
Expand Down
59 changes: 59 additions & 0 deletions internal/db/entities.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions internal/db/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions internal/repositories/mock/fixtures/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,34 @@ func WithFailedListRepositories(err error) func(RepoServiceMock) {
Return(nil, err).AnyTimes()
}
}

func WithSuccessfulListRepositoriesPaginated(
repositories []*models.EntityWithProperties,
nextCursor string,
) func(RepoServiceMock) {
return func(mock RepoServiceMock) {
mock.EXPECT().
ListRepositoriesPaginated(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).
Return(repositories, nextCursor, nil).AnyTimes()
}
Comment on lines +154 to +156

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

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

This fixture always returns uuid.Nil as nextCursor, so tests can’t cover the new behavior of returning a non-empty cursor and fetching subsequent pages. Consider accepting nextCursor as an argument and adding a test that asserts ListRepositoriesResponse.Cursor is set.

Copilot uses AI. Check for mistakes.
}

func WithFailedListRepositoriesPaginated(err error) func(RepoServiceMock) {
return func(mock RepoServiceMock) {
mock.EXPECT().
ListRepositoriesPaginated(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).
Return(nil, "", err).AnyTimes()
}
}
16 changes: 16 additions & 0 deletions internal/repositories/mock/service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

117 changes: 117 additions & 0 deletions internal/repositories/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package repositories
import (
"context"
"database/sql"
"encoding/base64"
"errors"
"fmt"

Expand Down Expand Up @@ -70,6 +71,16 @@ type RepositoryService interface {
providerID uuid.UUID,
) ([]*models.EntityWithProperties, error)

// ListRepositoriesPaginated retrieves repositories with cursor-based pagination.
// If cursor is empty, returns the first page. limit defaults to 100.
ListRepositoriesPaginated(
ctx context.Context,
projectID uuid.UUID,
providerID uuid.UUID,
cursor string,
limit int64,
) ([]*models.EntityWithProperties, string, error)

// GetRepositoryById retrieves a repository by its ID and project.
GetRepositoryById(ctx context.Context, repositoryID uuid.UUID, projectID uuid.UUID) (*pb.Repository, error)
// GetRepositoryByName retrieves a repository by its name, owner, project and provider (if specified).
Expand Down Expand Up @@ -210,6 +221,112 @@ func (r *repositoryService) ListRepositories(
return ents, nil
}

// DefaultListRepositoriesPageLimit is the default (and maximum) page size for
// ListRepositoriesPaginated when the caller does not request a specific value
// or requests one above the cap.
const DefaultListRepositoriesPageLimit = 100

func (r *repositoryService) ListRepositoriesPaginated(
ctx context.Context,
projectID uuid.UUID,
providerID uuid.UUID,
cursorStr string,
limit int64,
) (ents []*models.EntityWithProperties, nextCursor string, outErr error) {
if limit <= 0 || limit > DefaultListRepositoriesPageLimit {
limit = DefaultListRepositoriesPageLimit
}
Comment on lines +236 to +238

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you remove the upper check on limit here, you could remove ListRepositories and pass in either MAX_INT or some sentinel value (think -1) on the call from fetchRepositoriesForProvider in handlers_repositories.go. That would allow removing ListRepositories (without pagination) from the codebase altogether.


cursor, err := decodeCursor(cursorStr)
if err != nil {
return nil, "", err
}

tx, err := r.store.BeginTransaction()
if err != nil {
return nil, "", fmt.Errorf("error starting transaction: %w", err)
}

defer func() {
if outErr != nil {
if rbErr := tx.Rollback(); rbErr != nil && !errors.Is(rbErr, sql.ErrTxDone) {
zerolog.Ctx(ctx).Error().Err(rbErr).Msg("error rolling back transaction")
}
}
}()

qtx := r.store.GetQuerierWithTransaction(tx)

repoEnts, err := qtx.ListEntitiesByTypePaginated(ctx, db.ListEntitiesByTypePaginatedParams{
EntityType: db.EntitiesRepository,
ProviderID: providerID,
Projects: []uuid.UUID{projectID},
Cursor: cursor,
PageLimit: limit,
})
if err != nil {
return nil, "", fmt.Errorf("error fetching repositories: %w", err)
}

hasMore := int64(len(repoEnts)) > limit
if hasMore {
repoEnts = repoEnts[:limit]
}

ents = make([]*models.EntityWithProperties, 0, len(repoEnts))
var lastID uuid.UUID
for _, ent := range repoEnts {
ewp, err := r.hydrateEntityProperties(ctx, qtx, ent.ID)
if err != nil {
return nil, "", err
}
ents = append(ents, ewp)
lastID = ent.ID

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rather than setting lastID on every pass through the loop, why not use lastID := ents[len(ents)-1].ID?

}

if err := tx.Commit(); err != nil {
return nil, "", fmt.Errorf("error committing transaction: %w", err)
}

if hasMore && lastID != uuid.Nil {
nextCursor = base64.StdEncoding.EncodeToString(lastID[:])
}

return ents, nextCursor, nil
}

func (r *repositoryService) hydrateEntityProperties(
ctx context.Context,
qtx db.ExtendQuerier,
entityID uuid.UUID,
) (*models.EntityWithProperties, error) {
ewp, err := r.propSvc.EntityWithPropertiesByID(ctx, entityID,
service.CallBuilder().WithStoreOrTransaction(qtx))
if err != nil {
return nil, fmt.Errorf("error fetching properties for repository: %w", err)
}
if err := r.propSvc.RetrieveAllPropertiesForEntity(ctx, ewp, r.providerManager,
service.ReadBuilder().WithStoreOrTransaction(qtx).TolerateStaleData()); err != nil {
return nil, fmt.Errorf("error fetching properties for repository: %w", err)
}
return ewp, nil
}

func decodeCursor(cursorStr string) (uuid.UUID, error) {
if cursorStr == "" {
return uuid.Nil, nil
}
Comment on lines +316 to +318

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this needs a comment -- "empty string means start at the beginning" or suchlike.

cursorBytes, err := base64.StdEncoding.DecodeString(cursorStr)
if err != nil {
return uuid.Nil, fmt.Errorf("invalid cursor format: %w", err)
}
cursor, err := uuid.FromBytes(cursorBytes)
if err != nil {
return uuid.Nil, fmt.Errorf("invalid cursor format: %w", err)
}
return cursor, nil
}

func (r *repositoryService) GetRepositoryById(
ctx context.Context,
repositoryID uuid.UUID,
Expand Down
Loading