diff --git a/pkg/sdkserver/sdkserver.go b/pkg/sdkserver/sdkserver.go index eee6588455..043db885a3 100644 --- a/pkg/sdkserver/sdkserver.go +++ b/pkg/sdkserver/sdkserver.go @@ -19,10 +19,12 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "sync" "time" + "github.com/mennanov/fmutils" "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -1066,31 +1068,79 @@ 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") + 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) + } - gs, err := s.gameServer() + list, err := s.GetList(ctx, &beta.GetListRequest{Name: in.List.Name}) if err != nil { - return nil, err + + return nil, errors.Errorf("not found. %s List not found", list.Name) } s.gsUpdateMutex.Lock() defer s.gsUpdateMutex.Unlock() - 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) - } + // Removes any fields from the request object that are not included in the FieldMask Paths. + fmutils.Filter(in.List, in.UpdateMask.Paths) + + // The list will allow the current list to be overwritten + batchList := listUpdateRequest{} - if _, ok := gs.Status.Lists[name]; ok { - batchList := s.gsListUpdates[name] + // Only set the capacity if its included in the update mask paths + if slices.Contains(in.UpdateMask.Paths, "capacity") { 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 } - return nil, errors.Errorf("not found. %s List not found", name) + + // Only change the values if its included in the update mask paths + if slices.Contains(in.UpdateMask.Paths, "values") { + currList := list + + // Find values to remove from the current list + valuesToDelete := map[string]bool{} + for _, value := range currList.Values { + valueFound := false + for _, element := range in.List.Values { + if value == element { + valueFound = true + } + } + + if !valueFound { + valuesToDelete[value] = true + } + } + batchList.valuesToDelete = valuesToDelete + + // Find values that need to be added to the current list from the incomming list + valuesToAdd := []string{} + for _, value := range in.List.Values { + valueFound := false + for _, element := range currList.Values { + if value == element { + valueFound = true + } + } + + if !valueFound { + valuesToAdd = append(valuesToAdd, value) + } + } + batchList.valuesToAppend = valuesToAdd + } + + // Queue up the Update for later batch processing by updateLists. + s.gsListUpdates[list.Name] = batchList + s.workerqueue.Enqueue(cache.ExplicitKey(updateLists)) + return &beta.List{}, nil + } // AddListValue collapses all append a value to the end of a List requests into a single UpdateList request. diff --git a/pkg/sdkserver/sdkserver_test.go b/pkg/sdkserver/sdkserver_test.go index 8809db8488..441e80a90c 100644 --- a/pkg/sdkserver/sdkserver_test.go +++ b/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,37 @@ func TestSDKServerUpdateList(t *testing.T) { updated: false, expectedUpdatesQueueLen: 0, }, + // New test cases to test updating values + "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 diff --git a/site/content/en/docs/Guides/Client SDKs/rest.md b/site/content/en/docs/Guides/Client SDKs/rest.md index 29c1d16dd6..a706602f13 100644 --- a/site/content/en/docs/Guides/Client SDKs/rest.md +++ b/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.