Updating UpdateList to update the values on a list

pkg/sdkserver/sdkserver.go

@@ -23,8 +23,10 @@ import (
 	"sync"
 	"time"
 
+	"github.com/mennanov/fmutils"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"google.golang.org/protobuf/proto"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1066,6 +1068,12 @@ func (s *SDKServer) UpdateList(ctx context.Context, in *beta.UpdateListRequest)
 	if in == nil {
 		return nil, errors.Errorf("UpdateListRequest cannot be nil")
 	}
+	if in.List == nil || in.UpdateMask == nil {
+		return nil, errors.Errorf("invalid argument. List: %v and UpdateMask %v cannot be nil", in.List, in.UpdateMask)
+	}
+	if !in.UpdateMask.IsValid(in.List.ProtoReflect().Interface()) {
+		return nil, errors.Errorf("invalid argument. Field Mask Path(s): %v are invalid for List. Use valid field name(s): %v", in.UpdateMask.GetPaths(), in.List.ProtoReflect().Descriptor().Fields())
+	}
 
 	name := in.List.Name
 	s.logger.WithField("name", name).Debug("Update List -- Currently only used for Updating Capacity")
@@ -1077,18 +1085,36 @@ func (s *SDKServer) UpdateList(ctx context.Context, in *beta.UpdateListRequest)
 
 	s.gsUpdateMutex.Lock()
 	defer s.gsUpdateMutex.Unlock()
+	gsCopy := gs.DeepCopy()
 
 	if in.List.Capacity < 0 || in.List.Capacity > apiserver.ListMaxCapacity {
 		return nil, errors.Errorf("out of range. Capacity must be within range [0,1000]. Found Capacity: %d", in.List.Capacity)
 	}
 
-	if _, ok := gs.Status.Lists[name]; ok {
-		batchList := s.gsListUpdates[name]
-		batchList.capacitySet = &in.List.Capacity
-		s.gsListUpdates[name] = batchList
-		// Queue up the Update for later batch processing by updateLists.
-		s.workerqueue.Enqueue(cache.ExplicitKey(updateLists))
-		return &beta.List{}, nil
+	if list, ok := gsCopy.Status.Lists[name]; ok {
+		//capacity := list.Capacity
+		//s.logger.WithField("name", capacity).Debug("Update List -- Currently only used for Updating Capacity")
+
+		// Create *beta.List from *sdk.GameServer_Status_ListStatus for merging.
+		tmpList := &beta.List{Name: name, Capacity: list.Capacity, Values: list.Values}
+		// Removes any fields from the request object that are not included in the FieldMask Paths.
+		fmutils.Filter(in.List, in.UpdateMask.Paths)
+		// Removes any fields from the existing gameserver object that are included in the FieldMask Paths.
+		fmutils.Prune(tmpList, in.UpdateMask.Paths)
+		// Due due filtering and pruning all gameserver object field(s) contained in the FieldMask are overwritten by the request object field(s).
+		proto.Merge(tmpList, in.List)
+		// Silently truncate list values if Capacity < len(Values)
+		if tmpList.Capacity < int64(len(tmpList.Values)) {
+			tmpList.Values = append([]string{}, tmpList.Values[:tmpList.Capacity]...)
+		}
+
+		list.Capacity = tmpList.Capacity
+		list.Values = tmpList.Values
+		gsCopy.Status.Lists[name] = list
+		s.patchGameServer(ctx, gs, gsCopy)
+		gs.Status.Lists[name] = list // without this line all the unit tests fail. Does s.PatchGameServer really work as intended in a test environment?
+
+		return &beta.List{Name: name, Capacity: gs.Status.Lists[name].Capacity, Values: gs.Status.Lists[name].Values}, nil
 	}
 	return nil, errors.Errorf("not found. %s List not found", name)
 }

pkg/sdkserver/sdkserver_test.go

@@ -1674,9 +1674,11 @@ func TestSDKServerUpdateList(t *testing.T) {
 	require.NoError(t, err, "Can not parse FeatureCountsAndLists feature")
 
 	lists := map[string]agonesv1.ListStatus{
-		"foo": {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
-		"bar": {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
-		"baz": {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
+		"foo":  {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
+		"bar":  {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
+		"baz":  {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
+		"qux":  {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
+		"quux": {Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)},
 	}
 
 	fixtures := map[string]struct {
@@ -1729,6 +1731,36 @@ func TestSDKServerUpdateList(t *testing.T) {
 			updated:                 false,
 			expectedUpdatesQueueLen: 0,
 		},
+		"update values below capacity": {
+			listName: "qux",
+			request: &beta.UpdateListRequest{
+				List: &beta.List{
+					Name:     "qux",
+					Capacity: int64(100),
+					Values:   []string{"one", "two", "three", "four", "five", "six"},
+				},
+				UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"capacity", "values"}},
+			},
+			want:                    agonesv1.ListStatus{Values: []string{"one", "two", "three", "four", "five", "six"}, Capacity: int64(100)},
+			updateErr:               false,
+			updated:                 true,
+			expectedUpdatesQueueLen: 0,
+		},
+		"update values above capacity": {
+			listName: "quux",
+			request: &beta.UpdateListRequest{
+				List: &beta.List{
+					Name:     "quux",
+					Capacity: int64(4),
+					Values:   []string{"one", "two", "three", "four", "five", "six"},
+				},
+				UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"capacity", "values"}},
+			},
+			want:                    agonesv1.ListStatus{Values: []string{"one", "two", "three", "four"}, Capacity: int64(4)},
+			updateErr:               false,
+			updated:                 true,
+			expectedUpdatesQueueLen: 0,
+		},
 	}
 
 	// nolint:dupl  // Linter errors on lines are duplicate of TestSDKServerAddListValue, TestSDKServerRemoveListValue

site/content/en/docs/Guides/Client SDKs/rest.md

@@ -296,35 +296,17 @@ Response:
 ```
 
 ##### Beta: UpdateList
-This function updates the list's properties with the key `players`, such as its capacity and values, and returns the updated list details.
+This function updates the list's properties with the key `players`, such as its capacity and values, and returns the updated list details. Use `AddListValue` or `RemoveListValue` for modifying the `List.Values` field.
 
-**Note:** The behavior of this function differs between the Local SDK Server and the SDK Server.
-
-- **Local SDK Server:** This will overwrite both the capacity and the values with the update request values. Use `AddListValue` or `RemoveListValue` for modifying the `List.Values` field.
-- **SDK Server:** This will only update the capacity of the list and does not modify the values.
 
 ###### Example
-**Local SDK Server Example:**
-
 ```bash
 curl -d '{"capacity": "120", "values": ["player3", "player4"]}' -H "Content-Type: application/json" -X PATCH http://localhost:${AGONES_SDK_HTTP_PORT}/v1beta1/lists/players
 ```
-
 Response:
 ```json
 {"name":"players", "capacity":"120", "values":["player3", "player4"]}
 ```
-**SDK Server Example:**
-
-```bash
-curl -d '{"capacity": "120"}' -H "Content-Type: application/json" -X PATCH http://localhost:${AGONES_SDK_HTTP_PORT}/v1beta1/lists/players
-```
-
-Response:
-```json
-{"name":"players", "capacity":"120", "values":["player0", "player1", "player2"]}
-```
-
 ##### Beta: AddListValue
 This function adds a new value to a list with the key `players` and returns the list with this addition.