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

How to invalidate cahce #837

Closed
vytautas-pranskunas- opened this issue Mar 10, 2021 · 26 comments
Closed

How to invalidate cahce #837

vytautas-pranskunas- opened this issue Mar 10, 2021 · 26 comments
Labels
🐛 bug Something isn't working

Comments

@vytautas-pranskunas-
Copy link
Contributor

vytautas-pranskunas- commented Mar 10, 2021

Hello,

query as well as watchQuery returns cashed properties when fetching the list and this list contains other similar records with same ids but different values. (btw - while talking here i realized more about nature of this bug thats why it took more time to put cleaner description)

I gave enough examples and screenshots but can try to give better one :)

server returns this data:

[
  {
    'id': '1',
    'name': 'test',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
  {
    'id': '2',
    'name': 'test2',
    'nestedObject': {
         id: 'nestedId',
         value: '6'
    }
  },
]

client receives this data:

[
  {
    'id': '1',
    'name': 'test',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
  {
    'id': '2',
    'name': 'test2',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
]
  • names in both objects will be received correctly because top level id is different.
  • value will get same value because id is the same despite server sending different value
@vytautas-pranskunas- vytautas-pranskunas- added the 🐛 bug Something isn't working label Mar 10, 2021
@micimize
Copy link
Collaborator

what you described is how the client should work. The only fetch policy that should use the cache and ignore the network is cacheFirst. If it is not fetching the query then there is an issue, but I'll need actual code to tell if you're doing something... unorthodox.

My best guess is that you've created optimistic data in the cache somehow, that is being merged into the result you're retrieving from the server.

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 12, 2021

Ok, I am not sure what code do you need but here you go client code and fetching code:

GraphQLClient createGraphqlClient({
  @required String uri,
  String subscriptionUri,
}) {
  Link link = HttpLink(uri);

  var authTokenService = GetIt.instance<AuthTokenService>();
  link = LocaleLink().concat(link);
  link = authTokenService.graphQLFreshLink.concat(link);

  if (subscriptionUri != null) {
    final WebSocketLink websocketLink = WebSocketLink(subscriptionUri,
        config: SocketClientConfig(
            autoReconnect: true,
            delayBetweenReconnectionAttempts: Duration(seconds: 2),
            inactivityTimeout: null,
            initialPayload: () {
              return authTokenService.addAuthHeader(EnvVariables.currentHost);
            }));

    link = Link.split(
      (request) => request.isSubscription,
      websocketLink,
      link,
    );
  }

  link = errorLink.concat(link);

  final client = GraphQLClient(
    defaultPolicies: DefaultPolicies(query: Policies(fetch: FetchPolicy.noCache)),
    cache: GraphQLCache(store: HiveStore()),
    link: link,
  );

  return client;
}

I have added Policies(fetch: FetchPolicy.noCache) in order to be able to work. If i add any other policy like cacheNetwork (which i would prefer) it starts acting like described above

locale link

class LocaleLink extends Link {
  @override
  Stream<Response> request(
    Request request, [
    NextLink forward,
  ]) async* {
    request.variables.putIfAbsent('locale', () => AppLocalizations.locale.languageCode);

    yield* forward(request);
  }
}

and fresh link if from package fresh_graphql and loke like this

FreshLink<OAuth2Token>(
      tokenStorage: _inMemoryTokenStorage,
      refreshToken: _refreshToken,
      tokenHeader: _tokenHeader,
      shouldRefresh: (gql_exec.Response result) {
        if (!hasToken) {
          return false;
        }

        var hasAuthError = result.errors != null &&
            result.errors
                    .where((e) =>
                        e.extensions != null &&
                        e.extensions['code'] != null &&
                        e.extensions['code'] == 'UNAUTHENTICATED')
                    .toList()
                    .length !=
                0;
        return hasAuthError;
      },
    )

not sure if you need LocaleLink and graphQLFresh links?

and gql call look sliek this:

var results = await _graphQLClient
          .query(QueryOptions(document: getAllInquiries, variables: <String, dynamic>{
                'userId': _userStore.id,
                'currentPage': _store.currentPage,
           })).withTimeout();

where withTimeout is extension that terminates never ending request (I saw issue somewhere reported to you to setup time when not responding request should end but it is not implemented yet as far i know)

@micimize
Copy link
Collaborator

client.query doesn't really work with policies like cacheAndNetwork because it only returns a single result. If you want to use the cache result for optimism, and still fetch the results from the network, you'll need to use client.watchQuery, which returns an ObservableQuery with a stream of responses

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 12, 2021 via email

@micimize
Copy link
Collaborator

Check the result.source of your watchQuery results. You should see maybe a loading result, a cache result, and eventually a network result. If the network result doesn't have the data you're expecting, confirm that's what was sent by the network, and that no other operations were done by the client.

I consider it highly unlikely that the client is broken in this way, but it is always possible.

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 12, 2021

I hear what you are saying and will try soon watchQuery ant paste all responses but for now with query i get this:
this a response on client:

image

This is a screenshotof DB:
image

This is screenshot of postman
image

As you can see DB and postman gives approvedByCustomer: false while gql client gives true

It was enough for me to set cacheAndNetwork policy which according to what you said should not work at all with query :(

@vytautas-pranskunas-
Copy link
Contributor Author

I checked result source and it is saying - network but reuslt is same:
image

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 12, 2021

as i said - it is bug of cacheing because as sson as i updated _id of that object in DB i got imeediatly updated results:
image

p.s. i do rememeber in your prev library versions you were suggesting to use cache comparer where results were cached accodring to ids. I think it is related to that

So 2 things are clear:

  1. cache should not - but applies to query
  2. There is a bug in library that even source is network but ir clearly returns cached results and ivalidates them only when ID changes or value for all ids changing within response results (becase when i updated property within all response documents it has changed in cache also)

@micimize
Copy link
Collaborator

well, if you can distill the problem into a test case and PR it against beta, I'll take a look later

@vytautas-pranskunas-
Copy link
Contributor Author

Well, I cannot imagine how to do this because I need to host api that you have access do in order to change data and create whole client. But VĮ an site that you have testing env and with all code given above you can easily test it. You can ignore links because I tested without them.

I think this c should be a priority one because it looks like swiss issue keeping in mind that v query should not work with cache at all...

@micimize
Copy link
Collaborator

Tests are written with mocks. That is how we isolate and reproduce behavior: https://github.com/zino-app/graphql-flutter/blob/beta/packages/graphql/test/graphql_client_test.dart#L274

I think your issue can be best distilled with two subsequent networkOnly client.query calls with different mocked results.

@vytautas-pranskunas-
Copy link
Contributor Author

So I think you are the best candidate to test it. I would like to help but have to work on MVP hard. I hopw that before our release (2 month) thi sissue will be resolved and i can get benefits of caching and its invalidation :)

@micimize
Copy link
Collaborator

@vytautas-pranskunas- your time is not more valuable than mine. As you've said, if this is an issue then it is massive, and I will fix it, but I don't know that anyone else has this issue. I don't even know what version you're on. Creating a test case is the only way you can convince me that the mistake is with the library.

I consider the way you're behaving here very inconsiderate. You are acting like maintaining this project is my job, as well as interpreting your incredibly sloppy bug description that is almost indecipherable because of your lack of interest in correcting your spelling mistakes.

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 12, 2021

wooowww... lets step back a bit :) I did not mean that ;) take things easy...
Your time, of course, is same valuable for you as mine for me - thats without discussions.
I do not know if you are the owner and main maintainer of this package - i think i was talking the way that you are the one :) If not - then we had to made it clear from the beginner.

Comming from other side - i am maitainer and owner of 2 state managment libraries for React and Angular and if somebody reports a bug I consider it as mine responsibility to fix it over all. Thats might be the reason why my communication made you think that i am pushing fix on you - but again - if you not an owner then lets step back :)

My spelling mistakes - most of the time I am answering from phone with autocompletion... But you are right - i should pay more attention to it.

not sure what do you mean "sloppy" description of the bug because it is clear and can be described like this:

  • query as well as watchQuery returns cashed properties when fetching the list and this list contains other similar records with same ids but different values. (btw - while talking here i realized more about nature of this bug thats why it took more time to put cleaner description)

I gave enough examples and screenshots but can try to give better one :)

server returns this data:

[
  {
    'id': '1',
    'name': 'test',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
  {
    'id': '2',
    'name': 'test2',
    'nestedObject': {
         id: 'nestedId',
         value: '6'
    }
  },
]

client receives this data:

[
  {
    'id': '1',
    'name': 'test',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
  {
    'id': '2',
    'name': 'test2',
    'nestedObject': {
         id: 'nestedId',
         value: '5'
    }
  },
]
  • names in both objects will be received correctly because top level id is different.
  • value will get same value because id is the same despite server sending different value

Did this make more sense?

@vytautas-pranskunas-
Copy link
Contributor Author

btw I am on version:
graphql_flutter: ^4.0.0

@vytautas-pranskunas-
Copy link
Contributor Author

And one more thing: Convincing you it is not the purpose of all this ;) I just have reported a bug and it is up to maintainer to investigate and fix it or not. I am not demanding consumers of my librarires to get familiar with code i wrote, write tests and convince me - I just feel responsibility for others who have put trust and chosen my library :)
I can cooperate, help to investigate, reproduce but as i said at this time i do have very tide deadlines and just pshically cannot spent time on gathering knowledge how this library works.

Bwt - I am contributing to less time consuming libraries. For example yesterdays PR: https://github.com/404shades/Gender-Selection-Flutter/pulls

@micimize
Copy link
Collaborator

Apologies if I projected intent on you.

I added a test for what we were discussing before (9f3f1dd), but based on your update this is probably expected due to the normalization layer:

if I'm understanding correctly tour nestedObjects all have the same id, nestedId? If so, the reason
based on your update this is expected due to the normalization layer. You probably need to provide a custom dataIdFromObject to the cache:
https://pub.dev/documentation/graphql/latest/graphql/GraphQLCache-class.html

@micimize
Copy link
Collaborator

realized that there weren't very good docs for normalization, so I added some https://github.com/zino-app/graphql-flutter/tree/beta/packages/graphql#normalization

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 13, 2021

Hmm yes that's a thing. I am not really sure now which way to go because this is just one specific response from big app and to create custom global dataIdFromObject based on id and approvedByCustomer property would be not good. Because other requests does not have property. Unless there is a possibility to provide dataIdFromObject per request?

@micimize
Copy link
Collaborator

There isn't, but if it's a property of the nested object's data type you could just check for it in dataIdFromObject and treat everything else as usual, or use a type policy (though I don't really understand them that well).

Actually, what might be best here is for you to use parseString from the gql library, and not add __typename to the nested objects. That way they won't be normalized, and thus won't overwrite eachother, which it seems like might be what you're looking for (the gql(doc) helper we export automatically adds __typename to every node)

@vytautas-pranskunas-
Copy link
Contributor Author

Can you give e an example of parseString usage?

@micimize
Copy link
Collaborator

micimize commented Mar 13, 2021

See https://github.com/gql-dart/gql/tree/master/gql#packagegqllanguagedart. The only difference between it and our gql helper is the need to explicitly include __typename

@vytautas-pranskunas-
Copy link
Contributor Author

Hmm looks like hacking.
I wonder, if all my requests are network based why instead of mapping this lib, during normalization, adds cached values. I mean I would understand if days comes from caching...

Anyway it would be nice to have v some simple solution like type policy for normalization where we could define what is id for example this kind v of map:
IdMap: {
'property.nestedObject': ['_id', 'approvedByCustomer' ]
}

Where key is path to nested document and value - I'd key

@vytautas-pranskunas-
Copy link
Contributor Author

vytautas-pranskunas- commented Mar 13, 2021

This actually bigger problem than I thought especially for document db like MongoDB. For example I store profile picture of customer as part of other documents. So if I have orders collection which might contain records with old and new customer profile pictures but I will always get one picture (not sure which old or new) if I send them along with customerId 😕

@micimize
Copy link
Collaborator

In terms of controlling cache rereads, there is CacheRereadPolicy which I believe can be set per-operation:

CacheRereadPolicy determines whether and how cache data will be merged into the final QueryResult.data before it is returned. Possible options:

  • mergeOptimistic: Merge relevant optimistic data from the cache before returning.
  • ignoreOptimistic: Ignore optimistic data, but still allow for non-optimistic cache rebroadcasts if applicable.
  • ignoreAll: Ignore all cache data besides the result, and never rebroadcast the result, even if the underlying cache data changes.

But I think this is more a deeper-cutting data modeling problem. We base our normalization scheme largely on apollos, so you might find this useful: https://www.apollographql.com/blog/demystifying-cache-normalization/

@vytautas-pranskunas-
Copy link
Contributor Author

That's exactly what I am taking about - here is no problems with relational DB as they said - target is to not have data duplication. However in document db data duplication is common if that data belongs together... So I have to think how to tackle this...

Thank anyway for all 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants