Vulcan icon indicating copy to clipboard operation
Vulcan copied to clipboard

Make Vulcan database-agnostic

Open SachaG opened this issue 7 years ago • 16 comments

Let's talk about how to make it easier for Vulcan to support other databases.

I think the first step would be isolating the MongoDB connectors and taking them out of the default mutators.

SachaG avatar Feb 08 '18 01:02 SachaG

I started working on this here: https://github.com/VulcanJS/Vulcan/tree/connectors

The idea is that instead of calling Movies.find(...), you'll call Connectors[database].find(Movies, ...) where database defaults to mongo.

In the future, we'll be able to change it to, say, sql, and swap out the db (provided someone has written a sql implementation of find).

As an example here are the current Mongo connectors:

Connectors.mongo = {
  findOne: async (collection, documentId) => {
    return await collection.findOne(documentId);
  },
  find: async (collection, selector, options) => {
    return await collection.find(selector, options).fetch();
  },
  count: async (collection, selector, options) => {
    return await collection.find(selector, options).count();
  },
  new: async (collection, document) => {
    return await collection.insert(document);
  },
  edit: async (collection, documentId, modifier, options) => {
    return await collection.update(documentId, modifier, options);
  },
  remove: async (collection, documentId, options) => {
    return await collection.remove(documentId);
  },
}

I'm not in love with those specific action names by the way, it's probably something we'll want to harmonize with the GraphQL API (see #1859). So maybe create/update/delete instead of Mongo's insert/update/remove or this weird new/edit/remove mix?

I'm also making everything async/await for better forward compatibility (instead of relying on Meteor' s weird magic that lets you write const movies = Movies.find()).

SachaG avatar Feb 11 '18 04:02 SachaG

(Ah, good to know that that is Meteor magic. I have been really confused by how const movie = Movies.find() works, and why it works even in non-async contexts.)

I think this general approach makes sense, though I am worried that we are encouraging Mongo-like thinking on a deeper level than just the DB connector. The whole idea of collections with edit/update/insert functions feels very Mongo like, and maybe we want to make things more agnostic on that level as well (though that would be a much larger project).

Discordius avatar Feb 11 '18 05:02 Discordius

I like CRUD as the abstraction. There’s likely someone out there already doing something similar so might be worth looking into.

justinr1234 avatar Feb 11 '18 13:02 justinr1234

@Discordius I agree that we should be careful no to impose any mongo-isms on the architecture if the goal is to support many databases.

That said, I actually think the create/update/delete functions generalises fairly well to other databases.

In #1859 we discuss adopting the Prisma API for VulcanJS. Prisma currently only support relational databases, but we are working on expanding it to other db types including document, search, time-series, graph etc.

The following is a nested mutation that uses a SQL transaction to create a row in two tables. This is conceptually similar to inserting a nested document in mongo, but the performance characteristics and query capabilities are entirely different.

mutation {
  createMovie(data: {
    title: "Thor: Ragnarok",
    year: 2017
    director: {
      create: {
        name: "Taika Waititi"
      }
    }
  }){
    id
  }
}

@SachaG - is the selector in your example above specific to mongo or could that be a generic type that can be used by different implementations of find?

sorenbs avatar Feb 11 '18 16:02 sorenbs

@sorenbs currently it would be a Mongo selector but eventually it could become a generic selector object that works for multiple databases.

I guess that's actually another place where we could harmonize our way of doing things, since I'm guessing Prisma has the same kind of issues?

As for the nested mutation, that's a really cool concept. I don't really see myself implementing it in Vulcan though so if people need it I'd rather invest more time on making Vulcan and Prisma play well together :)

SachaG avatar Feb 12 '18 01:02 SachaG

FeathersJS has a clean interface close to the proposed one and they return promises.

find(params),
get(id, params)
create(data, params)
update(id, data, params)
patch(id, data, params)
remove(id, params)

The params can contain client: 'data' and query which is Vulcans combined selector and filter options. Feathers implements local hooks (before, after, error) and the same for global hooks. Feathers allows to multi only on patch with an id of null. The learnings are all there. Although Feathers has a GraphQL version, it is not an official package and hasn't been updated in a while. So I am not sure how this would tie in with the data layer or if that is even a goal since Feathers creates 6 endpoints for each service.

MHerszak avatar Feb 12 '18 03:02 MHerszak

Interesting. What's the difference between update and patch?

SachaG avatar Feb 12 '18 03:02 SachaG

Update only one document patch for multiple document updates. Basically a mongo update with options set to multi = true by default. However, you can drop in Mongo connector or Mongoose connector. And by that I mean it is connector independent.

MHerszak avatar Feb 12 '18 03:02 MHerszak

OK. And I'm assuming params in get(id, params) or update(id, data, params) is just some kind of options object?

Yeah that seems like a sensible API to me, I'll go with that.

SachaG avatar Feb 12 '18 04:02 SachaG

const params = { query: { _id: user._id } } and some client-side data to send extra variables to the backend. The query is the most essential one.

MHerszak avatar Feb 12 '18 04:02 MHerszak

A query =>

const { selector, options, client } =
        Collection.getParameters({ view, ...obj.getProps(rest) }, obj.service);
    // Assemble query and submit
    const query = { ...selector, ...options, $client: client };

MHerszak avatar Feb 12 '18 04:02 MHerszak

ok so this gives us:

Connectors.fooDB = {
  get: async (collection, documentId, options) => {
    return await ...;
  },
  find: async (collection, selector, options) => {
    return await ...;
  },
  count: async (collection, selector, options) => {
    return await ...;
  },
  create: async (collection, document, options) => {
    return await ...;
  },
  update: async (collection, documentId, modifier, options) => {
    return await ...;
  },
  delete: async (collection, documentId, options) => {
    return await ...;
  },
}

Maybe later we can also add upsert to the list.

SachaG avatar Feb 12 '18 09:02 SachaG

Actually maybe find should be list to maintain consistency with the resolvers and the client-facing API?

SachaG avatar Feb 17 '18 07:02 SachaG

Maybe update and remove should take a selector rather than a documentId? We have the case of wanting to remove all dummy users in Vulcan for example.

SachaG avatar Feb 17 '18 09:02 SachaG

If you want to be able to build an interface you would need to know what most Connectors work with. You could leave it up to the community but Vulcan needs to be able to provide a working interface. In my opinion, find would be better than list.

class MongoConnector {
   find(selector, options) {
      return this.collection.find(selector) // this is very vage because I would not know how to use SQL with Vulcan.
   }
   create()
   update()
   ...
}
// plug in
Collection.Connector.use(MongoConnector)
// usage
Collection.find(selector, options)

MHerszak avatar Feb 17 '18 14:02 MHerszak

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 23 '18 09:11 stale[bot]