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

Models or Connectors? #118

Closed
zvictor opened this issue Sep 1, 2016 · 13 comments
Closed

Models or Connectors? #118

zvictor opened this issue Sep 1, 2016 · 13 comments

Comments

@zvictor
Copy link
Contributor

zvictor commented Sep 1, 2016

I am confused with the use of the words models and connectors among all the docs of the Apollo project. It is hard for me to figure out what is currently implemented and what is just a draft proposal.

Apparently, right now, the term connector actually refers to the combination of what in the future will be split into the model and the connector concepts. Am I correct?

@stubailo
Copy link
Contributor

stubailo commented Sep 1, 2016

I think you're right. Here's how I think about it:

  1. Connectors deal with the backend-specific logic: running a query against a DB connection, or passing credentials to an API. You don't expect to change your connectors very often, and eventually they can be on npm for a variety of backends.
  2. Models deal with the application-specific logic: which query to run, and which endpoint to call. These will change all the time as you add more functionality to the API.

It's easy to see this distinction in the GitHunt app:

  1. The GitHub connector deals with API keys, caching results, etc: https://github.com/apollostack/GitHunt-API/blob/master/api/github/connector.js
  2. The GitHub model just specifies which URL to fetch: https://github.com/apollostack/GitHunt-API/blob/master/api/github/models.js
  3. Here's where the connector and models are initialized: https://github.com/apollostack/GitHunt-API/blob/83562ca7c18e65b110e9733a33b29268c6b2b92a/api/index.js#L89-L103

I think we should improve the documentation to make this concept a lot clearer, thanks for bringing this up. Does the above distinction make sense?

@zvictor
Copy link
Contributor Author

zvictor commented Sep 2, 2016

Thank you! The links were very helpful and allowed me to keep moving in the scaffolding of my project.

The concept of Connectors is clear now, but I see some issues in the use of the Models concept as it seems to be overlapping with some of the concepts in the repository pattern.

In the GitHunt example I can see the methods of the models always return new instances, and not data of a given instance. That's work for Repository instead. Check Users.getByLogin for instance.

I would expect the methods of a Model to only return data that belong to the instance. In other words, a "user" could only tell me his age, address, or whatever else similar to that. It would only be acceptable for a "user" to give me another "user" as response if I ask about relationships to other instances that belong to them.

@zvictor
Copy link
Contributor Author

zvictor commented Sep 2, 2016

The Author's model vs repository

Let's take the following resolve as example.

const author = {
  posts: (author, args, context) => {
    ...
  },
};

In my experiments here, the author argument is a plain Object. If it were an instance of an Author model, both following examples should be possible, otherwise only the latter.

const author = {
  posts: (author, args, context) => {
    return author.getPosts();
  },
};
const author = {
  posts: (author, args, context) => {
    return (new context.models.Author(author)).getPosts();
  },
};

However, if I understand correctly the way Apollo goes, I am supposed to call it like that:

const author = {
  posts: (author, args, context) => {
    return context.models.Post.find({ authorId: author.id })
  },
};

... and that's not a Model, but a Repository.

@stubailo
Copy link
Contributor

stubailo commented Sep 2, 2016

Yeah, perhaps the wording is wrong. In Rails they tended to get mixed together. Repository seems really vague though, is there something else we could call it that would be clearer?

@zvictor
Copy link
Contributor Author

zvictor commented Sep 2, 2016

I don't really know, I haven't had the need to use this pattern for a while. Could it be called collection? Idk. It could be even more confusing.

More important than the naming is to implement Models properly. It would be really great to be able to call author.getPosts() inside the resolver instead of sending information around. Doing that, we can put the repository inside the model class.

So, this is how it could be:

const john = new Author({firstName: 'John', lastName: 'Doe'}); // Author
const posts = john.getPosts(); // [Post]

...

const johnAgain = Author.repository.findOne(john._id); // Author
const postsAgain = Post.repository.find({ 'author._id': john._id}); // [Post]

@zvictor
Copy link
Contributor Author

zvictor commented Sep 2, 2016

Oh! Now I remember. Django calls it manager and use the term objects inside the class.

@stubailo
Copy link
Contributor

stubailo commented Sep 2, 2016

Nice! My only problem with this approach is that this will mean that a model needs to be aware of multiple backends. For example, you can imagine a situation where Author objects are stored in SQL, but their posts come from the Medium API or something (if your app is some kind of news aggregator).

It would be odd for me if john.getPosts, implemented on the Author class, had to know how to call the Medium API. That's why I prefer to return the post ID instead, and have the resolver handle calling the right repository to get the post.

@zvictor
Copy link
Contributor Author

zvictor commented Sep 2, 2016

this will mean that a model needs to be aware of multiple backends

will it need? the connector will still be there abstracting it.

class Author {

  ...

  getPosts() {
    return this.posts || Post.objects.find({ 'author._id': this._id});
  }
}

It goes this way:

Model --> Manager(or Repository) --> Connector --> Backend

@helfer
Copy link
Contributor

helfer commented Sep 2, 2016

My only problem with this approach is that this will mean that a model needs to be aware of multiple backends.

I think john.getPosts makes perfect sense. Whether that fetches the actual post objects or just creates skeleton objects which fetch the rest of their data when needed is an implementation decision which I think cannot just be made generally. Returning just skeleton objects leads to better separated code, but it is also more complex because you have to deal with the possibility that objects could have their data only partially loaded.

@stubailo
Copy link
Contributor

stubailo commented Sep 2, 2016

I think it's a matter of preference. We can change the example to be more model-like if it is helpful.

@zvictor
Copy link
Contributor Author

zvictor commented Sep 6, 2016

I made a proof of concept that I would like to share with you. It is object/model oriented and remove completely the use of resolvers in the business logic. There is a helper function that inspects the schemas and models and generate the resolvers automatically.

My main concern while making this was to keep a clear separation of business logic and application. Business is in /domain and the API in /server. There is another helper function that let the server do dependency injection, connecting the managers to the connectors, and that's all coming from application to business.

All the models have a default manager which receives a default connector as well. Both could be overwritten or expanded, this way e.g. a PersonManager could connect to a SQL backend while keeping the mongodb connector active. The same way, a Person model could have multiple managers, let's say Person.authors, Person.editors, and Person.people.

To keep separation of concerns I assumed the mongodb API as standard to communicate between all managers and connectors. We would need to review this point.

The example code can be found in the perfect graphql starter repository and the helpers are inside the brand new graph-object package.

@manuelfink
Copy link

@stubailo @helfer is there already any sample of a model - connector implementation as you imagine it?

Is the connector models design description the latest approach you would recommend?

I'm asking because in the samples are several different approaches implemented eg: In the apollo-server-tutorial resolvers directly access the orm models of sequelize. Thereas the the githunt-api resolvers use knex through a model.

Is the intention here to make connecters fully exchangeable and standardized?

In the githunt example also the database connectors are hard coded into the models. Thereas the connector models design description foresees a more generic approach. Connectors are injected into the model. If every connector would implement the same functionality they would be exchangeable. However I think this may be a lot of work on some connecters and may much easier achieved on business model logic.

@jferrettiboke
Copy link

@stubailo

Nice! My only problem with this approach is that this will mean that a model needs to be aware of multiple backends. For example, you can imagine a situation where Author objects are stored in SQL, but their posts come from the Medium API or something (if your app is some kind of news aggregator).

The Repository Pattern should be used to communicate with its Model. So, if you need more than this, you can use the Service Pattern. A service can use other services without being worried about the model. Service <-> Repository <-> Model

This can scale well, however if the app is small you might over-engineering the app with many layers of abstraction.

@helfer helfer closed this as completed Jun 28, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants