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

When evicting an entity from the cache, the list becomes empty when it is nested in an object. #498

Closed
ValentinVignal opened this issue Mar 6, 2023 · 4 comments · Fixed by #499

Comments

@ValentinVignal
Copy link
Contributor

ValentinVignal commented Mar 6, 2023

I have this code (you can also check out https://github.com/ValentinVignal/flutter_app_stable/tree/ferry/cache-and-fragment)

# schema.graphql

type Pokemon {
  id: ID!
  name: String!
}

type Query {
  pokemons: [Pokemon!]!
  nestedPokemons: [NestedPokemon!]!
}

type NestedPokemon {
  nested: Pokemon!
}

With 2 queries:

# pokemons.graphql
query Pokemons {
  pokemons {
    id
    name
  }
}
# nested_pokemons.graphql
query NestedPokemons {
  nestedPokemons {
    nested {
      id
      name
    }
  }
}

Notice that pokemons returns a list of Pokemon (with an id) while nestedPokemon returns a list of NestedPokemon pokemon (that doesn't have an id)

I have an application that:

  1. Queries both lists (nested and non-nested)
  2. Display sthem
  3. Evicts one by one each entity
Code
import 'package:built_collection/built_collection.dart';
import 'package:ferry/ferry.dart';
import 'package:ferry_flutter/ferry_flutter.dart';
import 'package:flutter/material.dart';
import 'package:flutter_app_stable/__generated__/schema.ast.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/nested_pokemons.data.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/nested_pokemons.req.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/nested_pokemons.var.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/pokemons.data.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/pokemons.req.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/pokemons.var.gql.dart';
import 'package:gql_exec/src/response.dart';

final client = Client(
  link: Link.function((request, [forward]) {
    if (request.operation.operationName == 'Pokemons') {
      return Stream.value(
        Response(
          response: const {},
          data: GPokemonsData((data) {
            data.pokemons = ListBuilder(List.generate(
                40,
                (i) => GPokemonsData_pokemons(
                      (pokemon) => pokemon
                        ..id = '$i'
                        ..name = 'Pokemon $i',
                    )));
          }).toJson(),
        ),
      );
    } else {
      return Stream.value(
        Response(
          response: const {},
          data: GNestedPokemonsData((data) {
            data.nestedPokemons = ListBuilder(List.generate(
                40,
                (i) => GNestedPokemonsData_nestedPokemons(
                      (pokemon) => pokemon.nested
                        ..id = '$i'
                        ..name = 'Pokemon $i',
                    )));
          }).toJson(),
        ),
      );
    }
  }),
);

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: Home(),
    );
  }
}

class Home extends StatelessWidget {
  const Home({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Column(
        children: [
          Expanded(
            child: Operation<GPokemonsData, GPokemonsVars>(
              client: client,
              operationRequest: GPokemonsReq(),
              builder: (context, result, _) {
                final pokemons = result?.data?.pokemons ?? BuiltList();

                return ListView.builder(
                  itemCount: pokemons.length,
                  itemBuilder: (context, index) {
                    final pokemon = pokemons[index];

                    return ListTile(
                      title: Text(pokemon.name),
                      subtitle: Text(pokemon.id),
                    );
                  },
                );
              },
            ),
          ),
          Expanded(
            child: Operation<GNestedPokemonsData, GNestedPokemonsVars>(
              client: client,
              operationRequest: GNestedPokemonsReq(),
              builder: (context, result, _) {
                final pokemons = result?.data?.nestedPokemons ?? BuiltList();

                return ListView.builder(
                  itemCount: pokemons.length,
                  itemBuilder: (context, index) {
                    final pokemon = pokemons[index];

                    return ListTile(
                      title: Text(pokemon.nested.name),
                      subtitle: Text(pokemon.nested.id),
                    );
                  },
                );
              },
            ),
          ),
        ],
      ),
      floatingActionButton: const FAB(),
    );
  }
}

class FAB extends StatefulWidget {
  const FAB({super.key});

  @override
  State<FAB> createState() => _FABState();
}

class _FABState extends State<FAB> {
  int id = 0;
  @override
  Widget build(BuildContext context) {
    return FloatingActionButton(
      onPressed: () {
        client.cache.evict('${Pokemon.name.value}:$id');
        id++;
      },
      child: const Icon(Icons.remove),
    );
  }
}

When an entity is deleted from the cache, the UI is rebuilt.

  • The non-nested list displays all the other entities expect for ones already evicted
  • The nested list becomes empty
Video
Screen.Recording.2023-03-06.at.3.50.31.PM.mov

I would expect both lists to behave as the first one (non-nested)

@knaeckeKami
Copy link
Collaborator

You can inspect what is happening by inspecting the contents of the cache. (e.g.

       client.cache.store.keys.forEach((key) {
          print(key);
        });
        print(client.cache.store.get('Query'));

The cache stores the results of the queries like this:

{ __typename: Query, 
  pokemons: [ {$ref: Pokemon:0}, {$ref: Pokemon:1}, {$ref: Pokemon:2}, ... ], 
  nestedPokemons: [
     {__typename: NestedPokemon, nested: {$ref: Pokemon:0}}, 
     {__typename: NestedPokemon, nested: {$ref: Pokemon:1}}, 
     ... 
] 

and then each entry like this:

Pokemon:5:  {__typename: Pokemon, id: 5, name: Pokemon 5}

So, when you remove a Pokemon entry from the cache, it is still referenced in the queries.

So, when the data is denormalized, this can lead to issues.

For lists, ferry just ignores the invalid reference, see

.where((data) => !isDanglingReference(data, config))

and

This is what you are seeing in the non-nested case.

For the nested case, the invalid reference happens in a nested object, and we throw a PartialDataException exception here since the referenced object is supposed to be non-nullable.

https://github.com/gql-dart/ferry/blob/f253e437d6ee1bce961dd857697f4f22f2a514e2/packages/normalize/lib/src/denormalize_node.dart#LL78C17-L78C37

I can see why this behavior can be confusing.

@knaeckeKami
Copy link
Collaborator

knaeckeKami commented Mar 6, 2023

So, evict() does not preserve referential integrity and should be used with caution.

I can imagine improvements in handling danging references though, ferry probably should treat nested objects in lists with dangling references the same is direct dangling references in lists.

I could also see a deleteCascade mode for eviction, where, similarly to the garbage collection method gc(), the whole graph of the cache is traversed so all objects containing references to the evicted entity are removed. This could be very expensive though if the cache is big.

For a workaround for now, you can use writeQuery() to manually remove the evicted item from the the nested query.

@ValentinVignal
Copy link
Contributor Author

ValentinVignal commented Mar 7, 2023

I can imagine improvements in handling danging references though, ferry probably should treat nested objects in lists with dangling references the same is direct dangling references in lists.

Yes it could be nice to do something like that:

.where((data) => !isDanglingReference(data, config) && !isPartial) 

where isPartial is true if the build of the element in the list throw a PartialDataException.

For a workaround for now, you can use writeQuery() to manually remove the evicted item from the nested query.

This is actually what I'm trying to move away from. I'm having a bunch of different queries that fetch a list of Pokemon. When deleting one, I don't really have a way or want to loop over all the cached queries (of potentially different types) to remove the deleted pokemon from the list. That's why I want to use .evict

@knaeckeKami
Copy link
Collaborator

knaeckeKami commented Mar 7, 2023

it could be nice to do something like that:

.where((data) => !isDanglingReference(data, config) && !isPartial

the .where((data) => !isDanglingReference(data, config) filter is fine IMO, and this is why this works in the non-nestd case. We would need an additional check check for the nested case, something like this:

Object? denormalizeNode({
  required SelectionSetNode? selectionSet,
  required Object? dataForNode,
  required NormalizationConfig config,
}) {
  if (dataForNode == null) return null;


  if (dataForNode is List) {
     final newList = <Object?>[];
     final reachableData =  dataForNode
         .where((data) => !isDanglingReference(data, config));
     for(final node in reachableDate) {
        try{
            final denormalizedSubNode =  denormalizeNode(
              selectionSet: selectionSet,
              dataForNode: node,
              config: config);
            newList.add(denormalizedSubNode);
        } on PartialDataException { 
            // ignore list items with partial data
        }
    }
    ...

 }

The config actually has a parameter to set whether partial data is allowed or not, but for ferry we have to set it to true, as ferry is strongly typed an partial data would lead to exceptions further down the line when trying the map the denormalized data to the typed objects.

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

Successfully merging a pull request may close this issue.

2 participants