Fix belongsTo response when fk is null #113

Closed
kris-at-tout opened this Issue Aug 23, 2012 · 3 comments

Comments

Projects
None yet
3 participants

This was originally only mentioned in a pull request (#85) that was closed for implementation issues. However the issue remains, and I couldn't find it in the open issues. So raising it here.

Currently on MySQL mocha returns a timeout if the fk is null. The proper response would be to return null.

Collaborator

anatoliychakkaev commented Aug 23, 2012

Probably issue with mysql adapter? This code:

            if (inst[fk] === this.id) {
                cb(null, inst);
            } else {
                cb(new Error('Permission denied'));
            }

Will invoke callback even if fk is null. If you provide unit test to help me reproduce issue I will fix it shortly.

Here is the test I tried adding to the common_test.js

it('should return null or an error if belongsTo foreignKey is null', function (test) {
     var passport = new Passport()
     // sync getter
     passport.owner(function (e, r) {
        console.log("---------------------");
        console.log(r);  
        console.log("---------------------");
     });
     passport.owner(function (e, r) {
         test.equal(r, null);
         test.done();
     });
});

I ran ONLY=mysql nodeunit test/common_test.js and it throws an uncaught exception:
/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:735
if (inst[fk] === this.id) {
^
TypeError: Cannot read property 'ownerId' of null
at Object.AbstractClass.belongsTo.finders.(anonymous function).__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js.(anonymous function) (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:735:21)
at Function.AbstractClass.find.__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js.(anonymous function) (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:301:9)
at MySQL. (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/sql.js:74:9)
at MySQL.query.__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/adapters/mysql.js.(anonymous function) (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/adapters/mysql.js:71:9)
at Query.Client.query (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/client.js:108:11)
at Query.EventEmitter.emit (events.js:85:17)
at Query._handlePacket (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/query.js:51:14)
at Client._handlePacket (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/client.js:319:14)
at Parser.EventEmitter.emit (events.js:88:17)
at Parser.write.emitPacket (/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/parser.js:71:14)

Kristine Robison | Senior Engineer | Tout
kris@tout.com | Skype: krobison | 408.390.8107

On Thursday, August 23, 2012 at 3:42 PM, Anatoliy Chakkaev wrote:

Probably issue with mysql adapter? This code:
if (inst[fk] === this.id) { cb(null, inst); } else { cb(new Error('Permission denied')); }

Will invoke callback even if fk is null. If you provide unit test to help me reproduce issue I will fix it shortly.


Reply to this email directly or view it on GitHub (#113 (comment)).

Collaborator

anatoliychakkaev commented Aug 24, 2012

I've pushed fix to repo.

On Fri, Aug 24, 2012 at 5:28 AM, Kristine Robison
notifications@github.comwrote:

Here is the test I tried adding to the common_test.js

it('should return null or an error if belongsTo foreignKey is null',
function (test) {
var passport = new Passport()
// sync getter
passport.owner(function (e, r) {
console.log("---------------------");
console.log(r);
console.log("---------------------");
});
passport.owner(function (e, r) {
test.equal(r, null);
test.done();
});
});

I ran ONLY=mysql nodeunit test/common_test.js and it throws an uncaught
exception:
/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:735

if (inst[fk] === this.id) {
^
TypeError: Cannot read property 'ownerId' of null
at Object.AbstractClass.belongsTo.finders.(anonymous
function).__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js.(anonymous
function)
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:735:21)

at
Function.AbstractClass.find.__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js.(anonymous
function)
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/abstract-class.js:301:9)

at MySQL.
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/sql.js:74:9)

at
MySQL.query.__cov./Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/adapters/mysql.js.(anonymous
function)
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/lib/adapters/mysql.js:71:9)

at Query.Client.query
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/client.js:108:11)

at Query.EventEmitter.emit (events.js:85:17)
at Query._handlePacket
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/query.js:51:14)

at Client._handlePacket
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/client.js:319:14)

at Parser.EventEmitter.emit (events.js:88:17)
at Parser.write.emitPacket
(/Users/kris/Development/filtered_stream/node_modules/jugglingdb/node_modules/mysql/lib/parser.js:71:14)

Kristine Robison | Senior Engineer | Tout
kris@tout.com | Skype: krobison | 408.390.8107

On Thursday, August 23, 2012 at 3:42 PM, Anatoliy Chakkaev wrote:

Probably issue with mysql adapter? This code:
if (inst[fk] === this.id) { cb(null, inst); } else { cb(new
Error('Permission denied')); }

Will invoke callback even if fk is null. If you provide unit test to
help me reproduce issue I will fix it shortly.


Reply to this email directly or view it on GitHub (
#113 (comment)).


Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/113#issuecomment-7989779.

@1602 1602 closed this Apr 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment