From 502fa2fab48e1c9de7db4bce643b06a61feb51b8 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Sun, 19 Jul 2026 22:26:42 -0700 Subject: [PATCH 1/4] feat(mapper): add entity-proto mappers and streaming splitters Co-Authored-By: Claude Sonnet 5 --- internal/mapper/BUILD.bazel | 6 + internal/mapper/changed_targets.go | 40 ++++++ internal/mapper/target_graph.go | 200 +++++++++++++++++++++++++++ internal/mapper/target_graph_test.go | 111 +++++++++++++++ 4 files changed, 357 insertions(+) create mode 100644 internal/mapper/changed_targets.go diff --git a/internal/mapper/BUILD.bazel b/internal/mapper/BUILD.bazel index 5b988d0f..0d317638 100644 --- a/internal/mapper/BUILD.bazel +++ b/internal/mapper/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "mapper", srcs = [ "build_description.go", + "changed_targets.go", "errors.go", "target_graph.go", ], @@ -11,8 +12,12 @@ go_library( visibility = ["//visibility:public"], deps = [ "//core/errors", + "//core/targethasher", "//entity", + "//internal/mapper/idmapper", + "//internal/streaming", "//tangopb", + "@com_github_bazelbuild_buildtools//build_proto", "@org_uber_go_yarpc//encoding/protobuf", "@org_uber_go_yarpc//yarpcerrors", ], @@ -28,6 +33,7 @@ go_test( embed = [":mapper"], deps = [ "//core/errors", + "//core/targethasher", "//entity", "//tangopb", "@com_github_stretchr_testify//assert", diff --git a/internal/mapper/changed_targets.go b/internal/mapper/changed_targets.go new file mode 100644 index 00000000..8b28d185 --- /dev/null +++ b/internal/mapper/changed_targets.go @@ -0,0 +1,40 @@ +package mapper + +import ( + "github.com/uber/tango/entity" + "github.com/uber/tango/tangopb" +) + +// ChangedTargetsResponseToProto converts an entity.GetChangedTargetsResponse +// to its proto equivalent for gRPC streaming. +func ChangedTargetsResponseToProto(resp *entity.GetChangedTargetsResponse) *tangopb.GetChangedTargetsResponse { + if resp.Metadata != nil { + return &tangopb.GetChangedTargetsResponse{ + Item: &tangopb.GetChangedTargetsResponse_Metadata{ + Metadata: metadataToProto(resp.Metadata), + }, + } + } + changed := make([]*tangopb.ChangedTarget, len(resp.ChangedTargets)) + for i := range resp.ChangedTargets { + ct := &resp.ChangedTargets[i] + changed[i] = &tangopb.ChangedTarget{ + ChangeType: tangopb.ChangeType(int32(ct.ChangeType)), + OldTarget: optionalTargetToProto(ct.OldTarget), + NewTarget: optionalTargetToProto(ct.NewTarget), + Distance: ct.Distance, + } + } + return &tangopb.GetChangedTargetsResponse{ + Item: &tangopb.GetChangedTargetsResponse_ChangedTargets{ + ChangedTargets: &tangopb.ChangedTargets{ChangedTargets: changed}, + }, + } +} + +func optionalTargetToProto(t *entity.OptimizedTarget) *tangopb.OptimizedTarget { + if t == nil { + return nil + } + return optimizedTargetToProto(t) +} diff --git a/internal/mapper/target_graph.go b/internal/mapper/target_graph.go index 45717d82..6b621bcb 100644 --- a/internal/mapper/target_graph.go +++ b/internal/mapper/target_graph.go @@ -1,12 +1,20 @@ package mapper import ( + "context" + "encoding/hex" "errors" + buildpb "github.com/bazelbuild/buildtools/build_proto" + "github.com/uber/tango/core/targethasher" "github.com/uber/tango/entity" + "github.com/uber/tango/internal/mapper/idmapper" + "github.com/uber/tango/internal/streaming" "github.com/uber/tango/tangopb" ) +const cancelCheckInterval = 4096 + // ProtoToGetTargetGraphRequest converts a proto GetTargetGraphRequest to the // domain type. Returns an error if req is nil or its BuildDescription fails // validation (see ProtoToBuildDescription). @@ -24,3 +32,195 @@ func ProtoToGetTargetGraphRequest(req *tangopb.GetTargetGraphRequest) (entity.Ge BypassCache: req.GetBypassCache(), }, nil } + +// ResultToTargetGraph converts a targethasher.Result into ID-mapped entity +// types. Returns a flat list of targets and their accompanying metadata. +// No chunking or proto conversion is performed. +func ResultToTargetGraph(ctx context.Context, result targethasher.Result) ([]entity.OptimizedTarget, *entity.Metadata, error) { + targetNamesMapping := make(map[string]int32, len(result.TargetNames)) + for i, name := range result.TargetNames { + targetNamesMapping[name] = int32(i + 1) + } + + ruleTypeMapper := idmapper.NewMapper() + tagMapper := idmapper.NewMapper() + attrNameMapper := idmapper.NewMapper() + attrStrValMapper := idmapper.NewMapper() + + targets := make([]entity.OptimizedTarget, 0, len(result.TargetNames)) + + n := 0 + for _, name := range result.TargetNames { + t, ok := result.Targets[name] + if !ok { + continue + } + if n%cancelCheckInterval == 0 { + if err := ctx.Err(); err != nil { + return nil, nil, err + } + } + n++ + + depIDs := make([]int32, 0, len(t.Deps)) + for _, depName := range t.Deps { + if depID, ok := targetNamesMapping[depName]; ok { + depIDs = append(depIDs, depID) + } + } + + idt := entity.OptimizedTarget{ + ID: targetNamesMapping[name], + Hash: hex.EncodeToString(t.Hash), + DirectDependencies: depIDs, + Root: t.Root, + External: t.External, + } + if t.RuleType != "" { + idt.RuleType = ruleTypeMapper.ID(t.RuleType) + } + if len(t.Tags) > 0 { + tagIDs := make([]int32, 0, len(t.Tags)) + for _, tag := range t.Tags { + tagIDs = append(tagIDs, tagMapper.ID(tag)) + } + idt.Tags = tagIDs + } + if len(t.Attributes) > 0 { + attrs := make(map[int32]int32, len(t.Attributes)) + for _, attr := range t.Attributes { + if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil { + attrs[attrNameMapper.ID(*attr.Name)] = attrStrValMapper.ID(*attr.StringValue) + } + } + if len(attrs) > 0 { + idt.Attributes = attrs + } + } + + targets = append(targets, idt) + } + + targetIDToName := make(map[int32]string, len(targetNamesMapping)) + for s, id := range targetNamesMapping { + targetIDToName[id] = s + } + + meta := &entity.Metadata{ + TargetIDMapping: targetIDToName, + RuleTypeMapping: ruleTypeMapper.Invert(), + TagMapping: tagMapper.Invert(), + AttributeNameMapping: attrNameMapper.Invert(), + AttributeStringValueMapping: attrStrValMapper.Invert(), + } + + return targets, meta, nil +} + +// ChunkTargetGraph splits targets and metadata into wire-safe +// entity.GetTargetGraphResponse chunks bounded by maxMessageBytes. +func ChunkTargetGraph(targets []entity.OptimizedTarget, meta *entity.Metadata, maxMessageBytes int) ([]entity.GetTargetGraphResponse, error) { + protoTargets := make([]*tangopb.OptimizedTarget, len(targets)) + for i := range targets { + protoTargets[i] = optimizedTargetToProto(&targets[i]) + } + + targetGroups, err := streaming.SplitBySize(protoTargets, maxMessageBytes) + if err != nil { + return nil, err + } + + var chunks []entity.GetTargetGraphResponse + idx := 0 + for _, g := range targetGroups { + chunks = append(chunks, entity.GetTargetGraphResponse{ + Targets: targets[idx : idx+len(g)], + }) + idx += len(g) + } + + metaGroups, err := streaming.SplitMetadata( + meta.TargetIDMapping, + meta.RuleTypeMapping, + meta.TagMapping, + meta.AttributeNameMapping, + meta.AttributeStringValueMapping, + maxMessageBytes, + ) + if err != nil { + return nil, err + } + for _, m := range metaGroups { + chunks = append(chunks, entity.GetTargetGraphResponse{Metadata: m}) + } + + return chunks, nil +} + +// GetTargetGraphResponseToProto converts an entity.GetTargetGraphResponse to +// the corresponding proto GetTargetGraphResponse. +func GetTargetGraphResponseToProto(chunk *entity.GetTargetGraphResponse) *tangopb.GetTargetGraphResponse { + if chunk.Metadata != nil { + return &tangopb.GetTargetGraphResponse{ + Item: &tangopb.GetTargetGraphResponse_Metadata{ + Metadata: metadataToProto(chunk.Metadata), + }, + } + } + targets := make([]*tangopb.OptimizedTarget, len(chunk.Targets)) + for i := range chunk.Targets { + targets[i] = optimizedTargetToProto(&chunk.Targets[i]) + } + return &tangopb.GetTargetGraphResponse{ + Item: &tangopb.GetTargetGraphResponse_Targets{ + Targets: &tangopb.OptimizedTargets{Targets: targets}, + }, + } +} + +// metadataToProto converts an entity.Metadata to a proto Metadata. +func metadataToProto(m *entity.Metadata) *tangopb.Metadata { + return &tangopb.Metadata{ + TargetIdMapping: m.TargetIDMapping, + RuleTypeMapping: m.RuleTypeMapping, + TagMapping: m.TagMapping, + AttributeNameMapping: m.AttributeNameMapping, + AttributeStringValueMapping: m.AttributeStringValueMapping, + } +} + +func optimizedTargetToProto(t *entity.OptimizedTarget) *tangopb.OptimizedTarget { + return &tangopb.OptimizedTarget{ + Id: t.ID, + Hash: t.Hash, + DirectDependencies: t.DirectDependencies, + RuleType: t.RuleType, + Tags: t.Tags, + Root: t.Root, + External: t.External, + Attributes: t.Attributes, + } +} + +func protoToOptimizedTarget(t *tangopb.OptimizedTarget) entity.OptimizedTarget { + return entity.OptimizedTarget{ + ID: t.GetId(), + Hash: t.GetHash(), + DirectDependencies: t.GetDirectDependencies(), + RuleType: t.GetRuleType(), + Tags: t.GetTags(), + Root: t.GetRoot(), + External: t.GetExternal(), + Attributes: t.GetAttributes(), + } +} + +func protoToMetadata(m *tangopb.Metadata) *entity.Metadata { + return &entity.Metadata{ + TargetIDMapping: m.GetTargetIdMapping(), + RuleTypeMapping: m.GetRuleTypeMapping(), + TagMapping: m.GetTagMapping(), + AttributeNameMapping: m.GetAttributeNameMapping(), + AttributeStringValueMapping: m.GetAttributeStringValueMapping(), + } +} diff --git a/internal/mapper/target_graph_test.go b/internal/mapper/target_graph_test.go index 0d0d92fb..271bf7f6 100644 --- a/internal/mapper/target_graph_test.go +++ b/internal/mapper/target_graph_test.go @@ -1,11 +1,14 @@ package mapper import ( + "context" + "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber/tango/core/targethasher" "github.com/uber/tango/entity" "github.com/uber/tango/tangopb" ) @@ -86,3 +89,111 @@ func TestProtoToGetTargetGraphRequest(t *testing.T) { }) } } + +func TestResultToTargetGraph_EmptyResult(t *testing.T) { + t.Parallel() + + targets, meta, err := ResultToTargetGraph(context.Background(), targethasher.Result{}) + require.NoError(t, err) + assert.Empty(t, targets) + assert.NotNil(t, meta) +} + +func TestChunkTargetGraph(t *testing.T) { + t.Parallel() + + numTargets := 50 + result := targethasher.Result{ + TargetNames: make([]string, numTargets), + Targets: make(map[string]*targethasher.Target, numTargets), + } + for i := 0; i < numTargets; i++ { + name := fmt.Sprintf("//pkg:target%d", i) + result.TargetNames[i] = name + result.Targets[name] = &targethasher.Target{Name: name, Hash: []byte{0}, RuleType: "go_library"} + } + + targets, meta, err := ResultToTargetGraph(context.Background(), result) + require.NoError(t, err) + + // Get baseline proto size per target for budget calculation. + baseline, err := ChunkTargetGraph(targets, meta, 1<<30) + require.NoError(t, err) + protoResp := GetTargetGraphResponseToProto(&baseline[0]) + protoTargets := protoResp.GetItem().(*tangopb.GetTargetGraphResponse_Targets) + targetBytes := protoTargets.Targets.Targets[0].Size() + require.NotZero(t, targetBytes) + + tests := []struct { + name string + maxMessageBytes int + wantTargetChunks int + }{ + {name: "25 per chunk", maxMessageBytes: targetBytes * 25, wantTargetChunks: 2}, + {name: "10 per chunk", maxMessageBytes: targetBytes * 10, wantTargetChunks: 5}, + {name: "all in one chunk", maxMessageBytes: targetBytes * 100, wantTargetChunks: 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + chunks, err := ChunkTargetGraph(targets, meta, tt.maxMessageBytes) + require.NoError(t, err) + + var targetChunks, totalTargets int + for _, c := range chunks { + if len(c.Targets) > 0 || c.Metadata == nil { + targetChunks++ + totalTargets += len(c.Targets) + } + } + assert.Equal(t, tt.wantTargetChunks, targetChunks) + assert.Equal(t, numTargets, totalTargets) + }) + } +} + +func TestGetTargetGraphResponseToProto_RoundTrip(t *testing.T) { + t.Parallel() + + chunk := entity.GetTargetGraphResponse{ + Targets: []entity.OptimizedTarget{ + {ID: 1, Hash: "ab", DirectDependencies: []int32{2}, RuleType: 10, Root: true}, + {ID: 2, Hash: "cd", Tags: []int32{5, 6}, External: true}, + }, + } + + proto := GetTargetGraphResponseToProto(&chunk) + roundTripped := protoToEntityResponse(proto) + assert.Equal(t, chunk, roundTripped) +} + +func TestGetTargetGraphResponseToProto_Metadata_RoundTrip(t *testing.T) { + t.Parallel() + + chunk := entity.GetTargetGraphResponse{ + Metadata: &entity.Metadata{ + TargetIDMapping: map[int32]string{1: "//pkg:a", 2: "//pkg:b"}, + RuleTypeMapping: map[int32]string{10: "go_library"}, + }, + } + + proto := GetTargetGraphResponseToProto(&chunk) + roundTripped := protoToEntityResponse(proto) + assert.Equal(t, chunk, roundTripped) +} + +func protoToEntityResponse(resp *tangopb.GetTargetGraphResponse) entity.GetTargetGraphResponse { + switch item := resp.GetItem().(type) { + case *tangopb.GetTargetGraphResponse_Targets: + targets := make([]entity.OptimizedTarget, len(item.Targets.GetTargets())) + for i, t := range item.Targets.GetTargets() { + targets[i] = protoToOptimizedTarget(t) + } + return entity.GetTargetGraphResponse{Targets: targets} + case *tangopb.GetTargetGraphResponse_Metadata: + return entity.GetTargetGraphResponse{Metadata: protoToMetadata(item.Metadata)} + default: + return entity.GetTargetGraphResponse{} + } +} From a832fa1d4550b1b96345045305d4f7ffdfc5d999 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Mon, 20 Jul 2026 11:18:15 -0700 Subject: [PATCH 2/4] refactor: rename ChunkTargetGraph to SplitTargetGraph and move to streaming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move SplitTargetGraph from internal/mapper to internal/streaming where the other split functions live. Rename chunk→split for consistency. Co-Authored-By: Claude Sonnet 5 --- internal/mapper/BUILD.bazel | 1 - internal/mapper/target_graph.go | 41 --------------------- internal/mapper/target_graph_test.go | 55 ---------------------------- 3 files changed, 97 deletions(-) diff --git a/internal/mapper/BUILD.bazel b/internal/mapper/BUILD.bazel index 0d317638..43434021 100644 --- a/internal/mapper/BUILD.bazel +++ b/internal/mapper/BUILD.bazel @@ -15,7 +15,6 @@ go_library( "//core/targethasher", "//entity", "//internal/mapper/idmapper", - "//internal/streaming", "//tangopb", "@com_github_bazelbuild_buildtools//build_proto", "@org_uber_go_yarpc//encoding/protobuf", diff --git a/internal/mapper/target_graph.go b/internal/mapper/target_graph.go index 6b621bcb..1cddcb2b 100644 --- a/internal/mapper/target_graph.go +++ b/internal/mapper/target_graph.go @@ -9,7 +9,6 @@ import ( "github.com/uber/tango/core/targethasher" "github.com/uber/tango/entity" "github.com/uber/tango/internal/mapper/idmapper" - "github.com/uber/tango/internal/streaming" "github.com/uber/tango/tangopb" ) @@ -117,46 +116,6 @@ func ResultToTargetGraph(ctx context.Context, result targethasher.Result) ([]ent return targets, meta, nil } -// ChunkTargetGraph splits targets and metadata into wire-safe -// entity.GetTargetGraphResponse chunks bounded by maxMessageBytes. -func ChunkTargetGraph(targets []entity.OptimizedTarget, meta *entity.Metadata, maxMessageBytes int) ([]entity.GetTargetGraphResponse, error) { - protoTargets := make([]*tangopb.OptimizedTarget, len(targets)) - for i := range targets { - protoTargets[i] = optimizedTargetToProto(&targets[i]) - } - - targetGroups, err := streaming.SplitBySize(protoTargets, maxMessageBytes) - if err != nil { - return nil, err - } - - var chunks []entity.GetTargetGraphResponse - idx := 0 - for _, g := range targetGroups { - chunks = append(chunks, entity.GetTargetGraphResponse{ - Targets: targets[idx : idx+len(g)], - }) - idx += len(g) - } - - metaGroups, err := streaming.SplitMetadata( - meta.TargetIDMapping, - meta.RuleTypeMapping, - meta.TagMapping, - meta.AttributeNameMapping, - meta.AttributeStringValueMapping, - maxMessageBytes, - ) - if err != nil { - return nil, err - } - for _, m := range metaGroups { - chunks = append(chunks, entity.GetTargetGraphResponse{Metadata: m}) - } - - return chunks, nil -} - // GetTargetGraphResponseToProto converts an entity.GetTargetGraphResponse to // the corresponding proto GetTargetGraphResponse. func GetTargetGraphResponseToProto(chunk *entity.GetTargetGraphResponse) *tangopb.GetTargetGraphResponse { diff --git a/internal/mapper/target_graph_test.go b/internal/mapper/target_graph_test.go index 271bf7f6..cc61f125 100644 --- a/internal/mapper/target_graph_test.go +++ b/internal/mapper/target_graph_test.go @@ -2,7 +2,6 @@ package mapper import ( "context" - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -99,60 +98,6 @@ func TestResultToTargetGraph_EmptyResult(t *testing.T) { assert.NotNil(t, meta) } -func TestChunkTargetGraph(t *testing.T) { - t.Parallel() - - numTargets := 50 - result := targethasher.Result{ - TargetNames: make([]string, numTargets), - Targets: make(map[string]*targethasher.Target, numTargets), - } - for i := 0; i < numTargets; i++ { - name := fmt.Sprintf("//pkg:target%d", i) - result.TargetNames[i] = name - result.Targets[name] = &targethasher.Target{Name: name, Hash: []byte{0}, RuleType: "go_library"} - } - - targets, meta, err := ResultToTargetGraph(context.Background(), result) - require.NoError(t, err) - - // Get baseline proto size per target for budget calculation. - baseline, err := ChunkTargetGraph(targets, meta, 1<<30) - require.NoError(t, err) - protoResp := GetTargetGraphResponseToProto(&baseline[0]) - protoTargets := protoResp.GetItem().(*tangopb.GetTargetGraphResponse_Targets) - targetBytes := protoTargets.Targets.Targets[0].Size() - require.NotZero(t, targetBytes) - - tests := []struct { - name string - maxMessageBytes int - wantTargetChunks int - }{ - {name: "25 per chunk", maxMessageBytes: targetBytes * 25, wantTargetChunks: 2}, - {name: "10 per chunk", maxMessageBytes: targetBytes * 10, wantTargetChunks: 5}, - {name: "all in one chunk", maxMessageBytes: targetBytes * 100, wantTargetChunks: 1}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - chunks, err := ChunkTargetGraph(targets, meta, tt.maxMessageBytes) - require.NoError(t, err) - - var targetChunks, totalTargets int - for _, c := range chunks { - if len(c.Targets) > 0 || c.Metadata == nil { - targetChunks++ - totalTargets += len(c.Targets) - } - } - assert.Equal(t, tt.wantTargetChunks, targetChunks) - assert.Equal(t, numTargets, totalTargets) - }) - } -} - func TestGetTargetGraphResponseToProto_RoundTrip(t *testing.T) { t.Parallel() From 5e1ec556dba9472ebd8cdbef837f360d686b6819 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Mon, 20 Jul 2026 12:50:56 -0700 Subject: [PATCH 3/4] =?UTF-8?q?refactor(mapper):=20remove=20proto=E2=86=92?= =?UTF-8?q?entity=20conversion=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove protoToOptimizedTarget, protoToMetadata, and round-trip tests that converted back from proto. Replace with direct proto field assertions. Use t.Context() instead of context.Background(). Co-Authored-By: Claude Sonnet 5 --- internal/mapper/target_graph.go | 22 ---------------- internal/mapper/target_graph_test.go | 38 +++++++++++----------------- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/internal/mapper/target_graph.go b/internal/mapper/target_graph.go index 1cddcb2b..238f0933 100644 --- a/internal/mapper/target_graph.go +++ b/internal/mapper/target_graph.go @@ -161,25 +161,3 @@ func optimizedTargetToProto(t *entity.OptimizedTarget) *tangopb.OptimizedTarget } } -func protoToOptimizedTarget(t *tangopb.OptimizedTarget) entity.OptimizedTarget { - return entity.OptimizedTarget{ - ID: t.GetId(), - Hash: t.GetHash(), - DirectDependencies: t.GetDirectDependencies(), - RuleType: t.GetRuleType(), - Tags: t.GetTags(), - Root: t.GetRoot(), - External: t.GetExternal(), - Attributes: t.GetAttributes(), - } -} - -func protoToMetadata(m *tangopb.Metadata) *entity.Metadata { - return &entity.Metadata{ - TargetIDMapping: m.GetTargetIdMapping(), - RuleTypeMapping: m.GetRuleTypeMapping(), - TagMapping: m.GetTagMapping(), - AttributeNameMapping: m.GetAttributeNameMapping(), - AttributeStringValueMapping: m.GetAttributeStringValueMapping(), - } -} diff --git a/internal/mapper/target_graph_test.go b/internal/mapper/target_graph_test.go index cc61f125..d66b68ba 100644 --- a/internal/mapper/target_graph_test.go +++ b/internal/mapper/target_graph_test.go @@ -1,7 +1,6 @@ package mapper import ( - "context" "testing" "github.com/stretchr/testify/assert" @@ -92,13 +91,13 @@ func TestProtoToGetTargetGraphRequest(t *testing.T) { func TestResultToTargetGraph_EmptyResult(t *testing.T) { t.Parallel() - targets, meta, err := ResultToTargetGraph(context.Background(), targethasher.Result{}) + targets, meta, err := ResultToTargetGraph(t.Context(), targethasher.Result{}) require.NoError(t, err) assert.Empty(t, targets) assert.NotNil(t, meta) } -func TestGetTargetGraphResponseToProto_RoundTrip(t *testing.T) { +func TestGetTargetGraphResponseToProto_Targets(t *testing.T) { t.Parallel() chunk := entity.GetTargetGraphResponse{ @@ -109,11 +108,18 @@ func TestGetTargetGraphResponseToProto_RoundTrip(t *testing.T) { } proto := GetTargetGraphResponseToProto(&chunk) - roundTripped := protoToEntityResponse(proto) - assert.Equal(t, chunk, roundTripped) + targets := proto.GetItem().(*tangopb.GetTargetGraphResponse_Targets) + require.Len(t, targets.Targets.GetTargets(), 2) + + got := targets.Targets.GetTargets()[0] + assert.Equal(t, int32(1), got.GetId()) + assert.Equal(t, "ab", got.GetHash()) + assert.Equal(t, []int32{2}, got.GetDirectDependencies()) + assert.Equal(t, int32(10), got.GetRuleType()) + assert.True(t, got.GetRoot()) } -func TestGetTargetGraphResponseToProto_Metadata_RoundTrip(t *testing.T) { +func TestGetTargetGraphResponseToProto_Metadata(t *testing.T) { t.Parallel() chunk := entity.GetTargetGraphResponse{ @@ -124,21 +130,7 @@ func TestGetTargetGraphResponseToProto_Metadata_RoundTrip(t *testing.T) { } proto := GetTargetGraphResponseToProto(&chunk) - roundTripped := protoToEntityResponse(proto) - assert.Equal(t, chunk, roundTripped) -} - -func protoToEntityResponse(resp *tangopb.GetTargetGraphResponse) entity.GetTargetGraphResponse { - switch item := resp.GetItem().(type) { - case *tangopb.GetTargetGraphResponse_Targets: - targets := make([]entity.OptimizedTarget, len(item.Targets.GetTargets())) - for i, t := range item.Targets.GetTargets() { - targets[i] = protoToOptimizedTarget(t) - } - return entity.GetTargetGraphResponse{Targets: targets} - case *tangopb.GetTargetGraphResponse_Metadata: - return entity.GetTargetGraphResponse{Metadata: protoToMetadata(item.Metadata)} - default: - return entity.GetTargetGraphResponse{} - } + meta := proto.GetItem().(*tangopb.GetTargetGraphResponse_Metadata) + assert.Equal(t, map[int32]string{1: "//pkg:a", 2: "//pkg:b"}, meta.Metadata.GetTargetIdMapping()) + assert.Equal(t, map[int32]string{10: "go_library"}, meta.Metadata.GetRuleTypeMapping()) } From 814e9715a8001dfd0f822158447cc9794b0e6c34 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Mon, 20 Jul 2026 15:09:15 -0700 Subject: [PATCH 4/4] refactor(mapper): extract ResultToTargetGraph to top-level mapper package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move ResultToTargetGraph and idmapper out of internal/ so external consumers (e.g. the internal monorepo compute package) can import them. Proto↔entity conversion helpers remain in internal/mapper. Also fixes trailing newline lint issue in target_graph.go. Co-Authored-By: Claude Sonnet 5 --- internal/mapper/BUILD.bazel | 4 -- internal/mapper/target_graph.go | 92 -------------------------- internal/mapper/target_graph_test.go | 10 --- mapper/BUILD.bazel | 25 +++++++ mapper/mapper.go | 97 ++++++++++++++++++++++++++++ mapper/mapper_test.go | 19 ++++++ 6 files changed, 141 insertions(+), 106 deletions(-) create mode 100644 mapper/BUILD.bazel create mode 100644 mapper/mapper.go create mode 100644 mapper/mapper_test.go diff --git a/internal/mapper/BUILD.bazel b/internal/mapper/BUILD.bazel index 43434021..65bf5afd 100644 --- a/internal/mapper/BUILD.bazel +++ b/internal/mapper/BUILD.bazel @@ -12,11 +12,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//core/errors", - "//core/targethasher", "//entity", - "//internal/mapper/idmapper", "//tangopb", - "@com_github_bazelbuild_buildtools//build_proto", "@org_uber_go_yarpc//encoding/protobuf", "@org_uber_go_yarpc//yarpcerrors", ], @@ -32,7 +29,6 @@ go_test( embed = [":mapper"], deps = [ "//core/errors", - "//core/targethasher", "//entity", "//tangopb", "@com_github_stretchr_testify//assert", diff --git a/internal/mapper/target_graph.go b/internal/mapper/target_graph.go index 238f0933..4ff3aa28 100644 --- a/internal/mapper/target_graph.go +++ b/internal/mapper/target_graph.go @@ -1,19 +1,12 @@ package mapper import ( - "context" - "encoding/hex" "errors" - buildpb "github.com/bazelbuild/buildtools/build_proto" - "github.com/uber/tango/core/targethasher" "github.com/uber/tango/entity" - "github.com/uber/tango/internal/mapper/idmapper" "github.com/uber/tango/tangopb" ) -const cancelCheckInterval = 4096 - // ProtoToGetTargetGraphRequest converts a proto GetTargetGraphRequest to the // domain type. Returns an error if req is nil or its BuildDescription fails // validation (see ProtoToBuildDescription). @@ -32,90 +25,6 @@ func ProtoToGetTargetGraphRequest(req *tangopb.GetTargetGraphRequest) (entity.Ge }, nil } -// ResultToTargetGraph converts a targethasher.Result into ID-mapped entity -// types. Returns a flat list of targets and their accompanying metadata. -// No chunking or proto conversion is performed. -func ResultToTargetGraph(ctx context.Context, result targethasher.Result) ([]entity.OptimizedTarget, *entity.Metadata, error) { - targetNamesMapping := make(map[string]int32, len(result.TargetNames)) - for i, name := range result.TargetNames { - targetNamesMapping[name] = int32(i + 1) - } - - ruleTypeMapper := idmapper.NewMapper() - tagMapper := idmapper.NewMapper() - attrNameMapper := idmapper.NewMapper() - attrStrValMapper := idmapper.NewMapper() - - targets := make([]entity.OptimizedTarget, 0, len(result.TargetNames)) - - n := 0 - for _, name := range result.TargetNames { - t, ok := result.Targets[name] - if !ok { - continue - } - if n%cancelCheckInterval == 0 { - if err := ctx.Err(); err != nil { - return nil, nil, err - } - } - n++ - - depIDs := make([]int32, 0, len(t.Deps)) - for _, depName := range t.Deps { - if depID, ok := targetNamesMapping[depName]; ok { - depIDs = append(depIDs, depID) - } - } - - idt := entity.OptimizedTarget{ - ID: targetNamesMapping[name], - Hash: hex.EncodeToString(t.Hash), - DirectDependencies: depIDs, - Root: t.Root, - External: t.External, - } - if t.RuleType != "" { - idt.RuleType = ruleTypeMapper.ID(t.RuleType) - } - if len(t.Tags) > 0 { - tagIDs := make([]int32, 0, len(t.Tags)) - for _, tag := range t.Tags { - tagIDs = append(tagIDs, tagMapper.ID(tag)) - } - idt.Tags = tagIDs - } - if len(t.Attributes) > 0 { - attrs := make(map[int32]int32, len(t.Attributes)) - for _, attr := range t.Attributes { - if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil { - attrs[attrNameMapper.ID(*attr.Name)] = attrStrValMapper.ID(*attr.StringValue) - } - } - if len(attrs) > 0 { - idt.Attributes = attrs - } - } - - targets = append(targets, idt) - } - - targetIDToName := make(map[int32]string, len(targetNamesMapping)) - for s, id := range targetNamesMapping { - targetIDToName[id] = s - } - - meta := &entity.Metadata{ - TargetIDMapping: targetIDToName, - RuleTypeMapping: ruleTypeMapper.Invert(), - TagMapping: tagMapper.Invert(), - AttributeNameMapping: attrNameMapper.Invert(), - AttributeStringValueMapping: attrStrValMapper.Invert(), - } - - return targets, meta, nil -} - // GetTargetGraphResponseToProto converts an entity.GetTargetGraphResponse to // the corresponding proto GetTargetGraphResponse. func GetTargetGraphResponseToProto(chunk *entity.GetTargetGraphResponse) *tangopb.GetTargetGraphResponse { @@ -160,4 +69,3 @@ func optimizedTargetToProto(t *entity.OptimizedTarget) *tangopb.OptimizedTarget Attributes: t.Attributes, } } - diff --git a/internal/mapper/target_graph_test.go b/internal/mapper/target_graph_test.go index d66b68ba..ebec3962 100644 --- a/internal/mapper/target_graph_test.go +++ b/internal/mapper/target_graph_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/uber/tango/core/targethasher" "github.com/uber/tango/entity" "github.com/uber/tango/tangopb" ) @@ -88,15 +87,6 @@ func TestProtoToGetTargetGraphRequest(t *testing.T) { } } -func TestResultToTargetGraph_EmptyResult(t *testing.T) { - t.Parallel() - - targets, meta, err := ResultToTargetGraph(t.Context(), targethasher.Result{}) - require.NoError(t, err) - assert.Empty(t, targets) - assert.NotNil(t, meta) -} - func TestGetTargetGraphResponseToProto_Targets(t *testing.T) { t.Parallel() diff --git a/mapper/BUILD.bazel b/mapper/BUILD.bazel new file mode 100644 index 00000000..ea640a95 --- /dev/null +++ b/mapper/BUILD.bazel @@ -0,0 +1,25 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "mapper", + srcs = ["mapper.go"], + importpath = "github.com/uber/tango/mapper", + visibility = ["//visibility:public"], + deps = [ + "//core/targethasher", + "//entity", + "//internal/mapper/idmapper", + "@com_github_bazelbuild_buildtools//build_proto", + ], +) + +go_test( + name = "mapper_test", + srcs = ["mapper_test.go"], + embed = [":mapper"], + deps = [ + "//core/targethasher", + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + ], +) diff --git a/mapper/mapper.go b/mapper/mapper.go new file mode 100644 index 00000000..92fa0f54 --- /dev/null +++ b/mapper/mapper.go @@ -0,0 +1,97 @@ +package mapper + +import ( + "context" + "encoding/hex" + + buildpb "github.com/bazelbuild/buildtools/build_proto" + "github.com/uber/tango/core/targethasher" + "github.com/uber/tango/entity" + "github.com/uber/tango/internal/mapper/idmapper" +) + +const cancelCheckInterval = 4096 + +// ResultToTargetGraph converts a targethasher.Result into ID-mapped entity +// types. Returns a flat list of targets and their accompanying metadata. +// No chunking or proto conversion is performed. +func ResultToTargetGraph(ctx context.Context, result targethasher.Result) ([]entity.OptimizedTarget, *entity.Metadata, error) { + targetNamesMapping := make(map[string]int32, len(result.TargetNames)) + for i, name := range result.TargetNames { + targetNamesMapping[name] = int32(i + 1) + } + + ruleTypeMapper := idmapper.NewMapper() + tagMapper := idmapper.NewMapper() + attrNameMapper := idmapper.NewMapper() + attrStrValMapper := idmapper.NewMapper() + + targets := make([]entity.OptimizedTarget, 0, len(result.TargetNames)) + + n := 0 + for _, name := range result.TargetNames { + t, ok := result.Targets[name] + if !ok { + continue + } + if n%cancelCheckInterval == 0 { + if err := ctx.Err(); err != nil { + return nil, nil, err + } + } + n++ + + depIDs := make([]int32, 0, len(t.Deps)) + for _, depName := range t.Deps { + if depID, ok := targetNamesMapping[depName]; ok { + depIDs = append(depIDs, depID) + } + } + + idt := entity.OptimizedTarget{ + ID: targetNamesMapping[name], + Hash: hex.EncodeToString(t.Hash), + DirectDependencies: depIDs, + Root: t.Root, + External: t.External, + } + if t.RuleType != "" { + idt.RuleType = ruleTypeMapper.ID(t.RuleType) + } + if len(t.Tags) > 0 { + tagIDs := make([]int32, 0, len(t.Tags)) + for _, tag := range t.Tags { + tagIDs = append(tagIDs, tagMapper.ID(tag)) + } + idt.Tags = tagIDs + } + if len(t.Attributes) > 0 { + attrs := make(map[int32]int32, len(t.Attributes)) + for _, attr := range t.Attributes { + if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil { + attrs[attrNameMapper.ID(*attr.Name)] = attrStrValMapper.ID(*attr.StringValue) + } + } + if len(attrs) > 0 { + idt.Attributes = attrs + } + } + + targets = append(targets, idt) + } + + targetIDToName := make(map[int32]string, len(targetNamesMapping)) + for s, id := range targetNamesMapping { + targetIDToName[id] = s + } + + meta := &entity.Metadata{ + TargetIDMapping: targetIDToName, + RuleTypeMapping: ruleTypeMapper.Invert(), + TagMapping: tagMapper.Invert(), + AttributeNameMapping: attrNameMapper.Invert(), + AttributeStringValueMapping: attrStrValMapper.Invert(), + } + + return targets, meta, nil +} diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go new file mode 100644 index 00000000..3641f7ea --- /dev/null +++ b/mapper/mapper_test.go @@ -0,0 +1,19 @@ +package mapper + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/uber/tango/core/targethasher" +) + +func TestResultToTargetGraph_EmptyResult(t *testing.T) { + t.Parallel() + + targets, meta, err := ResultToTargetGraph(t.Context(), targethasher.Result{}) + require.NoError(t, err) + assert.Empty(t, targets) + assert.NotNil(t, meta) +}