Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: is implementing a patch application function reasonable? #171

Open
icamys opened this issue Mar 14, 2024 · 2 comments
Open

Question: is implementing a patch application function reasonable? #171

icamys opened this issue Mar 14, 2024 · 2 comments

Comments

@icamys
Copy link
Contributor

icamys commented Mar 14, 2024

I was looking at how to implement data patching, and it appears that a working algorithm looks as follows:

  1. Get the patch operations in the HTTP handler (the package already does this);
  2. Get data you want to patch from a data store and transform it to a map[string]interface{};
  3. Apply the patch operations to the data;
  4. Validate the patched data using Schema.Validate() function. If not valid, return a patch error;
  5. Transform the patched data to resource and return the resource.

If this is how the package is designed to work, it seems reasonable to implement a generic function that does data patching. Something like this:

// Apply applies the patch operation to the provided data.
func (o *PatchOperation) Apply (data map[string]interface{})

Then, the handler will have a shape like this:

func (h *Handler) Patch(r *http.Request, id string, operations []scim.PatchOperation) (scim.Resource, error) {
	entity := h.dataStore.Get(id)

        data := userEntity.ToMap()

        for _, op := range operations {
                op.Apply(data)
        }

        if err := h.schema.Validate(data); err != nil {
                // return an appropriate error
        }

       entity := newEntityFromData(data)
       h.dataStore.Save(entity)

       resource := newResourceFromEntity(entity)

	return resource, nil
}
@q-uint
Copy link
Collaborator

q-uint commented Mar 15, 2024

Hi @icamys,

The reason we have not immediately added it, is because we use custom structs to represent data, and do not use maps for resource types. So implementing PATCH for it made more sense at the time than converting it to a map and back to do this.

It's important to note that every user's data store varies, and while this feature may not be universally applicable, it could certainly benefit certain users.

@icamys
Copy link
Contributor Author

icamys commented Mar 15, 2024

@q-uint Thank you for the response. It makes sense.

Although the data store varies, I suppose, in most cases, the data model returned from the data store can be transformed into a map. At least, if theoretically there were a function that automatically applies a patch to a map, creating functions that convert data models to maps and back would be beneficial for the users of this package.

What's the current recommended approach to applying patches? Is it something like this?:

var entity *User

// for each patch operation
switch op {
case "add":
    addAttribute(entity, path, value)
case "remove":
    removeAttribute(entity, path, value)
case "replace":
    replaceAttribute(entity, path, value)
}

// where addAttribute, removeAttribute, and replaceAttribute know 
// how to apply a certain patch to a *User entity type

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

No branches or pull requests

2 participants