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

Indexes and cascade behaviours TBD #88

Closed
pandaiolo opened this issue Mar 21, 2014 · 31 comments
Closed

Indexes and cascade behaviours TBD #88

pandaiolo opened this issue Mar 21, 2014 · 31 comments
Assignees
Labels

Comments

@pandaiolo
Copy link
Contributor

As discussed in https://groups.google.com/forum/#!topic/loopbackjs/z7bW9UFJzCU

I recently noticed some issues or sub-optimal behaviour with loopback models storage in the database :

  • How index are created (for exampe: mongodb foreign key are not indexed, at least for many-to-many)
  • How interface tables items are dealt with, for example deleting links when a parent object is deleted (cascade)

Example : when i remove items in table Product and/or Category, via loopback API, items in product and category collection are deleted, but not productcategory collection

In a RDBMS, I suspect the best practice is to define FK and cascade behaviour in the DB engine, but in noSQL like mongodb, should not this be taken care of by Loopback ?

@innerdaze
Copy link

+1 for a configurable cascade delete feature

@fabien
Copy link
Contributor

fabien commented May 29, 2014

+1 seems to be mandatory indeed

@michael-lee
Copy link

+1

@fabien
Copy link
Contributor

fabien commented Jul 9, 2014

Current workaround involves a Model.on('deleted', function(id) { ... } event handler. It works, but should be integrated more tightly.

@raymondfeng
Copy link
Contributor

  1. We'll fix the code to automatically set index to true for foreign key properties. This way, automigrate/autoupdate will have opportunity to create indexes or foreign key constraints.
  2. We may introduce a cascade option when a hasOne/hasMany relation is defined. Do we also want to support the option per request? If so, we probably have to add a new arg (options) to some of the CRUD methods.

@socketwiz
Copy link

Hey guys, new loopback user here. I've come to the point where I need a cascading delete feature and in my quest to find it I've stumbled upon this post. I was just curious if anything was ever created to address this?

$ slc --version
strongloop v2.10.1 (node v0.10.22)

@jonathan-casarrubias
Copy link

Any progress on this request? this is not just a trivial enhancement, it actually generates bugs when the relation map is not removed in cascade, if you try to use the include filter, it breaks if the relation map still exists.

events.js:72
        throw er; // Unhandled 'error' event
              ^
TypeError: Cannot read property '__cachedRelations' of null
    at defineCachedRelations (/root/production/com.cariloop.ws/node_modules/loopback-datasource-juggler/lib/utils.js:288:11)
    at /root/production/com.cariloop.ws/node_modules/loopback-datasource-juggler/lib/include.js:204:11
    at /root/production/com.cariloop.ws/node_modules/loopback-datasource-juggler/lib/scope.js:70:7
    at /root/production/com.cariloop.ws/node_modules/loopback-datasource-juggler/lib/dao.js:839:7
    at /root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/lib/mongodb.js:546:7
    at handleCallback (/root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/node_modules/mongodb/lib/utils.js:93:12)
    at /root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/node_modules/mongodb/lib/cursor.js:571:16
    at handleCallback (/root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:229:5)
    at Cursor.next (/root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:507:7)
    at fetchDocs (/root/production/com.cariloop.ws/node_modules/loopback-connector-mongodb/node_modules/mongodb/lib/cursor.js:567:10)```

@raymondfeng raymondfeng self-assigned this Feb 5, 2015
@mrjrdnthms
Copy link

+1 I really need an option to enable Cascading Deletes

@violet-day
Copy link

+1

3 similar comments
@mevillalo
Copy link

+1

@gruzilla
Copy link

+1

@spencerfdavis
Copy link

👍

@pulkitsinghal
Copy link
Contributor

@ritch and @raymondfeng - it sounds like it has been almost ready for a long time, where does it stand in the pipeline? https://waffle.io/strongloop/loopback

@cndreiter
Copy link

+1

@CjS77
Copy link

CjS77 commented Sep 22, 2015

I've written a mixin that pretty much covers the use cases described here. In particular, in MyModel.js if you write

...
"mixins": {
    "Cascade": {
        "order": {      // order is a hasMany relationship
            "unlink": false
        },
        "products": {  // products is a hasAndBelongsToMany relationship
            "unlink": true
        }
    }
}

If unlink is true, then when a record of id n in MyModel is deleted, only the links to the model are deleted (i.e. the references in the through model that point back to MyModel.n are removed).

If unlink is false, then the cascade delete proceeds as one might expect, by deleting records on the other side of the relation that would otherwise be orphaned.

If there's interest, I can submit a pull request -- my only question is: to which project? the built-in mixins seem to live in the datasource-juggler project, but it could also be put in the loopback/server/mixins directory of the loopback project. Or it could live as a standalone module. But to write tests, one needs the whole loopback infrastructure, which makes deployment a bit blech. Thoughts?

@JonnyBGod
Copy link

+1

@michaelsframe
Copy link

+1, any word on the status of this?

@el-davo
Copy link

el-davo commented Dec 26, 2015

+1, Im in serious need of such a feature. Any status updates?

@devonatdomandtom
Copy link

@CjS77 would you have a gist handy for the mixin you've written by chance?

@CjS77
Copy link

CjS77 commented Jan 7, 2016

Wroks for me. YMMV :)

https://gist.github.com/CjS77/44b2f75c0ec468f590d0

@devonatdomandtom
Copy link

@CjS77 Cheers for that!

@zbarbuto
Copy link
Contributor

I found that @CjS77 's solution worked only for a single relation. As soon as there were multiple relations to cascade it would only delete the first relation as the first resolved delete would call next();

I've made a slight change which waits for all deletes to resolve before calling next:
https://gist.github.com/zbarbuto/add938efd9653c7c6c14

@giowe
Copy link

giowe commented Jan 20, 2016

+1 !!!

@jimutt
Copy link

jimutt commented Jan 30, 2016

+1 What's the status of this issue? I really hope that a solid cascade-support will be implemented soon as I find it quite valuable for more complex systems.

@tspoke
Copy link

tspoke commented Jul 21, 2016

+1

@ataraxus
Copy link

ataraxus commented Aug 1, 2016

+1 No changes on the status?

@dancingshell
Copy link

@raymondfeng it seems like foreign key indexes are still not being created, did that ever get merged in? Thanks!

@hussainak
Copy link

hussainak commented Oct 2, 2016

Just in case if people still want this, The code below walks through the tree of relations and destroys them, use at your own risk.
`

/**
 * Created by hussainak on 30/09/2016.
 */
var _ = require('lodash');
var async = require('async');
function DeleteDeepMixin(Model) {

  Model.deleteDeep = function (modelId, cb) {
    var modelnamesExclude = ['Model1', 'Model1', 'Member', 'Role', 'RoleMapping'];
    if(_.includes(modelnamesExclude, Model.modelName)){
      console.log("Model Return", Model.modelName);
      return;
    }

    //Add exceptions to the class based on modelName
    // console.log("Model", Model.modelName, modelId);
    var self = Model;
    var includes = Object.keys(self.definition.settings.relations);
    var models = Model.app.models;
    var paraArr = [];
    // console.log('inc', includes);
    self.findById(modelId, {include: includes}, function(error, modelResult){
      if(error){
        cb(error);
      }
      else{
        if(!_.isEmpty(modelResult)){
          self.destroyAll({id: modelResult.id}, function (error, result) {
            if(error){
              console.log("DESTROY ERR", error);
              cb(error);
            }
            else{
              // console.log("DESTROY RESULT", result);
              var modelInstance = JSON.parse(JSON.stringify(modelResult));
              var relations = self.definition.settings.relations;
              // console.log('rel', relations);
              //once the main object model relations are found,
              // start by deleting the main model and in a parallel operation delete deep others.

               _.forOwn(relations, function(relation, name) {
                // console.log(modelInstance);
                // console.log('rel', relation);
                if(relation.type === "hasMany"){
                  // console.log('rel', relation.type);
                  var modelJSON = modelInstance[name];
                  if(_.isArray(modelJSON) && modelJSON.length > 0){
                    _.each(modelJSON, function(model) {
                      if (!_.isEmpty(model)) {
                        // console.log("model def", relation.model, model.id);
                        // console.log("Model", models[relation.model].modelName);
                        paraArr.push(async.apply(models[relation.model].deleteDeep, model.id, cb))
                      }
                    });
                  }
                  else{
                    if(!_.isEmpty(modelJSON)){
                      // console.log("model 2", relation.model, modelJSON.id);
                      paraArr.push(async.apply(models[relation.model].deleteDeep, modelJSON.id, cb))
                    }
                  }
                }
              });
              if(paraArr.length > 0){
                async.parallel(paraArr, function (error, results) {
                  if (error){
                    cb(error);
                  }else{
                    cb(null, results);
                  }
                });
              } else {
                cb();
              }
            }
          });
        }else{
          cb();
        }

      }
    });
  };
};

module.exports = DeleteDeepMixin;

`

@rocknrolla777
Copy link

I have created npm package
https://www.npmjs.com/package/loopback-cascade-delete-mixin

@timlind
Copy link

timlind commented Apr 12, 2017

This needs to happen on the connector / SQL definition layer, so if a row is deleted manually or via an external admin tool then it cascades @superkhau

Without the cascade client code will easily break when it expects included properties that don't come through.

@stale stale bot added the stale label Aug 23, 2017
@stale stale bot closed this as completed Sep 6, 2017
@AlexScotland
Copy link

AlexScotland commented Feb 13, 2020

A Co-worker of mine pointed out that each Loopback foreignKey element has an onUpdate and onDelete attribute.

Using those, we can add onDelete:"cascade", and this will add the cascade functionality built into MySQL.

No where in Loopback documentation is this mentioned, so if we can have that added, that'd be nice.

tl;dr: onDelete:"cascade" is cascade delete without any external mixins.

*quick edit
it is worth noting that we are using the MySQL connector for Loopback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests