multiple hasMany join rows created

[bvallelunga]

ORM code

models.users = require("./users")(db, models);
models.users.groups = require("./users/groups")(db, models);

models.users.groups.hasOne("owner", models.users);
models.users.groups.hasMany("users", models.users, {}, {reverse: "groups", autoFetch: true, autoFetchLimit: 1});

3 tables created: users, users_groups, users_group_users

when ever i call user.save() have making a change to the user model another row gets added to the users_group_users table. @dresende and @SPARTAN563 do you have any thoughts on this problem. Is it bug or am i incorrectly using some part of the code?

users_group_users table before user.save()

+----------+----------------+
| users_id | users_group_id |
+----------+----------------+
|        1 |            125 |
|        1 |            126 |
+----------+----------------+

users_group_users table after user.save()

+----------+----------------+
| users_id | users_group_id |
+----------+----------------+
|        1 |            125 |
|        1 |            126 |
|        1 |            125 |
|        1 |            126 |
+----------+----------------+

[notheotherben]

Sounds to me like it isn't creating a compound primary key in _users_groupusers over the _usersid and _users_groupid fields. I'm afraid I can't remember whether ORM attempts to add the entries each time and ignores failure codes for collisions, or if it attempts to determine changes locally - if it's local then there's likely some kind of error in the way ORM is handling things which needs to be fixed.

Could you throw together a Gist which demonstrates the behaviour on a smaller scale to allow us to troubleshoot it?

[bvallelunga]

here is a working gist https://gist.github.com/bvallelunga/9463037

[notheotherben]

Thanks, I'll look at it as soon as I have a free moment (might be a little while)

[bvallelunga]

okay sounds good. Thanks for the help

[bvallelunga]

@SPARTAN563 another issue i have been having with the hasMany is that it does not give a valid response on the hasUsers. Every time i call the function, it always returns false. I updated the gist to show that also. Thanks again for the help.

[bvallelunga]

@SPARTAN563 it seems like @dxg last commit fixed all the problems for this issue 3d1d026. The only problem left that should be a quick fix. In the users.groups model i have this hook which should remove all the users associated before removal, but instead it does nothing. Any suggestions?

beforeRemove: function() {
    this.removeUsers(this.users, lib.error.capture);
}

Edit: it turns out for some reason the beforeRemove hook is not being called. Nothing to do with removeUsers method.

[bvallelunga]

After @dxg last commit I am not able to get the beforeRemove & afterRemove hooks to fire on a model that has a hasMany association. This is the users.groups model from above:

var rand = require("generate-key");

module.exports = function (db, models) {
    return db.define("users_group", {
        pub_id: {
            type: "text"
        },
        name: {
            type: "text",
            required: true
        },
        description: String,
        private: {
            type: "boolean",
            required: true,
            defaultValue: false
        }
    }, {
        timestamp: true,
        hooks: {
            beforeCreate: function() {
                this.pub_id = rand.generateKey(Math.floor(Math.random() * 15) + 15);
            },
            afterCreate: function(success) {
                var _this = this;

                if(success) {
                    models.users.get(_this.owner_id, function(error, user) {
                        if(!error && user) {
                            user.addGroups(_this, lib.error.capture);
                        } else {
                            lib.error.capture(error);
                        }
                    });
                }
            },
            afterRemove: function(success) {
                if(success) {
                    this.removeUsers(this.users, lib.error.capture);
                }
            }
        },
        validations: {
            pub_id: db.enforce.unique()
        }
    });
}

[dxg]

I've had to modify your gist to make it work, and added some remove hooks.

The hooks fire for me. Can you modify the gist to fail? Output:

null true
before remove
after remove
null

[bvallelunga]

i can not get it to fail. It must be something on my end. Thanks for the help

[bvallelunga]

so i found the problem

before: remove hooks NOT triggered

models.users.groups.find({
    pub_id: req.param("group"),
    owner_id: req.session.user.id
}).remove(function(error) {
    ....
});

after: remove hooks triggered

models.users.groups.find({
    pub_id: req.param("group"),
    owner_id: req.session.user.id
}, 1, function(error, groups) {
    groups[0].remove();
});

[dxg]

The chained remove operates on a list of items rather than individual ones. Because of this, it does a batch deletion rather than instantiating every single item - and without instantiating, it can't run hooks.

I've added a note to the readme about it. It's kind of confusing. It would make sense to make this behaviour switchable:

.find().remove(false) // Fast; Don't run any hooks
.find().remove(fireHooks: false) // Fast; Don't run any hooks
.find().remove() // Slow but predictable; Instantiate each item and run hooks

Thoughts?

[bvallelunga]

@dxg i like the direction. It would make sense if the second argument was a callback. I also think regardless of if hooks is set to false, the orm should remove any rows in the join table if the model has an hasMany association. Because when I looked at the database after the .remove() was called, it confused me on why the join tables rows associated with the model just deleted where not also removed. What confused me may also have confused others.

.find().remove(false, callback) //Boolean
.find().remove({ hooks: false }, callback) //Object
.find().remove(callback) //None, then first argument is callback

[dxg]

Agreed; items from join tables should be nuked regardless. Sort of like foreign key constraints.. but those can be tricky to set up.

[bvallelunga]

it looks like another user was confused by the .find().remove() #471

[dxg]

Let's continue any discussion there since the title more fits the issue. If the original issue you described above is now resolved, please close this issue.

[bvallelunga]

this issue has been fixed so I am closing. The notion of making .find().remove() trigger hooks has been moved to issue #471