Skip to content

Commit

Permalink
Refactor domain handling to use typed responses and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
tsutsu3 committed Dec 7, 2024
1 parent d8d9fac commit 3319e21
Show file tree
Hide file tree
Showing 8 changed files with 344 additions and 101 deletions.
7 changes: 7 additions & 0 deletions lib/functions/convert.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'package:pi_hole_client/models/domain.dart';

List<Domain> parseDomainList(dynamic jsonList) {
return (jsonList as List<dynamic>)
.map((item) => Domain.fromJson(item as Map<String, dynamic>))
.toList();
}
22 changes: 19 additions & 3 deletions lib/gateways/api_gateway_interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,27 @@ abstract interface class ApiGateway {
String domain, String list);

/// Fetches domain lists (whitelist, blacklist, and regex-based lists) from a Pi-hole server.
Future getDomainLists();
///
/// ### Returns
/// - A [GetDomainLists] object containing the result of the get domain lists query.
Future<GetDomainLists> getDomainLists();

/// Removes a domain from a specific list on a Pi-hole server.
Future removeDomainFromList(Domain domain);
///
/// ### Parameters
/// - [domain]: The domain to remove from the list.
///
/// ### Returns
/// - A [RemoveDomainFromListResponse] object containing the result of the remove domain from list query.
Future<RemoveDomainFromListResponse> removeDomainFromList(Domain domain);

/// Adds a domain to a specified list on a Pi-hole server.
Future addDomainToList(Map<String, dynamic> domainData);
///
/// ### Parameters
/// - [domainData]: The domain data to add to the list.
///
/// ### Returns
/// - An [AddDomainToListResponse] object containing the result of the add domain to list query.
Future<AddDomainToListResponse> addDomainToList(
Map<String, dynamic> domainData);
}
126 changes: 44 additions & 82 deletions lib/gateways/v5/api_gateway_v5.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:convert';
import 'dart:io';
import 'package:http/http.dart';
import 'package:http/http.dart' as http;
import 'package:pi_hole_client/functions/convert.dart';
import 'package:pi_hole_client/models/app_log.dart';
import 'package:pi_hole_client/models/domain.dart';
import 'package:pi_hole_client/functions/encode_basic_auth.dart';
Expand Down Expand Up @@ -450,27 +451,8 @@ class ApiGatewayV5 implements ApiGateway {
///
/// This method retrieves the whitelist, regex whitelist, blacklist, and regex blacklist
/// from the specified Pi-hole server. Each list is processed and returned in a structured format.
///
/// ### Parameters:
/// - `server` (`Server`): The server object containing the Pi-hole address, token, and optional basic authentication credentials.
///
/// ### Returns:
/// - `Map<String, dynamic>`: A result object with the following keys
/// - `result`: A string indicating the outcome of the operation (`success`, `no_connection`, `ssl_error`, `auth_error`, `error`).
/// - `data`: A map containing the following
/// - `whitelist`: A list of `Domain` objects representing the whitelist.
/// - `whitelistRegex`: A list of `Domain` objects representing the regex whitelist.
/// - `blacklist`: A list of `Domain` objects representing the blacklist.
/// - `blacklistRegex`: A list of `Domain` objects representing the regex blacklist.
///
/// ### Exceptions:
/// - `SocketException`: Network issues prevent connection to the server.
/// - `TimeoutException`: The request times out.
/// - `HandshakeException`: SSL/TLS handshake fails.
/// - `FormatException`: Malformed response body or unexpected data format.
/// - General exceptions: Any other errors encountered during execution.
@override
Future getDomainLists() async {
Future<GetDomainLists> getDomainLists() async {
Map<String, String>? headers;
if (checkBasicAuth(server.basicAuthUser, server.basicAuthPassword) ==
true) {
Expand Down Expand Up @@ -508,32 +490,30 @@ class ApiGatewayV5 implements ApiGateway {
results[1].statusCode == 200 &&
results[2].statusCode == 200 &&
results[3].statusCode == 200) {
return {
'result': 'success',
'data': {
'whitelist': jsonDecode(results[0].body)['data']
.map((item) => Domain.fromJson(item)),
'whitelistRegex': jsonDecode(results[1].body)['data']
.map((item) => Domain.fromJson(item)),
'blacklist': jsonDecode(results[2].body)['data']
.map((item) => Domain.fromJson(item)),
'blacklistRegex': jsonDecode(results[3].body)['data']
.map((item) => Domain.fromJson(item)),
}
};
return GetDomainLists(
result: APiResponseType.success,
data: DomainListResult(
whitelist: parseDomainList(jsonDecode(results[0].body)['data']),
whitelistRegex:
parseDomainList(jsonDecode(results[1].body)['data']),
blacklist: parseDomainList(jsonDecode(results[2].body)['data']),
blacklistRegex:
parseDomainList(jsonDecode(results[3].body)['data']),
),
);
} else {
return {'result': 'error'};
return GetDomainLists(result: APiResponseType.error);

Check warning on line 505 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L505

Added line #L505 was not covered by tests
}
} on SocketException {
return {'result': 'no_connection'};
return GetDomainLists(result: APiResponseType.socket);

Check warning on line 508 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L508

Added line #L508 was not covered by tests
} on TimeoutException {
return {'result': 'no_connection'};
return GetDomainLists(result: APiResponseType.timeout);

Check warning on line 510 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L510

Added line #L510 was not covered by tests
} on HandshakeException {
return {'result': 'ssl_error'};
return GetDomainLists(result: APiResponseType.sslError);

Check warning on line 512 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L512

Added line #L512 was not covered by tests
} on FormatException {
return {'result': 'auth_error'};
return GetDomainLists(result: APiResponseType.authError);

Check warning on line 514 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L514

Added line #L514 was not covered by tests
} catch (e) {
return {'result': 'error'};
return GetDomainLists(result: APiResponseType.error);
}
}

Expand All @@ -543,23 +523,9 @@ class ApiGatewayV5 implements ApiGateway {
/// from the given list, which can be one of the following: whitelist, blacklist,
/// regex whitelist, or regex blacklist. The operation's success or failure is determined
/// by the server's response.
///
/// ### Parameters:
/// - `server` (`Server`): The server object containing the Pi-hole address, token, and optional basic authentication credentials.
/// - `domain` (`Domain`): The domain object to remove from the list.
///
/// ### Returns:
/// - `Map<String, dynamic>`: A result object with the following keys
/// - `result`: A string indicating the outcome of the operation (`success`, `not_exists`, `socket`, `timeout`, `ssl_error`, `error`).
/// - `message`: A string indicating the reason for the operation's outcome.
///
/// ### Exceptions:
/// - `SocketException`: Network issues prevent connection to the server.
/// - `TimeoutException`: The request times out.
/// - `HandshakeException`: SSL/TLS handshake fails.
/// - General exceptions: Any other errors encountered during execution.
@override
Future removeDomainFromList(Domain domain) async {
Future<RemoveDomainFromListResponse> removeDomainFromList(
Domain domain) async {
String getType(int type) {
switch (type) {
case 0:
Expand Down Expand Up @@ -591,25 +557,27 @@ class ApiGatewayV5 implements ApiGateway {
if (response.statusCode == 200) {
final json = jsonDecode(response.body);
if (json.runtimeType == List<dynamic>) {
return {'result': 'error', 'message': 'not_exists'};
return RemoveDomainFromListResponse(

Check warning on line 560 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L560

Added line #L560 was not covered by tests
result: APiResponseType.error, message: 'not_exists');
} else {
if (json['success'] == true) {
return {'result': 'success'};
return RemoveDomainFromListResponse(
result: APiResponseType.success);
} else {
return {'result': 'error'};
return RemoveDomainFromListResponse(result: APiResponseType.error);

Check warning on line 567 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L567

Added line #L567 was not covered by tests
}
}
} else {
return {'result': 'error'};
return RemoveDomainFromListResponse(result: APiResponseType.error);

Check warning on line 571 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L571

Added line #L571 was not covered by tests
}
} on SocketException {
return {'result': 'socket'};
return RemoveDomainFromListResponse(result: APiResponseType.noConnection);

Check warning on line 574 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L574

Added line #L574 was not covered by tests
} on TimeoutException {
return {'result': 'timeout'};
return RemoveDomainFromListResponse(result: APiResponseType.noConnection);

Check warning on line 576 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L576

Added line #L576 was not covered by tests
} on HandshakeException {
return {'result': 'ssl_error'};
return RemoveDomainFromListResponse(result: APiResponseType.sslError);

Check warning on line 578 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L578

Added line #L578 was not covered by tests
} catch (e) {
return {'result': 'error'};
return RemoveDomainFromListResponse(result: APiResponseType.error);
}
}

Expand All @@ -619,16 +587,9 @@ class ApiGatewayV5 implements ApiGateway {
/// such as the whitelist, blacklist, regex whitelist, or regex blacklist. It checks the server's
/// response to determine whether the operation was successful or if the domain already exists
/// in the list.
///
/// ### Parameters:
/// - `server` (`Server`): The server object containing the Pi-hole address, token, and optional basic authentication credentials.
/// - `domainData` (`Map<String, dynamic>`): A map containing the following keys
///
/// ### Returns:
/// - `Map<String, dynamic>`: A result object with the following keys
/// - `result`: A string indicating the outcome of the operation (`success`, `already_added`, `no_connection`, `ssl_error`, `auth_error`, `error`).
@override
Future addDomainToList(Map<String, dynamic> domainData) async {
Future<AddDomainToListResponse> addDomainToList(
Map<String, dynamic> domainData) async {
try {
final response = await httpClient(
method: 'get',
Expand All @@ -641,32 +602,33 @@ class ApiGatewayV5 implements ApiGateway {
if (response.statusCode == 200) {
final json = jsonDecode(response.body);
if (json.runtimeType == List<dynamic>) {
return {'result': 'error'};
return AddDomainToListResponse(result: APiResponseType.error);

Check warning on line 605 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L605

Added line #L605 was not covered by tests
} else {
if (json['success'] == true &&
json['message'] == 'Added ${domainData['domain']}') {
return {'result': 'success'};
return AddDomainToListResponse(result: APiResponseType.success);
} else if (json['success'] == true &&
json['message'] ==
'Not adding ${domainData['domain']} as it is already on the list') {
return {'result': 'already_added'};
return AddDomainToListResponse(
result: APiResponseType.alreadyAdded);
} else {
return {'result': 'error'};
return AddDomainToListResponse(result: APiResponseType.error);

Check warning on line 616 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L616

Added line #L616 was not covered by tests
}
}
} else {
return {'result': 'error'};
return AddDomainToListResponse(result: APiResponseType.error);

Check warning on line 620 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L620

Added line #L620 was not covered by tests
}
} on SocketException {
return {'result': 'no_connection'};
return AddDomainToListResponse(result: APiResponseType.noConnection);

Check warning on line 623 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L623

Added line #L623 was not covered by tests
} on TimeoutException {
return {'result': 'no_connection'};
return AddDomainToListResponse(result: APiResponseType.noConnection);

Check warning on line 625 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L625

Added line #L625 was not covered by tests
} on HandshakeException {
return {'result': 'ssl_error'};
return AddDomainToListResponse(result: APiResponseType.sslError);

Check warning on line 627 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L627

Added line #L627 was not covered by tests
} on FormatException {
return {'result': 'auth_error'};
return AddDomainToListResponse(result: APiResponseType.authError);

Check warning on line 629 in lib/gateways/v5/api_gateway_v5.dart

View check run for this annotation

Codecov / codecov/patch

lib/gateways/v5/api_gateway_v5.dart#L629

Added line #L629 was not covered by tests
} catch (e) {
return {'result': 'error'};
return AddDomainToListResponse(result: APiResponseType.error);
}
}
}
46 changes: 45 additions & 1 deletion lib/models/gateways.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:pi_hole_client/models/app_log.dart';
import 'package:pi_hole_client/models/domain.dart';
import 'package:pi_hole_client/models/log.dart';
import 'package:pi_hole_client/models/overtime_data.dart';
import 'package:pi_hole_client/models/realtime_status.dart';
Expand All @@ -10,7 +11,8 @@ enum APiResponseType {
socket,
timeout,
sslError,
error
error,
alreadyAdded
}

/// A response object for the login query.
Expand Down Expand Up @@ -116,3 +118,45 @@ class SetWhiteBlacklistResponse {
this.data,
});
}

class DomainListResult {
final List<Domain> whitelist;
final List<Domain> whitelistRegex;
final List<Domain> blacklist;
final List<Domain> blacklistRegex;

DomainListResult({
required this.whitelist,
required this.whitelistRegex,
required this.blacklist,
required this.blacklistRegex,
});
}

class GetDomainLists {
final APiResponseType result;
final DomainListResult? data;

GetDomainLists({
required this.result,
this.data,
});
}

class RemoveDomainFromListResponse {
final APiResponseType result;
final String? message;

RemoveDomainFromListResponse({
required this.result,
this.message,
});
}

class AddDomainToListResponse {
final APiResponseType result;

AddDomainToListResponse({
required this.result,
});
}
11 changes: 6 additions & 5 deletions lib/providers/domains_list_provider.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:pi_hole_client/constants/enums.dart';
import 'package:flutter/material.dart';
import 'package:pi_hole_client/models/gateways.dart';

import 'package:pi_hole_client/models/server.dart';
import 'package:pi_hole_client/models/domain.dart';
Expand Down Expand Up @@ -101,18 +102,18 @@ class DomainsListProvider with ChangeNotifier {
Future fetchDomainsList(Server server) async {
final apiGateway = serversProvider?.selectedApiGateway;
final result = await apiGateway?.getDomainLists();
if (result['result'] == 'success') {
if (result?.result == APiResponseType.success) {
final List<Domain> whitelist = [
...result['data']['whitelist'],
...result['data']['whitelistRegex']
...result!.data!.whitelist,
...result.data!.whitelistRegex
];
_whitelistDomains = whitelist;
_filteredWhitelistDomains =
whitelist.where((i) => i.domain.contains(_searchTerm)).toList();

final List<Domain> blacklist = [
...result['data']['blacklist'],
...result['data']['blacklistRegex']
...result.data!.blacklist,
...result.data!.blacklistRegex
];
_blacklistDomains = blacklist;
_filteredBlacklistDomains =
Expand Down
9 changes: 5 additions & 4 deletions lib/screens/domains/domains.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// ignore_for_file: use_build_context_synchronously

import 'package:flutter/material.dart';
import 'package:pi_hole_client/models/gateways.dart';
import 'package:provider/provider.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';

Expand Down Expand Up @@ -80,16 +81,16 @@ class _DomainListsWidgetState extends State<DomainListsWidget>

process.close();

if (result['result'] == 'success') {
if (result?.result == APiResponseType.success) {
domainsListProvider.removeDomainFromList(domain);

showSnackBar(
appConfigProvider: appConfigProvider,
label: AppLocalizations.of(context)!.domainRemoved,
color: Colors.green);
} else if (result['result'] == 'error' &&
result['message'] != null &&
result['message'] == 'not_exists') {
} else if (result!.result == APiResponseType.error &&
result.message != null &&
result.message == 'not_exists') {
showSnackBar(
appConfigProvider: appConfigProvider,
label: AppLocalizations.of(context)!.domainNotExists,
Expand Down
Loading

0 comments on commit 3319e21

Please sign in to comment.