Skip to content

Conversation

@zakirhussain
Copy link

For patch request, It is actually modifying reference which is received from the DB

For patch request, It is actually modifying reference which will be received from the DB
@zakirhussain
Copy link
Author

I have tested and verified for in memory DB

atheriel added a commit to posit-dev/go-scim that referenced this pull request Apr 21, 2022
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:

    version := ref.MetaVersionOrEmpty()
    if len(version) > 0 && m.db[id].MetaVersionOrEmpty() != version {
    	return spec.ErrConflict
    }

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 commit) yields failures like as the following:

    === 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 imulab#80. This
commit expands on that work to include unit test changes to catch a
regression and (hopefully) a better explanation on the underlying cause
of the issue.

Co-authored-by: Zakir Hussain <pro.zakir24@gmail.com>
Signed-off-by: Aaron Jacobs <aaron.jacobs@rstudio.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant