csync / csync-server

Apache License 2.0
14 stars 4 forks source link

Delete Wildcard #32

Closed boopt2 closed 7 years ago

boopt2 commented 7 years ago

Resolves #6 Proposed Changes:

DCO1.1 Signed-off-by: Thomas Boop tgboop@us.ibm.com

boopt2 commented 7 years ago

Hi boopt2,

Thanks for submitting this pull request!

I can confirm that the DCO1.1 sign-off has been included. It is okay to process this pull request.

dco-bot

mkistler commented 7 years ago

What is the expected behavior of a delete with wildcard when the caller has delete permissions on some keys matching the wildcard but not on some others?

mkistler commented 7 years ago

What is the expected behavior of a delete with wildcard with a lower cts than one of the matching keys in the database?

boopt2 commented 7 years ago

@mkistler The expected behavior for a delete on a set of keys where some you have permission and some you do not would be that all of the keys you have permission for are deleted and it returns the highest. Looking through the code though this probably won't work, I'll update the PR with a fix.

For your second question, if the delete has a lower cts, then the the matching key should not be deleted. However, I think the error handling may be a little more complex then this pr has it as. Currently, if one errors out it looks like it will throw an error, which is probably what we do not want.

boopt2 commented 7 years ago

How do the following edge cases look: @mkistler @kevinlai If you try to delete a path with nothing there, you receive a CannotDeleteNonExistingPath error If you try to delete 5 elements, and you only have permission for three, you delete the 3 and it silently fails on the other two. It then returns the highest vts of the three you actually deleted. If you try to delete If you have permissions to delete all 5 nodes, it deletes all 5 and returns the highest vts of the nodes

However, I'm confused on the following case: You try to delete 5 nodes, and they all fail for a combination of either outdated VTS or no permissions to delete. This could be 5 outdated vts or 5 no permission, or any mix. Can we create a new error that is just a simple Wildcard delete did not delete any entries?