Loopback: investigate soft delete feature

[clarkbw]

This is a fork of the issue strongloop/loopback-component-push#57 and the related pull request strongloop/loopback-component-push#72 where a quick patch was created to investigate implementing a soft delete for the strongloop/loopback-component-push module.

@raymondfeng mentioned that this would make sense to generalize and bring up to the loopback level so I'm opening this issue for that.

I'd appreciate any comments from @karlmikko who mentioned here https://github.com/strongloop/loopback/issues/222#issuecomment-46518381 implementing a soft delete via a boot script.

I'm assuming we want a field, perhaps configurable, which looks something like _is_deleted that defaults to false and is changed to true where we intend to delete. When this soft delete option is turned on for a Model the find and upsert methods likely want to filter for instances where the _is_deleted property is true.

Here's an example model definition with a possible option:

{
  "name": "Customer",
  "description": "A Customer model representing our customers.",
  "base": "User",
  "softDelete" : true
  }
}

And this could also allow implementers to specify the attribute name so they can match this up with data they might already have.

{
  "name": "Customer",
  "description": "A Customer model representing our customers.",
  "base": "User",
  "softDelete" : {
    "attribute": "isDeleted"
  }
  }
}

From a quick search I see a couple options:

• Use the PersistedModel setup method to wrap the destroyAll functions changing from a destroyAll to updateAll call passing { _is_deleted : true }
• Add a _default scope_ to the model so our find methods all work as expected.
• Instead of default scope we could use the PersistedModel setup to wrap the find / upsert methods to filter for the soft delete attribute

Below is just for clarity, we would set the default scope programmatically, but this is what the model definition would look like with a soft delete default scope.

{
  "name": "Customer",
  "description": "A Customer model representing our customers.",
  "base": "User",
  "scope": {
    "where": {
      "_is_deleted": false
    }
  }
}

I also looked at doing this from the connector level but I don't think connectors should be aware of options in the models, they are better kept simple to just implement the required CRUD methods.

[raymondfeng]

+1 to explore the idea to use softDelete setting.

[seriousben]

We have easily implemented softDelete in our application in:

• Adding a softDelete method to models
• Disabling the remote del /:id method
• Adding a remote del /:id method calling softDelete
• Ensuring remotely fetched objects never include deleted objects (after hooks on find*)
• adding a default query criteria on 'access' when isDeleted is not in the query

Things to think about:

• a generic softDelete should allow setting deleteBy, deletedCause maybe using a hook

[fabien]

@raymondfeng @seriousben this is a snapshot of the code I have been using - it's generalized as a Mixin, and uses the new access operation hook:

https://gist.github.com/fabien/1585f7be8a1c05e5ec21

The code has been updated recently, but the gist does not reflect this yet. If there's enough interest I'll see if I can release it as a proper npm package.

Suggestions for getting rid of the ugly hack are welcome ;)

[seriousben]

Really cool stuff in there, thanks @fabien. I really think having this implemented as a mixin is the way to go and as soon I have some time will give you better feedback by implementing that way as well.

@raymondfeng How awesome would it be to enable mixins from the json model defintion file? And how awesome would it be if those mixins could be provided by third party npm modules a bit like the middlewares can be.

{
  "name": "Customer",
  "description": "A Customer model representing our customers.",
  "mixins": {
    "timestamps": {
      "updatedPropertyName": "updatedAt",
      "createdPropertyName": "createdAt"
    },
    "softDelete": {
      // options example
      "deletedPropertyName": "isDeleted",
      "deletedTimestampPropertyName": "deletedAt",
      "deleteByPropertyName": "deletedBy"
    }
  }
}

[fabien]

@seriousben thanks for the heads-up. AFAIK this is already working (or perhaps this is still pending with this: https://github.com/strongloop/loopback-boot/pull/111 ?) - I'm not really sure, because I'm waiting forever to have this patch merged ... I have been using the patched code since last year though, which enabled me to extract all kinds of 'traits' as mixins.

Confirmed: this functionality has been merged in a long time ago, and it works: https://github.com/strongloop/loopback-datasource-juggler/blob/093a3070522f93015b3af888921a26a713754375/lib/model-builder.js#L484

('mixins' were originally called 'plugins', but this is the relevant commit: https://github.com/strongloop/loopback-datasource-juggler/commit/1a4e8863ef1bb2104964146931017f04fd5d94bf)

[clarkbw]

enable mixins from the json model defintion file

This is exactly how my example mixin works, see: https://github.com/clarkbw/loopback-ds-timestamp-mixin

[karlmikko]

I agree with keeping this as a mixin as it will make adding more datasources in the future easier. Would be interesting to consider how an immutable data store like http://www.datomic.com/ with a builtin notion of time and history would be implemented. As timestamps would need to be shimmed if you wanted a similar data structure.

[clarkbw]

@fabien is there anything I can do to help with the code for this?

[fabien]

@clarkbw I'm currently flooded with (client) work, I'm afraid. Apart from giving the pending PR https://github.com/strongloop/loopback-boot/pull/111 a shot, you might be able to create the mixin as an npm module, based on the code I'm using.

I can post the most recent version as a gist - if you don't mind, it will be as-is; it will need some tweaks because of the internal dependencies I'm using. But in this particular case, I don't think I'm using anything special, except for lodash, async and such. Let me know!

[eugenehp]

@altsang how far have you moved with implementation of this feature?

[altsang]

i'm supportive of this feature but our maintainers are inundated with the work they have supporting other customer related features

[gausie]

I've been working on this as a mixin today. Not finished yet, but take a look!

https://github.com/gausie/loopback-softdelete-mixin

[jdhrivas]

Any updates on this feature?

[clarkbw]

I've been working on this as a mixin today. Not finished yet, but take a look!

https://github.com/gausie/loopback-softdelete-mixin

I'm not longer actively working on this feature, I'd recommend taking a look at the mixin from @gausie