neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

Cannot build a new instance of `...` without an object #575

Closed nodesocket closed 8 years ago

nodesocket commented 8 years ago

I am completely stumped. All the sudden I started getting the error:

Cannot build a new instance of `accounts` without an object

I narrowed the error down to this specific line (when I comment it out things work).

const Account = require('./Account.js');
Project.belongsTo(Account, 'account', 'account', 'id');

Any idea what could be causing this problem?

neumino commented 8 years ago

You probably did something like new Account() or new Account(null) somewhere

nodesocket commented 8 years ago

@neumino I thought the same thing, but I have verified that I am passing a payload into new Account().

Also, as soon as I comment out the call to belongsTo() it works.

const Account = require('./Account.js');
Project.belongsTo(Account, 'account', 'account', 'id'); //comment this line out, and it works
nodesocket commented 8 years ago

@neumino I am completely stumped on this.

As soon as I comment out Project.belongsTo() the error goes away. I need that relationship though. Here is what I have:

Models

Account
Project
Event

Relationships

Account.hasMany(Project, 'projects', 'id', 'account');

Project.belongsTo(Account, 'account', 'account', 'id');
Project.hasMany(Event, 'events', 'id', 'project');

Event.belongsTo(Project, 'project', 'project', 'id');
neumino commented 8 years ago

What's the stacktrace? How do you declare your models?

nodesocket commented 8 years ago

There is no stacktrace that I see bubbled up. Here are the three models I have defined. Note that I use a custom abstraction in ../lib/orm.js. Let me know if you want to see that as well.

Account.js

'use strict';

const orm = require('../lib/orm.js');
const type = orm.type;
const uuid = require('node-uuid');

const Account = module.exports = orm.createModel('accounts', {
    id: type.string().uuid(4).default(() => {
        return uuid.v4();
    }),
    object: type.virtual().default(() => {
        return 'account';
    }),
    created: type.date().default(orm.r.now())
}, {
    enforce_extra: 'strict' // jshint ignore:line
});

const Project = require('./Project.js');

Account.hasMany(Project, 'projects', 'id', 'account');

Project.js

'use strict';

const orm = require('../lib/orm.js');
const type = orm.type;
const uuid = require('node-uuid');

const Project = module.exports = orm.createModel('projects', {
    id: type.string().uuid(4).default(() => {
        return uuid.v4();
    }),
    account: type.string().uuid(4).required().allowNull(false),
    label: type.string().required(),
    object: type.virtual().default(() => {
        return 'project';
    }),
    created: type.date().default(orm.r.now())
}, {
    enforce_extra: 'strict' // jshint ignore:line
});

const Event = require('./Event.js');
const Account = require('./Account.js');

Project.belongsTo(Account, 'account', 'account', 'id');  // breaks here. if line commented out works.
Project.hasMany(Event, 'events', 'id', 'project');

Event.js

'use strict';

const orm = require('../lib/orm.js');
const type = orm.type;
const uuid = require('node-uuid');

const Event = module.exports = orm.createModel('events', {
    id: type.string().uuid(4).default(() => {
        return uuid.v4();
    }),
    project: type.string().uuid(4).required().allowNull(false),
    object: type.virtual().default(() => {
        return 'event';
    }),
    created: type.date().default(orm.r.now()),
    heuristics: type.object().required()
}, {
    enforce_extra: 'strict' // jshint ignore:line
});

const Project = require('./Project.js');

Event.belongsTo(Project, 'project', 'project', 'id'); // breaks here. if line commented out works.
nodesocket commented 8 years ago

@neumino sorry to bug you, but this issue has me completely stumped. Any ideas?

nodesocket commented 8 years ago

Is this stack trace helpful at all?

Error: Cannot build a new instance of `accounts` without an object
    at new model (/Users/justin/Sites/MyApp/api/node_modules/thinky/lib/model.js:98:13)
    at /Users/justin/Sites/MyApp/api/node_modules/thinky/lib/model.js:114:22
    at Object.loopKeys (/Users/justin/Sites/MyApp/api/node_modules/thinky/lib/util.js:200:16)
    at new model (/Users/justin/Sites/MyApp/api/node_modules/thinky/lib/model.js:108:10)
nodesocket commented 8 years ago

This line looks promising?

https://github.com/neumino/thinky/blob/master/lib/model.js#L98

neumino commented 8 years ago

I'm pretty sure it's because you use account to store a string and the joined document. You need different fields.

On Thu, Oct 6, 2016, 15:29 Justin Keller notifications@github.com wrote:

This line looks promising?

https://github.com/neumino/thinky/blob/master/lib/model.js#L98

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/neumino/thinky/issues/575#issuecomment-252106661, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZOu9LBXTLq925vf5i4BIZS82d8q3-Oks5qxXZDgaJpZM4KKmBI .

nodesocket commented 8 years ago

Can you explain a bit further?

For project, it should store account which is a foreign key, pointing to the account that owns the project. account is a string and specially a uuid.

neumino commented 8 years ago

In Project.belongsTo, you use account to set the the joined document but also use it to set the foreign key. You need two different fields here. You probably want Project.b elongsTo(Account, 'account', 'accountId', 'id')

nodesocket commented 8 years ago

The field name is called account though.

Project.belongsTo(Account, 'account', 'account', 'id')
const Project = module.exports = orm.createModel('projects', {
    id: type.string().uuid(4).default(() => {
        return uuid.v4();
    }),
    account: type.string().uuid(4).required().allowNull(false),
    label: type.string().required(),
    object: type.virtual().default(() => {
        return 'project';
    }),
    created: type.date().default(orm.r.now())
}, {
    enforce_extra: 'strict' // jshint ignore:line
});
nodesocket commented 8 years ago

Sorry to keep bothering about this error, but I am completely stumped.

I tried rename foreign key fields to accoundId, projectId like:

Project.belongsTo(Account, 'account', 'accountId', 'id')

Still the same error:

Error: Cannot build a new instance of `accounts` without an object
neumino commented 8 years ago

This should work:

const Project = module.exports = orm.createModel('projects', {
    id: type.string().uuid(4).default(() => {
        return uuid.v4();
    }),
    accountId: type.string().uuid(4).required().allowNull(false),
    label: type.string().required(),
    object: type.virtual().default(() => {
        return 'project';
    }),
    created: type.date().default(orm.r.now())
}, {
    enforce_extra: 'strict' // jshint ignore:line
});

Project.belongsTo(Account, 'account', 'accountId', 'id');

If you have old data where the account field is set to a string, you will still get the same error. You need to clean your data first

Clean your data:

Projects.replace(function(doc) {
  doc.merge({accountId: doc('account')}).without('account')
}).run()

You could also just use this relation, though it's a bit less clean IMO.

Project.belongsTo(Account, 'joinedAccount', 'account', 'id')
nodesocket commented 8 years ago

@neumino confirm switching all foreign keys from account to accountId fixed the problem. This is a bug though right? You should be able to name foreign key fields whatever you want right?

nodesocket commented 8 years ago

Confirm, switching to using a different nomenclature _:

Project.belongsTo(Account, '_account', 'account', 'id');
Project.hasMany(Event, '_events', 'id', 'project');

Works. Is this something that is able to be fixed?

neumino commented 8 years ago

No, I don't think it's reasonable to expect people to use a field to store both a foreign key and a document. This means that some magic will happen and that they won't have access to their foreign key.