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

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

Open
bobobo1618 opened this issue Oct 6, 2014 · 18 comments
Labels

Comments

@bobobo1618
Copy link

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:

  • Give up and put them in the same file
  • Try and split them into two and fail because they require each other and cause a circular import
  • Break it up into 3 files with a separate file requiring them both and setting the relations

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
Copy link
Owner

neumino commented Oct 6, 2014

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
Copy link
Author

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
Copy link
Owner

neumino commented Oct 6, 2014

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
Copy link
Author

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
Copy link
Author

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

@neumino
Copy link
Owner

neumino commented Oct 6, 2014

@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
Copy link
Author

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
Copy link
Owner

neumino commented Oct 6, 2014

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
Copy link
Author

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
Copy link
Owner

neumino commented Oct 6, 2014

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!

@neumino neumino reopened this Oct 6, 2014
@bobobo1618
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

Any progress? +1

@neumino
Copy link
Owner

neumino commented May 28, 2015

Not really,

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

@xpepermint
Copy link

Thanks @neumino, you are very kind!

@neumino neumino added the feature label Aug 9, 2015
@neumino
Copy link
Owner

neumino commented Aug 9, 2015

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
Copy link

itsmunim commented Apr 5, 2017

@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.

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

6 participants