From 8b2207e6735fb6885b0bebee6db883432f4d8852 Mon Sep 17 00:00:00 2001
From: Oscar Muhr <oscar.muhr@einride.tech>
Date: Fri, 3 Nov 2023 08:20:33 +0100
Subject: [PATCH] feat(fieldbehavior): ignore immutable in validation if not in
 field mask

AIP-203
"When a service receives an immutable field in an update request (or similar),
even if included in the update mask, the service should ignore the field if the value matches,
but should error with INVALID_ARGUMENT if a change is requested"
---
 fieldbehavior/fieldbehavior_test.go | 18 ++++++++++++++++--
 fieldbehavior/immutable.go          |  4 ++--
 fieldbehavior/required.go           |  9 +++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/fieldbehavior/fieldbehavior_test.go b/fieldbehavior/fieldbehavior_test.go
index 9934f43cd5..19f4bc91e5 100644
--- a/fieldbehavior/fieldbehavior_test.go
+++ b/fieldbehavior/fieldbehavior_test.go
@@ -141,6 +141,20 @@ func TestValidateImmutableFieldsWithMask(t *testing.T) {
 		err := ValidateImmutableFieldsWithMask(req, req.GetUpdateMask())
 		assert.NilError(t, err)
 	})
+	t.Run("no error when immutable field not part of fieldmask", func(t *testing.T) {
+		t.Parallel()
+		req := &examplefreightv1.UpdateShipmentRequest{
+			Shipment: &examplefreightv1.Shipment{
+				ExternalReferenceId: "external-reference-id",
+				OriginSite:          "shippers/shipper1/sites/site1",
+			},
+			UpdateMask: &fieldmaskpb.FieldMask{
+				Paths: []string{"shipment.origin_site"},
+			},
+		}
+		err := ValidateImmutableFieldsWithMask(req, req.GetUpdateMask())
+		assert.NilError(t, err)
+	})
 	t.Run("errors when wildcard fieldmask used", func(t *testing.T) {
 		t.Parallel()
 		req := &examplefreightv1.UpdateShipmentRequest{
@@ -170,7 +184,7 @@ func TestValidateImmutableFieldsWithMask(t *testing.T) {
 				ExternalReferenceId: "I am immutable!",
 			},
 			UpdateMask: &fieldmaskpb.FieldMask{
-				Paths: []string{""},
+				Paths: []string{},
 			},
 		}
 		err := ValidateImmutableFieldsWithMask(req, req.GetUpdateMask())
@@ -187,7 +201,7 @@ func TestValidateImmutableFieldsWithMask(t *testing.T) {
 				},
 			},
 			UpdateMask: &fieldmaskpb.FieldMask{
-				Paths: []string{""},
+				Paths: []string{},
 			},
 		}
 		err := ValidateImmutableFieldsWithMask(req, req.GetUpdateMask())
diff --git a/fieldbehavior/immutable.go b/fieldbehavior/immutable.go
index b8b9144093..33f64f45a5 100644
--- a/fieldbehavior/immutable.go
+++ b/fieldbehavior/immutable.go
@@ -10,7 +10,7 @@ import (
 )
 
 // ValidateImmutableFieldsWithMask returns a validation error if the message
-// or field mask contains a field that is immutable.
+// or field mask contains a field that is immutable and a change to an immutable field is requested.
 // This can be used when validating update requests and want to return
 // INVALID_ARGUMENT to the user.
 // If you want to ignore immutable fields rather than error then use ClearFields().
@@ -29,7 +29,7 @@ func validateImmutableFields(m protoreflect.Message, mask *fieldmaskpb.FieldMask
 		}
 
 		currPath += string(field.Name())
-		if isImmutable(field) && (hasPath(mask, currPath) || m.Has(field)) {
+		if isImmutable(field) && hasPath(mask, currPath) {
 			return fmt.Errorf("field is immutable: %s", currPath)
 		}
 
diff --git a/fieldbehavior/required.go b/fieldbehavior/required.go
index 5205c9cc26..3ddd2768d3 100644
--- a/fieldbehavior/required.go
+++ b/fieldbehavior/required.go
@@ -76,7 +76,16 @@ func validateRequiredFields(reflectMessage protoreflect.Message, mask *fieldmask
 	return nil
 }
 
+func isEmpty(mask *fieldmaskpb.FieldMask) bool {
+	paths := mask.GetPaths()
+	return mask == nil ||
+		len(paths) == 0
+}
+
 func hasPath(mask *fieldmaskpb.FieldMask, needle string) bool {
+	if isEmpty(mask) {
+		return true
+	}
 	for _, straw := range mask.GetPaths() {
 		if straw == "*" || straw == needle {
 			return true