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.

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

feat(getallassets) : add support for include_fields in request to limit the response. #49

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ help: ##@help show this help
NAME="github.com/goto/compass"
VERSION=$(shell git describe --always --tags 2>/dev/null)
COVERFILE="/tmp/compass.coverprofile"
PROTON_COMMIT := "a6b2821e8ddd1127a63d3b376f860990d58931da"
PROTON_COMMIT := "0db56803e2f75591fe9d1cd461122ebcbaeda8c6"

TOOLS_MOD_DIR = ./tools
TOOLS_DIR = $(abspath ./.tools)
Expand Down
62 changes: 39 additions & 23 deletions core/asset/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,35 @@ import (
)

type Filter struct {
Types []Type
Services []string
Size int
Offset int
SortBy string `validate:"omitempty,oneof=name type service created_at updated_at"`
SortDirection string `validate:"omitempty,oneof=asc desc"`
QueryFields []string
Query string
Data map[string][]string
Types []Type
Services []string
Size int
Offset int
SortBy string `validate:"omitempty,oneof=name type service created_at updated_at"`
SortDirection string `validate:"omitempty,oneof=asc desc"`
QueryFields []string
Query string
Data map[string][]string
IncludeFields []string
IncludeUserInfo bool
}

func (f *Filter) Validate() error {
return validator.ValidateStruct(f)
}

type filterBuilder struct {
types string
services string
q string
qFields string
data map[string]string
size int
offset int
sortBy string
sortDirection string
types string
services string
q string
qFields string
data map[string]string
size int
offset int
sortBy string
sortDirection string
includeFields []string
includeUserInfo bool
}

func NewFilterBuilder() *filterBuilder {
Expand Down Expand Up @@ -83,13 +87,25 @@ func (fb *filterBuilder) SortDirection(sortDirection string) *filterBuilder {
return fb
}

func (fb *filterBuilder) IncludeFields(includeFields []string) *filterBuilder {
fb.includeFields = includeFields
return fb
}

func (fb *filterBuilder) IncludeUserInfo(includeUserInfo bool) *filterBuilder {
fb.includeUserInfo = includeUserInfo
return fb
}

func (fb *filterBuilder) Build() (Filter, error) {
flt := Filter{
Size: fb.size,
Offset: fb.offset,
SortBy: fb.sortBy,
SortDirection: fb.sortDirection,
Query: fb.q,
Size: fb.size,
Offset: fb.offset,
SortBy: fb.sortBy,
SortDirection: fb.sortDirection,
Query: fb.q,
IncludeFields: fb.includeFields,
IncludeUserInfo: fb.includeUserInfo,
}

if len(fb.data) != 0 {
Expand Down
2 changes: 2 additions & 0 deletions internal/server/v1beta1/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ func (server *APIServer) GetAllAssets(ctx context.Context, req *compassv1beta1.G
SortBy(req.GetSort()).
SortDirection(req.GetDirection()).
Data(req.GetData()).
IncludeFields(req.IncludeFields).
IncludeUserInfo(req.IncludeUserInfo).
Build()
if err != nil {
return nil, status.Error(codes.InvalidArgument, bodyParserErrorMsg(err))
Expand Down
1 change: 1 addition & 0 deletions internal/server/v1beta1/asset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func TestGetAllAssets(t *testing.T) {
{
Description: "should return status OK along with list of assets",
ExpectStatus: codes.OK,
Request: &compassv1beta1.GetAllAssetsRequest{},
Setup: func(ctx context.Context, as *mocks.AssetService, _ *mocks.UserService) {
as.EXPECT().GetAllAssets(ctx, asset.Filter{}, false).Return([]asset.Asset{
{ID: "testid-1"},
Expand Down
45 changes: 41 additions & 4 deletions internal/store/postgres/asset_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,57 @@ type AssetRepository struct {

// GetAll retrieves list of assets with filters
func (r *AssetRepository) GetAll(ctx context.Context, flt asset.Filter) ([]asset.Asset, error) {
builder := r.getAssetSQL().Offset(uint64(flt.Offset))
size := flt.Size
fields := flt.IncludeFields

var builder sq.SelectBuilder
if len(fields) == 0 {
builder = r.getAssetSQL().Offset(uint64(flt.Offset))
Comment on lines +29 to +33
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these changes necessary since search assets can also serve a similar purpose using ES?

Last time we came across changing these functionalities - we were aligning on enhancing the searchAssets API itself and consolidate both APIs into one. Context - raystack#201 (comment)

@anjali9791 @sudo-suhas

PS. search API also supports pagination now.

Choose a reason for hiding this comment

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

Thanks for bringing this up @bsushmith. In my initial discussion with @anjali9791 I missed bringing it up.

} else {
updatedFields := make([]string, len(fields))
ifIDNotIncluded := true
for i, field := range fields {
if strings.ToLower(field) == "id" {
ifIDNotIncluded = false
} else {
updatedFields[i] = "a." + field
}
}

if ifIDNotIncluded {
updatedFields = append(updatedFields, "a.id")
}

if size > 0 {
builder = r.getAssetSQL().Limit(uint64(size)).Offset(uint64(flt.Offset))
if flt.IncludeUserInfo {
userFields := []string{
"u.id as \"updated_by.id\"",
"u.uuid as \"updated_by.uuid\"",
"u.email as \"updated_by.email\"",
"u.provider as \"updated_by.provider\"",
"u.created_at as \"updated_by.created_at\"",
"u.updated_at as \"updated_by.updated_at\"",
}
updatedFields = append(updatedFields, userFields...)
}

builder = sq.Select(updatedFields...).From("assets as a").
LeftJoin("users u ON a.updated_by = u.id").
Offset(uint64(flt.Offset))
}

if flt.Size > 0 {
builder = builder.Limit(uint64(flt.Size))
}

builder = r.BuildFilterQuery(builder, flt)
builder = r.buildOrderQuery(builder, flt)

query, args, err := builder.PlaceholderFormat(sq.Dollar).ToSql()
if err != nil {
return nil, fmt.Errorf("error building query: %w", err)
}

var ams []*AssetModel

err = r.client.db.SelectContext(ctx, &ams, query, args...)
if err != nil {
return nil, fmt.Errorf("error getting asset list: %w", err)
Expand Down
46 changes: 46 additions & 0 deletions internal/store/postgres/asset_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,52 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
}
})

r.Run("should return when user has selected some fields", func() {
results, err := r.repository.GetAll(r.ctx, asset.Filter{
Data: map[string][]string{
"properties.dependencies": {"_nonempty"},
"entity": {"gotocompany"},
"urn": {"j-xcvcx"},
"country": {"vn"},
},
IncludeFields: []string{"urn"},
})
r.Require().NoError(err)

expectedURNs := []string{"nine-mock"}

r.Equal(len(expectedURNs), len(results))
for i := range results {
r.Equal(expectedURNs[i], results[i].URN)
r.NotEmpty(results[i].ID)
r.Equal("", results[i].Name)
}
})

r.Run("should return when user info is reequired in response with selected fields", func() {
results, err := r.repository.GetAll(r.ctx, asset.Filter{
Data: map[string][]string{
"properties.dependencies": {"_nonempty"},
"entity": {"gotocompany"},
"urn": {"j-xcvcx"},
"country": {"vn"},
},
IncludeFields: []string{"urn"},
IncludeUserInfo: true,
})
r.Require().NoError(err)
expectedURNs := []string{"nine-mock"}
r.Equal(len(expectedURNs), len(results))
for i := range results {
r.Equal(expectedURNs[i], results[i].URN)
r.NotEmpty(results[i].ID)
r.Equal("", results[i].Name)
r.Equal(assets[8].UpdatedBy.ID, results[i].UpdatedBy.ID)
r.Equal(assets[8].UpdatedBy.UUID, results[i].UpdatedBy.UUID)
r.Equal(assets[8].UpdatedBy.Email, results[i].UpdatedBy.Email)
}
})

r.Run("should override default size using GetConfig.Size", func() {
size := 6

Expand Down
12 changes: 12 additions & 0 deletions proto/compass.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ paths:
in: query
required: false
type: boolean
- name: include_fields
in: query
required: false
type: array
items:
type: string
collectionFormat: multi
- name: include_user_info
description: if true user information will be added in the response else not
in: query
required: false
type: boolean
tags:
- Asset
put:
Expand Down
Loading
Loading