diff --git a/packages/postgrest/lib/src/postgrest_builder.dart b/packages/postgrest/lib/src/postgrest_builder.dart index a768c511d..4e10d45ac 100644 --- a/packages/postgrest/lib/src/postgrest_builder.dart +++ b/packages/postgrest/lib/src/postgrest_builder.dart @@ -486,12 +486,14 @@ class PostgrestBuilder implements Future { } } - // Workaround for https://github.com/supabase/supabase-flutter/issues/560 - if (_maybeSingle && method == HttpMethod.get && body is List) { + // maybeSingle() fetches the result as a list and enforces the + // at-most-one-row constraint here, so that zero rows never produce a + // PostgREST 406. + if (_maybeSingle && body is List) { if (body.length > 1) { final exception = PostgrestApiException( - // https://github.com/PostgREST/postgrest/blob/a867d79c42419af16c18c3fb019eba8df992626f/src/PostgREST/Error.hs#L553 statusCode: 406, + errorCode: 'PGRST116', details: 'Results contain ${body.length} rows, application/vnd.pgrst.object+json requires 1 row', hint: null, @@ -562,10 +564,6 @@ class PostgrestBuilder implements Future { statusCode: response.statusCode, details: response.reasonPhrase, ); - - if (_maybeSingle) { - return _handleMaybeSingleError(response, error); - } } } else { error = PostgrestApiException( @@ -582,30 +580,6 @@ class PostgrestBuilder implements Future { throw error; } - /// When [_maybeSingle] is true, check whether error details contain - /// 'Results contain 0 rows' then - /// return PostgrestResponse with null data - T _handleMaybeSingleError( - http.Response response, - PostgrestApiException error, - ) { - if (error.details is String && - (error.details as String).contains('Results contain 0 rows')) { - if (_count != null && response.request!.method != HttpMethod.head.value) { - if (_converter != null) { - return PostgrestResponse(data: _converter(null as R), count: 0) - as T; - } - return PostgrestResponse(data: null as S, count: 0) as T; - } - if (_converter != null) { - return _converter(null as R) as T; - } - return null as T; - } - throw error; - } - @override Stream asStream() { final controller = StreamController.broadcast(); diff --git a/packages/postgrest/lib/src/postgrest_transform_builder.dart b/packages/postgrest/lib/src/postgrest_transform_builder.dart index 9e2321c8f..8740e63cf 100644 --- a/packages/postgrest/lib/src/postgrest_transform_builder.dart +++ b/packages/postgrest/lib/src/postgrest_transform_builder.dart @@ -206,17 +206,13 @@ class PostgrestTransformBuilder extends RawPostgrestBuilder { /// (e.g. using `eq` on a UNIQUE column or `limit(1)`), /// otherwise this will result in an error. PostgrestTransformBuilder maybeSingle() { - // Temporary fix for https://github.com/supabase/supabase-flutter/issues/560 - // Issue persists e.g. for `.insert([...]).select().maybeSingle()` - final newHeaders = {..._headers}; - newHeaders['Accept'] = _method == HttpMethod.get - ? 'application/json' - : 'application/vnd.pgrst.object+json'; - + // The single-row constraint is enforced client-side instead of via the + // `application/vnd.pgrst.object+json` Accept header, so that a request + // matching zero rows resolves to `null` without PostgREST answering 406 + // and polluting the API logs. return PostgrestTransformBuilder( _copyWithType( maybeSingle: true, - headers: newHeaders, ), ); } diff --git a/packages/postgrest/test/maybe_single_test.dart b/packages/postgrest/test/maybe_single_test.dart index 5248cc631..e1aa819e6 100644 --- a/packages/postgrest/test/maybe_single_test.dart +++ b/packages/postgrest/test/maybe_single_test.dart @@ -4,59 +4,81 @@ import 'package:http/http.dart'; import 'package:postgrest/postgrest.dart'; import 'package:test/test.dart'; -/// Mimics PostgREST returning the "0 rows" error that `maybeSingle()` treats as -/// an empty result rather than a failure. -class ZeroRowsHttpClient extends BaseClient { +/// Records the requests it receives and answers each one with the given body +/// and headers, mimicking PostgREST answering a `maybeSingle()` request that +/// is fetched as a plain JSON list. +class RecordingHttpClient extends BaseClient { + RecordingHttpClient({ + required this.responseBody, + this.responseHeaders = const {}, + }); + + final Object responseBody; + final Map responseHeaders; + final List requests = []; + @override Future send(BaseRequest request) async { + requests.add(request); return StreamedResponse( - Stream.value( - utf8.encode( - jsonEncode({ - 'code': 'PGRST116', - 'details': 'Results contain 0 rows', - 'hint': null, - 'message': 'JSON object requested, multiple (or no) rows returned', - }), - ), - ), - 406, + Stream.value(utf8.encode(jsonEncode(responseBody))), + 200, + headers: responseHeaders, request: request, ); } } -/// Mimics PostgREST rejecting a `maybeSingle()` request because more than one -/// row matched, which is a real failure rather than an empty result. -class MultipleRowsHttpClient extends BaseClient { +/// Mimics PostgREST answering with a genuine error, which `maybeSingle()` +/// must surface unchanged rather than swallow. +class ErrorHttpClient extends BaseClient { @override Future send(BaseRequest request) async { return StreamedResponse( Stream.value( utf8.encode( jsonEncode({ - 'code': 'PGRST116', - 'details': - 'Results contain 2 rows, application/vnd.pgrst.object+json ' - 'requires 1 row', - 'hint': 'Ask for more rows', - 'message': 'JSON object requested, multiple (or no) rows returned', + 'code': '42501', + 'details': 'Policy check failed', + 'hint': 'Check your RLS policies', + 'message': 'permission denied for table users', }), ), ), - 406, + 403, request: request, ); } } void main() { + test('maybeSingle() does not override the Accept header', () async { + final httpClient = RecordingHttpClient(responseBody: []); + final postgrest = PostgrestClient( + 'https://example.com', + httpClient: httpClient, + ); + + await postgrest.from('users').select().maybeSingle(); + await postgrest.from('users').update({'name': 'x'}).select().maybeSingle(); + + for (final request in httpClient.requests) { + expect( + request.headers['Accept'], + isNot('application/vnd.pgrst.object+json'), + ); + } + }); + test( 'maybeSingle().count() returns null data and count 0 when no rows match', () async { final postgrest = PostgrestClient( 'https://example.com', - httpClient: ZeroRowsHttpClient(), + httpClient: RecordingHttpClient( + responseBody: [], + responseHeaders: {'content-range': '*/0'}, + ), ); final response = await postgrest @@ -71,29 +93,49 @@ void main() { }, ); - test( - 'maybeSingle() keeps the reported code and hint on a real error', - () async { - final postgrest = PostgrestClient( - 'https://example.com', - httpClient: MultipleRowsHttpClient(), - ); + test('maybeSingle() throws when a write returns more than one row', () async { + final postgrest = PostgrestClient( + 'https://example.com', + httpClient: RecordingHttpClient( + responseBody: [ + {'name': 'a'}, + {'name': 'b'}, + ], + ), + ); - await expectLater( - () => postgrest.from('users').select().maybeSingle(), - throwsA( - isA() - .having((e) => e.statusCode, 'statusCode', 406) - .having((e) => e.errorCode, 'errorCode', 'PGRST116') - .having((e) => e.hint, 'hint', 'Ask for more rows') - .having( - (e) => e.details, - 'details', - 'Results contain 2 rows, application/vnd.pgrst.object+json ' - 'requires 1 row', - ), - ), - ); - }, - ); + await expectLater( + () => + postgrest.from('users').update({'name': 'x'}).select().maybeSingle(), + throwsA( + isA() + .having((e) => e.statusCode, 'statusCode', 406) + .having((e) => e.errorCode, 'errorCode', 'PGRST116') + .having( + (e) => e.details, + 'details', + 'Results contain 2 rows, application/vnd.pgrst.object+json ' + 'requires 1 row', + ), + ), + ); + }); + + test('maybeSingle() surfaces a real error unchanged', () async { + final postgrest = PostgrestClient( + 'https://example.com', + httpClient: ErrorHttpClient(), + ); + + await expectLater( + () => postgrest.from('users').select().maybeSingle(), + throwsA( + isA() + .having((e) => e.statusCode, 'statusCode', 403) + .having((e) => e.errorCode, 'errorCode', '42501') + .having((e) => e.hint, 'hint', 'Check your RLS policies') + .having((e) => e.details, 'details', 'Policy check failed'), + ), + ); + }); }