feat(301): add undelete pattern, replace soft-delete #365
Conversation
toumorokoshi
force-pushed
the
yft/undelete
branch
2 times, most recently
from
1ec3c4e to
0e222ac
Compare
The current soft-delete pattern has the undesirable side effect of adding special-casing and additional query parameters to the standard methods, as well as possibly impacting their behavior. Introducing undelete, which is a resource oriented solution that achieves much of the same user journeys: - Restoring resources after being deleted. - Listing / getting deleted resources to figure out what is restorable. fixes aep-dev#111.
toumorokoshi
force-pushed
the
yft/undelete
branch
from
0e222ac to
549f57b
Compare
There was a problem hiding this comment.
I understand the issues soft delete causes for declarative APIs, but to introduce a separate resource type seems to me like an overcorrection.
Speaking for our use cases (business systems, small data, lots of separate entities and collections, big total API surface) we use the current soft delete pattern on all our APIs and it's working well. Adding 3 separate methods and a separate resource type with separate resource paths seems like it would introduce quite a bit of overhead compared to the current pattern.
But then we also don't make use of declarative APIs today (no real need for it yet), so we are not impacted by the problem this solution optimizes for.
Have we completely ruled out more subtle tweaks to soft deletes that would make the pattern more declarative-friendly? You mentioned in the other PR the case where:
A resource "create" throwing an error because the resource is soft-deleted, and therefore exists. But a get would return a 404.
What if we solved that case by having Create take precedence over soft deletes and overwrite the previously soft deleted resource?
|
Thanks for the feedback! I think a couple thoughts:
Is it a significant overhead to add these? I guess I imagined that these could also be implemented with the same underlying data storage, so therefore it's just route overhead which is mostly boilerplate.
The issue I have with this idea is that it many be recoverable enough for some users. e.g. I could imagine the following flow:
Basically: there is a continued desire to have some way to recover a database, even if it was permanently deleted. At minimum the idea of overwriting a soft-deleted record will likely require an explicit confirmation to provide guard against what most users expect. |
When I thought about this, it didn't seem feasible to implement both collections with the same table - or that would introduce other edge cases similar to the existing "overwrite deleted data" edge case. The soft deleted record would occupy the primary key that the new record would want to use. But even with two tables (one for active, one for deleted) how to handle when the second active record for a user-specified ID gets soft deleted, and the deleted table already occupies the first deleted version? Refuse to create the new record, or overwrite the previous soft deleted record? Or have some other internal UUID identifier scheme in the deleted table that makes it possible to have many resources with the same user-specified ID? I guess what I'm trying to say is - it seems like this pattern has other trade-offs that are not immediately apparent, and it makes implementation complex in other ways than the existing one. I don't see anyone speaking up for this pattern who have already used it in production and can attest that the edge cases can and have all been worked out. Is the pattern battle tested already? |
Yep, I agree. I tried to implement this with aep-dev/aepc#78 and found the same issue.
The way I viewed this is that both records would co-exist. And if you wanted to bring back the old version, you could call undelete, which would effectively replace the current version. Although I honestly think at this point it's more of a snapshot that it would be called "soft delete"
Sure, I think that argument is fair. So maybe we can move to the other solution: allowing create to overwrite the soft-deleted resource. That would indeed fix the declarative issue. But my question would be: if one overwrites a soft-deleted resource, rather than reject the request because there's a soft deleted copy, is that desirable? For example, one case is to restore cryptographic keys: if someone does a create, they will overwrite the old entry, without rejecting the request. is that desirable? Also a slightly separate topic, but @odsod have you looked at the existing soft-delete pattern and made sure that aligned with your needs based on the resources you have that have implemented it? @thegagne similar question to you. Just want to make sure that we agree that the existing soft delete pattern is indeed useful, and will continue to be useful with the create-overwrites pattern. |
|
Our current use case disables crypto keys before deletion. For us, it's not truly a delete, more of a disable. It can be re-enabled until actual deletion. Since we are using envelope encryption a new key (with new id) can be used to take its place. For our usage, I think updating a field like |
| @@ -1,5 +1,8 @@ | |||
| # Soft delete | |||
|
|
|||
| **NOTE: this pattern is now deprecated, and is no longer valid. Please use | |||
There was a problem hiding this comment.
We haven't created our first edition, so I don't think we should be "deprecating" AEPs. Let's delete it.
To help ensure that declarative clients do not have a degraded user experience with resources that implement soft-delete, specify that create overwrites an existing soft-deleted resource, or has a query parameter that allows a client to specify overwrite behavior. This ensures that a declarative tool can still properly apply a resource, overwriting it instead of requiring an out of band force delete or having the user modify the ID of the resource. fixes #111 (which includes some significant debate and context on why this approach was chosen). Inlined description: In #365, there was additional discussion or the viability of the pattern. @odsod brings up valid points that the cost to implement my proposed pattern is quite a bit different (requires two different collections), and perhaps implements a sort of "snapshot" resource collection than a soft-deleted resource, whose main intention is primarily around having a resource that exists until some expiry date rather than a snapshot that should be recovered. In addition, most who have joined AEP live meetings have raised valid examples of soft deleted resources in their APIs (e.g. cryptographic keys in Azure), and therefore the omissions of this pattern might invalidate those resources. As such, combined with the primary motivation here to eliminate the poor UX with declarative clients which may need to overwrite the resource, this change acts as a more surgical fix to ensure that a create behaves like any other resource (specifically allowing the recreation of one), even when using the soft delete pattern.
|
closed in favor of #371 |
4 participants
The current soft-delete pattern has the undesirable side effect of adding special-casing and additional query parameters to the standard methods, as well as possibly impacting their behavior.
Introducing undelete, which is a resource oriented solution that achieves much of the same user journeys:
fixes #111.
🍱 Types of changes
What types of changes does your code introduce to AEP? Put an
xin the boxesthat apply
📋 Your checklist for this pull request
Please review the AEP Style and Guidance for
contributing to this repository.
General
references AEPs
correctly.
(usually
prettier -w .)./scripts/fix.py💝 Thank you!