From 16885b3a03d4412aee1a12611a454570ddd6041d Mon Sep 17 00:00:00 2001 From: Josh Humphries <2035234+jhump@users.noreply.github.com> Date: Fri, 23 Feb 2024 08:33:25 -0500 Subject: [PATCH] add bufimage.CloneImage --- private/bufpkg/bufimage/bufimage.go | 39 ++++++++ private/bufpkg/bufimage/image_test.go | 122 ++++++++++++++++++++++++++ 2 files changed, 161 insertions(+) diff --git a/private/bufpkg/bufimage/bufimage.go b/private/bufpkg/bufimage/bufimage.go index 4bf5e36e03..eac7619c94 100644 --- a/private/bufpkg/bufimage/bufimage.go +++ b/private/bufpkg/bufimage/bufimage.go @@ -23,6 +23,7 @@ import ( "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/descriptorpb" "google.golang.org/protobuf/types/pluginpb" ) @@ -122,6 +123,44 @@ func NewImage(imageFiles []ImageFile) (Image, error) { return newImage(imageFiles, false) } +// CloneImage returns a deep copy of the given image. +func CloneImage(image Image) (Image, error) { + originalFiles := image.Files() + imageFiles := make([]ImageFile, len(originalFiles)) + for i, originalFile := range originalFiles { + clonedFile, err := CloneImageFile(originalFile) + if err != nil { + return nil, err + } + imageFiles[i] = clonedFile + } + return NewImage(imageFiles) +} + +// CloneImageFile returns a deep copy of the given image file. +func CloneImageFile(imageFile ImageFile) (ImageFile, error) { + clonedProto := proto.Clone(imageFile.FileDescriptorProto()) + clonedDescriptor, ok := clonedProto.(*descriptorpb.FileDescriptorProto) + if !ok { + // Shouldn't actually be possible... + return nil, fmt.Errorf("failed to clone image file %q: input %T; clone is %T but expecting %T", + imageFile.Path(), imageFile, clonedProto, (*descriptorpb.FileDescriptorProto)(nil)) + } + originalUnusedDeps := imageFile.UnusedDependencyIndexes() + unusedDeps := make([]int32, len(originalUnusedDeps)) + copy(unusedDeps, originalUnusedDeps) + // The other attributes are already immutable, so we don't need to copy them. + return NewImageFile( + clonedDescriptor, + imageFile.ModuleIdentity(), + imageFile.Commit(), + imageFile.ExternalPath(), + imageFile.IsImport(), + imageFile.IsSyntaxUnspecified(), + unusedDeps, + ) +} + // MergeImages returns a new Image for the given Images. ImageFiles // treated as non-imports in at least one of the given Images will // be treated as non-imports in the returned Image. The first non-import diff --git a/private/bufpkg/bufimage/image_test.go b/private/bufpkg/bufimage/image_test.go index 1265b7d507..b1c15cdeb8 100644 --- a/private/bufpkg/bufimage/image_test.go +++ b/private/bufpkg/bufimage/image_test.go @@ -15,12 +15,16 @@ package bufimage import ( + "reflect" "testing" imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/types/descriptorpb" ) func TestMergeImagesWithImports(t *testing.T) { @@ -137,3 +141,121 @@ func TestMergeImagesOrdered(t *testing.T) { } assert.Equal(t, []string{"a.proto", "b.proto", "c.proto", "d.proto"}, paths) } + +func TestCloneImage(t *testing.T) { + t.Parallel() + protoImage := &imagev1.Image{ + File: []*imagev1.ImageFile{ + { + Syntax: proto.String("proto3"), + Name: proto.String("a.proto"), + Dependency: []string{"b.proto", "c.proto"}, + Package: proto.String("abc.def"), + MessageType: []*descriptorpb.DescriptorProto{ + { + Name: proto.String("Msg"), + Field: []*descriptorpb.FieldDescriptorProto{ + { + Name: proto.String("id"), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Number: proto.Int32(1), + Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(), + JsonName: proto.String("id"), + }, + { + Name: proto.String("en"), + Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(), + Number: proto.Int32(2), + Type: descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(), + TypeName: proto.String(".abc.def.Enum"), + JsonName: proto.String("en"), + }, + }, + }, + }, + BufExtension: &imagev1.ImageFileExtension{ + IsImport: proto.Bool(false), + IsSyntaxUnspecified: proto.Bool(false), + UnusedDependency: []int32{1}, + ModuleInfo: &imagev1.ModuleInfo{ + Name: &imagev1.ModuleName{ + Remote: proto.String("buf.build"), + Owner: proto.String("foo"), + Repository: proto.String("bar"), + }, + }, + }, + }, + { + Syntax: proto.String("proto3"), + Name: proto.String("b.proto"), + Package: proto.String("abc.def"), + EnumType: []*descriptorpb.EnumDescriptorProto{ + { + Name: proto.String("Enum"), + Value: []*descriptorpb.EnumValueDescriptorProto{ + { + Name: proto.String("ZERO"), + Number: proto.Int32(0), + }, + { + Name: proto.String("ONE"), + Number: proto.Int32(1), + }, + }, + }, + }, + BufExtension: &imagev1.ImageFileExtension{ + IsImport: proto.Bool(true), + IsSyntaxUnspecified: proto.Bool(false), + ModuleInfo: &imagev1.ModuleInfo{ + Name: &imagev1.ModuleName{ + Remote: proto.String("buf.build"), + Owner: proto.String("foo"), + Repository: proto.String("baz"), + }, + }, + }, + }, + { + Syntax: proto.String("proto2"), + Name: proto.String("c.proto"), + BufExtension: &imagev1.ImageFileExtension{ + IsImport: proto.Bool(true), + IsSyntaxUnspecified: proto.Bool(true), + ModuleInfo: &imagev1.ModuleInfo{ + Name: &imagev1.ModuleName{ + Remote: proto.String("buf.build"), + Owner: proto.String("foo"), + Repository: proto.String("baz"), + }, + }, + }, + }, + }, + } + + image, err := NewImageForProto(protoImage) + require.NoError(t, err) + + clone, err := CloneImage(image) + require.NoError(t, err) + + // Test that they are equal by comparing their proto versions + protoClone := ImageToProtoImage(clone) + + require.Empty(t, cmp.Diff(protoImage, protoClone, protocmp.Transform())) + + // Verify the pointer values are different + for _, imageFile := range image.Files() { + cloneFile := clone.GetFile(imageFile.Path()) + require.NotSame(t, imageFile.FileDescriptorProto(), cloneFile.FileDescriptorProto()) + unused := reflect.ValueOf(imageFile.UnusedDependencyIndexes()).Pointer() + cloneUnused := reflect.ValueOf(cloneFile.UnusedDependencyIndexes()).Pointer() + if unused != 0 || cloneUnused != 0 { + // They can both be nil. But otherwise must not be equal since that + // means the backing arrays are shared. + require.NotEqual(t, unused, cloneUnused) + } + } +}