Skip to content

Commit 414593a

Browse files
sudo-suhassudo-suhas
authored
refactor!: stop usage of deprecated fields in LineageNode (#178)
- Stop usage of v1beta1.LineageNode.{type, service}. The URN is now sufficient to uniquely identify the node. - Modify interfaces and service to stop usage of core/asset.LineageNode, just use the URN. - core/asset.Service struct. - core/asset/LineageRepository interface. - Remove the type core/asset.LineageEdge. BREAKING CHANGES: - Types modified - core/asset.Service, core/asset/LineageRepository. - Type removed - core/asset.LineageEdge.
1 parent 28ca981 commit 414593a

16 files changed

Lines changed: 2152 additions & 1787 deletions

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
NAME="github.com/odpf/compass"
22
VERSION=$(shell git describe --always --tags 2>/dev/null)
33
COVERFILE="/tmp/compass.coverprofile"
4-
PROTON_COMMIT := "c3fd1594a3d2e7b9df2f2050cce36c72c29fe4b2"
4+
PROTON_COMMIT := "838f2a8c9ddc8fa6dfbd6f3ebe6201e76e2368f2"
55
.PHONY: all build test clean install proto
66

77
all: build

‎core/asset/lineage.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ type LineageQuery struct {
2727

2828
//go:generate mockery --name=LineageRepository -r --case underscore --with-expecter --structname=LineageRepository --filename=lineage_repository.go --output=./mocks
2929
type LineageRepository interface {
30-
GetGraph(ctx context.Context, node LineageNode, query LineageQuery) (LineageGraph, error)
31-
Upsert(ctx context.Context, node LineageNode, upstreams, downstreams []LineageNode) error
30+
GetGraph(ctx context.Context, urn string, query LineageQuery) (LineageGraph, error)
31+
Upsert(ctx context.Context, urn string, upstreams, downstreams []string) error
3232
}
3333

3434
type LineageGraph []LineageEdge
@@ -43,9 +43,3 @@ type LineageEdge struct {
4343
// Prop is a map containing extra information about the edge
4444
Prop map[string]interface{} `json:"prop"`
4545
}
46-
47-
type LineageNode struct {
48-
URN string `json:"urn"`
49-
Type Type `json:"type"`
50-
Service string `json:"service"`
51-
}

‎core/asset/mocks/lineage_repository.go‎

Lines changed: 35 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎core/asset/service.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (s *Service) GetAllAssets(ctx context.Context, flt Filter, withTotal bool)
3838
return assets, totalCount, nil
3939
}
4040

41-
func (s *Service) UpsertAsset(ctx context.Context, ast *Asset, upstreams, downstreams []LineageNode) (string, error) {
41+
func (s *Service) UpsertAsset(ctx context.Context, ast *Asset, upstreams, downstreams []string) (string, error) {
4242
var assetID string
4343
var err error
4444

@@ -52,13 +52,7 @@ func (s *Service) UpsertAsset(ctx context.Context, ast *Asset, upstreams, downst
5252
return assetID, err
5353
}
5454

55-
node := LineageNode{
56-
URN: ast.URN,
57-
Type: ast.Type,
58-
Service: ast.Service,
59-
}
60-
61-
if err := s.lineageRepository.Upsert(ctx, node, upstreams, downstreams); err != nil {
55+
if err := s.lineageRepository.Upsert(ctx, ast.URN, upstreams, downstreams); err != nil {
6256
return assetID, err
6357
}
6458

@@ -114,8 +108,8 @@ func (s *Service) AddProbe(ctx context.Context, assetURN string, probe *Probe) e
114108
return s.assetRepository.AddProbe(ctx, assetURN, probe)
115109
}
116110

117-
func (s *Service) GetLineage(ctx context.Context, node LineageNode, query LineageQuery) (LineageGraph, error) {
118-
return s.lineageRepository.GetGraph(ctx, node, query)
111+
func (s *Service) GetLineage(ctx context.Context, urn string, query LineageQuery) (LineageGraph, error) {
112+
return s.lineageRepository.GetGraph(ctx, urn, query)
119113
}
120114

121115
func (s *Service) GetTypes(ctx context.Context, flt Filter) (map[Type]int, error) {

‎core/asset/service_test.go‎

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -161,35 +161,13 @@ func TestService_GetTypes(t *testing.T) {
161161

162162
func TestService_UpsertAsset(t *testing.T) {
163163
sampleAsset := &asset.Asset{ID: "some-id", URN: "some-urn", Type: asset.TypeDashboard, Service: "some-service"}
164-
sampleNodes1 := []asset.LineageNode{
165-
{
166-
URN: "1-urn-1",
167-
Type: asset.TypeJob,
168-
Service: "service-1",
169-
},
170-
{
171-
URN: "1-urn-2",
172-
Type: asset.TypeJob,
173-
Service: "service-1",
174-
},
175-
}
176-
sampleNodes2 := []asset.LineageNode{
177-
{
178-
URN: "2-urn-1",
179-
Type: asset.TypeTopic,
180-
Service: "service-2",
181-
},
182-
{
183-
URN: "2-urn-2",
184-
Type: asset.TypeJob,
185-
Service: "service-2",
186-
},
187-
}
164+
sampleNodes1 := []string{"1-urn-1", "1-urn-2"}
165+
sampleNodes2 := []string{"2-urn-1", "2-urn-2"}
188166
type testCase struct {
189167
Description string
190168
Asset *asset.Asset
191-
Upstreams []asset.LineageNode
192-
Downstreams []asset.LineageNode
169+
Upstreams []string
170+
Downstreams []string
193171
Err error
194172
ReturnedID string
195173
Setup func(context.Context, *mocks.AssetRepository, *mocks.DiscoveryRepository, *mocks.LineageRepository)
@@ -223,11 +201,7 @@ func TestService_UpsertAsset(t *testing.T) {
223201
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
224202
ar.EXPECT().Upsert(ctx, sampleAsset).Return(sampleAsset.ID, nil)
225203
dr.EXPECT().Upsert(ctx, *sampleAsset).Return(nil)
226-
lr.EXPECT().Upsert(ctx, asset.LineageNode{
227-
URN: sampleAsset.URN,
228-
Type: sampleAsset.Type,
229-
Service: sampleAsset.Service,
230-
}, sampleNodes1, sampleNodes2).Return(errors.New("unknown error"))
204+
lr.EXPECT().Upsert(ctx, sampleAsset.URN, sampleNodes1, sampleNodes2).Return(errors.New("unknown error"))
231205
},
232206
Err: errors.New("unknown error"),
233207
ReturnedID: sampleAsset.ID,
@@ -240,11 +214,7 @@ func TestService_UpsertAsset(t *testing.T) {
240214
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
241215
ar.EXPECT().Upsert(ctx, sampleAsset).Return(sampleAsset.ID, nil)
242216
dr.EXPECT().Upsert(ctx, *sampleAsset).Return(nil)
243-
lr.EXPECT().Upsert(ctx, asset.LineageNode{
244-
URN: sampleAsset.URN,
245-
Type: sampleAsset.Type,
246-
Service: sampleAsset.Service,
247-
}, sampleNodes1, sampleNodes2).Return(nil)
217+
lr.EXPECT().Upsert(ctx, sampleAsset.URN, sampleNodes1, sampleNodes2).Return(nil)
248218
},
249219
Err: nil,
250220
ReturnedID: sampleAsset.ID,
@@ -586,15 +556,15 @@ func TestService_GetLineage(t *testing.T) {
586556
Description: `should return error if the GetGraph function return error`,
587557
ID: assetID,
588558
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
589-
lr.EXPECT().GetGraph(ctx, asset.LineageNode{}, asset.LineageQuery{}).Return(asset.LineageGraph{}, errors.New("error fetching graph"))
559+
lr.EXPECT().GetGraph(ctx, "", asset.LineageQuery{}).Return(asset.LineageGraph{}, errors.New("error fetching graph"))
590560
},
591561
Err: errors.New("error fetching graph"),
592562
},
593563
{
594564
Description: `should return no error if graph nodes are returned`,
595565
ID: assetID,
596566
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
597-
lr.EXPECT().GetGraph(ctx, asset.LineageNode{}, asset.LineageQuery{}).Return(asset.LineageGraph{}, nil)
567+
lr.EXPECT().GetGraph(ctx, "", asset.LineageQuery{}).Return(asset.LineageGraph{}, nil)
598568
},
599569
Err: nil,
600570
},
@@ -611,7 +581,7 @@ func TestService_GetLineage(t *testing.T) {
611581
}
612582

613583
svc := asset.NewService(mockAssetRepo, mockDiscoveryRepo, mockLineageRepo)
614-
_, err := svc.GetLineage(ctx, asset.LineageNode{}, asset.LineageQuery{})
584+
_, err := svc.GetLineage(ctx, "", asset.LineageQuery{})
615585
if err != nil && errors.Is(tc.Err, err) {
616586
t.Fatalf("got error %v, expected error was %v", err, tc.Err)
617587
}

‎internal/server/v1beta1/asset.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ type StatsDClient interface {
2525
}
2626

2727
type AssetService interface {
28-
GetAllAssets(context.Context, asset.Filter, bool) ([]asset.Asset, uint32, error)
28+
GetAllAssets(ctx context.Context, flt asset.Filter, withTotal bool) ([]asset.Asset, uint32, error)
2929
GetAssetByID(ctx context.Context, id string) (asset.Asset, error)
3030
GetAssetByVersion(ctx context.Context, id string, version string) (asset.Asset, error)
3131
GetAssetVersionHistory(ctx context.Context, flt asset.Filter, id string) ([]asset.Asset, error)
32-
UpsertAsset(context.Context, *asset.Asset, []asset.LineageNode, []asset.LineageNode) (string, error)
33-
DeleteAsset(context.Context, string) error
32+
UpsertAsset(ctx context.Context, ast *asset.Asset, upstreams, downstreams []string) (string, error)
33+
DeleteAsset(ctx context.Context, id string) error
3434

35-
GetLineage(ctx context.Context, node asset.LineageNode, query asset.LineageQuery) (asset.LineageGraph, error)
35+
GetLineage(ctx context.Context, urn string, query asset.LineageQuery) (asset.LineageGraph, error)
3636
GetTypes(ctx context.Context, flt asset.Filter) (map[asset.Type]int, error)
3737

3838
SearchAssets(ctx context.Context, cfg asset.SearchConfig) (results []asset.SearchResult, err error)
@@ -348,13 +348,13 @@ func (server *APIServer) upsertAsset(
348348
return "", status.Error(codes.InvalidArgument, err.Error())
349349
}
350350

351-
upstreams := []asset.LineageNode{}
351+
upstreams := make([]string, 0, len(reqUpstreams))
352352
for _, pb := range reqUpstreams {
353-
upstreams = append(upstreams, lineageNodeFromProto(pb))
353+
upstreams = append(upstreams, pb.Urn)
354354
}
355-
downstreams := []asset.LineageNode{}
355+
downstreams := make([]string, 0, len(reqDownstreams))
356356
for _, pb := range reqDownstreams {
357-
downstreams = append(downstreams, lineageNodeFromProto(pb))
357+
downstreams = append(downstreams, pb.Urn)
358358
}
359359

360360
assetID, err = server.assetService.UpsertAsset(ctx, &ast, upstreams, downstreams)

‎internal/server/v1beta1/asset_test.go‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,8 @@ func TestUpsertAsset(t *testing.T) {
433433
as.EXPECT().UpsertAsset(
434434
ctx,
435435
mock.AnythingOfType("*asset.Asset"),
436-
mock.AnythingOfType("[]asset.LineageNode"),
437-
mock.AnythingOfType("[]asset.LineageNode"),
436+
mock.AnythingOfType("[]string"),
437+
mock.AnythingOfType("[]string"),
438438
).Return("", expectedErr)
439439
},
440440
Request: validPayload,
@@ -452,18 +452,13 @@ func TestUpsertAsset(t *testing.T) {
452452
Data: map[string]interface{}{},
453453
Owners: []user.User{{ID: "id", UUID: "", Email: "email@email.com", Provider: "provider"}},
454454
}
455-
upstreams := []asset.LineageNode{
456-
{URN: "upstream-1", Type: asset.TypeJob, Service: "optimus"},
457-
}
458-
downstreams := []asset.LineageNode{
459-
{URN: "downstream-1", Type: asset.TypeDashboard, Service: "metabase"},
460-
{URN: "downstream-2", Type: asset.TypeDashboard, Service: "tableau"},
461-
}
455+
upstreams := []string{"upstream-1"}
456+
downstreams := []string{"downstream-1", "downstream-2"}
462457

463458
assetWithID := ast
464459
assetWithID.ID = assetID
465460

466-
as.EXPECT().UpsertAsset(ctx, &ast, upstreams, downstreams).Return(assetWithID.ID, nil).Run(func(ctx context.Context, ast *asset.Asset, upstreams, downstreams []asset.LineageNode) {
461+
as.EXPECT().UpsertAsset(ctx, &ast, upstreams, downstreams).Return(assetWithID.ID, nil).Run(func(ctx context.Context, ast *asset.Asset, upstreams, downstreams []string) {
467462
ast.ID = assetWithID.ID
468463
})
469464
},
@@ -642,7 +637,7 @@ func TestUpsertPatchAsset(t *testing.T) {
642637
Setup: func(ctx context.Context, as *mocks.AssetService) {
643638
expectedErr := errors.New("unknown error")
644639
as.EXPECT().GetAssetByID(ctx, "test dagger").Return(currentAsset, nil)
645-
as.EXPECT().UpsertAsset(ctx, mock.AnythingOfType("*asset.Asset"), mock.AnythingOfType("[]asset.LineageNode"), mock.AnythingOfType("[]asset.LineageNode")).Return("1234-5678", expectedErr)
640+
as.EXPECT().UpsertAsset(ctx, mock.AnythingOfType("*asset.Asset"), mock.AnythingOfType("[]string"), mock.AnythingOfType("[]string")).Return("1234-5678", expectedErr)
646641
},
647642
Request: validPayload,
648643
ExpectStatus: codes.Internal,
@@ -659,19 +654,14 @@ func TestUpsertPatchAsset(t *testing.T) {
659654
Data: map[string]interface{}{},
660655
Owners: []user.User{{ID: "id", UUID: "", Email: "email@email.com", Provider: "provider"}},
661656
}
662-
upstreams := []asset.LineageNode{
663-
{URN: "upstream-1", Type: asset.TypeJob, Service: "optimus"},
664-
}
665-
downstreams := []asset.LineageNode{
666-
{URN: "downstream-1", Type: asset.TypeDashboard, Service: "metabase"},
667-
{URN: "downstream-2", Type: asset.TypeDashboard, Service: "tableau"},
668-
}
657+
upstreams := []string{"upstream-1"}
658+
downstreams := []string{"downstream-1", "downstream-2"}
669659

670660
assetWithID := patchedAsset
671661
assetWithID.ID = assetID
672662

673663
as.EXPECT().GetAssetByID(ctx, "test dagger").Return(currentAsset, nil)
674-
as.EXPECT().UpsertAsset(ctx, &patchedAsset, upstreams, downstreams).Return(assetWithID.ID, nil).Run(func(ctx context.Context, ast *asset.Asset, upstreams, downstreams []asset.LineageNode) {
664+
as.EXPECT().UpsertAsset(ctx, &patchedAsset, upstreams, downstreams).Return(assetWithID.ID, nil).Run(func(ctx context.Context, ast *asset.Asset, upstreams, downstreams []string) {
675665
patchedAsset.ID = assetWithID.ID
676666
})
677667
},

0 commit comments

Comments
 (0)