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

Consider Moor cache #402

Closed
micimize opened this issue Sep 2, 2019 · 17 comments
Closed

Consider Moor cache #402

micimize opened this issue Sep 2, 2019 · 17 comments
Labels
enhancement New feature or request 📦 cache Relates to caching functionality wontfix This will not be worked on

Comments

@micimize
Copy link
Collaborator

micimize commented Sep 2, 2019

I'm not sure how hard this would be, but using a moor database as a cache seems like a really powerful approach to me

@mainawycliffe
Copy link
Collaborator

I think it is worth giving it a try

@Sewdn
Copy link

Sewdn commented Sep 26, 2019

This would be much more stable, because now there are several issues with saving cache to persistance layer from memory and restoring it again in several lifecycle methods of the app.

A cache provider writing and reading straight from and to a reactive persisted store would mean much less overhead and much less circumstances (eg app crashes) were you would/could loose cache.

@mainawycliffe
Copy link
Collaborator

mainawycliffe commented Oct 4, 2019

Moor DB supports flutter for web, and I think Cache is the one stopping us from support flutter for web. We can follow the steps here to use Moor for both flutter for native apps and web. The one gotcha is that it uses WebAssembly, which while the adoption is great will exclude some browsers out there.

@micimize
Copy link
Collaborator Author

micimize commented Oct 6, 2019

One challenge is getting a sane inheritance chain in dart with fragments, but I think this trick should work:

https://gist.github.com/micimize/ec9df3c1df23f415621fd3da7e81209e
// base type
class Foo {
  String _foo;
  int _bar;
}

//fragment exposing foo
class FooFragment extends Foo {
  String get foo => _foo;
  set foo(String value) => _foo = value;

  FooFragment({String foo}) {
    this.foo = foo;
  }
}

@klavs
Copy link
Collaborator

klavs commented Oct 7, 2019

Here's something to consider

schema.graphql

type Query {
  movie(id: ID!): Movie
}

enum DescriptionKind {
  SENTENCE
  PARAGRAPH
  PAGE
}

type Movie {
  id: ID!
  title(locale: String!): String!
  description(locale: String!, kind: DescriptionKind!): String
}

get_movie.graphql

query GetMovie($id: ID!){
  movie(id: $id) {
    id
    originalTitle: title(locale: "en")
    originalShortDescription: description(locale: "en", kind: SENTENCE)
    localTitle: title(locale: "lv")
    localShortDescription: description(locale: "lv", kind: SENTENCE)
    localDescription: description(locale: "lv", kind: PARAGRAPH)
    localFullDescription: description(locale: "lv", kind: PAGE)
  }
}

Here you'd need to cache:

{
  "MOVIE:MOVIE_ID": {
    title(en): ...
    title(lv): ...
    description(en, SENTENCE): ...
    description(lv, SENTENCE): ...
    description(lv, PARAGRAPH): ...
    description(lv, PAGE): ...
  }
}

Meaning that every parameterised field must be treated as some kind of collection with a key of variable count of dimensions.

@klavs
Copy link
Collaborator

klavs commented Oct 7, 2019

With that I guess I wanted to say that it is ok to model the cache in a type-safe manner, but we should remember that the types used in the cache will not be the same types the users are going to use.

With the previous example, users will expect something like this:

class GetMovieResponse {
  String id;
  String originalTitle;
  String originalShortDescription;
  String localTitle;
  String localShortDescription;
  String localDescription;
  String localFullDescription;
}

Also, "type-strict" cache demands that all the queries and the schema is known at build-time. There's no problem with schema changing in a compatible way, but there's a problem if somehow a query is changed after the build – the cache would not be able to store a field that was not in the schema at the time of the build.

Not to discourage anyone, but It feels to me that a client-side cache should be more NoSQL than SQL.

@micimize
Copy link
Collaborator Author

micimize commented Oct 7, 2019

@klavs schema needs to be known, but not necessarily operations. If we parse operations at runtime we could use that to write to normalize into the correct client-side table. But yeah, using moor basically means generating an entire relational database schema for the client based on your graphql schema.

I think we'd want to do this with methods as well, so description(locale, description) becomes something like

String description(String locale, DescriptionKind kind) =>
  _descriptionColumn[Tuple(locale, kind)]

And we store the previous results of the method in a custom map-like sql type (unless it is a relation to another type)

Still... sounds hard.

@klavs
Copy link
Collaborator

klavs commented Oct 7, 2019

@micimize you're right, but if we let users use arbitrary queries, they may run into a query containing a field not in that build-time schema and then it all falls apart.

That description example is quite trivial comparing to what is possible. GraphQL input types may even be nested in quite advanced ways.

@smkhalsa
Copy link
Contributor

I'm wondering how moor will perform relative to other persistent datastore options. It seems that it's built on sqlite. What about using hive? It also supports query streams and seems highly performant. It also supports web out of the box (using IndexedDB, so it should avoid the browser compatibility issues mentioned by @mainawycliffe above).

@mainawycliffe
Copy link
Collaborator

It seems we have a number of choices when it comes to cache, which IMO is a good thing.

@mainawycliffe
Copy link
Collaborator

There is also sembast, looks good too. I am thinking we should pick one package maybe two, that will work for everyone, cover almost all needs, and then we add some tutorials on implementing a custom cache.

@JasCodes
Copy link

I agree with @mainawycliffe sembast is more versatile option.

Thx

@micimize
Copy link
Collaborator Author

we should implement multiple caches as different packages like graphql_cache_sembast, graphql_cache_hive. I especially wouldn't consider adding a moor cache to core.

@mainawycliffe
Copy link
Collaborator

mainawycliffe commented Oct 24, 2019

Yea, I agree, multiple packages makes sense and also opens the opportunity for third-party community-supported caches, not just those at the core.

@mainawycliffe
Copy link
Collaborator

mainawycliffe commented Oct 31, 2019

Starting from next week, I am going to build a hive cache package and add to the list of packages in the packages directory - graphql_cache_hive will be the name. I have created an issue for that (#456).

In case we decide to use it at the core, then we can just import the package.

@micimize
Copy link
Collaborator Author

This would be a significant undertaking. I might one day still do it, but it would belong in a different / 3rd party project

@SalahAdDin
Copy link

There is also sembast, looks good too. I am thinking we should pick one package maybe two, that will work for everyone, cover almost all needs, and then we add some tutorials on implementing a custom cache.

It seems not to be possible soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 📦 cache Relates to caching functionality wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

7 participants