search

imulab / go-scim

Building blocks for servers implementing Simple Cloud Identity Management v2
MIT License
145 stars 56 forks source link

Fix PATCH conflicts when `meta.version` is present #87

Open atheriel opened 2 years ago

atheriel commented 2 years ago

At present all PATCH requests for resources that have a meta.version attribute will fail with ErrConflict because the PatchService.Do() method incorrectly mutates the current resource instead of its replacement.

This error was not caught by existing unit tests because they do not set a meta.version attribute and db.Memory().Replace() has the following check: https://github.com/imulab/go-scim/blob/fec7838ed1c454d176f882e8c6cc73f094068697/pkg/v2/db/memory.go#L76-L79

This has the effect of permitting replacements when there is no version present -- which is the case for the existing unit tests but not real APIs. Simply adding a meta.version attribute to unit tests (as in this PR) yields failures like as the following for all tests:

=== RUN   TestPatchService/TestDo/patch_to_make_a_difference
    patch_test.go:99:
            Error Trace:    patch_test.go:99
                                        patch_test.go:366
            Error:          Expected nil, but got: &spec.Error{Status:412, Type:"conflict"}
            Test:           TestPatchService/TestDo/patch_to_make_a_difference

An identical fix was originally proposed by @zakirhussain in #80. This PR expands on that work to include unit test changes to catch a regression and (hopefully) a better explanation of the underlying cause of the issue.