Make api_request's HTTP client injectable + add request-shape tests

lib/utils/api_request.dart

@@ -3,10 +3,26 @@ import 'dart:io';
 
 import 'package:app/exceptions/exceptions.dart';
 import 'package:app/utils/preferences.dart' as preferences;
+import 'package:flutter/foundation.dart';
 import 'package:http/http.dart' as Http;
 
 enum HttpMethod { get, post, patch, put, delete }
 
+/// The HTTP client used by [request] and the per-method helpers below.
+/// Production code uses a single shared [Http.Client]. Tests can swap
+/// in their own client (typically a `MockClient` from
+/// `package:http/testing.dart`) via [setHttpClientForTesting].
+Http.Client _client = Http.Client();
+
+/// Replace the HTTP client used by [request]. Test-only.
+@visibleForTesting
+void setHttpClientForTesting(Http.Client client) => _client = client;
+
+/// Reset the HTTP client to a fresh default. Test-only — call from
+/// `tearDown` so the override doesn't leak across tests.
+@visibleForTesting
+void resetHttpClientForTesting() => _client = Http.Client();
+
 Future<dynamic> request(
   HttpMethod method,
   String path, {
@@ -26,31 +42,31 @@ Future<dynamic> request(
 
   switch (method) {
     case HttpMethod.get:
-      response = await Http.get(uri, headers: headers);
+      response = await _client.get(uri, headers: headers);
       break;
     case HttpMethod.post:
-      response = await Http.post(
+      response = await _client.post(
         uri,
         headers: headers,
         body: json.encode(data),
       );
       break;
     case HttpMethod.patch:
-      response = await Http.patch(
+      response = await _client.patch(
         uri,
         headers: headers,
         body: json.encode(data),
       );
       break;
     case HttpMethod.put:
-      response = await Http.put(
+      response = await _client.put(
         uri,
         headers: headers,
         body: json.encode(data),
       );
       break;
     case HttpMethod.delete:
-      response = await Http.delete(
+      response = await _client.delete(
         uri,
         headers: headers,
         body: json.encode(data),

test/helpers/api_test_setup.dart

@@ -0,0 +1,108 @@
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:app/utils/api_request.dart' as api;
+import 'package:app/utils/preferences.dart' as preferences;
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:get_storage/get_storage.dart';
+import 'package:http/http.dart' as Http;
+import 'package:http/testing.dart';
+
+/// One-time bring-up for any test that touches `api_request.dart`:
+/// initialises `GetStorage` (which is needed by `preferences`) and
+/// stubs the `path_provider` channel so it works in widget tests.
+///
+/// Each test isolate gets its own temp directory so the
+/// `./Preferences.gs` file lock doesn't collide when several provider
+/// test files run in parallel.
+///
+/// Call from `setUpAll`.
+Future<void> initApiTestEnvironment() async {
+  TestWidgetsFlutterBinding.ensureInitialized();
+  final tmp = await Directory.systemTemp.createTemp('koel_test_');
+  // GetStorage.init() pulls the docs dir via path_provider, which has
+  // no plugin implementation in widget tests. Stub the channel so it
+  // falls back to a writable location.
+  const channel = MethodChannel('plugins.flutter.io/path_provider');
+  TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
+      .setMockMethodCallHandler(channel, (call) async => tmp.path);
+  await GetStorage.init('Preferences');
+}
+
+/// Sets stable values for the preferences the api_request layer reads.
+/// Pair with [tearDownApiTest] to clear them again.
+void setUpApiTest() {
+  preferences.host = 'https://koel.test';
+  preferences.apiToken = 'tok';
+}
+
+/// Resets the http client and clears preferences. Call from `tearDown`.
+void tearDownApiTest() {
+  preferences.host = null;
+  preferences.apiToken = null;
+  api.resetHttpClientForTesting();
+}
+
+/// Records every outgoing request and replies with a configurable
+/// status / body. Each captured entry exposes the URL, method and
+/// decoded JSON body for assertions.
+class CapturingClient {
+  final List<CapturedRequest> requests = [];
+  int _status = 200;
+  String _body = '{}';
+
+  late final MockClient client = MockClient((request) async {
+    final raw = request.body;
+    requests.add(CapturedRequest(
+      method: request.method,
+      url: request.url.toString(),
+      // Lowercase keys: http's internal request.headers is
+      // case-insensitive, but a plain Map<String, String> copy isn't
+      // — normalise so test lookups don't depend on the package's
+      // storage casing.
+      headers: {
+        for (final entry in request.headers.entries)
+          entry.key.toLowerCase(): entry.value,
+      },
+      rawBody: raw,
+    ));
+    return Http.Response(_body, _status);
+  });
+
+  /// Sets the response the next (and subsequent) requests will see.
+  void willReturn({int status = 200, Object? json}) {
+    _status = status;
+    _body = json == null ? '' : jsonEncode(json);
+  }
+
+  /// Sets a status with a literal text body (use for non-JSON / 4xx /
+  /// 5xx fixtures).
+  void willReturnRaw({int status = 200, String body = ''}) {
+    _status = status;
+    _body = body;
+  }
+
+  /// Installs this client as the api_request module's active client.
+  void install() => api.setHttpClientForTesting(client);
+}
+
+class CapturedRequest {
+  final String method;
+  final String url;
+  final Map<String, String> headers;
+  final String rawBody;
+
+  CapturedRequest({
+    required this.method,
+    required this.url,
+    required this.headers,
+    required this.rawBody,
+  });
+
+  Map<String, dynamic>? get jsonBody {
+    if (rawBody.isEmpty) return null;
+    final decoded = jsonDecode(rawBody);
+    return decoded is Map<String, dynamic> ? decoded : null;
+  }
+}

test/providers/album_provider_test.dart

@@ -0,0 +1,103 @@
+import 'package:app/exceptions/exceptions.dart';
+import 'package:app/models/album.dart';
+import 'package:app/providers/album_provider.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import '../helpers/api_test_setup.dart';
+
+void main() {
+  setUpAll(initApiTestEnvironment);
+  setUp(setUpApiTest);
+  tearDown(tearDownApiTest);
+
+  group('AlbumProvider.toggleFavorite', () {
+    test('flips optimistically, posts the right body, persists on 200',
+        () async {
+      final http = CapturingClient()..install();
+
+      final album = Album.fake(id: 7, favorite: false);
+      final provider = AlbumProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await provider.toggleFavorite(album);
+
+      // Single request, expected URL/method/body.
+      expect(http.requests, hasLength(1));
+      final req = http.requests.single;
+      expect(req.method, 'POST');
+      expect(req.url, 'https://koel.test/api/favorites/toggle');
+      expect(req.jsonBody, {'type': 'album', 'id': 7});
+
+      // Optimistic flip is preserved on success.
+      expect(album.favorite, isTrue);
+      // One notify before the await, none after on success.
+      expect(notifyCount, 1);
+    });
+
+    test('rolls back, notifies again, and rethrows on failure', () async {
+      CapturingClient()..willReturn(status: 500)..install();
+
+      final album = Album.fake(id: 8, favorite: true);
+      final provider = AlbumProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await expectLater(
+        provider.toggleFavorite(album),
+        throwsA(isA<HttpResponseException>()),
+      );
+
+      // The optimistic flip was reverted.
+      expect(album.favorite, isTrue);
+      // Two notifies: one after the optimistic flip, one after the
+      // rollback.
+      expect(notifyCount, 2);
+    });
+  });
+
+  group('AlbumProvider.update', () {
+    test('PUTs the new fields and merges the server response', () async {
+      final http = CapturingClient()
+        ..willReturn(json: {'name': 'New Name', 'year': '1969'})
+        ..install();
+
+      final album = Album.fake(id: 4)
+        ..name = 'Old Name'
+        ..year = 1965;
+      final provider = AlbumProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await provider.update(album, name: 'New Name', year: 1969);
+
+      final req = http.requests.single;
+      expect(req.method, 'PUT');
+      expect(req.url, 'https://koel.test/api/albums/4');
+      expect(req.jsonBody, {'name': 'New Name', 'year': 1969});
+
+      // Server response is the source of truth for the merged values.
+      expect(album.name, 'New Name');
+      expect(album.year, 1969);
+      expect(notifyCount, 1);
+    });
+
+    test('treats a null year in the response as null on the model',
+        () async {
+      CapturingClient()
+        ..willReturn(json: {'name': 'No Year', 'year': null})
+        ..install();
+
+      final album = Album.fake(id: 5)
+        ..name = 'X'
+        ..year = 2000;
+
+      await AlbumProvider().update(album, name: 'No Year', year: null);
+
+      expect(album.year, isNull);
+    });
+  });
+}

test/providers/artist_provider_test.dart

@@ -0,0 +1,78 @@
+import 'package:app/exceptions/exceptions.dart';
+import 'package:app/models/artist.dart';
+import 'package:app/providers/artist_provider.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import '../helpers/api_test_setup.dart';
+
+void main() {
+  setUpAll(initApiTestEnvironment);
+  setUp(setUpApiTest);
+  tearDown(tearDownApiTest);
+
+  group('ArtistProvider.toggleFavorite', () {
+    test('flips optimistically, posts the right body, persists on 200',
+        () async {
+      final http = CapturingClient()..install();
+
+      final artist = Artist.fake(id: 9, favorite: false);
+      final provider = ArtistProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await provider.toggleFavorite(artist);
+
+      final req = http.requests.single;
+      expect(req.method, 'POST');
+      expect(req.url, 'https://koel.test/api/favorites/toggle');
+      expect(req.jsonBody, {'type': 'artist', 'id': 9});
+
+      expect(artist.favorite, isTrue);
+      expect(notifyCount, 1);
+    });
+
+    test('rolls back, notifies again, and rethrows on failure', () async {
+      final http = CapturingClient()..willReturn(status: 500)..install();
+
+      final artist = Artist.fake(id: 10, favorite: true);
+      final provider = ArtistProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await expectLater(
+        provider.toggleFavorite(artist),
+        throwsA(isA<HttpResponseException>()),
+      );
+
+      expect(artist.favorite, isTrue);
+      expect(http.requests, hasLength(1));
+      expect(notifyCount, 2);
+    });
+  });
+
+  group('ArtistProvider.update', () {
+    test('PUTs the new name and merges the server response', () async {
+      final http = CapturingClient()
+        ..willReturn(json: {'name': 'Renamed'})
+        ..install();
+
+      final artist = Artist.fake(id: 11)..name = 'Old';
+      final provider = ArtistProvider();
+
+      var notifyCount = 0;
+      provider.addListener(() => notifyCount++);
+
+      await provider.update(artist, name: 'Renamed');
+
+      final req = http.requests.single;
+      expect(req.method, 'PUT');
+      expect(req.url, 'https://koel.test/api/artists/11');
+      expect(req.jsonBody, {'name': 'Renamed'});
+
+      expect(artist.name, 'Renamed');
+      expect(notifyCount, 1);
+    });
+  });
+}