Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 5 additions & 31 deletions packages/postgrest/lib/src/postgrest_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -486,12 +486,14 @@ class PostgrestBuilder<T, S, R> implements Future<T> {
}
}

// 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',
Comment thread
coderabbitai[bot] marked this conversation as resolved.
details:
'Results contain ${body.length} rows, application/vnd.pgrst.object+json requires 1 row',
hint: null,
Expand Down Expand Up @@ -562,10 +564,6 @@ class PostgrestBuilder<T, S, R> implements Future<T> {
statusCode: response.statusCode,
details: response.reasonPhrase,
);

if (_maybeSingle) {
return _handleMaybeSingleError(response, error);
}
}
} else {
error = PostgrestApiException(
Expand All @@ -582,30 +580,6 @@ class PostgrestBuilder<T, S, R> implements Future<T> {
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<S>(data: _converter(null as R), count: 0)
as T;
}
return PostgrestResponse<S>(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<T> asStream() {
final controller = StreamController<T>.broadcast();
Expand Down
12 changes: 4 additions & 8 deletions packages/postgrest/lib/src/postgrest_transform_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -206,17 +206,13 @@ class PostgrestTransformBuilder<T> extends RawPostgrestBuilder<T, T, T> {
/// (e.g. using `eq` on a UNIQUE column or `limit(1)`),
/// otherwise this will result in an error.
PostgrestTransformBuilder<PostgrestMap?> 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,
),
);
}
Expand Down
140 changes: 91 additions & 49 deletions packages/postgrest/test/maybe_single_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> responseHeaders;
final List<BaseRequest> requests = [];

@override
Future<StreamedResponse> 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<StreamedResponse> 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
Expand All @@ -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<PostgrestApiException>()
.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<PostgrestApiException>()
.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<PostgrestApiException>()
.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'),
),
);
});
}