From 6b310cebb5d5ce45eecab6975de203adf7f072e8 Mon Sep 17 00:00:00 2001
From: tidwall <joshbaker77@gmail.com>
Date: Wed, 19 Oct 2022 04:36:17 -0700
Subject: [PATCH] Fixed zero-field to deleting existing field

---
 internal/collection/collection.go | 31 ----------------------------
 internal/server/crud.go           | 16 +++++++++++++--
 tests/keys_test.go                | 34 ++++++++++++++++++++-----------
 3 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/internal/collection/collection.go b/internal/collection/collection.go
index df68aad4..b8de7951 100644
--- a/internal/collection/collection.go
+++ b/internal/collection/collection.go
@@ -166,37 +166,6 @@ func (c *Collection) Set(obj *object.Object) (prev *object.Object) {
 	return prev
 }
 
-// SetMerged works just like Set but it will merge the new object fields and
-// the previous object fields and create a newer object that is then set into
-// the collection. The newer object is returned.
-func (c *Collection) SetMerged(obj *object.Object,
-) (prev, newObj *object.Object) {
-	prev, _ = c.objs.Set(obj.ID(), obj)
-	if prev != nil {
-		// Check if at least one field exists from the previous object and
-		// merge the two field lists, then re-set the new object. Otherwise,
-		// we stick with the current object.
-		// TODO: check if the old object has fields that new object does not
-		// and only reset those.
-		ofields := prev.Fields()
-		var reset bool
-		ofields.Scan(func(f field.Field) bool {
-			reset = true
-			return false
-		})
-		if reset {
-			obj.Fields().Scan(func(f field.Field) bool {
-				ofields = ofields.Set(f)
-				return true
-			})
-			obj = object.New(obj.ID(), obj.Geo(), obj.Expires(), ofields)
-			c.objs.Set(obj.ID(), obj)
-		}
-	}
-	c.setFill(prev, obj)
-	return prev, obj
-}
-
 func (c *Collection) setFill(prev, obj *object.Object) {
 	if prev != nil {
 		if prev.IsSpatial() {
diff --git a/internal/server/crud.go b/internal/server/crud.go
index d5658edd..a0fc727d 100644
--- a/internal/server/crud.go
+++ b/internal/server/crud.go
@@ -745,8 +745,20 @@ func (s *Server) cmdSET(msg *Message) (resp.Value, commandDetails, error) {
 		}
 	}
 
-	obj := object.New(id, oobj, ex, field.MakeList(fields))
-	old, obj := col.SetMerged(obj)
+	var flist field.List
+	if len(fields) > 0 {
+		old := col.Get(id)
+		if old != nil {
+			flist = old.Fields()
+		}
+		for _, f := range fields {
+			flist = flist.Set(f)
+		}
+	} else {
+		flist = field.MakeList(fields)
+	}
+	obj := object.New(id, oobj, ex, flist)
+	old := col.Set(obj)
 
 	// >> Response
 
diff --git a/tests/keys_test.go b/tests/keys_test.go
index d172f501..44031696 100644
--- a/tests/keys_test.go
+++ b/tests/keys_test.go
@@ -438,18 +438,28 @@ func keys_SET_EX_test(mc *mockServer) (err error) {
 }
 
 func keys_FIELDS_test(mc *mockServer) error {
-	return mc.DoBatch([][]interface{}{
-		{"SET", "mykey", "myid1a", "FIELD", "a", 1, "POINT", 33, -115}, {"OK"},
-		{"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1]]`},
-		{"SET", "mykey", "myid1a", "FIELD", "a", "a", "POINT", 33, -115}, {"OK"},
-		{"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a a]]`},
-		{"SET", "mykey", "myid1a", "FIELD", "a", 1, "FIELD", "b", 2, "POINT", 33, -115}, {"OK"},
-		{"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`},
-		{"SET", "mykey", "myid1a", "FIELD", "b", 2, "POINT", 33, -115}, {"OK"},
-		{"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`},
-		{"SET", "mykey", "myid1a", "FIELD", "b", 2, "FIELD", "a", "1", "FIELD", "c", 3, "POINT", 33, -115}, {"OK"},
-		{"GET", "mykey", "myid1a", "WITHFIELDS"}, {`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2 c 3]]`},
-	})
+	return mc.DoBatch(
+		Do("SET", "mykey", "myid1a", "FIELD", "a", 1, "POINT", 33, -115).OK(),
+		Do("GET", "mykey", "myid1a", "WITHFIELDS").Str(`[{"type":"Point","coordinates":[-115,33]} [a 1]]`),
+		Do("SET", "mykey", "myid1a", "FIELD", "a", "a", "POINT", 33, -115).OK(),
+		Do("GET", "mykey", "myid1a", "WITHFIELDS").Str(`[{"type":"Point","coordinates":[-115,33]} [a a]]`),
+		Do("SET", "mykey", "myid1a", "FIELD", "a", 1, "FIELD", "b", 2, "POINT", 33, -115).OK(),
+		Do("GET", "mykey", "myid1a", "WITHFIELDS").Str(`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`),
+		Do("SET", "mykey", "myid1a", "FIELD", "b", 2, "POINT", 33, -115).OK(),
+		Do("GET", "mykey", "myid1a", "WITHFIELDS").Str(`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2]]`),
+		Do("SET", "mykey", "myid1a", "FIELD", "b", 2, "FIELD", "a", "1", "FIELD", "c", 3, "POINT", 33, -115).OK(),
+		Do("GET", "mykey", "myid1a", "WITHFIELDS").Str(`[{"type":"Point","coordinates":[-115,33]} [a 1 b 2 c 3]]`),
+
+		Do("GET", "fleet", "truck1", "WITHFIELDS").JSON().Err("key not found"),
+		Do("SET", "fleet", "truck1", "FIELD", "speed", "0", "POINT", "-112", "33").JSON().OK(),
+		Do("GET", "fleet", "truck1", "WITHFIELDS").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[33,-112]}}`),
+		Do("SET", "fleet", "truck1", "FIELD", "speed", "1", "POINT", "-112", "33").JSON().OK(),
+		Do("GET", "fleet", "truck1", "WITHFIELDS").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[33,-112]},"fields":{"speed":1}}`),
+		Do("SET", "fleet", "truck1", "FIELD", "speed", "0", "POINT", "-112", "33").JSON().OK(),
+		Do("GET", "fleet", "truck1", "WITHFIELDS").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[33,-112]}}`),
+		Do("SET", "fleet", "truck1", "FIELD", "speed", "2", "POINT", "-112", "33").JSON().OK(),
+		Do("GET", "fleet", "truck1", "WITHFIELDS").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[33,-112]},"fields":{"speed":2}}`),
+	)
 }
 
 func keys_PDEL_test(mc *mockServer) error {