neumino / thinky

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

Allow setting of relations using a string reference rather than a Model object reference #129

Open bobobo1618 opened 9 years ago

bobobo1618 commented 9 years ago

At the moment if you want a relation between two models you have to do something like:

var A = thinky.createModel('Thing', {schema goes here});
var B = thinky.createModel('Thing2', {schema goes here});
var A.belongsTo(B, 'bs', 'bId', 'id');
var B.hasMany(A, 'as', 'id', 'bId');

If you want to split these across multiple files, you have to:

Or maybe I'm wrong and I've spent too much time in Python :P

The way Django handles this is by allowing the relations to be specified using a string rather than the Model itself. This way you could have:

var A = thinky.createModel('Thing', {schema goes here});
var A.belongsTo('B', 'bs', 'bId', 'id');

----------------------------------

var B = thinky.createModel('Thing2', {schema goes here});
var B.hasMany('A', 'as', 'id', 'bId');
neumino commented 9 years ago

Hello @bobobo1618

I wrote about circular references, relations and how to import thinky here: http://thinky.io/documentation/architecture/

I think this should solve your problem.

That being said, it could be nice to pass a string instead of a model. It requires a bit of code, but nothing too hard on top of my head.

bobobo1618 commented 9 years ago

I had a look at that but (and it may be an issue with me using CoffeeScript or my inexperience) it doesn't appear to be working.

thinky = require './thinky'

Attachment = thinky.createModel 'Attachment', {
    id: String
    type: String
    meta: {
        _type: Object
    }
    hash: String
    tmpPath: String
    backendId: String
    status: Number
    fileId: String
    userId: String
}

File = require './file'
Attachment.belongsTo File, 'file', 'fileId', 'id'

module.exports = Attachment
thinky = require './thinky'

File = thinky.createModel 'File', {
    id: String
    type: String
    hash: String
    phash: String
    tmpPath: String
    name: String
    tags: [String]
    backendId: String
    status: Number
    userId: String
}

Attachment = require './attachment'
File.hasMany Attachment, 'attachments', 'id', 'fileId'

module.exports = File

Gives me

Error: First argument of `belongsTo` must be a Model

console.dir tells me that File is {} after that require.

neumino commented 9 years ago

I'm sorry, I didn't pay attention when I wrote this article. Can you try this:

thinky = require './thinky'

Attachment = thinky.createModel 'Attachment', {
    id: String
    type: String
    meta: {
        _type: Object
    }
    hash: String
    tmpPath: String
    backendId: String
    status: Number
    fileId: String
    userId: String
}
module.exports = Attachment

File = require './file'
Attachment.belongsTo File, 'file', 'fileId', 'id'
thinky = require './thinky'

File = thinky.createModel 'File', {
    id: String
    type: String
    hash: String
    phash: String
    tmpPath: String
    name: String
    tags: [String]
    backendId: String
    status: Number
    userId: String
}
module.exports = File

Attachment = require './attachment'
File.hasMany Attachment, 'attachments', 'id', 'fileId'

This should work I think.

bobobo1618 commented 9 years ago

Yep, that works, thanks!

I'd still prefer to reference using strings though. Does thinky have a nice object that stores models that I can conveniently hack into the relation functions?

bobobo1618 commented 9 years ago

Ah, don't worry. Didn't realise how clean and easy the code is. I'll send a pull request later :)

neumino commented 9 years ago

@bobobo1618 -- the thinky module stores all the models in the models field -- See https://github.com/neumino/thinky/blob/master/lib/thinky.js#L96

bobobo1618 commented 9 years ago

Dammit. Grabbing the model from the Thinky instance works of course but createModel has to have been called first, which brings the same issue back.

I'm happy to just leave it as is for now so I'll close the issue.

neumino commented 9 years ago

I think it should be possible to delay things until someone call getJoin. I'll give this idea a try later this week.

Let me know if you have more questions

bobobo1618 commented 9 years ago

Another idea: What if we defined relations in the schema? I'd actually like that a lot more. Something like:

File = thinky.createModel 'File', {
    id: String
    type: String
    hash: String
    attachments: {
        _type: hasMany
        localJoin: 'id'
        foreignJoin: 'fileId'
        foreignModel: 'Attachment'
    }
}

Attachment = thinky.createModel 'Attachment', {
    id: String
    type: String
    file: {
        _type: 'belongsTo'
        localJoin: 'fileId'
        foreignJoin: 'id'
        foreignModel: 'File'
    }
}

Adding something like foreignProperty on one end that defines how the access works on the other model would be nice too, as it means you only have to define the relation once, instead of reversing/duplicating it on the other end.

neumino commented 9 years ago

Oh, I didn't think about that. I like the idea.

I'm reopening the issue to make sure that I don't forget about it. I think there's something interesting to explorer here.

Thanks @bobobo1618 for the suggestion!

bobobo1618 commented 9 years ago

As a Django user this comes with some bias but it might be worth looking into the way they do things with model definitions (pretty much what I just described with the addition of a few events/hooks on the model) :)

chrisfosterelli commented 9 years ago

This is a great idea! The suggestion @neumino had works great as well, but isn't super intuitive and I was stuck for a little bit before I came across this. I'm sure it's caught some other people as well.

simonratner commented 9 years ago

For what it's worth, this is approximately the approach I took; it is a simple loader that delays defining relationships until all models have been loaded.

(Typing this from memory, forgive any obvious errors.)

models/A.js:

module.exports = function(thinky) {
  var A = thinky.createModel("A", ...);
  A.defineRelations = function() {
    A.hasMany(thinky.models.B, ...);
  };
  return A;
};

models/B.js:

module.exports = function(thinky) {
  var B = thinky.createModel("B", ...);
  B.defineRelations = function() {
    B.belongsTo(thinky.models.A, ...);
  };
  return B;
};

models/index.js:

var thinky = require("thinky")(options);
fs.readDirSync(pathToModels).forEach(function(file) {
  if (file != "index.js") {
    require(path.join(pathToModels, file))(thinky);
  }
});
for (var name in thinky.models) {
  if (typeof thinky.models[name].defineRelations == "function") {
    thinky.models[name].defineRelations();
  }
}
module.exports = thinky.models;

Elsewhere:

var A = require("./models").A;
xpepermint commented 9 years ago

Any progress? +1

neumino commented 9 years ago

Not really,

I'm pretty busy with reqlite for the moment, I'll try to schedule that in my free time

xpepermint commented 9 years ago

Thanks @neumino, you are very kind!

neumino commented 9 years ago

This is actually a bit tricky to do because we would register the creation of an index before the creation of the table. It's basically shuffling quite some code, which again is not that hard, but tedious and error prone-ish.

itsmunim commented 7 years ago

@neumino in the meantime, it would be great if you modify the article you wrote on architecture- since you didn't include your suggestion there.