Skip to content

Commit 8ead7c7

Browse files
committed
fix(postgrest): override limit and offset instead of appending duplicates
limit() and range() built their query params with appendSearchParams, so chaining or re-applying them on a shared builder emitted duplicate, single-valued params, e.g. select().limit(5).range(0,9) produced limit=5&limit=10&offset=0 and limit(1).limit(2) produced limit=1&limit=2. PostgREST treats limit/offset as single-valued, so override them (last call wins), matching postgrest-js and the select()/order() handling in the same file.
1 parent 9dc0b18 commit 8ead7c7

3 files changed

Lines changed: 77 additions & 6 deletions

File tree

packages/postgrest/lib/src/postgrest_builder.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,11 @@ class PostgrestBuilder<T, S, R> implements Future<T> {
402402
/// Get new Uri with overridden queryParams
403403
///
404404
/// [url] may be used to update based on a different url than the current one
405-
Uri overrideSearchParams(String key, String value) {
406-
final searchParams = Map<String, dynamic>.of(_url.queryParametersAll);
405+
Uri overrideSearchParams(String key, String value, [Uri? url]) {
406+
final searchParams =
407+
Map<String, dynamic>.of((url ?? _url).queryParametersAll);
407408
searchParams[key] = value;
408-
return _url.replace(queryParameters: searchParams);
409+
return (url ?? _url).replace(queryParameters: searchParams);
409410
}
410411

411412
/// Convert list filter to query params string

packages/postgrest/lib/src/postgrest_transform_builder.dart

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ class PostgrestTransformBuilder<T> extends RawPostgrestBuilder<T, T, T> {
106106
PostgrestTransformBuilder<T> limit(int count, {String? referencedTable}) {
107107
final key = referencedTable == null ? 'limit' : '$referencedTable.limit';
108108

109-
final url = appendSearchParams(key, '$count');
109+
// `limit` is single-valued in PostgREST, so a later call must replace an
110+
// earlier one rather than appending a second `limit` query param.
111+
final url = overrideSearchParams(key, '$count');
110112
return PostgrestTransformBuilder(copyWithUrl(url));
111113
}
112114

@@ -134,8 +136,11 @@ class PostgrestTransformBuilder<T> extends RawPostgrestBuilder<T, T, T> {
134136
final keyLimit =
135137
referencedTable == null ? 'limit' : '$referencedTable.limit';
136138

137-
var url = appendSearchParams(keyOffset, '$from');
138-
url = appendSearchParams(keyLimit, '${to - from + 1}', url);
139+
// `offset` and `limit` are single-valued in PostgREST; override so a
140+
// later range()/limit() call replaces the earlier value instead of
141+
// appending a duplicate query param.
142+
var url = overrideSearchParams(keyOffset, '$from');
143+
url = overrideSearchParams(keyLimit, '${to - from + 1}', url);
139144
return PostgrestTransformBuilder(copyWithUrl(url));
140145
}
141146

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import 'dart:convert';
2+
3+
import 'package:http/http.dart';
4+
import 'package:postgrest/postgrest.dart';
5+
import 'package:test/test.dart';
6+
7+
/// Captures the URL of the request the builder generates and returns an empty
8+
/// result set, so we can assert on the query string without a live server.
9+
class _CapturingClient extends BaseClient {
10+
Uri? lastUrl;
11+
12+
@override
13+
Future<StreamedResponse> send(BaseRequest request) async {
14+
lastUrl = request.url;
15+
return StreamedResponse(
16+
Stream.value(utf8.encode('[]')),
17+
200,
18+
request: request,
19+
headers: {'content-type': 'application/json'},
20+
);
21+
}
22+
}
23+
24+
void main() {
25+
late _CapturingClient mock;
26+
late PostgrestClient client;
27+
28+
setUp(() {
29+
mock = _CapturingClient();
30+
client = PostgrestClient('http://localhost:3000', httpClient: mock);
31+
});
32+
33+
test('a later limit() replaces the earlier one', () async {
34+
await client.from('t').select().limit(1).limit(2);
35+
36+
// `limit` is single-valued in PostgREST; the second call must win rather
37+
// than emitting `limit=1&limit=2`.
38+
expect(mock.lastUrl!.queryParametersAll['limit'], ['2']);
39+
});
40+
41+
test('range() overrides a preceding limit() instead of duplicating it',
42+
() async {
43+
await client.from('t').select().limit(5).range(0, 9);
44+
45+
expect(mock.lastUrl!.queryParametersAll['limit'], ['10']);
46+
expect(mock.lastUrl!.queryParametersAll['offset'], ['0']);
47+
});
48+
49+
test('a later range() replaces the earlier one', () async {
50+
await client.from('t').select().range(0, 9).range(10, 19);
51+
52+
expect(mock.lastUrl!.queryParametersAll['offset'], ['10']);
53+
expect(mock.lastUrl!.queryParametersAll['limit'], ['10']);
54+
});
55+
56+
test('referencedTable limit is scoped and single-valued', () async {
57+
await client
58+
.from('t')
59+
.select('messages(*)')
60+
.limit(1, referencedTable: 'messages')
61+
.limit(2, referencedTable: 'messages');
62+
63+
expect(mock.lastUrl!.queryParametersAll['messages.limit'], ['2']);
64+
});
65+
}

0 commit comments

Comments
 (0)