Merge pull request #6 from ivixvi/feat-attibutes-changed

feat: check attributes is changed
ivixvi · Jul 8, 2024 · 2803587 · 2803587
2 parents c57df3c + 88ac1a0
commit 2803587
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 29 deletions.
diff --git a/example/handlers.go b/example/handlers.go
@@ -96,17 +96,20 @@ func (h testResourceHandler) Patch(r *http.Request, id string, operations []scim
 
 	// Apply PATCH operations
 	var err error
+	var changed bool
 	for _, op := range operations {
-		data.resourceAttributes, err = h.patcher.Apply(op, data.resourceAttributes)
+		data.resourceAttributes, changed, err = h.patcher.Apply(op, data.resourceAttributes)
 		if err != nil {
 			return scim.Resource{}, err
 		}
 	}
 
 	// store resource
-	now := time.Now()
-	data.meta.LastModified = &now
-	h.data[id] = data
+	if changed {
+		now := time.Now()
+		data.meta.LastModified = &now
+		h.data[id] = data
+	}
 
 	return scim.Resource{
 		ID:         id,

diff --git a/scimpatch.go b/scimpatch.go
@@ -1,6 +1,7 @@
 package scimpatch
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/elimity-com/scim"
@@ -26,9 +27,9 @@ func NewPatcher(s schema.Schema, extentions []schema.Schema) *Patcher {
 }
 
 // Apply は RFC7644 3.5.2.  Modifying with PATCH の実装です。
-// data に op が適用された ResourceAttributes を返却します。
+// data に op が適用された ResourceAttributes と実際に適用されたかどうかの真偽値を返却します。
 // see. https://datatracker.ietf.org/doc/html/rfc7644#section-3.5.2
-func (p *Patcher) Apply(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, error) {
+func (p *Patcher) Apply(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, bool, error) {
 	switch strings.ToLower(op.Op) {
 	case scim.PatchOperationAdd:
 		return p.add(op, data)
@@ -37,50 +38,55 @@ func (p *Patcher) Apply(op scim.PatchOperation, data scim.ResourceAttributes) (s
 	case scim.PatchOperationRemove:
 		return p.remove(op, data)
 	}
-	return data, nil
+	return data, false, nil
 }
 
 // add は RFC7644 3.5.2.1. Add Operation の実装です。
-// data に op が適用された ResourceAttributes を返却します。
+// data に op が適用された ResourceAttributes と実際に適用されたかどうかの真偽値を返却します。
 // see. https://datatracker.ietf.org/doc/html/rfc7644#section-3.5.2.1
 // 基本は Validated な op を想定しているため、エラーハンドリングは属性を確認するうえで対応することになる最小限のチェックとなっています。
-func (p *Patcher) add(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, error) {
-	return data, nil
+func (p *Patcher) add(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, bool, error) {
+	return data, false, nil
 }
 
 // replace は RFC7644 3.5.2.3. Replace Operation の実装です。
-// data に op が適用された ResourceAttributes を返却します。
+// data に op が適用された ResourceAttributes と実際に適用されたかどうかの真偽値を返却します。
 // see. https://datatracker.ietf.org/doc/html/rfc7644#section-3.5.2.3
 // 基本は Validated な op を想定しているため、エラーハンドリングは属性を確認するうえで対応することになる最小限のチェックとなっています。
-func (p *Patcher) replace(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, error) {
-	return data, nil
+func (p *Patcher) replace(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, bool, error) {
+	return data, false, nil
 }
 
 // remove は RFC7644 3.5.2.2. Remove Operation の実装です。
-// data に op が適用された ResourceAttributes を返却します。
+// data に op が適用された ResourceAttributes と実際に適用されたかどうかの真偽値を返却します。
 // see. https://datatracker.ietf.org/doc/html/rfc7644#section-3.5.2.2
 // 基本は Validated な op を想定しているため、エラーハンドリングは属性を確認するうえで対応することになる最小限のチェックとなっています。
-func (p *Patcher) remove(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, error) {
+func (p *Patcher) remove(op scim.PatchOperation, data scim.ResourceAttributes) (scim.ResourceAttributes, bool, error) {
+	var changed = false
 	if op.Path == nil {
 		// If "path" is unspecified, the operation fails with HTTP status code 400 and a "scimType" error code of "noTarget".
-		return scim.ResourceAttributes{}, errors.ScimErrorNoTarget
+		return scim.ResourceAttributes{}, false, errors.ScimErrorNoTarget
 	}
 	// Resolve Attribute
 	attrName := op.Path.AttributePath.AttributeName
 	attr, ok := p.containsAttribute(attrName)
 	if !ok {
-		return scim.ResourceAttributes{}, errors.ScimErrorInvalidPath
+		return scim.ResourceAttributes{}, false, errors.ScimErrorInvalidPath
 	}
 	if cannotBePatched(op.Op, attr) {
-		return scim.ResourceAttributes{}, errors.ScimErrorMutability
+		return scim.ResourceAttributes{}, false, errors.ScimErrorMutability
 	}
 	scopedMap := p.getScopedMap(op, data)
 	switch {
 	case !attr.HasSubAttributes() && !attr.MultiValued():
-		delete(scopedMap, attrName)
+		fmt.Printf("%v, %s", scopedMap, attrName)
+		if _, ok := scopedMap[attrName]; ok {
+			delete(scopedMap, attrName)
+			changed = true
+		}
 	}
 	data = p.setScopedMap(op, data, scopedMap)
-	return data, nil
+	return data, changed, nil
 }
 
 // containsAttribute は attrName がサーバーで利用されているスキーマの属性名として適切かを確認し、取得します。

diff --git a/scimpatch_test.go b/scimpatch_test.go
@@ -25,10 +25,11 @@ func path(s string) *filter.Path {
 func TestPatcher_Apply(t *testing.T) {
 	// Define the test cases
 	testCases := []struct {
-		name     string
-		op       scim.PatchOperation
-		data     scim.ResourceAttributes
-		expected scim.ResourceAttributes
+		name            string
+		op              scim.PatchOperation
+		data            scim.ResourceAttributes
+		expected        scim.ResourceAttributes
+		expectedChanged bool
 	}{
 		{
 			name: "Add operation",
@@ -42,6 +43,7 @@ func TestPatcher_Apply(t *testing.T) {
 			expected: scim.ResourceAttributes{
 				"displayName": "Bob Green",
 			},
+			expectedChanged: false,
 		},
 		{
 			name: "Replace operation",
@@ -55,6 +57,7 @@ func TestPatcher_Apply(t *testing.T) {
 			expected: scim.ResourceAttributes{
 				"displayName": "Bob Green",
 			},
+			expectedChanged: false,
 		},
 		{
 			name: "Remove operation - Core Singular Attribute",
@@ -65,7 +68,18 @@ func TestPatcher_Apply(t *testing.T) {
 			data: scim.ResourceAttributes{
 				"displayName": "Bob Green",
 			},
-			expected: scim.ResourceAttributes{},
+			expected:        scim.ResourceAttributes{},
+			expectedChanged: true,
+		},
+		{
+			name: "Remove operation - Core Singular Attribute Not Changed.",
+			op: scim.PatchOperation{
+				Op:   "remove",
+				Path: path("displayName"),
+			},
+			data:            scim.ResourceAttributes{},
+			expected:        scim.ResourceAttributes{},
+			expectedChanged: false,
 		},
 		{
 			name: "Remove operation - Extention Singular Attribute - All Removed.",
@@ -78,7 +92,8 @@ func TestPatcher_Apply(t *testing.T) {
 					"department": "2B Sales",
 				},
 			},
-			expected: scim.ResourceAttributes{},
+			expected:        scim.ResourceAttributes{},
+			expectedChanged: true,
 		},
 		{
 			name: "Remove operation - Extention Singular Attribute - Partially Removed.",
@@ -97,6 +112,35 @@ func TestPatcher_Apply(t *testing.T) {
 					"division": "Sales",
 				},
 			},
+			expectedChanged: true,
+		},
+		{
+			name: "Remove operation - Extention Singular Attribute - URI Prefix not exists.",
+			op: scim.PatchOperation{
+				Op:   "remove",
+				Path: path("urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department"),
+			},
+			data:            scim.ResourceAttributes{},
+			expected:        scim.ResourceAttributes{},
+			expectedChanged: false,
+		},
+		{
+			name: "Remove operation - Extention Singular Attribute - URI Prefix exists.",
+			op: scim.PatchOperation{
+				Op:   "remove",
+				Path: path("urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department"),
+			},
+			data: scim.ResourceAttributes{
+				"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": map[string]interface{}{
+					"division": "Sales",
+				},
+			},
+			expected: scim.ResourceAttributes{
+				"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": map[string]interface{}{
+					"division": "Sales",
+				},
+			},
+			expectedChanged: false,
 		},
 	}
 
@@ -107,11 +151,14 @@ func TestPatcher_Apply(t *testing.T) {
 			patcher := scimpatch.NewPatcher(schema.CoreUserSchema(), []schema.Schema{schema.ExtensionEnterpriseUser()})
 
 			// Apply the PatchOperation
-			result, err := patcher.Apply(tc.op, tc.data)
+			result, changed, err := patcher.Apply(tc.op, tc.data)
 			if err != nil {
 				t.Fatalf("Apply() returned an unexpected error: %v", err)
 			}
-
+			// Check if the result matches the expected data
+			if changed != tc.expectedChanged {
+				t.Errorf("actual: %v, expected: %v", changed, tc.expectedChanged)
+			}
 			// Check if the result matches the expected data
 			if !(fmt.Sprint(result) == fmt.Sprint(tc.expected)) {
 				t.Errorf("actual: %v, expected: %v", result, tc.expected)
@@ -144,7 +191,7 @@ func TestPatcher_ApplyError(t *testing.T) {
 			patcher := scimpatch.Patcher{}
 
 			// Apply the PatchOperation
-			_, err := patcher.Apply(tc.op, scim.ResourceAttributes{})
+			_, _, err := patcher.Apply(tc.op, scim.ResourceAttributes{})
 			if err == nil {
 				t.Fatalf("Apply() not returned error")
 			}