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

Redundant REST API calls for fetching an association (Supabase Integration) #433

Closed
devj3ns opened this issue Sep 5, 2024 · 2 comments
Closed

Comments

@devj3ns
Copy link
Contributor

devj3ns commented Sep 5, 2024

Context: #399, #429 (comment)

In my project using the new Supabase integration, I have a model Project and a model Customer:

@ConnectOfflineFirstWithSupabase( supabaseConfig: SupabaseSerializable(tableName: 'projects'))
class Project extends OfflineFirstWithSupabaseModel {

  ...
  
  @Supabase(name: 'customer', toGenerator: '%INSTANCE_PROPERTY%.id.value')
  @Sqlite(index: true)
  final Customer customer;
}
@ConnectOfflineFirstWithSupabase( supabaseConfig: SupabaseSerializable(tableName: 'customers'))
class Customer extends OfflineFirstWithSupabaseModel {

  ...
  
  @Sqlite(unique: true, index: true)
  @Supabase(unique: true)
  final UUID id;
}

Currently, when I fetch a list of projects with await repository.getBatched<Project>() Brick fetches all the projects with their associated customers in one REST Request:

https://[project_id]/rest/v1/projects?select=id,customer:customers!customer(id,first_name)&limit=50

But unfortunately, for each project, a separate REST Request is sent to fetch the customer:

https://[project_id]/rest/v1/customers?select=id,first_name&id=eq.xyz&limit=1

This means, when I batch fetch 50 projects, 50 REST requests to the customers' endpoint are sent. But because we already joined the customers in the REST Request in which the projects were fetched, this is redundant.

The generated Project adapter code looks like follows:

(Note that since I removed the @OfflineFirst(where:) annotation, see #429 (comment), this is not the part where the redundant REST call is triggered).

Future<Project> _$ProjectFromSupabase(Map<String, dynamic> data,
    {required SupabaseProvider provider,
    OfflineFirstWithSupabaseRepository? repository}) async {
    return Project(
        ....
         customer: await CustomerAdapter().fromSupabase(data['customer'],
          provider: provider, repository: repository),
        ....
      ),
}
Future<Project> _$ProjectFromSqlite(Map<String, dynamic> data,
    {required SqliteProvider provider,
    OfflineFirstWithSupabaseRepository? repository}) async {
    return Project(
        ...
        customer: (await repository!.getAssociation<Customer>(
            Query.where(
                'primaryKey', data['customer_Customer_brick_id'] as int,
                limit1: true),
          ))!.first,
         ...
     ),
}

The repository!.getAssociation triggers the redundant REST API requests. In #429 (comment) I wrote that I think this part should be generated differently (only fetch the customer locally) to prevent the REST Call. But I am not sure if this would not lead to other issues.

Because of #429 (comment), I tried using seedOnly. This helps to reduce the number of requests to fetch a customer sent to the REST API. But there are still happening many unnecessary requests.

I created two videos in which you can see all the requests made. Context: I am using the alwaysHydrate policy.

seedOnly: false: https://www.loom.com/share/18709cae44c24046a4a6abf7c4a84ac9
seedOnly: true: https://www.loom.com/share/39b6ac74e95d4035b65664ac1569378f

I hope this helps. Thanks in advance 🙏

@tshedor
Copy link
Collaborator

tshedor commented Sep 6, 2024

Context: I am using the alwaysHydrate policy.

Ah. This is critical context......Brick is doing what you're asking it to do. It's going to always hydrate all the way down through associations, per #371 and the fix in #372.

I've created a test case in #434 to ensure that Brick does store associations correctly. So your problem is due to this policy. I believe the best case scenario fix would be to override the getAssociation method:

class MyRepository extends OfflineFirstWithSupabaseRepository {
  @override
  Future<List<TModel>?> getAssociation<TModel extends OfflineFirstWithSupabaseModel>(Query query) async {
    logger.finest('#getAssociation: $TModel $query');
    final results = await get<TModel>(
      query: query,
      // This removes the semi-stateful _latestGetPolicy
      policy: OfflineFirstGetPolicy.awaitRemoteWhenNoneExist,
    );
    if (results.isEmpty) return null;
    return results;
  }
}

@devj3ns
Copy link
Contributor Author

devj3ns commented Sep 6, 2024

Oh, this makes sense!

The fix is working great, thanks @tshedor!

@devj3ns devj3ns closed this as completed Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants