From 408c40b3aff122ac4299e9524ae59b907e0ae781 Mon Sep 17 00:00:00 2001 From: Vishesh Handa Date: Sat, 12 Jun 2021 14:27:33 +0200 Subject: [PATCH] GitHostApis: Use Result class Instead of throwing exceptions. There are so so many edge cases which are failing. --- lib/apis/api_fakes.dart | 32 +++++++++----- lib/apis/githost.dart | 15 ++++--- lib/apis/github.dart | 80 +++++++++++++++++++++------------- lib/apis/gitlab.dart | 81 +++++++++++++++++++++-------------- lib/experiments/oauthapp.dart | 2 +- lib/setup/autoconfigure.dart | 6 +-- lib/setup/repo_selector.dart | 5 ++- 7 files changed, 137 insertions(+), 84 deletions(-) diff --git a/lib/apis/api_fakes.dart b/lib/apis/api_fakes.dart index 422d79bd..b0d8508f 100644 --- a/lib/apis/api_fakes.dart +++ b/lib/apis/api_fakes.dart @@ -16,22 +16,32 @@ class GitHubFake implements GitHost { Future launchOAuthScreen() async {} @override - Future getUserInfo() async {} - @override - Future addDeployKey(String sshPublicKey, String repoFullName) async {} - - @override - Future createRepo(String name) async { - return GitHub.repoFromJson({}); + Future> getUserInfo() async { + var ex = Exception("Not Implemented"); + return Result.fail(ex); } @override - Future getRepo(String name) async { - return GitHub.repoFromJson({}); + Future> addDeployKey( + String sshPublicKey, String repoFullName) async { + var ex = Exception("Not Implemented"); + return Result.fail(ex); } @override - Future> listRepos() async { + Future> createRepo(String name) async { + var ex = Exception("Not Implemented"); + return Result.fail(ex); + } + + @override + Future> getRepo(String name) async { + var ex = Exception("Not Implemented"); + return Result.fail(ex); + } + + @override + Future>> listRepos() async { List list = jsonDecode(data); var repos = []; list.forEach((dynamic d) { @@ -40,6 +50,6 @@ class GitHubFake implements GitHost { repos.add(repo); }); - return repos; + return Result(repos); } } diff --git a/lib/apis/githost.dart b/lib/apis/githost.dart index ff788526..e352b2f8 100644 --- a/lib/apis/githost.dart +++ b/lib/apis/githost.dart @@ -1,6 +1,9 @@ import 'dart:async'; import 'package:collection/collection.dart'; +import 'package:dart_git/utils/result.dart'; + +export 'package:dart_git/utils/result.dart'; typedef OAuthCallback = void Function(GitHostException?); @@ -8,11 +11,11 @@ abstract class GitHost { void init(OAuthCallback oAuthCallback); Future launchOAuthScreen(); - Future getUserInfo(); - Future> listRepos(); - Future createRepo(String name); - Future getRepo(String name); - Future addDeployKey(String sshPublicKey, String repoFullName); + Future> getUserInfo(); + Future>> listRepos(); + Future> createRepo(String name); + Future> getRepo(String name); + Future> addDeployKey(String sshPublicKey, String repoFullName); } class UserInfo { @@ -99,6 +102,8 @@ class GitHostException implements Exception { static const CreateRepoFailed = GitHostException("CreateRepoFailed"); static const DeployKeyFailed = GitHostException("DeployKeyFailed"); static const GetRepoFailed = GitHostException("GetRepoFailed"); + static const HttpResponseFail = GitHostException("HttpResponseFail"); + static const JsonDecodingFail = GitHostException("JsonDecodingFail"); final String cause; const GitHostException(this.cause); diff --git a/lib/apis/github.dart b/lib/apis/github.dart index 98221dbf..1797f507 100644 --- a/lib/apis/github.dart +++ b/lib/apis/github.dart @@ -11,6 +11,8 @@ import 'package:url_launcher/url_launcher.dart'; import 'package:gitjournal/utils/logger.dart'; import 'githost.dart'; +// FIXME: Handle for edge cases of json.decode + class GitHub implements GitHost { static const _clientID = "aa3072cbfb02b1db14ed"; static const _clientSecret = "010d303ea99f82330f2b228977cef9ddbf7af2cd"; @@ -79,9 +81,10 @@ class GitHub implements GitHost { } @override - Future> listRepos() async { + Future>> listRepos() async { if (_accessCode.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = @@ -96,11 +99,12 @@ class GitHub implements GitHost { var response = await http.get(url, headers: headers); if (response.statusCode != 200) { - Log.d("Github listRepos: Invalid response " + + Log.e("Github listRepos: Invalid response " + response.statusCode.toString() + ": " + response.body); - return []; + var ex = GitHostException.HttpResponseFail; + return Result.fail(ex); } List list = jsonDecode(response.body); @@ -112,13 +116,14 @@ class GitHub implements GitHost { }); // FIXME: Sort these based on some criteria - return repos; + return Result(repos); } @override - Future createRepo(String name) async { + Future> createRepo(String name) async { if (_accessCode.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = Uri.parse("https://api.github.com/user/repos"); @@ -135,35 +140,39 @@ class GitHub implements GitHost { var response = await http.post(url, headers: headers, body: json.encode(data)); if (response.statusCode != 201) { - Log.d("Github createRepo: Invalid response " + + Log.e("Github createRepo: Invalid response " + response.statusCode.toString() + ": " + response.body); if (response.statusCode == 422) { if (response.body.contains("name already exists")) { - throw GitHostException.RepoExists; + var ex = GitHostException.RepoExists; + return Result.fail(ex); } } - throw GitHostException.CreateRepoFailed; + var ex = GitHostException.CreateRepoFailed; + return Result.fail(ex); } Log.d("GitHub createRepo: " + response.body); Map map = json.decode(response.body); - return repoFromJson(map); + return Result(repoFromJson(map)); } @override - Future getRepo(String name) async { + Future> getRepo(String name) async { if (_accessCode.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } - var userInfo = await getUserInfo(); - if (userInfo == null) { - throw Exception("GitHub UserInfo not found. This is bad"); + var userInfoR = await getUserInfo(); + if (userInfoR.isFailure) { + return fail(userInfoR); } + var userInfo = userInfoR.getOrThrow(); var owner = userInfo.username; var url = Uri.parse("https://api.github.com/repos/$owner/$name"); @@ -173,23 +182,29 @@ class GitHub implements GitHost { var response = await http.get(url, headers: headers); if (response.statusCode != 200) { - Log.d("Github getRepo: Invalid response " + + Log.e("Github getRepo: Invalid response " + response.statusCode.toString() + ": " + response.body); - throw GitHostException.GetRepoFailed; + var ex = GitHostException.GetRepoFailed; + return Result.fail(ex); } Log.d("GitHub getRepo: " + response.body); - Map map = json.decode(response.body); - return repoFromJson(map); + try { + Map map = json.decode(response.body); + return Result(repoFromJson(map)); + } on Exception catch (ex, st) { + return Result.fail(ex, st); + } } @override - Future addDeployKey(String sshPublicKey, String repo) async { + Future> addDeployKey(String sshPublicKey, String repo) async { if (_accessCode.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = Uri.parse("https://api.github.com/repos/$repo/keys"); @@ -212,11 +227,12 @@ class GitHub implements GitHost { response.statusCode.toString() + ": " + response.body); - throw GitHostException.DeployKeyFailed; + var ex = GitHostException.DeployKeyFailed; + return Result.fail(ex); } Log.d("GitHub addDeployKey: " + response.body); - return json.decode(response.body); + return Result(null); } static GitHostRepo repoFromJson(Map parsedJson) { @@ -261,9 +277,10 @@ class GitHub implements GitHost { } @override - Future getUserInfo() async { + Future> getUserInfo() async { if (_accessCode.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = Uri.parse("https://api.github.com/user"); @@ -278,7 +295,8 @@ class GitHub implements GitHost { response.statusCode.toString() + ": " + response.body); - return null; + var ex = GitHostException.HttpResponseFail; + return Result.fail(ex); } Map? map = jsonDecode(response.body); @@ -287,14 +305,16 @@ class GitHub implements GitHost { response.statusCode.toString() + ": " + response.body); - return null; + + var ex = GitHostException.JsonDecodingFail; + return Result.fail(ex); } - return UserInfo( + return Result(UserInfo( name: map['name'], email: map['email'], username: map['login'], - ); + )); } String _buildAuthHeader() { diff --git a/lib/apis/gitlab.dart b/lib/apis/gitlab.dart index bea794ec..f4487bde 100644 --- a/lib/apis/gitlab.dart +++ b/lib/apis/gitlab.dart @@ -12,6 +12,8 @@ import 'package:url_launcher/url_launcher.dart'; import 'package:gitjournal/utils/logger.dart'; import 'githost.dart'; +// FIXME: Handle for edge cases of json.decode + class GitLab implements GitHost { static const _clientID = "faf33c3716faf05bfb701b1b31e36c83a23c3ec2d7161f4ff00fba2275524d09"; @@ -67,9 +69,10 @@ class GitLab implements GitHost { } @override - Future> listRepos() async { + Future>> listRepos() async { if (_accessCode!.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } // FIXME: pagination! @@ -82,11 +85,12 @@ class GitLab implements GitHost { var response = await http.get(url); if (response.statusCode != 200) { - Log.d("GitLab listRepos: Invalid response " + + Log.e("GitLab listRepos: Invalid response " + response.statusCode.toString() + ": " + response.body); - return []; + var ex = GitHostException.HttpResponseFail; + return Result.fail(ex); } List list = jsonDecode(response.body); @@ -98,13 +102,14 @@ class GitLab implements GitHost { }); // FIXME: Sort these based on some criteria - return repos; + return Result(repos); } @override - Future createRepo(String name) async { + Future> createRepo(String name) async { if (_accessCode!.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = Uri.parse( @@ -121,58 +126,64 @@ class GitLab implements GitHost { var response = await http.post(url, headers: headers, body: json.encode(data)); if (response.statusCode != 201) { - Log.d("GitLab createRepo: Invalid response " + + Log.e("GitLab createRepo: Invalid response " + response.statusCode.toString() + ": " + response.body); if (response.statusCode == 400) { if (response.body.contains("has already been taken")) { - throw GitHostException.RepoExists; + var ex = GitHostException.RepoExists; + return Result.fail(ex); } } - throw GitHostException.CreateRepoFailed; + var ex = GitHostException.CreateRepoFailed; + return Result.fail(ex); } Log.d("GitLab createRepo: " + response.body); Map map = json.decode(response.body); - return repoFromJson(map); + return Result(repoFromJson(map)); } @override - Future getRepo(String name) async { + Future> getRepo(String name) async { if (_accessCode!.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } - var userInfo = await getUserInfo(); - if (userInfo == null) { - throw Exception("GitLab no userInfo found!!"); + var userInfoR = await getUserInfo(); + if (userInfoR.isFailure) { + return fail(userInfoR); } + var userInfo = userInfoR.getOrThrow(); var repo = userInfo.username + '%2F' + name; var url = Uri.parse( "https://gitlab.com/api/v4/projects/$repo?access_token=$_accessCode"); var response = await http.get(url); if (response.statusCode != 200) { - Log.d("GitLab getRepo: Invalid response " + + Log.e("GitLab getRepo: Invalid response " + response.statusCode.toString() + ": " + response.body); - throw GitHostException.GetRepoFailed; + var ex = GitHostException.GetRepoFailed; + return Result.fail(ex); } Log.d("GitLab getRepo: " + response.body); Map map = json.decode(response.body); - return repoFromJson(map); + return Result(repoFromJson(map)); } @override - Future addDeployKey(String sshPublicKey, String repo) async { + Future> addDeployKey(String sshPublicKey, String repo) async { if (_accessCode!.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } repo = repo.replaceAll('/', '%2F'); @@ -192,15 +203,16 @@ class GitLab implements GitHost { var response = await http.post(url, headers: headers, body: json.encode(data)); if (response.statusCode != 201) { - Log.d("GitLab addDeployKey: Invalid response " + + Log.e("GitLab addDeployKey: Invalid response " + response.statusCode.toString() + ": " + response.body); - throw GitHostException.DeployKeyFailed; + var ex = GitHostException.DeployKeyFailed; + return Result.fail(ex); } Log.d("GitLab addDeployKey: " + response.body); - return json.decode(response.body); + return Result(null); } static GitHostRepo repoFromJson(Map parsedJson) { @@ -245,9 +257,10 @@ class GitLab implements GitHost { } @override - Future getUserInfo() async { + Future> getUserInfo() async { if (_accessCode!.isEmpty) { - throw GitHostException.MissingAccessCode; + var ex = GitHostException.MissingAccessCode; + return Result.fail(ex); } var url = @@ -255,27 +268,31 @@ class GitLab implements GitHost { var response = await http.get(url); if (response.statusCode != 200) { - Log.d("GitLab getUserInfo: Invalid response " + + Log.e("GitLab getUserInfo: Invalid response " + response.statusCode.toString() + ": " + response.body); - return null; + + var ex = GitHostException.HttpResponseFail; + return Result.fail(ex); } Map? map = jsonDecode(response.body); if (map == null || map.isEmpty) { - Log.d("GitLab getUserInfo: jsonDecode Failed " + + Log.e("GitLab getUserInfo: jsonDecode Failed " + response.statusCode.toString() + ": " + response.body); - return null; + + var ex = GitHostException.JsonDecodingFail; + return Result.fail(ex); } - return UserInfo( + return Result(UserInfo( name: map['name'], email: map['email'], username: map['username'], - ); + )); } } diff --git a/lib/experiments/oauthapp.dart b/lib/experiments/oauthapp.dart index ac8b9cad..be8e50f2 100644 --- a/lib/experiments/oauthapp.dart +++ b/lib/experiments/oauthapp.dart @@ -48,7 +48,7 @@ class OAuthAppState extends State { child: const Text("List Repos"), onPressed: () async { try { - var repos = await githost!.listRepos(); + var repos = await githost!.listRepos().getOrThrow(); for (var repo in repos) { print(repo); } diff --git a/lib/setup/autoconfigure.dart b/lib/setup/autoconfigure.dart index e1c7b2b9..60561f78 100644 --- a/lib/setup/autoconfigure.dart +++ b/lib/setup/autoconfigure.dart @@ -64,12 +64,12 @@ class GitHostSetupAutoConfigurePageState _message = tr('setup.autoconfigure.readUser'); }); - userInfo = await gitHost!.getUserInfo(); + var userInfo = await gitHost!.getUserInfo().getOrThrow(); var settings = Provider.of(context, listen: false); - if (userInfo != null && userInfo.name.isNotEmpty) { + if (userInfo.name.isNotEmpty) { settings.gitAuthor = userInfo.name; } - if (userInfo != null && userInfo.email.isNotEmpty) { + if (userInfo.email.isNotEmpty) { settings.gitAuthorEmail = userInfo.email; } settings.save(); diff --git a/lib/setup/repo_selector.dart b/lib/setup/repo_selector.dart index 6a0ab292..d6806a7b 100644 --- a/lib/setup/repo_selector.dart +++ b/lib/setup/repo_selector.dart @@ -80,7 +80,7 @@ class GitHostSetupRepoSelectorState extends State { Log.d("Starting RepoSelector"); try { - var allRepos = await widget.gitHost.listRepos(); + var allRepos = await widget.gitHost.listRepos().getOrThrow(); allRepos.sort((GitHostRepo a, GitHostRepo b) { if (a.updatedAt != null && b.updatedAt != null) { return a.updatedAt!.compareTo(b.updatedAt!); @@ -210,7 +210,8 @@ class GitHostSetupRepoSelectorState extends State { try { var repoName = _textController.text.trim(); - var repo = await widget.gitHost.createRepo(repoName); + var repo = + await widget.gitHost.createRepo(repoName).getOrThrow(); widget.onDone(repo); return; } catch (e, stacktrace) {