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

Apollo 2.3.0 does not trigger cache changes when cache is populated first time. #2531

Closed
piotr-pawlowski opened this issue Aug 21, 2020 · 18 comments

Comments

@piotr-pawlowski
Copy link

Summary
Apollo 2.3.0 does not trigger cache changes when cache is populated first time. On 2.2.0 version everything is fine.

Version
2.3.0.

@martinbonnin
Copy link
Contributor

Thanks for the bug report! Can you share more details about the scenario?

  • How is the cache populated? Through a query? Or using the imperative store APIs (store.write, ...) ?
  • How are changes detected? A query watcher? Something else?

@piotr-pawlowski
Copy link
Author

The cache is populated from query and changes are detected with ApolloStore.RecordChangeSubscriber.

@martinbonnin
Copy link
Contributor

Thanks!

This seems to work for me. Can you double check that you're holding the reference to the ApolloStore.RecordChangeSubscriber ? It's stored internally in a WeakHashMap so if you pass an object or anonymous class, chances are it will be garbage collected before the listener has a chance to fire.

@piotr-pawlowski
Copy link
Author

I am switching from 2.2.0 (I do not use 2.2.2 because in this version cache is not populated at all, database is always empty) to 2.3.0, compiling, running and then my app stops working properly. My app starts foreground service to fetch data from cloud and save it in cache and at the same time view (activity/fragment) is waiting for data to be loaded from cache. The cache is populated (I see that apollo.db file is updated) but there is no event trigged about updated cache.

@martinbonnin
Copy link
Contributor

martinbonnin commented Aug 21, 2020

Where do you store the reference to the RecordChangeSubscriber? It should be in a long lived object:

class MyFragment: Fragment() {
    val subscriber = object : ApolloStore.RecordChangeSubscriber {
        override fun onCacheRecordsChanged(changedRecordKeys: Set<String>) {
            println("onCacheRecordsChanged $changedRecordKeys")
        }

    }
    
    override fun onCreateView() {
        apolloClient.apolloStore.subscribe(subscriber)
    } 
}

The below will not work:

class MyFragment: Fragment() {   
    override fun onCreateView() {
        val subscriber = object : ApolloStore.RecordChangeSubscriber {
            override fun onCacheRecordsChanged(changedRecordKeys: Set<String>) {
                println("onCacheRecordsChanged $changedRecordKeys")
            }
        }
        apolloClient.apolloStore.subscribe(subscriber)
        // the subscriber will be garbage collected soon after this method finishes and the listener will not be called
    } 
}

@piotr-pawlowski
Copy link
Author

ApolloStore.RecordChangeSubscriber is subscribed in viewModel so it lives as long as relead fragment lives.

@martinbonnin
Copy link
Contributor

I made a small snippet there: https://github.com/martinbonnin/apollo-android-samples/blob/7d971f94b5506d5293b7f9243aa820a6bc3d5a6f/cache-record-subscriber/src/test/kotlin/MainTest.kt#L38

Can you see anything that's different from from your setup there?

@piotr-pawlowski
Copy link
Author

I have checked all my code and there is one more component (singleton created and subscribed in Application.class) and it observes cache with query. Unfortunately it is not triggered too. Maybe is it a problem with deprecated responseFetchers?

val disposable = apolloClient.query(Query()).responseFetcher(ApolloResponseFetchers.CACHE_ONLY).watcher().rx().doOnNext { 
/* log here */ }.subscribe()

Query is exactly the same as fetched from network.

@martinbonnin
Copy link
Contributor

The main differences regarding cache that I can think of are #2422 #2412 and #2402 but even with background writes, I cannot reproduce this issue.

@martinbonnin
Copy link
Contributor

I guess the next step is put a breakpoint in RealApolloStore.publish and see if it's reached To determine whether it's an issue upstream (somehow cache changes not being detected) or downstream (listeners not being notified)

@piotr-pawlowski
Copy link
Author

Yeah, I gonna test it with debugger and try to find an issue. Thanks for your help!

@martinbonnin
Copy link
Contributor

This might be linked to #2537 although that other issues apparently also happens with 2.2.0

@piotr-pawlowski
Copy link
Author

I have found the problem:

In RealApolloStore:

@Override public void publish(@NotNull final Set<String> changedKeys) {
    checkNotNull(changedKeys, "changedKeys == null");

    if (changedKeys.isEmpty()) {
      return;
    }

    Set<RecordChangeSubscriber> iterableSubscribers;
    synchronized (this) {
      iterableSubscribers = new LinkedHashSet<>(subscribers);
    }

    for (RecordChangeSubscriber subscriber : iterableSubscribers) {
      subscriber.onCacheRecordsChanged(changedKeys);
    }
  }

changedKeys is empty.

@martinbonnin
Copy link
Contributor

@piotr-pawlowski there is a potential fix in #2538.
Can you check the snapshots to see if it applies here?

@piotr-pawlowski
Copy link
Author

I tested version 2.3.1 and it doesnt work too :(

@martinbonnin
Copy link
Contributor

This is expected, the patch isn't released yet. The latest snapshot is 2.3.2-SNAPSHOT. To use it, you'll have to enable the snapshots repository: https://github.com/apollographql/apollo-android#releases

repositories {
  maven { 
    url = uri("https://oss.sonatype.org/content/repositories/snapshots/")
  }
}

@piotr-pawlowski
Copy link
Author

Ok, version 2.3.2-SNAPSHOT seems to work properly.
Are you gonna fix the issue #2532 too?

@martinbonnin
Copy link
Contributor

Ok, version 2.3.2-SNAPSHOT seems to work properly.

Nice! Glad to hear that

Are you gonna fix the issue #2532 too?

This bug looks different, let's move the discussion over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants