Skip to content

Commit

Permalink
fix(supabase): build errors and query transformer errors (#430)
Browse files Browse the repository at this point in the history
  • Loading branch information
tshedor authored Sep 1, 2024
1 parent fe6c1d7 commit b43b27a
Show file tree
Hide file tree
Showing 15 changed files with 250 additions and 92 deletions.
2 changes: 2 additions & 0 deletions docs/supabase/query.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
| `'orderBy'` | `String` | Use field names not column names and always specify direction.For example, given a `final DateTime createdAt;` field: `{'orderBy': 'createdAt ASC'}`. |
| `'orderByReferencedTable'` | `String?` | Forwards to Supabase's `referencedTable` [property](https://supabase.com/docs/reference/dart/order) |

?> The `ReferencedTable` params are awkward but necessary to not collide with other providers (like `SqliteProvider`) that also use `orderBy` and `limit`. While a `foreign_table.foreign_column` syntax is more Supabase-like, it is not supported in `orderBy` and `limit`.

## `where:`

Brick currently does not support all of Supabase's filtering methods. Consider the associated `Compare` enum value to Supabase's method when building a Brick query:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ homepage: https://github.com/GetDutchie/brick/tree/main/packages/brick_offline_f
issue_tracker: https://github.com/GetDutchie/brick/issues
repository: https://github.com/GetDutchie/brick

version: 0.1.0
version: 0.1.0+1

environment:
sdk: ">=3.0.0 <4.0.0"
Expand Down
34 changes: 34 additions & 0 deletions packages/brick_sqlite/test/query_sql_transformer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,40 @@ void main() {
expect(statement, sqliteQuery.statement);
sqliteStatementExpectation(statement);
});

// This behavior is not explicitly supported - field names should be used.
// This is considered functionality behavior and is not guaranteed in
// future Brick releases.
// https://github.com/GetDutchie/brick/issues/429
test('incorrectly cased columns are forwarded as is', () async {
final statement =
'SELECT DISTINCT `DemoModel`.* FROM `DemoModel` ORDER BY complex_field_name DESC';
final sqliteQuery = QuerySqlTransformer<DemoModel>(
modelDictionary: dictionary,
query: Query(providerArgs: {'orderBy': 'complex_field_name DESC'}),
);
await db.rawQuery(sqliteQuery.statement, sqliteQuery.values);

expect(statement, sqliteQuery.statement);
sqliteStatementExpectation(statement);
});

// This behavior is not explicitly supported because table names are autogenerated
// and not configurable. This is considered functionality behavior and is not
// guaranteed in future Brick releases.
// https://github.com/GetDutchie/brick/issues/429
test('ordering by association is forwarded as is', () async {
final statement =
'SELECT DISTINCT `DemoModel`.* FROM `DemoModel` ORDER BY other_table.complex_field_name DESC';
final sqliteQuery = QuerySqlTransformer<DemoModel>(
modelDictionary: dictionary,
query: Query(providerArgs: {'orderBy': 'other_table.complex_field_name DESC'}),
);
await db.rawQuery(sqliteQuery.statement, sqliteQuery.values);

expect(statement, sqliteQuery.statement);
sqliteStatementExpectation(statement);
});
});

group('#values', () {
Expand Down
2 changes: 2 additions & 0 deletions packages/brick_supabase/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Connecting [Brick](https://github.com/GetDutchie/brick) with Supabase.
| `'orderBy'` | `String` | Use field names not column names and always specify direction.For example, given a `final DateTime createdAt;` field: `{'orderBy': 'createdAt ASC'}`. |
| `'orderByReferencedTable'` | `String?` | Forwards to Supabase's `referencedTable` [property](https://supabase.com/docs/reference/dart/order) |

:bulb: The `ReferencedTable` params are awkward but necessary to not collide with other providers (like `SqliteProvider`) that also use `orderBy` and `limit`. While a `foreign_table.foreign_column` syntax is more Supabase-like, it is not supported in `orderBy` and `limit`.

### `where:`

Brick currently does not support all of Supabase's filtering methods. Consider the associated `Compare` enum value to Supabase's method when building a Brick query:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,12 @@ class QuerySupabaseTransformer<_Model extends SupabaseModel> {
);
}

final queryKey = (leadingAssociations != null ? '${leadingAssociations.join('.')}.' : '') +
definition.columnName;

return [
{
definition.columnName:
'${_compareToSearchParam(condition.compare)}.${leadingAssociations != null ? '${leadingAssociations.join('.')}.' : ''}${condition.value}',
queryKey: '${_compareToSearchParam(condition.compare)}.${condition.value}',
}
];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/brick_supabase/lib/src/supabase_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ abstract mixin class SupabaseAdapter<TModel extends SupabaseModel> implements Ad
bool get ignoreDuplicates;

/// Used for upserts; forwards to Supabase's `onConflict`
String? get onConflict;
String? get onConflict => null;

/// Declared by the [SupabaseSerializable] `tableName` property
String get supabaseTableName;
Expand Down
39 changes: 28 additions & 11 deletions packages/brick_supabase/lib/src/supabase_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,47 +87,49 @@ class SupabaseProvider implements Provider<SupabaseModel> {
/// when `Room` is upserted, `Pillow` is upserted and then `Bed` is upserted.
@override
Future<TModel> upsert<TModel extends SupabaseModel>(instance, {query, repository}) async {
final adapter = modelDictionary.adapterFor[TModel]!;
final output = await adapter.toSupabase(instance, provider: this, repository: repository);

return await _recursiveAssociationUpsert(
instance,
output,
type: TModel,
query: query,
repository: repository,
) as TModel;
}

Future<SupabaseModel> _upsertByType(
SupabaseModel instance, {
Map<String, dynamic> serializedInstance, {
required Type type,
Query? query,
ModelRepository<SupabaseModel>? repository,
}) async {
assert(modelDictionary.adapterFor.containsKey(type));

final adapter = modelDictionary.adapterFor[type]!;
final output = await adapter.toSupabase(instance, provider: this, repository: repository);

final queryTransformer =
QuerySupabaseTransformer(adapter: adapter, modelDictionary: modelDictionary, query: query);

final builder = adapter.uniqueFields.fold(client.from(adapter.supabaseTableName).upsert(output),
(acc, uniqueFieldName) {
final builder = adapter.uniqueFields.fold(
client.from(adapter.supabaseTableName).upsert(serializedInstance), (acc, uniqueFieldName) {
final columnName = adapter.fieldsToSupabaseColumns[uniqueFieldName]!.columnName;
if (output.containsKey(columnName)) {
return acc.eq(columnName, output[columnName]);
if (serializedInstance.containsKey(columnName)) {
return acc.eq(columnName, serializedInstance[columnName]);
}
return acc;
});
final resp = await builder.select(queryTransformer.selectFields).limit(1).maybeSingle();

if (resp == null) {
throw StateError('Upsert of $instance failed');
throw StateError('Upsert of $type failed');
}

return adapter.fromSupabase(resp, repository: repository, provider: this);
}

Future<SupabaseModel> _recursiveAssociationUpsert(
SupabaseModel instance, {
Map<String, dynamic> serializedInstance, {
required Type type,
Query? query,
ModelRepository<SupabaseModel>? repository,
Expand All @@ -139,13 +141,28 @@ class SupabaseProvider implements Provider<SupabaseModel> {
.where((a) => a.association && a.associationType != null);

for (final association in associations) {
if (!serializedInstance.containsKey(association.columnName)) {
continue;
}

if (serializedInstance[association.columnName] is! Map) {
continue;
}

await _recursiveAssociationUpsert(
instance,
Map<String, dynamic>.from(serializedInstance[association.columnName]),
type: association.associationType!,
query: query,
repository: repository,
);
serializedInstance.remove(association.columnName);
}
return await _upsertByType(instance, type: type, query: query, repository: repository);

return await _upsertByType(
serializedInstance,
type: type,
query: query,
repository: repository,
);
}
}
3 changes: 2 additions & 1 deletion packages/brick_supabase/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ homepage: https://github.com/GetDutchie/brick/tree/main/packages/brick_supabase
issue_tracker: https://github.com/GetDutchie/brick/issues
repository: https://github.com/GetDutchie/brick

version: 0.1.1+2
version: 0.1.1+5

environment:
sdk: ">=3.0.0 <4.0.0"
Expand All @@ -18,3 +18,4 @@ dependencies:
dev_dependencies:
lints: ^2.0.1
test: ^1.16.5
collection: ">=1.15.0 <2.0.0"
16 changes: 15 additions & 1 deletion packages/brick_supabase/test/__mocks__.dart
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Future<Map<String, dynamic>> _$DemoNestedAssociationModelToSupabase(
return <String, dynamic>{
'id': instance.id,
'name': instance.name,
'nested': await _$DemoAssociationModelToSupabase(instance.nested),
};
}

Expand Down Expand Up @@ -176,10 +177,23 @@ DemoAssociationModel _$DemoAssociationModelFromSupabase(Map<String, dynamic> jso
);
}

Future<Map<String, dynamic>> _$DemoAssociationModelToSupabase(DemoAssociationModel instance) async {
Future<Map<String, dynamic>> _$DemoAssociationModelToSupabase(
DemoAssociationModel instance, {
provider,
repository,
}) async {
return <String, dynamic>{
'id': instance.id,
'name': instance.name,
'assocs': await Future.wait<Map<String, dynamic>>(
instance.assocs
?.map(
(s) => DemoModelAdapter().toSupabase(s, provider: provider, repository: repository),
)
.toList() ??
[],
),
'assoc': await _$DemoModelToSupabase(instance.assoc),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void main() {

expect(
select.query,
'select=id,name,assoc:demos!assoc_id(id,name,age),assocs:demos!assocs_id(id,name,age)&name=eq.assoc.Thomas',
'select=id,name,assoc:demos!assoc_id(id,name,age),assocs:demos!assocs_id(id,name,age)&assoc.name=eq.Thomas',
);
});
});
Expand Down
Loading

0 comments on commit b43b27a

Please sign in to comment.