Skip to content

Commit

Permalink
feat: change flow of deletion assets and refactor codes based on feed…
Browse files Browse the repository at this point in the history
…backs
  • Loading branch information
Muhammad Luthfi Fahlevi committed Aug 20, 2024
1 parent 4abd20d commit a77cf72
Show file tree
Hide file tree
Showing 29 changed files with 294 additions and 372 deletions.
7 changes: 4 additions & 3 deletions core/asset/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import (
"time"

"github.com/goto/compass/core/user"
"github.com/goto/compass/pkg/queryexpr"
"github.com/r3labs/diff/v2"
)

type Repository interface {
GetAll(context.Context, Filter) ([]Asset, error)
GetCount(context.Context, Filter) (int, error)
GetCountByQueryExpr(context.Context, string, bool) (int, error)
GetCountByQueryExpr(ctx context.Context, queryExpr queryexpr.ExprStr) (int, error)
GetByID(ctx context.Context, id string) (Asset, error)
GetByURN(ctx context.Context, urn string) (Asset, error)
GetVersionHistory(ctx context.Context, flt Filter, id string) ([]Asset, error)
Expand All @@ -22,7 +23,7 @@ type Repository interface {
Upsert(ctx context.Context, ast *Asset) (string, error)
DeleteByID(ctx context.Context, id string) error
DeleteByURN(ctx context.Context, urn string) error
DeleteByQueryExpr(ctx context.Context, queryExpr string) ([]string, error)
DeleteByQueryExpr(ctx context.Context, queryExpr queryexpr.ExprStr) ([]string, error)
AddProbe(ctx context.Context, assetURN string, probe *Probe) error
GetProbes(ctx context.Context, assetURN string) ([]Probe, error)
GetProbesWithFilter(ctx context.Context, flt ProbesFilter) (map[string][]Probe, error)
Expand All @@ -42,7 +43,7 @@ type Asset struct {
Owners []user.User `json:"owners,omitempty" diff:"owners"`
CreatedAt time.Time `json:"created_at" diff:"-"`
UpdatedAt time.Time `json:"updated_at" diff:"-"`
RefreshedAt time.Time `json:"refreshed_at" diff:"-"`
RefreshedAt *time.Time `json:"refreshed_at" diff:"-"`
Version string `json:"version" diff:"-"`
UpdatedBy user.User `json:"updated_by" diff:"-"`
Changelog diff.Changelog `json:"changelog,omitempty" diff:"-"`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,50 +1,68 @@
package queryexpr
package asset

import (
"fmt"
"strings"

"github.com/goto/compass/core/asset"
generichelper "github.com/goto/compass/pkg/generic_helper"
"github.com/goto/compass/pkg/queryexpr"
)

var assetJSONTagsSchema = generichelper.GetJSONTags(Asset{})

type DeleteAssetExpr struct {
ExprStr
queryexpr.ExprStr
}

func (d DeleteAssetExpr) ToQuery() (string, error) {
return d.ExprStr.ToQuery()
}

func (d DeleteAssetExpr) Validate() error {
identifiersWithOperator, err := GetIdentifiersMap(d.ExprStr.String())
identifiersWithOperator, err := queryexpr.GetIdentifiersMap(d.ExprStr.String())
if err != nil {
return err
}

isExist := func(jsonTag string) bool {
return identifiersWithOperator[jsonTag] != ""
}
mustExist := isExist("refreshed_at") && isExist("type") && isExist("service")
if !mustExist {
return fmt.Errorf("must exists these identifiers: refreshed_at, type, and service")
if err := d.isRequiredIdentifiersExist(identifiersWithOperator); err != nil {
return err
}

isOperatorEqualsOrIn := func(jsonTag string) bool {
return identifiersWithOperator[jsonTag] == "==" || strings.ToUpper(identifiersWithOperator[jsonTag]) == "IN"
}
if !isOperatorEqualsOrIn("type") || !isOperatorEqualsOrIn("service") {
return fmt.Errorf("identifier type and service must be equals (==) or IN operator")
if err := d.isUsingRightOperator(identifiersWithOperator); err != nil {
return err
}

return d.isAllIdentifiersExistInStruct(identifiersWithOperator)
}

func (DeleteAssetExpr) isAllIdentifiersExistInStruct(identifiersWithOperator map[string]string) error {
identifiers := generichelper.GetMapKeys(identifiersWithOperator)
jsonTagsSchema := generichelper.GetJSONTags(asset.Asset{})
for _, identifier := range identifiers {
isFieldValid := generichelper.Contains(jsonTagsSchema, identifier)
isFieldValid := generichelper.Contains(assetJSONTagsSchema, identifier)
if !isFieldValid {
return fmt.Errorf("%s is not a valid identifier", identifier)
}
}
return nil
}

func (DeleteAssetExpr) isUsingRightOperator(identifiersWithOperator map[string]string) error {
isOperatorEqualsOrIn := func(jsonTag string) bool {
return identifiersWithOperator[jsonTag] == "==" || strings.ToUpper(identifiersWithOperator[jsonTag]) == "IN"
}
if !isOperatorEqualsOrIn("type") || !isOperatorEqualsOrIn("service") {
return fmt.Errorf("identifier type and service must be equals (==) or IN operator")
}
return nil
}

func (DeleteAssetExpr) isRequiredIdentifiersExist(identifiersWithOperator map[string]string) error {
isExist := func(jsonTag string) bool {
return identifiersWithOperator[jsonTag] != ""
}
mustExist := isExist("refreshed_at") && isExist("type") && isExist("service")
if !mustExist {
return fmt.Errorf("must exists these identifiers: refreshed_at, type, and service")
}
return nil
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package queryexpr_test
package asset_test

import (
"errors"
"testing"

queryexpr "github.com/goto/compass/pkg/query_expr"
"github.com/goto/compass/core/asset"
"github.com/goto/compass/pkg/queryexpr"
"github.com/stretchr/testify/assert"
)

Expand All @@ -21,23 +22,23 @@ func TestDeleteAssetExpr_ToQuery(t *testing.T) {
}{
{
name: "convert to SQL query",
exprStr: queryexpr.DeleteAssetExpr{
exprStr: asset.DeleteAssetExpr{
ExprStr: &sqlExpr,
},
want: "((name = 'John') OR (service NOT IN ('test1', 'test2', 'test3')))",
wantErr: false,
},
{
name: "convert to ES query",
exprStr: queryexpr.DeleteAssetExpr{
exprStr: asset.DeleteAssetExpr{
ExprStr: &esExpr,
},
want: `{"query":{"bool":{"should":[{"term":{"name":"John"}},{"bool":{"must_not":[{"terms":{"service":["test1","test2","test3"]}}]}}]}}}`,
wantErr: false,
},
{
name: "got error due to wrong syntax",
exprStr: queryexpr.DeleteAssetExpr{
exprStr: asset.DeleteAssetExpr{
ExprStr: &wrongExpr,
},
want: "",
Expand All @@ -46,7 +47,7 @@ func TestDeleteAssetExpr_ToQuery(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := queryexpr.DeleteAssetExpr{
d := asset.DeleteAssetExpr{
ExprStr: tt.exprStr,
}
got, err := d.ToQuery()
Expand All @@ -72,7 +73,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error get identifiers map",
exprStrFn: func() queryexpr.ExprStr {
wrongExpr := queryexpr.SQLExpr("findLast(")
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &wrongExpr,
}
},
Expand All @@ -83,7 +84,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error miss refreshed_at not exist",
exprStrFn: func() queryexpr.ExprStr {
missRefreshedAt := queryexpr.SQLExpr(`updated_at < "2023-12-12 23:59:59" && type == "table" && service in ["test1","test2","test3"]`)
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &missRefreshedAt,
}
},
Expand All @@ -94,7 +95,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error miss type not exist",
exprStrFn: func() queryexpr.ExprStr {
missType := queryexpr.SQLExpr(`refreshed_at < "2023-12-12 23:59:59" && service in ["test1","test2","test3"]`)
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &missType,
}
},
Expand All @@ -105,7 +106,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error miss service not exist",
exprStrFn: func() queryexpr.ExprStr {
missService := queryexpr.SQLExpr(`refreshed_at < "2023-12-12 23:59:59" && type == "table"`)
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &missService,
}
},
Expand All @@ -116,7 +117,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error wrong operator for type identifier",
exprStrFn: func() queryexpr.ExprStr {
wrongTypeOperator := queryexpr.SQLExpr(`refreshed_at < "2023-12-12 23:59:59" && type != "table" && service in ["test1","test2","test3"]`)
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &wrongTypeOperator,
}
},
Expand All @@ -127,7 +128,7 @@ func TestDeleteAssetExpr_Validate(t *testing.T) {
name: "error wrong operator for service identifier",
exprStrFn: func() queryexpr.ExprStr {
wrongServiceOperator := queryexpr.SQLExpr(`refreshed_at < "2023-12-12 23:59:59" && type != "table" && service not in ["test1","test2","test3"]`)
return queryexpr.DeleteAssetExpr{
return asset.DeleteAssetExpr{
ExprStr: &wrongServiceOperator,
}
},
Expand Down
6 changes: 6 additions & 0 deletions core/asset/delete_assets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package asset

type DeleteAssetsRequest struct {
QueryExpr string
DryRun bool
}
2 changes: 1 addition & 1 deletion core/asset/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type DiscoveryRepository interface {
Upsert(context.Context, Asset) error
DeleteByID(ctx context.Context, assetID string) error
DeleteByURN(ctx context.Context, assetURN string) error
DeleteByQueryExpr(ctx context.Context, filterQuery string) error
DeleteByQueryExpr(ctx context.Context, queryExpr string) error
Search(ctx context.Context, cfg SearchConfig) (results []SearchResult, err error)
Suggest(ctx context.Context, cfg SearchConfig) (suggestions []string, err error)
GroupAssets(ctx context.Context, cfg GroupConfig) (results []GroupResult, err error)
Expand Down
2 changes: 1 addition & 1 deletion core/asset/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ var (
ErrEmptyID = errors.New("asset does not have ID")
ErrProbeExists = errors.New("asset probe already exists")
ErrEmptyURN = errors.New("asset does not have URN")
ErrEmptyQuery = errors.New("query must exist to filtering assets")
ErrEmptyQuery = errors.New("query is empty")
ErrUnknownType = errors.New("unknown type")
ErrNilAsset = errors.New("nil asset")
)
Expand Down
51 changes: 26 additions & 25 deletions core/asset/mocks/asset_repository.go

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

Loading

0 comments on commit a77cf72

Please sign in to comment.