Fix the bug that property merge new tags failed (apache#347)

zesiar0 · Oct 31, 2023 · facef3d · facef3d
1 parent 0b55f26
commit facef3d
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 5 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -9,6 +9,10 @@ Release Notes.
 - Support etcd client authentication.
 - Implement Local file system.
 
+### Bugs
+
+- Fix the bug that property merge new tags failed.
+
 ## 0.5.0
 
 ### Features

diff --git a/banyand/metadata/schema/property.go b/banyand/metadata/schema/property.go
@@ -177,16 +177,20 @@ func (e *etcdSchemaRegistry) mergeProperty(ctx context.Context, key string, prop
 		return false, 0, 0, err
 	}
 	merge := func(existed *propertyv1.Property) (*propertyv1.Property, error) {
-		tags := make([]*modelv1.Tag, len(property.Tags))
-		copy(tags, property.Tags)
+		tags := make([]*modelv1.Tag, 0)
 		for i := 0; i < int(tagsNum); i++ {
-			t := tags[0]
-			tags = tags[1:]
+			t := property.Tags[i]
+			tagExisted := false
 			for _, et := range existed.Tags {
 				if et.Key == t.Key {
 					et.Value = t.Value
+					tagExisted = true
+					break
 				}
 			}
+			if !tagExisted {
+				tags = append(tags, t)
+			}
 		}
 		existed.Tags = append(existed.Tags, tags...)
 		existed.LeaseId = leaseID

diff --git a/pkg/node/maglev_test.go b/pkg/node/maglev_test.go
@@ -30,7 +30,7 @@ import (
 
 const (
 	dataNodeTemplate = "data-node-%d"
-	targetEpsilon    = 0.03
+	targetEpsilon    = 0.1
 )
 
 func TestMaglevSelector(t *testing.T) {

diff --git a/test/integration/standalone/other/property_test.go b/test/integration/standalone/other/property_test.go
@@ -98,6 +98,75 @@ var _ = Describe("Property application", func() {
 			{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v22"}}}},
 		}))
 	})
+	It("applies properties with new tags", func() {
+		md := &propertyv1.Metadata{
+			Container: &commonv1.Metadata{
+				Name:  "p",
+				Group: "g",
+			},
+			Id: "1",
+		}
+		resp, err := client.Apply(context.Background(), &propertyv1.ApplyRequest{Property: &propertyv1.Property{
+			Metadata: md,
+			Tags: []*modelv1.Tag{
+				{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v1"}}}},
+			},
+		}})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(resp.Created).To(BeTrue())
+		Expect(resp.TagsNum).To(Equal(uint32(1)))
+		resp, err = client.Apply(context.Background(), &propertyv1.ApplyRequest{Property: &propertyv1.Property{
+			Metadata: md,
+			Tags: []*modelv1.Tag{
+				{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v2"}}}},
+				{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v22"}}}},
+			},
+		}})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(resp.Created).To(BeFalse())
+		Expect(resp.TagsNum).To(Equal(uint32(2)))
+		got, err := client.Get(context.Background(), &propertyv1.GetRequest{Metadata: md})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(got.Property.Tags).To(Equal([]*modelv1.Tag{
+			{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v2"}}}},
+			{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v22"}}}},
+		}))
+	})
+	It("applies null tag", func() {
+		md := &propertyv1.Metadata{
+			Container: &commonv1.Metadata{
+				Name:  "p",
+				Group: "g",
+			},
+			Id: "1",
+		}
+		resp, err := client.Apply(context.Background(), &propertyv1.ApplyRequest{Property: &propertyv1.Property{
+			Metadata: md,
+			Tags: []*modelv1.Tag{
+				{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v1"}}}},
+				{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Null{}}},
+			},
+		}})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(resp.Created).To(BeTrue())
+		Expect(resp.TagsNum).To(Equal(uint32(2)))
+		resp, err = client.Apply(context.Background(), &propertyv1.ApplyRequest{Property: &propertyv1.Property{
+			Metadata: md,
+			Tags: []*modelv1.Tag{
+				{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v2"}}}},
+				{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v22"}}}},
+			},
+		}})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(resp.Created).To(BeFalse())
+		Expect(resp.TagsNum).To(Equal(uint32(2)))
+		got, err := client.Get(context.Background(), &propertyv1.GetRequest{Metadata: md})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(got.Property.Tags).To(Equal([]*modelv1.Tag{
+			{Key: "t1", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v2"}}}},
+			{Key: "t2", Value: &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: "v22"}}}},
+		}))
+	})
 	It("applies a property with TTL", func() {
 		md := &propertyv1.Metadata{
 			Container: &commonv1.Metadata{