Supabase Integration (WIP)

[tshedor]

Please use this issue to follow progress on the Supabase integration. This issue will be opened for comments when the branch is considered stable enough for early testing, so consider subscribing to Notifications. Related: #359

brick_supabase

• Dev (eng(supabase): add supabase and offline first with supabase packages #403)
• CI (eng(supabase): add supabase and offline first with supabase packages #403)
• Publish (eng(supabase): add supabase and offline first with supabase packages #403)
• Validate (bug(supabase-integration): Invalid argument(s): Field primaryKey on Customer is not serialized by Supabase #424 bug(supabase-integration): Supabase query includes join even though there is no associationForeignKey defined #425 Issues with Querying Associated Documents and REST Parameter Mapping in Supabase Integration #429 Issue upserting SupabaseModel with association #431 fix(supabase): remove foreignKey property from supabase annotation #432 test(supabase): ensure supabase stores associations; add testing classes #434)
• Tests (eng(supabase): add supabase tests and skips #405 eng(supabase): add query supabase transformer tests #408 fix(supabase): resolve all skipped brick_supabase tests #415 fix(supabase): build errors and query transformer errors #430)
• Example (eng(supabase): add example file #412)
• Doc site documentation (doc(supabase): add doc site pages #413)

brick_offline_first_with_supabase

• Publish (eng(supabase): add supabase and offline first with supabase packages #403)
• CI (eng(supabase): add supabase and offline first with supabase packages #403)
• Queue (feat(supabase): add rest's offline queue to supabase #410)
• Validate (fix(supabase): scope table name to supabase only #416 #417 fix(brick_offline_first): add toSupabase method to OfflineFirstSerdes #423 bug(supabase-integration): Invalid argument(s): Field primaryKey on Customer is not serialized by Supabase #424 fix(supabase): do not declare nulls on column definition #426 fix(supabase): remove foreignKey property from supabase annotation #432 test(supabase): ensure supabase stores associations; add testing classes #434 fix(supabase): add default list of reattempt codes #437 [Supabase integration]: Null check exception thrown in PostgrestBuilder when upserting while being offline #439 Supabase requests to non rest endpoints are added to the offline request queue and cause problems #440 Supabase: PostgrestException thrown when fetching data while being offline #441 doc(supabase): add authentication caveat #440 #442 fix(supabase): handle offline errors #441 #443)
• Tests (test(supabase): ensure supabase stores associations; add testing classes #434)
• Example (feat(supabase): add rest's offline queue to supabase #410)
• Doc site documentation (doc(supabase): add doc site pages #413 doc(supabase): replace example_supabase #414 doc(supabase): add authentication caveat #440 #442)

brick_offline_first_with_supabase_build

• Publish (eng(supabase): add generator and offline_first build #404)
• CI (eng(supabase): add generator and offline_first build #404)
• Validate (bug(supabase-integration): OfflineFirstWithSupabaseAdapter fields are generated twice #416 fix(supabase): do not declare nulls on column definition #426 brick_offline_first_with_supabase_build v0.1.0 from pub.dev throws an exception when starting the build_runner #427)
• Tests (eng(supabase): add offline first with supabase build tests #407 fix(supabase): resolve breaking CI dependency failures #420)

brick_supabase_generators

• Dev (eng(supabase): add generator and offline_first build #404)
• CI (eng(supabase): add generator and offline_first build #404)
• Publish (eng(supabase): add generator and offline_first build #404)
• Validate (fix: add missing overrides to the supabase generator #422 fix(supabase): do not declare nulls on column definition #426 bug: SupabaseAdapter is missing the onConflict override #428)
• Tests (eng(supabase): add supabase_generators tests #406)

---

brick_supabase_abstract

No longer needed; using the supabase_dart library

• Dev (eng(supabase): add abstract packages #402)
• Publish (eng(supabase): add abstract packages #402)
• Validate

brick_offline_first_with_supabase_abstract

No longer needed; using the supabase_dart library

• Dev (eng(supabase): add abstract packages #402)
• Publish (eng(supabase): add abstract packages #402)
• Validate

[devj3ns]

I just migrated my code from using the RestProvider for Supabase implementation to the new first party Supabase Integration. While doing this, I encountered some issues, as described below:

1. I could not use brick_offline_first_from_supabase v0.1.0 from pub.dev, as it does not include the latest changes (e.g., the additions inside the OfflineFirstWithSupabaseRepository class). I used the version from GitHub instead
2. I found a bug in generating the adapters: bug(supabase-integration): OfflineFirstWithSupabaseAdapter fields are generated twice #416
3. I was unable to use the OfflineFirstWithSupabaseRepository.clientQueue method as seen in the example because it does not allow to pass the reattemptForStatusCodes, databaseName, processingInterval and serialProcessing attributes. More info below (*)

---

(*): I had to use the following code when initializing the OfflineFirstWithSupabaseRepository offlineRequestQueue. This requires importing the brick_offline_first_with_rest package. While this is acceptable, I would prefer not to import the REST package when using the Supabase integration.

offlineRequestQueue: RestOfflineRequestQueue(
    client: RestOfflineQueueClient(
        http.Client(),
        reattemptForStatusCodes: [400, 401, 403, 404 405,408,409,429,500501,502,503,504],
        RestRequestSqliteCacheManager(
              'offline_queue.sqlite',
              databaseFactory: kIsWeb ? databaseFactoryFfiWeb : databaseFactory,
              processingInterval: const Duration(seconds: 5),
              serialProcessing: true,
        ),
  ),
),

---

When the adapter issue is resolved, I can run the app and provide additional feedback.

[tshedor]

@devj3ns Regarding "While this is acceptable, I would prefer not to import the REST package when using the Supabase integration," I largely agree. Do you think it'd be better to move this logic to the brick_offline_first package? It is somewhat REST specific and the brick_offline_first_with_rest doesn't have any other heavy dependencies.

I wrestled with this and felt recycling the code was a suitable tradeoff for the amount of testing/stability behind the REST queue, but I'm less attached to where the code properly lives (as long as the decision can be defended).

[tshedor]

I was unable to use the OfflineFirstWithSupabaseRepository.clientQueue method as seen in the example because it does not allow to pass the reattemptForStatusCodes, databaseName, processingInterval and serialProcessing attributes. More info below (*)

Are all of these status codes necessary from the Supabase API? If they are, they should be a default value in the argument.

Latest versions have been published off the #420 (yo @mateominato tell the Dutchie team) branch:

brick_offline_first_with_supabase: 0.1.0+1
brick_supabase: 0.1.1
brick_supabase_generators: 0.1.1

---

For easier testing, it's best to clone this repo locally, git checkout supabase-integration, then reference any brick packages via path in dependency_overrides in your app's pubspec. Once you clone it, run dart pub get; dart run melos bootsrap in the root so that all related packages install from path instead of remote.

[devj3ns]

@devj3ns Regarding "While this is acceptable, I would prefer not to import the REST package when using the Supabase integration," I largely agree. Do you think it'd be better to move this logic to the brick_offline_first package? It is somewhat REST specific and the brick_offline_first_with_rest doesn't have any other heavy dependencies.

I wrestled with this and felt recycling the code was a suitable tradeoff for the amount of testing/stability behind the REST queue, but I'm less attached to where the code properly lives (as long as the decision can be defended).

I think reusing the rest_offline_queue code for the Supabase package is totally fine for now. And I would personally not move it to the brick_offline_first package because it is REST related.

When using the OfflineFirstWithSupabaseRepository.clientQueue method, one does not have to add the rest package as a direct dependency, so in my opinion it is okay as it is.

[devj3ns]

Are all of these status codes necessary from the Supabase API? If they are, they should be a default value in the argument.

@tshedor For my app, they are necessary because I want the app user to know when some data could not be upserted when processing the offline queue (no matter why). I think it depends on the app and conflict resolution strategy.

I would personally definitely add HTTP 401 (Unauthorized) to the default reattempt status codes for the Supabase queue, as this is the one returned when the JWT expired and that's a case where a reattempt is definitely important.

When being precise, HTTP 409 (Conflict) would rather be a "rejectedStatusCode" which had to be handled by the app user instead of reattempting it. But that is another topic which is somewhat related to #393, but unfortunately, I currently do not have much time to think about improving the offline request queue.

[tshedor]

@devj3ns I think we're finally close to a stable release. I've prepared a PR (#436) that updates all new packages to the 1.0.0 version.

I'd like this to go through the routine with your app and testing for another week. If there are no new bugs, let's call it stable on Friday, Sept 13.

[devj3ns]

@tshedor That timeline aligns perfectly with my plan. Next week, I'll be running additional tests to ensure that all the app's functionality works seamlessly with the Supabase integration. Once confirmed, I’ll create a new release of my app for production use.

I'll report back next week, but my feeling is that everything is looking good so far.

Before making a stable release, however, I would definitely suggest adjusting the default reattempt status codes. As mentioned in my previous comment. At a minimum HTTP 401 (Unauthorized) should be added. This status occurs when the client's JWT has expired, which can happen e. g. on the first request after the client comes back online.

[tshedor]

@devj3ns Done, merged, and published in #437

[devj3ns]

Nice 👍

I think we should add a note to the docs, that when this list includes status codes like 409 (Conflict) or 403 (Forbidden), which could mean that the request is permanently rejected by the Database and no reattempt will succeed, it can block other requests in the queue from sending. What do you think @tshedor?

Currently, the biggest thing missing from brick (in my opinion) is handling what happens when a request is permanently rejected by the Remote Provider and all reattempts fail (e.g. HTTP 409 or 403). In my app, which uses serial processing (because of association dependencies) I added a check which detects a blocked request queue and gives the user the ability to discard the rejected request to unblock the queue.

When I am done with the file upload thing (on which I will write something to the other issue next week), I will come back to the "more stateful" request queue topic (related: #393).

[tshedor]

I think we should add a note to the docs, that when this list includes status codes like 409 (Conflict) or 403 (Forbidden), which could mean that the request is permanently rejected by the Database and no reattempt will succeed, it can block other requests in the queue from sending.

This feels a little "water is wet" but if you think it would be useful to include then we should include it. Mind opening a PR?

Currently, the biggest thing missing from brick (in my opinion) is handling what happens when a request is permanently rejected by the Remote Provider and all reattempts fail

Yeah, I know this has been bothering you for a while. Do you want to lead the development of the feature since you're closest to it? I'd be more comfortable with this implementation in Supabase because the remote provider is known versus a GraphQL or Rest provider where, in theory, most applications would be using their own, manageable API and would therefore control the status codes.

[tshedor]

@devj3ns How'd this week go? Any further bugs?

[devj3ns]

Hey @tshedor, unfortunately, I was very busy with other tasks this week, but I already blocked some hours tomorrow morning to further test the supabase integration.

I will report back until tomorrow 10 UTC.

[devj3ns]

This feels a little "water is wet" but if you think it would be useful to include then we should include it. Mind opening a PR?

Yes, I’ll go ahead and open a small PR on Monday.

Do you want to lead the development of the feature since you're closest to it?

I'd be happy to do that!

I'd be more comfortable with this implementation in Supabase because the remote provider is known versus a GraphQL or Rest provider where, in theory, most applications would be using their own, manageable API and would therefore control the status codes.

I completely agree! I’ll comment on #393 once I’m ready to start on this, which should be around mid-October.

[devj3ns]

If there are no new bugs, let's call it stable on Friday, Sept 13.

@tshedor Unfortunately, there is an issue with the Supabase integration when upserting while being offline: #439

[devj3ns]

@tshedor, I found two more issues with the Supabase integration: #440 #441

[tshedor]

Since #418 is merged and all packages are published, I'm going to close this issue.

@devj3ns let me know if you're able to smoothly transition using packages from pub.dev . Small release updates can land on #444 . Everything is on the 1.0.0 release except for the offline first with rest stuff, but that's all been published too.