-
Notifications
You must be signed in to change notification settings - Fork 149
feat: Flutter SDK update for version 19.1.0 #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/services/teams_test.dart (2)
14-23: Signature mismatch:Client.calloverride must be non‑nullable and capture named args.Base is
Future<Response> call(HttpMethod method, {...}). UsingHttpMethod?breaks override; also forward named args for accurate stubbing.- @override - Future<Response> call( - HttpMethod? method, { - String path = '', - Map<String, String> headers = const {}, - Map<String, dynamic> params = const {}, - ResponseType? responseType, - }) async { - return super.noSuchMethod(Invocation.method(#call, [method]), - returnValue: Response()); - } + @override + Future<Response> call( + HttpMethod method, { + String path = '', + Map<String, String> headers = const {}, + Map<String, dynamic> params = const {}, + ResponseType? responseType, + }) async { + return super.noSuchMethod( + Invocation.method( + #call, + [method], + { + #path: path, + #headers: headers, + #params: params, + #responseType: responseType, + }, + ), + returnValue: Response(), + ); + }
36-45: Signature mismatch:chunkedUploadnamed params are required and non‑nullable.Match the base API and propagate named args in the invocation.
- @override - Future<Response> chunkedUpload({ - String? path, - Map<String, dynamic>? params, - String? paramName, - String? idParamName, - Map<String, String>? headers, - Function(UploadProgress)? onProgress, - }) async { - return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); - } + @override + Future<Response> chunkedUpload({ + required String path, + required Map<String, dynamic> params, + required String paramName, + required String idParamName, + required Map<String, String> headers, + Function(UploadProgress)? onProgress, + }) async { + return super.noSuchMethod( + Invocation.method(#chunkedUpload, [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + ); + }
🧹 Nitpick comments (86)
lib/src/models/team_list.dart (2)
11-11: Constructor one‑liner is fine; consider const + defensive immutability.Making the constructor
constand storing an unmodifiable view improves immutability without breaking API.- TeamList({required this.total, required this.teams}); + const TeamList({required this.total, required List<Team> teams}) + : teams = List.unmodifiable(teams);
21-21: Serialization LGTM; consider tighter return typing.
toMapis correct. If feasible across the codebase, preferMap<String, Object?>to avoid overly broaddynamic.- Map<String, dynamic> toMap() { + Map<String, Object?> toMap() { return {"total": total, "teams": teams.map((p) => p.toMap()).toList()}; }test/src/models/file_list_test.dart (1)
15-17: PreferisEmptymatcher for lists in tests.Clearer intent and better failure messages.
- expect(result.files, []); + expect(result.files, isEmpty);test/src/models/document_test.dart (1)
21-28: Minor: useisEmptyfor empty permissions.Improves readability and diffs on failure.
- expect(result.$permissions, []); + expect(result.$permissions, isEmpty);test/src/models/locale_test.dart (1)
20-27: UseisTruefor booleans.Slightly clearer than comparing to
true.- expect(result.eu, true); + expect(result.eu, isTrue);lib/src/models/algo_phpass.dart (1)
8-16: Optional: make the constructor const.This class is immutable; marking the ctor const enables compile‑time instances in tests and callers without breaking changes.
Apply:
- AlgoPhpass({required this.type}); + const AlgoPhpass({required this.type});lib/services/avatars.dart (1)
30-36: Project param added — client strips nulls; DRY helper recommended
- Confirmed: lib/src/client_mixin.dart removes null params via params.removeWhere((k, v) => v == null), so 'project': client.config['project'] will not be sent when null.
- Consistent with rest of SDK: other services already include 'project' in params (e.g., lib/services/storage.dart, lib/services/account.dart, lib/src/realtime_mixin.dart), so Avatars matching that pattern is correct.
- Optional DRY helper (inside Avatars or a shared Service helper):
Map<String, dynamic> _withProject(Map<String, dynamic> params) => {...params, 'project': client.config['project']};Example usage:
- final Map<String, dynamic> params = { - 'width': width, - 'height': height, - 'quality': quality, - 'project': client.config['project'], - }; + final Map<String, dynamic> params = + _withProject({'width': width, 'height': height, 'quality': quality});test/src/models/identity_test.dart (1)
23-33: Avoid test tokens that trigger secret scanners.Gitleaks flags these base64‑looking strings. Use obviously fake placeholders to prevent false positives.
Apply this diff:
- expect(result.providerAccessToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerAccessToken, 'not-a-secret-access-token'); - expect(result.providerRefreshToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerRefreshToken, 'not-a-secret-refresh-token');Optionally, add a gitleaks allowlist for test fixtures in a follow-up.
test/services/graphql_test.dart (4)
27-33: Include named args in noSuchMethod for webAuth.Passing named args improves Mockito matching and future‑proofs the mock.
Apply this diff:
- Future webAuth( - Uri? url, - { - String? callbackUrlScheme, - } - ) async { - return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); - } + Future webAuth( + Uri? url, { + String? callbackUrlScheme, + }) async { + return super.noSuchMethod( + Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), + returnValue: 'done', + ); + }
44-44: Mock chunkedUpload should use named args in Invocation.Align with the method’s named parameters so stubs/verifications match reliably.
Apply this diff:
- return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); + return super.noSuchMethod( + Invocation.method(#chunkedUpload, [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + );
58-69: Strengthen stubbing and add minimal assertions for query().Match named args and verify the call to make the test meaningful.
Apply this diff:
- when(client.call( - HttpMethod.post, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.post, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data)); - - final response = await graphql.query( - query: {}, - ); + final response = await graphql.query(query: {}); + expect(response, isNotNull); + verify(client.call( + HttpMethod.post, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).called(1);Additionally, consider updating MockClient.call to forward named args in its noSuchMethod Invocation:
// outside this hunk (optional hardening) return super.noSuchMethod( Invocation.method(#call, [method], { #path: path, #headers: headers, #params: params, #responseType: responseType, }), returnValue: Response(), );
71-82: Mirror the same robustness for mutation().Add named‑arg matchers and a basic verify.
Apply this diff:
- when(client.call( - HttpMethod.post, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.post, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data)); - - final response = await graphql.mutation( - query: {}, - ); + final response = await graphql.mutation(query: {}); + expect(response, isNotNull); + verify(client.call( + HttpMethod.post, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).called(1);lib/src/models/algo_bcrypt.dart (1)
8-16: Prefer strict casting over.toString()in fromMap.
.toString()can silently turnnullinto the string"null". Cast instead to fail fast if the payload is malformed.- factory AlgoBcrypt.fromMap(Map<String, dynamic> map) { - return AlgoBcrypt(type: map['type'].toString()); - } + factory AlgoBcrypt.fromMap(Map<String, dynamic> map) { + return AlgoBcrypt(type: map['type'] as String); + }lib/services/graphql.dart (2)
13-14: Type thequeryparam asMap<String, dynamic>for both methods.Improves API clarity and static checks.
- Future query({required Map query}) async { + Future query({required Map<String, dynamic> query}) async { ... - Future mutation({required Map query}) async { + Future mutation({required Map<String, dynamic> query}) async {Also applies to: 34-35
20-25: Call formatting LGTM; minor doc nit above.The multi-line
client.callis clearer. Nit: The docstring on thequerymethod says "mutation" — please correct.- /// Execute a GraphQL mutation. + /// Execute a GraphQL query.Also applies to: 41-46
lib/src/models/algo_sha.dart (1)
8-16: Same.toString()concern as bcrypt.Cast to
Stringto avoid accidental"null".- factory AlgoSha.fromMap(Map<String, dynamic> map) { - return AlgoSha(type: map['type'].toString()); - } + factory AlgoSha.fromMap(Map<String, dynamic> map) { + return AlgoSha(type: map['type'] as String); + }lib/src/client.dart (1)
30-30: Factory formatting LGTM; tiny typo in doc above.Nit on Line 13: "cunked" → "chunked".
- /// The size for cunked uploads in bytes. + /// The size for chunked uploads in bytes.test/services/teams_test.dart (4)
5-7: Avoid importing fromsrc/; rely on public exports.Using internal
src/paths is brittle.appwrite.dartalready re-exports what you need.-import 'package:appwrite/enums.dart' as enums; -import 'package:appwrite/src/enums.dart'; -import 'package:appwrite/src/response.dart'; +// Use public exports via appwrite.dart
64-67: Update Mockito stubs to match named args.After fixing overrides to forward named args, include
anyNamedforpath/params/headersin stubs.Example:
-when(client.call( - HttpMethod.get, -)).thenAnswer((_) async => Response(data: data)); +when(client.call( + HttpMethod.get, + path: anyNamed('path'), + params: anyNamed('params'), + headers: anyNamed('headers'), +)).thenAnswer((_) async => Response(data: data));Replicate for POST/PUT/PATCH/DELETE cases.
Also applies to: 85-88, 108-111, 130-133, 146-149, 162-165, 191-194, 221-224, 251-254, 268-271, 296-299, 315-318, 332-334
143-155: Add assertions for delete flows.Currently no expectations are verified. At minimum, assert completion.
- final response = await teams.delete( - teamId: '<TEAM_ID>', - ); + await expectLater( + teams.delete(teamId: '<TEAM_ID>'), + completes, + );Also applies to: 265-277
58-73: Strengthen coverage inlist()test.Optionally assert decoded values (e.g.,
total), not just the type.- expect(response, isA<models.TeamList>()); + expect(response, isA<models.TeamList>()); + expect(response.total, 5);lib/src/models/execution.dart (2)
35-35: Doc wording nit.Consider "allowlisted" instead of "whitelisted".
93-99: Null-safety for header lists.Guard against missing
requestHeaders/responseHeadersto avoid NPEs when fields are absent.- requestHeaders: List<Headers>.from( - map['requestHeaders'].map((p) => Headers.fromMap(p)), - ), + requestHeaders: List<Headers>.from( + (map['requestHeaders'] ?? []).map((p) => Headers.fromMap(p)), + ), ... - responseHeaders: List<Headers>.from( - map['responseHeaders'].map((p) => Headers.fromMap(p)), - ), + responseHeaders: List<Headers>.from( + (map['responseHeaders'] ?? []).map((p) => Headers.fromMap(p)), + ),.github/workflows/format.yml (2)
13-15: YAML lint nit: extra spaces after colon.
image: dart:stablehas two spaces; some linters flag this.- image: dart:stable + image: dart:stable
16-25: Optional: make format step deterministic.
Consider failing instead of pushing when formatting differs on forks, or at least make intent explicit.Example alternatives:
- Use
dart format --set-exit-if-changed .and let contributors push fixes locally.- Keep auto-commit but add a note in CONTRIBUTING.md about the bot formatting pushes.
test/src/models/algo_md5_test.dart (1)
14-15: Style nit: prefer explicit matcher.
expect(result.type, 'md5')is valid, butequals('md5')is clearer.- expect(result.type, 'md5'); + expect(result.type, equals('md5'));test/services/avatars_test.dart (3)
25-33: More accurate noSuchMethod invocations for named args.
BothwebAuthandchunkedUploadhave named parameters, butInvocation.methodis constructed with positional args only. Build the named-args map for closer Mockito matching (helps future verifications).- Future webAuth( - Uri? url, - { - String? callbackUrlScheme, - } - ) async { - return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); - } + Future webAuth( + Uri? url, { + String? callbackUrlScheme, + }) async { + return super.noSuchMethod( + Invocation.method(#webAuth, [], { + #url: url, + #callbackUrlScheme: callbackUrlScheme, + }), + returnValue: 'done', + ); + }- Future<Response> chunkedUpload({ + Future<Response> chunkedUpload({ String? path, Map<String, dynamic>? params, String? paramName, String? idParamName, Map<String, String>? headers, Function(UploadProgress)? onProgress, }) async { - return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); + return super.noSuchMethod( + Invocation.method(#chunkedUpload, [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + ); }
58-77: Stub with named-arg matchers for resilience.
Current stubs ignore named args; more robust to match withanyNamedto avoid breakage as service methods evolve (e.g., addedprojectparam).- when(client.call( - HttpMethod.get, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.get, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data));Repeat for other tests in this file.
58-58: Readability: avoid multiple statements on one line.
Long one‑liners hinder diffs. Break into standard multi‑line blocks.- test('test method getBrowser()', () async {final Uint8List data = Uint8List.fromList([]); + test('test method getBrowser()', () async { + final Uint8List data = Uint8List.fromList([]);test/src/models/jwt_test.dart (1)
8-9: Secret scanning false positive risk.
Hardcoded JWT string triggers Gitleaks. Replace with a benign placeholder to keep CI green.- final model = Jwt( - jwt: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c', - ); + final model = Jwt(jwt: 'header.payload.signature'); // non-sensitive test tokenAnd update the assertion accordingly.
lib/src/cookie_manager.dart (1)
14-27: Simplify async flow; avoid then/catch on awaited future.Keeps control flow clearer and avoids nested callbacks.
- FutureOr<http.BaseRequest> onRequest(http.BaseRequest request) async { - await cookieJar - .loadForRequest(request.url) - .then((cookies) { - var cookie = getCookies(cookies); - if (cookie.isNotEmpty) { - request.headers.addAll({HttpHeaders.cookieHeader: cookie}); - } - return request; - }) - .catchError((e, stackTrace) { - return request; - }); - return request; - } + FutureOr<http.BaseRequest> onRequest(http.BaseRequest request) async { + try { + final cookies = await cookieJar.loadForRequest(request.url); + final cookie = getCookies(cookies); + if (cookie.isNotEmpty) { + request.headers[HttpHeaders.cookieHeader] = cookie; + } + } catch (_) {/* no-op */} + return request; + }test/services/tables_db_test.dart (2)
178-191: Add an assertion for deleteRow().Currently no expectations; assert response or verify the call.
final response = await tablesDB.deleteRow( databaseId: '<DATABASE_ID>', tableId: '<TABLE_ID>', rowId: '<ROW_ID>', ); + expect(response, isNotNull);
69-75: Strengthen test fidelity: verify method, path, and params.Mock only on
HttpMethod.getallows false positives. Verify the constructed request.final response = await tablesDB.listRows( databaseId: '<DATABASE_ID>', tableId: '<TABLE_ID>', ); expect(response, isA<models.RowList>()); + verify(client.call( + HttpMethod.get, + path: argThat(contains('/tablesdb/')), + headers: anyNamed('headers'), + params: anyNamed('params'), + )).called(1);lib/services/teams.dart (2)
60-73: Encode path parameters.Raw
replaceAllcan break paths if IDs contain reserved chars. EncodeteamId(and other IDs across this file).- final String apiPath = '/teams/{teamId}'.replaceAll('{teamId}', teamId); + final String apiPath = '/teams/{teamId}' + .replaceAll('{teamId}', Uri.encodeComponent(teamId));Apply similarly wherever
{teamId}/{membershipId}appear.
337-356: Prefer typed prefs map.Use
Map<String, dynamic>to avoid dynamic keys/values at the API boundary.- Future<models.Preferences> updatePrefs({ - required String teamId, - required Map prefs, - }) async { + Future<models.Preferences> updatePrefs({ + required String teamId, + required Map<String, dynamic> prefs, + }) async {lib/services/tables_db.dart (3)
73-89: Encode all dynamic path segments.Encode
{databaseId},{tableId},{rowId}to prevent path issues.- final String apiPath = - '/tablesdb/{databaseId}/tables/{tableId}/rows/{rowId}' - .replaceAll('{databaseId}', databaseId) - .replaceAll('{tableId}', tableId) - .replaceAll('{rowId}', rowId); + final String apiPath = + '/tablesdb/{databaseId}/tables/{tableId}/rows/{rowId}' + .replaceAll('{databaseId}', Uri.encodeComponent(databaseId)) + .replaceAll('{tableId}', Uri.encodeComponent(tableId)) + .replaceAll('{rowId}', Uri.encodeComponent(rowId));Do the same in createRow/deleteRow/decrementRowColumn/incrementRowColumn (including
{column}).
201-210: Encode{column}as well.Column names may include spaces or symbols; encode to be safe.
- .replaceAll('{column}', column); + .replaceAll('{column}', Uri.encodeComponent(column));
231-240: Encode{column}(increment).- .replaceAll('{column}', column); + .replaceAll('{column}', Uri.encodeComponent(column));test/services/messaging_test.dart (1)
94-106: Add an assertion for deleteSubscriber().Ensure the call returned (or verify it was invoked).
final response = await messaging.deleteSubscriber( topicId: '<TOPIC_ID>', subscriberId: '<SUBSCRIBER_ID>', ); + expect(response, isNotNull);lib/services/messaging.dart (2)
15-18: Encode{topicId}in path.- final String apiPath = '/messaging/topics/{topicId}/subscribers'.replaceAll( - '{topicId}', - topicId, - ); + final String apiPath = '/messaging/topics/{topicId}/subscribers'.replaceAll( + '{topicId}', + Uri.encodeComponent(topicId), + );
43-45: Encode{topicId}and{subscriberId}.- .replaceAll('{topicId}', topicId) - .replaceAll('{subscriberId}', subscriberId); + .replaceAll('{topicId}', Uri.encodeComponent(topicId)) + .replaceAll('{subscriberId}', Uri.encodeComponent(subscriberId));lib/services/account.dart (1)
1297-1309: Encode{provider}and keep params robust.Guard against reserved chars in provider.
- final String apiPath = '/account/tokens/oauth2/{provider}'.replaceAll( - '{provider}', - provider.value, - ); + final String apiPath = '/account/tokens/oauth2/{provider}'.replaceAll( + '{provider}', + Uri.encodeComponent(provider.value), + );test/services/account_test.dart (2)
955-999: Add assertion for updateMagicURLSession().Keep consistency with other tests.
final response = await account.updateMagicURLSession( userId: '<USER_ID>', secret: '<SECRET>', ); - expect(response, isA<models.Session>()); + expect(response, isA<models.Session>());
1195-1206: Add assertion for deleteSession().Currently no expectations.
final response = await account.deleteSession( sessionId: '<SESSION_ID>', ); + expect(response, isNotNull);lib/services/functions.dart (2)
11-29: listExecutions: API path/shape look correct; tiny nit on null param pruningAll good. Optional: avoid sending
'queries': nullifclient.calldoesn’t already strip nulls.
38-69: createExecution: type theheadersparam; make content-type map const
- Prefer a typed header map for better DX.
- Use a const map for headers.
Apply:
- Map? headers, + Map<String, dynamic>? headers, @@ - final Map<String, String> apiHeaders = {'content-type': 'application/json'}; + final Map<String, String> apiHeaders = const {'content-type': 'application/json'};Please confirm tightening the type won’t break existing callers (Map? → Map<String, dynamic>?).
test/services/functions_test.dart (2)
69-73: Add verify() assertions to check paths are correctStrengthens the tests by validating endpoint paths.
final response = await functions.listExecutions( functionId: '<FUNCTION_ID>', ); expect(response, isA<models.ExecutionList>()); +verify(client.call( + HttpMethod.get, + path: '/functions/<FUNCTION_ID>/executions', + params: anyNamed('params'), + headers: anyNamed('headers'), + responseType: anyNamed('responseType'), +)); @@ final response = await functions.createExecution( functionId: '<FUNCTION_ID>', ); expect(response, isA<models.Execution>()); +verify(client.call( + HttpMethod.post, + path: '/functions/<FUNCTION_ID>/executions', + params: anyNamed('params'), + headers: anyNamed('headers'), + responseType: anyNamed('responseType'), +)); @@ final response = await functions.getExecution( functionId: '<FUNCTION_ID>', executionId: '<EXECUTION_ID>', ); expect(response, isA<models.Execution>()); +verify(client.call( + HttpMethod.get, + path: '/functions/<FUNCTION_ID>/executions/<EXECUTION_ID>', + params: anyNamed('params'), + headers: anyNamed('headers'), + responseType: anyNamed('responseType'), +));Also applies to: 102-106, 135-140
21-23: Optional: also forward named args for call(...) override (outside changed hunk)For completeness, mirror the pattern used above so future stubs with named args work:
@override Future<Response> call( HttpMethod? method, { String path = '', Map<String, String> headers = const {}, Map<String, dynamic> params = const {}, ResponseType? responseType, }) async { return super.noSuchMethod( Invocation.method(#call, [method], { #path: path, #headers: headers, #params: params, #responseType: responseType, }), returnValue: Response(), ); }test/src/models/continent_list_test.dart (1)
15-17: Prefer clearer matchers for emptiness
expect(result.continents, isEmpty);reads clearer than comparing to[].Apply:
- expect(result.total, 5); - expect(result.continents, []); + expect(result.total, 5); + expect(result.continents, isEmpty);test/src/models/continent_test.dart (1)
15-17: Nit: indentation only — looks goodNo behavioral change; assertions remain correct.
Optionally align indentation consistently with project formatter.
test/src/models/country_test.dart (1)
15-17: Prefer clearer equality for readabilityKeep, or switch to a single-line style for consistency with formatter.
- expect(result.name, 'United States'); - expect(result.code, 'US'); + expect(result.name, 'United States'); + expect(result.code, 'US');lib/src/models/language_list.dart (2)
11-11: Make constructorconstfor better compile-time usageAllows const instantiation and minor perf wins.
- LanguageList({required this.total, required this.languages}); + const LanguageList({required this.total, required this.languages});
16-18: Harden fromMap: null-safety, typing, and immutabilityCurrent code assumes non-null list and leaves the list mutable.
- languages: List<Language>.from( - map['languages'].map((p) => Language.fromMap(p)), - ), + languages: List<Language>.unmodifiable( + ((map['languages'] ?? []) as List) + .map((p) => Language.fromMap(p as Map<String, dynamic>)), + ),(Optional) also coerce total safely:
// total: (map['total'] as num).toInt(),test/src/models/preferences_test.dart (1)
13-13: Add assertions — test currently doesn’t verify anythingRound‑trip without checks loses value.
final map = model.toMap(); final result = Preferences.fromMap(map); - + expect(result.data, isEmpty); + expect(result.toMap(), {'data': {}});test/src/models/phone_list_test.dart (1)
15-17: UseisEmptyfor list emptinessMore idiomatic than
[]comparison.- expect(result.total, 5); - expect(result.phones, []); + expect(result.total, 5); + expect(result.phones, isEmpty);lib/src/models/session_list.dart (2)
11-11: Make constructorconstConsistent with other immutable models; enables const usage.
- SessionList({required this.total, required this.sessions}); + const SessionList({required this.total, required this.sessions});
16-18: Defensive copy + null-safety in fromMapAvoid NPEs when
sessionsis absent/null and prevent external mutation.- sessions: List<Session>.from( - map['sessions'].map((p) => Session.fromMap(p)), - ), + sessions: List<Session>.unmodifiable( + ((map['sessions'] ?? []) as List) + .map((p) => Session.fromMap(p as Map<String, dynamic>)), + ),(Optional) coerce total:
// total: (map['total'] as num).toInt(),test/src/models/locale_code_test.dart (1)
15-17: Indentation nit — fine to keepNo behavior change; consistent formatting preferred.
- expect(result.code, 'en-us'); - expect(result.name, 'US'); + expect(result.code, 'en-us'); + expect(result.name, 'US');lib/src/models/identity_list.dart (1)
17-18: Guard against null identities in fromMap.Avoid
NoSuchMethodErrorif server ever returnsnullinstead of[].Apply this diff:
- identities: List<Identity>.from( - map['identities'].map((p) => Identity.fromMap(p)), - ), + identities: List<Identity>.from( + ((map['identities'] as List?) ?? const []) + .map((p) => Identity.fromMap(p)), + ),lib/src/models/log_list.dart (1)
15-17: Consider null-safe parsing for logs as well.Mirror the defensive pattern to avoid surprises on malformed payloads.
Suggested change:
- logs: List<Log>.from(map['logs'].map((p) => Log.fromMap(p))), + logs: List<Log>.from( + ((map['logs'] as List?) ?? const []) + .map((p) => Log.fromMap(p)), + ),lib/services/locale.dart (1)
22-27: Multiline call refactor looks good; consider const empty maps.No behavior change—nice. As a tiny alloc win, you can use const maps for empty params/headers in these methods.
Example (apply similarly across methods):
- final Map<String, dynamic> apiParams = {}; - final Map<String, String> apiHeaders = {}; + const Map<String, dynamic> apiParams = <String, dynamic>{}; + const Map<String, String> apiHeaders = <String, String>{};Also applies to: 41-46, 60-65, 79-84, 98-103, 117-122, 137-142, 156-161
test/src/models/membership_list_test.dart (1)
15-17: Fix indentation to satisfy formatter/analyzer.These lines are mis-indented and may fail lint/format checks.
- expect(result.total, 5); - expect(result.memberships, []); - }); + expect(result.total, 5); + expect(result.memberships, []); + });lib/src/models/execution_list.dart (1)
11-11: Consider returning a non-growable list for immutability.Models are typically immutable; prevent accidental growth and save a small amount of overhead.
- ExecutionList({required this.total, required this.executions}); + ExecutionList({required this.total, required this.executions}); @@ - executions: List<Execution>.from( - map['executions'].map((p) => Execution.fromMap(p)), - ), + executions: List<Execution>.from( + (map['executions'] as List).map((p) => Execution.fromMap(p)), + growable: false, + ),Also applies to: 17-19
lib/src/models/continent_list.dart (1)
11-11: Make list non-growable to match immutable model intent.- ContinentList({required this.total, required this.continents}); + ContinentList({required this.total, required this.continents}); @@ - continents: List<Continent>.from( - map['continents'].map((p) => Continent.fromMap(p)), - ), + continents: List<Continent>.from( + (map['continents'] as List).map((p) => Continent.fromMap(p)), + growable: false, + ),Also applies to: 17-19
lib/src/models/membership_list.dart (1)
11-11: Prefer non-growable list for memberships.- MembershipList({required this.total, required this.memberships}); + MembershipList({required this.total, required this.memberships}); @@ - memberships: List<Membership>.from( - map['memberships'].map((p) => Membership.fromMap(p)), - ), + memberships: List<Membership>.from( + (map['memberships'] as List).map((p) => Membership.fromMap(p)), + growable: false, + ),Also applies to: 17-19
lib/src/models/country_list.dart (1)
11-11: Lock down countries list to non-growable.- CountryList({required this.total, required this.countries}); + CountryList({required this.total, required this.countries}); @@ - countries: List<Country>.from( - map['countries'].map((p) => Country.fromMap(p)), - ), + countries: List<Country>.from( + (map['countries'] as List).map((p) => Country.fromMap(p)), + growable: false, + ),Also applies to: 16-19
lib/query.dart (1)
185-191: Nit: tighten value typing in geo-distance helpers.Consider
List<num>(orList<double>) instead ofList<dynamic>forvaluesto improve type-safety without breaking behavior.- List<dynamic> values, + List<num> values,Also applies to: 195-201, 205-211, 215-221
lib/src/models/country.dart (1)
14-14: Minor: avoid'null'strings when source fields are absent.Using
.toString()on null yields the literal'null'. Prefer null-aware fallbacks.- return Country(name: map['name'].toString(), code: map['code'].toString()); + return Country( + name: (map['name'] as String?) ?? '', + code: (map['code'] as String?) ?? '', + );lib/src/models/document_list.dart (1)
16-18: Harden fromMap against nulls/typesDefensive casts avoid runtime errors if payload varies.
- documents: List<Document>.from( - map['documents'].map((p) => Document.fromMap(p)), - ), + documents: (map['documents'] as List?) + ?.map((p) => Document.fromMap(p as Map<String, dynamic>)) + .toList() ?? + const <Document>[],Also consider:
total: (map['total'] as num).toInt()for robustness.lib/src/realtime_io.dart (1)
35-39: Avoid shadowing the mixin field_websokUse a local name like
channelto reduce confusion.- final _websok = IOWebSocketChannel( + final channel = IOWebSocketChannel( (client as ClientIO).selfSigned ? await _connectForSelfSignedCert(uri, headers) : await WebSocket.connect(uri.toString(), headers: headers), ); - return _websok; + return channel;lib/services/databases.dart (5)
12-18: Deprecation message update LGTM; omit null query paramMessage reads well. Small tweak: avoid sending
queries=null.
- Verify target API name:
TablesDB.listRows.- final Map<String, dynamic> apiParams = {'queries': queries}; + final Map<String, dynamic> apiParams = { + if (queries != null) 'queries': queries, + };Also applies to: 24-24, 28-33
43-46: createDocument: deprecation text LGTM; prefer typed MapUse
Map<String, dynamic>fordatato aid users and analyzers.- Future<models.Document> createDocument({ + Future<models.Document> createDocument({ required String databaseId, required String collectionId, required String documentId, - required Map data, + required Map<String, dynamic> data, List<String>? permissions, }) async {Confirm
TablesDB.createRownaming.Also applies to: 63-70
78-85: getDocument: deprecation text LGTM; omit null query paramSame null-queries suggestion as above.
- final Map<String, dynamic> apiParams = {'queries': queries}; + final Map<String, dynamic> apiParams = { + if (queries != null) 'queries': queries, + };Verify
TablesDB.getRownaming.Also applies to: 92-92, 96-101
111-119: upsertDocument: deprecation text LGTM; type the data map- required Map data, + required Map<String, dynamic> data,Confirm
TablesDB.upsertRownaming.Also applies to: 131-138
146-154: updateDocument: deprecation text LGTM; type the data map- Map? data, + Map<String, dynamic>? data,Verify
TablesDB.updateRownaming.Also applies to: 166-173
lib/src/realtime_mixin.dart (2)
73-123: Stream listener refactor LGTM; guard against closed controllersMinor safety: avoid add/addError on closed controllers.
- subscription.controller.add(message); + if (!subscription.controller.isClosed) { + subscription.controller.add(message); + }- subscription.controller.addError(err, stack); + if (!subscription.controller.isClosed) { + subscription.controller.addError(err, stack); + }
149-152: Ternary readability (optional)Consider simple if/else for maintainability.
test/services/storage_test.dart (3)
27-33: Forward named args in MockClient.webAuthEnsure Mockito sees named args.
- ) async { - return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); + ) async { + return super.noSuchMethod( + Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), + returnValue: 'done', + );
44-44: Forward named args in MockClient.chunkedUploadImproves stub matching and clarity.
- return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); + return super.noSuchMethod( + Invocation.method(#chunkedUpload, const [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + );
179-193: Assert request shape for project param (optional)Since SDK adds
projectto download endpoints, verify it in tests.verify(client.call( HttpMethod.get, path: anyNamed('path'), headers: anyNamed('headers'), params: argThat(containsPair('project', 'testproject'), named: 'params'), ));test/services/databases_test.dart (6)
27-33: Forward named args in MockClient.webAuthSame rationale as storage tests.
- ) async { - return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); + ) async { + return super.noSuchMethod( + Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), + returnValue: 'done', + );
44-44: Forward named args in MockClient.chunkedUploadAligns with Mockito’s handling of named parameters.
- return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); + return super.noSuchMethod( + Invocation.method(#chunkedUpload, const [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + );
58-75: listDocuments test coverage LGTMConsider verifying path/params via
verify(...)as in storage tests.
77-101: createDocument test LGTMAs above, optional verify on method/path.
103-126: getDocument test LGTMOptional: verify
documentIdembedded in path.
179-193: deleteDocument test LGTMOptionally assert that response is empty string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
.github/workflows/format.yml(2 hunks)README.md(1 hunks)lib/query.dart(3 hunks)lib/services/account.dart(4 hunks)lib/services/avatars.dart(7 hunks)lib/services/databases.dart(7 hunks)lib/services/functions.dart(4 hunks)lib/services/graphql.dart(2 hunks)lib/services/locale.dart(8 hunks)lib/services/messaging.dart(1 hunks)lib/services/storage.dart(3 hunks)lib/services/tables_db.dart(7 hunks)lib/services/teams.dart(11 hunks)lib/src/client.dart(1 hunks)lib/src/client_browser.dart(1 hunks)lib/src/client_io.dart(1 hunks)lib/src/client_mixin.dart(1 hunks)lib/src/cookie_manager.dart(2 hunks)lib/src/enums.dart(1 hunks)lib/src/models/algo_bcrypt.dart(1 hunks)lib/src/models/algo_md5.dart(1 hunks)lib/src/models/algo_phpass.dart(1 hunks)lib/src/models/algo_sha.dart(1 hunks)lib/src/models/continent.dart(2 hunks)lib/src/models/continent_list.dart(1 hunks)lib/src/models/country.dart(1 hunks)lib/src/models/country_list.dart(1 hunks)lib/src/models/currency_list.dart(1 hunks)lib/src/models/document_list.dart(1 hunks)lib/src/models/execution.dart(2 hunks)lib/src/models/execution_list.dart(1 hunks)lib/src/models/file_list.dart(2 hunks)lib/src/models/headers.dart(2 hunks)lib/src/models/identity_list.dart(1 hunks)lib/src/models/jwt.dart(1 hunks)lib/src/models/language.dart(2 hunks)lib/src/models/language_list.dart(1 hunks)lib/src/models/locale_code.dart(2 hunks)lib/src/models/locale_code_list.dart(1 hunks)lib/src/models/log_list.dart(2 hunks)lib/src/models/membership_list.dart(1 hunks)lib/src/models/mfa_recovery_codes.dart(2 hunks)lib/src/models/mfa_type.dart(2 hunks)lib/src/models/phone_list.dart(2 hunks)lib/src/models/preferences.dart(1 hunks)lib/src/models/row_list.dart(2 hunks)lib/src/models/session_list.dart(1 hunks)lib/src/models/team_list.dart(2 hunks)lib/src/realtime_io.dart(2 hunks)lib/src/realtime_mixin.dart(3 hunks)lib/src/realtime_response.dart(1 hunks)lib/src/realtime_response_connected.dart(2 hunks)lib/src/realtime_stub.dart(1 hunks)pubspec.yaml(1 hunks)test/query_test.dart(2 hunks)test/services/account_test.dart(2 hunks)test/services/avatars_test.dart(2 hunks)test/services/databases_test.dart(2 hunks)test/services/functions_test.dart(2 hunks)test/services/graphql_test.dart(2 hunks)test/services/locale_test.dart(2 hunks)test/services/messaging_test.dart(2 hunks)test/services/storage_test.dart(2 hunks)test/services/tables_db_test.dart(2 hunks)test/services/teams_test.dart(2 hunks)test/src/models/algo_argon2_test.dart(1 hunks)test/src/models/algo_bcrypt_test.dart(1 hunks)test/src/models/algo_md5_test.dart(1 hunks)test/src/models/algo_phpass_test.dart(1 hunks)test/src/models/algo_scrypt_modified_test.dart(1 hunks)test/src/models/algo_scrypt_test.dart(1 hunks)test/src/models/algo_sha_test.dart(1 hunks)test/src/models/continent_list_test.dart(1 hunks)test/src/models/continent_test.dart(1 hunks)test/src/models/country_list_test.dart(1 hunks)test/src/models/country_test.dart(1 hunks)test/src/models/currency_list_test.dart(1 hunks)test/src/models/currency_test.dart(1 hunks)test/src/models/document_list_test.dart(1 hunks)test/src/models/document_test.dart(1 hunks)test/src/models/execution_list_test.dart(1 hunks)test/src/models/execution_test.dart(1 hunks)test/src/models/file_list_test.dart(1 hunks)test/src/models/file_test.dart(1 hunks)test/src/models/headers_test.dart(1 hunks)test/src/models/identity_list_test.dart(1 hunks)test/src/models/identity_test.dart(1 hunks)test/src/models/jwt_test.dart(1 hunks)test/src/models/language_list_test.dart(1 hunks)test/src/models/language_test.dart(1 hunks)test/src/models/locale_code_list_test.dart(1 hunks)test/src/models/locale_code_test.dart(1 hunks)test/src/models/locale_test.dart(1 hunks)test/src/models/log_list_test.dart(1 hunks)test/src/models/log_test.dart(1 hunks)test/src/models/membership_list_test.dart(1 hunks)test/src/models/membership_test.dart(1 hunks)test/src/models/mfa_challenge_test.dart(1 hunks)test/src/models/mfa_factors_test.dart(1 hunks)test/src/models/mfa_recovery_codes_test.dart(1 hunks)test/src/models/mfa_type_test.dart(1 hunks)test/src/models/phone_list_test.dart(1 hunks)test/src/models/phone_test.dart(1 hunks)test/src/models/preferences_test.dart(1 hunks)test/src/models/row_list_test.dart(1 hunks)test/src/models/row_test.dart(1 hunks)test/src/models/session_list_test.dart(1 hunks)
⛔ Files not processed due to max files limit (7)
- test/src/models/session_test.dart
- test/src/models/subscriber_test.dart
- test/src/models/target_test.dart
- test/src/models/team_list_test.dart
- test/src/models/team_test.dart
- test/src/models/token_test.dart
- test/src/models/user_test.dart
🧰 Additional context used
🪛 Gitleaks (8.28.0)
test/src/models/identity_test.dart
[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/src/models/jwt_test.dart
[high] 8-8: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 14-14: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
test/src/models/algo_scrypt_modified_test.dart
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/services/account_test.dart
[high] 428-428: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 430-430: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 474-474: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 476-476: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 874-874: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 876-876: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 918-918: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 920-920: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 964-964: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 966-966: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1022-1022: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1024-1024: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1068-1068: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1070-1070: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1114-1114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1116-1116: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1159-1159: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1161-1161: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 188-188: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 YAMLlint (1.37.1)
.github/workflows/format.yml
[warning] 14-14: too many spaces after colon
(colons)
| uses: EndBug/add-and-commit@v9.1.4 | ||
| with: | ||
| add: '["lib", "test"]' | ||
| add: lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential regression: only committing lib/ changes now.
Previously both lib/ and test/ were added; with add: lib, formatted test files won’t be committed, causing drift and CI flakiness. Re-include test/.
Apply one of the following:
- add: lib
+ add: '["lib","test"]'or
- add: lib
+ add: |
+ lib
+ test📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| add: lib | |
| add: '["lib","test"]' |
🤖 Prompt for AI Agents
.github/workflows/format.yml around line 32: the workflow currently only adds
the lib directory (add: lib), which omits formatted test files and can cause
drift; update the add setting so both lib and test directories are
staged/committed (for example, use an array or list that includes both "lib" and
"test") so formatted test files are also committed by the workflow.
| headers: streamedResponse.headers.map( | ||
| (k, v) => k.toLowerCase() == 'content-type' | ||
| ? MapEntry(k, 'text/plain') | ||
| : MapEntry(k, v), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
Normalize header keys to lowercase to preserve case-insensitive lookups on 204 path
Map.map creates a plain Map and preserves original header casing, so lookups like res.headers['content-type'] and res.headers['x-appwrite-warning'] can fail on the 204 path (while fromStream typically yields case-insensitive headers). Normalize keys to lowercase for consistency.
- headers: streamedResponse.headers.map(
- (k, v) => k.toLowerCase() == 'content-type'
- ? MapEntry(k, 'text/plain')
- : MapEntry(k, v),
- ),
+ headers: {
+ for (final e in streamedResponse.headers.entries)
+ e.key.toLowerCase():
+ e.key.toLowerCase() == 'content-type' ? 'text/plain' : e.value,
+ },Run to confirm we consistently use lowercase header keys elsewhere:
🏁 Script executed:
#!/bin/bash
# Find header lookups that assume lowercase keys
rg -nP -C2 "headers\[['\"]([a-z0-9-]+)['\"]\]"Length of output: 2993
Normalize response header keys to lowercase
Confirmed multiple consumers read lowercase header keys; normalize the response headers from streamedResponse to lowercase to avoid lookup failures.
Locations: lib/src/client_mixin.dart:20,82,88,101; lib/src/client_browser.dart:205,250.
- headers: streamedResponse.headers.map(
- (k, v) => k.toLowerCase() == 'content-type'
- ? MapEntry(k, 'text/plain')
- : MapEntry(k, v),
- ),
+ headers: {
+ for (final e in streamedResponse.headers.entries)
+ e.key.toLowerCase():
+ e.key.toLowerCase() == 'content-type' ? 'text/plain' : e.value,
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| headers: streamedResponse.headers.map( | |
| (k, v) => k.toLowerCase() == 'content-type' | |
| ? MapEntry(k, 'text/plain') | |
| : MapEntry(k, v), | |
| ), | |
| headers: { | |
| for (final e in streamedResponse.headers.entries) | |
| e.key.toLowerCase(): | |
| e.key.toLowerCase() == 'content-type' ? 'text/plain' : e.value, | |
| }, |
| scheme: response.request!.url.scheme, | ||
| host: response.request!.url.host, | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Save cookies with the exact response URL (prevents over‑broad Path=/).
Using only scheme+host when saving can widen cookie scope to /. Persist against response.request!.url.
- Uri(
- scheme: response.request!.url.scheme,
- host: response.request!.url.host,
- ),
+ response.request!.url,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scheme: response.request!.url.scheme, | |
| host: response.request!.url.host, | |
| ), | |
| response.request!.url, |
🤖 Prompt for AI Agents
In lib/src/cookie_manager.dart around lines 45 to 47, cookies are being saved
using only scheme and host which can broaden their path to “/”; update the
save/persist call to use the full response.request!.url (including path and
query) as the cookie's URL so the stored cookie preserves the exact path scope
from the response URL; adjust any method parameters to accept a Uri or string
URL and pass response.request!.url instead of constructing from scheme+host.
| factory Preferences.fromMap(Map<String, dynamic> map) { | ||
| return Preferences( | ||
| data: map, | ||
| ); | ||
| return Preferences(data: map); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocker: fromMap nests prefs incorrectly; use map['data'] with safe cast.
Current code sets data to the whole map, which yields double-nested {"data":{"data":...}} when deserializing server responses. Fix to extract the inner data and default to {}.
Apply:
- factory Preferences.fromMap(Map<String, dynamic> map) {
- return Preferences(data: map);
- }
+ factory Preferences.fromMap(Map<String, dynamic> map) {
+ final raw = map['data'];
+ return Preferences(
+ data: raw is Map ? Map<String, dynamic>.from(raw) : <String, dynamic>{},
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| factory Preferences.fromMap(Map<String, dynamic> map) { | |
| return Preferences( | |
| data: map, | |
| ); | |
| return Preferences(data: map); | |
| } | |
| factory Preferences.fromMap(Map<String, dynamic> map) { | |
| final raw = map['data']; | |
| return Preferences( | |
| data: raw is Map ? Map<String, dynamic>.from(raw) : <String, dynamic>{}, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In lib/src/models/preferences.dart around lines 9-11, the Preferences.fromMap
factory currently assigns the entire input map to the data field, causing
double-nesting; change it to extract the inner map via map['data'] with a safe
cast and fallback to an empty map (e.g., treat null or non-map values as {}),
ensuring data is a Map<String, dynamic> rather than the whole envelope.
| when(client.call( | ||
| HttpMethod.get, | ||
| )).thenAnswer((_) async => Response(data: data)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stub named args in when(...) to ensure the stub matches the real call
Without matching named args, the stub may not hit and fall back to default Response().
when(client.call(
- HttpMethod.get,
+ HttpMethod.get,
+ path: anyNamed('path'),
+ params: anyNamed('params'),
+ headers: anyNamed('headers'),
+ responseType: anyNamed('responseType'),
)).thenAnswer((_) async => Response(data: data));
@@
when(client.call(
- HttpMethod.post,
+ HttpMethod.post,
+ path: anyNamed('path'),
+ params: anyNamed('params'),
+ headers: anyNamed('headers'),
+ responseType: anyNamed('responseType'),
)).thenAnswer((_) async => Response(data: data));
@@
when(client.call(
- HttpMethod.get,
+ HttpMethod.get,
+ path: anyNamed('path'),
+ params: anyNamed('params'),
+ headers: anyNamed('headers'),
+ responseType: anyNamed('responseType'),
)).thenAnswer((_) async => Response(data: data));Also applies to: 97-99, 130-132
🤖 Prompt for AI Agents
In test/services/functions_test.dart around lines 64-66 (and similarly at 97-99
and 130-132), the Mockito stub uses when(client.call(HttpMethod.get)) without
including the named parameters, so it may not match the real invocation and
falls back to a default Response; update the stubs to include the same named
arguments used in the real calls (e.g., include named params like method:,
path:, queryParameters:, body: or any other named args) when calling
when(...).thenAnswer(...) so the matcher exactly matches the invocation and
returns the intended Response.
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass named arguments to noSuchMethod for webAuth.
Current stub drops the named param; use namedArgs to avoid matcher mismatches.
Apply this diff:
- Future webAuth(
- Uri? url,
- {
- String? callbackUrlScheme,
- }
- ) async {
- return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
- }
+ Future webAuth(
+ Uri? url, {
+ String? callbackUrlScheme,
+ }) async {
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
+ }Additionally, mirror this pattern for call(...) to forward its named args (example outside changed range):
@override
Future<Response> call(
HttpMethod? method, {
String path = '',
Map<String, String> headers = const {},
Map<String, dynamic> params = const {},
ResponseType? responseType,
}) async {
return super.noSuchMethod(
Invocation.method(#call, [method], {
#path: path,
#headers: headers,
#params: params,
#responseType: responseType,
}),
returnValue: Response(),
);
}🤖 Prompt for AI Agents
In test/services/locale_test.dart around lines 27 to 33, the mocked webAuth stub
calls super.noSuchMethod with only positional args and drops the named parameter
callbackUrlScheme; update the Invocation to include namedArgs mapping
(#callbackUrlScheme: callbackUrlScheme) so matchers receive the named argument.
Also apply the same forwarding pattern to the call(...) mock: invoke
super.noSuchMethod with Invocation.method including positional args and a
namedArgs map for path, headers, params, and responseType, and supply an
appropriate returnValue.
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); | ||
| return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forward named args in chunkedUpload’s noSuchMethod.
Currently passes them positionally; this can break stubbing and verifications.
Apply this diff:
- return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return super.noSuchMethod(Invocation.method(#chunkedUpload, [path, params, paramName, idParamName, headers]), returnValue: Response(data: {})); | |
| return super.noSuchMethod( | |
| Invocation.method(#chunkedUpload, const [], { | |
| #path: path, | |
| #params: params, | |
| #paramName: paramName, | |
| #idParamName: idParamName, | |
| #headers: headers, | |
| #onProgress: onProgress, | |
| }), | |
| returnValue: Response(data: {}), | |
| ); |
🤖 Prompt for AI Agents
In test/services/locale_test.dart around line 44, the noSuchMethod stub for
chunkedUpload is forwarding all arguments positionally which loses named
argument semantics; update the Invocation.method call to pass only true
positional args in the positional list and supply a namedArguments map using
Symbol keys for the named parameters (e.g. #paramName, #idParamName) mapping to
the local variables, keeping the same returnValue.
| when(client.call( | ||
| HttpMethod.get, | ||
| )).thenAnswer((_) async => Response(data: data)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mockito stubs should match named arguments.
These when(client.call(HttpMethod.get,)) stubs won’t match real calls that pass named args (path/headers/params/responseType). Use anyNamed matchers.
Apply this pattern (example for the first occurrence):
- when(client.call(
- HttpMethod.get,
- )).thenAnswer((_) async => Response(data: data));
+ when(client.call(
+ HttpMethod.get,
+ path: anyNamed('path'),
+ headers: anyNamed('headers'),
+ params: anyNamed('params'),
+ responseType: anyNamed('responseType'),
+ )).thenAnswer((_) async => Response(data: data));Repeat the same matcher shape for the other when(client.call(...)) blocks in this file so they match actual invocations.
Also applies to: 86-88, 103-105, 120-122, 137-139, 154-156, 171-173, 188-190
🤖 Prompt for AI Agents
In test/services/locale_test.dart around lines 69 to 71, the Mockito stub uses
when(client.call(HttpMethod.get,)) which will not match calls that supply named
arguments; update the stub to match named args by using anyNamed matchers for
path, headers, params, responseType (e.g., when(client.call(HttpMethod.get,
path: anyNamed('path'), headers: anyNamed('headers'), params:
anyNamed('params'), responseType: anyNamed('responseType'))).thenAnswer(...)).
Apply the same pattern for the other when(client.call(...)) blocks at lines
86-88, 103-105, 120-122, 137-139, 154-156, 171-173, and 188-190 so each stub
uses anyNamed for every named parameter so they correctly match real
invocations.
| @override | ||
| Future webAuth( | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signature mismatch: webAuth must accept non‑nullable Uri.
Align with Client.webAuth(Uri url, {String? callbackUrlScheme}) and forward the named arg.
- @override
- Future webAuth(
- Uri? url,
- {
- String? callbackUrlScheme,
- }
- ) async {
- return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
- }
+ @override
+ Future webAuth(
+ Uri url, {
+ String? callbackUrlScheme,
+ }) async {
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| Uri? url, | |
| { | |
| String? callbackUrlScheme, | |
| } | |
| ) async { | |
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), | |
| returnValue: 'done', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In test/services/teams_test.dart around lines 25 to 33, the mock override
defines webAuth with a nullable Uri and fails to forward the named
callbackUrlScheme; change the signature to webAuth(Uri url, {String?
callbackUrlScheme}) (Uri non‑nullable) and call super.noSuchMethod with the
named argument forwarded (Invocation.method(#webAuth, [url],
{#callbackUrlScheme: callbackUrlScheme})) and the appropriate returnValue so the
mock properly matches Client.webAuth.
| uses: EndBug/add-and-commit@v9.1.4 | ||
| with: | ||
| add: '["lib", "test"]' | ||
| add: lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excepted? to only format lib and not tests?
This PR contains updates to the Flutter SDK for version 19.1.0.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
Tests