From 69a3c0dfc4fbe1fb3751fad449dc015adbf8ace9 Mon Sep 17 00:00:00 2001 From: Rob Becker Date: Tue, 15 Sep 2026 22:37:53 -0600 Subject: [PATCH] fix(services): align bound service operations with OpenAPI REST contracts Co-authored-by: Cursor --- lib/src/common/activity_service.dart | 31 +- lib/src/common/authorizations_service.dart | 20 +- lib/src/common/checks_service.dart | 30 +- lib/src/common/gists_service.dart | 15 +- lib/src/common/git_service.dart | 39 ++- lib/src/common/issues_service.dart | 40 +-- lib/src/common/misc_service.dart | 12 +- lib/src/common/orgs_service.dart | 76 +++-- lib/src/common/repos_service.dart | 74 ++++- lib/src/common/search_service.dart | 38 +-- lib/src/common/util/utils.dart | 43 +-- test/contract/service_contracts_test.dart | 339 +++++++++++++++++++++ test/git_test.dart | 8 +- 13 files changed, 605 insertions(+), 160 deletions(-) create mode 100644 test/contract/service_contracts_test.dart diff --git a/lib/src/common/activity_service.dart b/lib/src/common/activity_service.dart index f97aefcf..09830823 100644 --- a/lib/src/common/activity_service.dart +++ b/lib/src/common/activity_service.dart @@ -146,7 +146,7 @@ class ActivityService extends Service { /// /// API docs: https://developer.github.com/v3/activity/notifications/#mark-as-read Future markNotificationsRead({DateTime? lastRead}) { - final data = {}; + final data = {}; if (lastRead != null) { data['last_read_at'] = lastRead.toIso8601String(); @@ -167,7 +167,7 @@ class ActivityService extends Service { RepositorySlug slug, { DateTime? lastRead, }) { - final data = {}; + final data = {}; if (lastRead != null) { data['last_read_at'] = lastRead.toIso8601String(); @@ -185,7 +185,7 @@ class ActivityService extends Service { /// /// API docs: https://developer.github.com/v3/activity/notifications/#view-a-single-thread Future getThread(String threadId) => - github.getJSON('/notification/threads/$threadId', + github.getJSON('/notifications/threads/$threadId', statusCode: StatusCodes.OK, convert: Notification.fromJson); /// Mark the specified notification thread as read. @@ -244,21 +244,17 @@ class ActivityService extends Service { /// Stars the specified repository for the currently authenticated user. /// /// API docs: https://developer.github.com/v3/activity/starring/#star-a-repository - Future star(RepositorySlug slug) { - return github.request('PUT', '/user/starred/${slug.fullName}', - headers: {'Content-Length': '0'}).then((response) { - return null; - }); + Future star(RepositorySlug slug) async { + await github.request('PUT', '/user/starred/${slug.fullName}', + statusCode: 204, headers: {'Content-Length': '0'}); } /// Unstars the specified repository for the currently authenticated user. /// /// API docs: https://developer.github.com/v3/activity/starring/#unstar-a-repository - Future unstar(RepositorySlug slug) { - return github.request('DELETE', '/user/starred/${slug.fullName}', - headers: {'Content-Length': '0'}).then((response) { - return null; - }); + Future unstar(RepositorySlug slug) async { + await github.request('DELETE', '/user/starred/${slug.fullName}', + statusCode: 204, headers: {'Content-Length': '0'}); } /// Lists the watchers of the specified repository. @@ -315,10 +311,10 @@ class ActivityService extends Service { /// Deletes a Repository Subscription /// /// API docs: https://developer.github.com/v3/activity/watching/#delete-a-repository-subscription - Future deleteRepositorySubscription(RepositorySlug slug) { + Future deleteRepositorySubscription(RepositorySlug slug) { return github.request('DELETE', '/repos/${slug.fullName}/subscription', headers: {'Content-Length': '0'}).then((response) { - return null; + return; }); } } @@ -355,7 +351,8 @@ class EventPoller { _lastFetched = response.headers['ETag']; - final json = List>.from(jsonDecode(response.body)); + final json = List>.from( + jsonDecode(response.body) as Iterable); if (!(onlyNew && _timer == null)) { for (final item in json) { @@ -399,7 +396,7 @@ class EventPoller { return _controller!.stream; } - Future stop() { + Future stop() { if (_timer == null) { throw Exception('Polling not started.'); } diff --git a/lib/src/common/authorizations_service.dart b/lib/src/common/authorizations_service.dart index 68b80c25..235d292b 100644 --- a/lib/src/common/authorizations_service.dart +++ b/lib/src/common/authorizations_service.dart @@ -2,31 +2,27 @@ import 'dart:async'; import 'package:github/src/common.dart'; -/// The [AuthorizationsService] handles communication with authorizations related methods -/// of the GitHub API. +/// The [AuthorizationsService] handles communication with legacy OAuth authorizations. /// -/// Note: You can only access this API via Basic Authentication using your -/// username and password, not tokens. +/// **Deprecated**: The OAuth Authorizations API was sunset by GitHub. +/// Use modern OAuth web application flows or GitHub Apps instead. /// -/// API docs: https://developer.github.com/v3/oauth_authorizations/ +/// API docs: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#oauth-authorizations-api +@Deprecated( + 'The OAuth Authorizations API has been sunset by GitHub. Use modern OAuth or GitHub Apps.') class AuthorizationsService extends Service { AuthorizationsService(super.github); /// Lists all authorizations. - /// - /// API docs: https://developer.github.com/v3/oauth_authorizations/#list-your-authorizations + @Deprecated('The OAuth Authorizations API has been sunset by GitHub.') Stream listAuthorizations() { return PaginationHelper(github) .objects('GET', '/authorizations', Authorization.fromJson); } /// Fetches an authorization specified by [id]. - /// - /// API docs: https://developer.github.com/v3/oauth_authorizations/#get-a-single-authorization + @Deprecated('The OAuth Authorizations API has been sunset by GitHub.') Future getAuthorization(int id) => github.getJSON('/authorizations/$id', statusCode: 200, convert: Authorization.fromJson); - - // TODO: Implement remaining API methods of authorizations: - // See https://developer.github.com/v3/oauth_authorizations/ } diff --git a/lib/src/common/checks_service.dart b/lib/src/common/checks_service.dart index 21feff81..7763ee7b 100644 --- a/lib/src/common/checks_service.dart +++ b/lib/src/common/checks_service.dart @@ -58,9 +58,18 @@ class CheckRunsService extends Service { CheckRunOutput? output, List? actions, }) async { - assert(conclusion != null || - (completedAt == null && status != CheckRunStatus.completed)); - assert(actions == null || actions.length <= 3); + if (status == CheckRunStatus.completed && conclusion == null) { + throw ArgumentError.value( + conclusion, 'conclusion', 'Required when status is completed'); + } + if (completedAt != null && conclusion == null) { + throw ArgumentError.value( + conclusion, 'conclusion', 'Required when completedAt is provided'); + } + if (actions != null && actions.length > 3) { + throw ArgumentError.value( + actions.length, 'actions', 'A maximum of 3 actions are accepted'); + } return github.postJSON, CheckRun>( '/repos/${slug.fullName}/check-runs', statusCode: StatusCodes.CREATED, @@ -109,9 +118,18 @@ class CheckRunsService extends Service { CheckRunOutput? output, List? actions, }) async { - assert(conclusion != null || - (completedAt == null && status != CheckRunStatus.completed)); - assert(actions == null || actions.length <= 3); + if (status == CheckRunStatus.completed && conclusion == null) { + throw ArgumentError.value( + conclusion, 'conclusion', 'Required when status is completed'); + } + if (completedAt != null && conclusion == null) { + throw ArgumentError.value( + conclusion, 'conclusion', 'Required when completedAt is provided'); + } + if (actions != null && actions.length > 3) { + throw ArgumentError.value( + actions.length, 'actions', 'A maximum of 3 actions are accepted'); + } return github.requestJson, CheckRun>( 'PATCH', '/repos/${slug.fullName}/check-runs/${checkRunToUpdate.id}', diff --git a/lib/src/common/gists_service.dart b/lib/src/common/gists_service.dart index 05ac62bd..5af7e4a8 100644 --- a/lib/src/common/gists_service.dart +++ b/lib/src/common/gists_service.dart @@ -56,7 +56,7 @@ class GistsService extends Service { String? description, bool public = false, }) { - final map = {'files': {}}; + final map = {'files': {}}; if (description != null) { map['description'] = description; @@ -64,7 +64,7 @@ class GistsService extends Service { map['public'] = public; - final f = {}; + final f = {}; for (final key in files.keys) { f[key] = {'content': files[key]}; @@ -111,7 +111,8 @@ class GistsService extends Service { map['files'] = f; } - return github.postJSON( + return github.requestJson, Gist>( + 'PATCH', '/gists/$id', statusCode: 200, body: GitHubJson.encode(map), @@ -125,7 +126,9 @@ class GistsService extends Service { /// /// API docs: https://developer.github.com/v3/gists/#star-a-gist Future starGist(String id) { - return github.request('POST', '/gists/$id/star').then((response) { + return github + .request('PUT', '/gists/$id/star', statusCode: 204) + .then((response) { return response.statusCode == 204; }); } @@ -176,7 +179,9 @@ class GistsService extends Service { /// API docs: https://developer.github.com/v3/gists/comments/#create-a-comment Future createComment(String gistId, CreateGistComment request) { return github.postJSON('/gists/$gistId/comments', - body: GitHubJson.encode(request), convert: GistComment.fromJson); + body: GitHubJson.encode(request), + convert: GistComment.fromJson, + statusCode: 201); } // TODO: Implement editComment: https://developer.github.com/v3/gists/comments/#edit-a-comment diff --git a/lib/src/common/git_service.dart b/lib/src/common/git_service.dart index 338dbeba..32e75f6c 100644 --- a/lib/src/common/git_service.dart +++ b/lib/src/common/git_service.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:convert'; import 'package:github/src/common.dart'; @@ -77,10 +76,11 @@ class GitService extends Service { /// API docs: https://developer.github.com/v3/git/refs/#create-a-reference Future createReference( RepositorySlug slug, String ref, String? sha) { + final formattedRef = ref.startsWith('refs/') ? ref : 'refs/$ref'; return github.postJSON('/repos/${slug.fullName}/git/refs', convert: GitReference.fromJson, statusCode: StatusCodes.CREATED, - body: GitHubJson.encode({'ref': ref, 'sha': sha})); + body: GitHubJson.encode({'ref': formattedRef, 'sha': sha})); } /// Updates a reference in a repository. @@ -92,25 +92,25 @@ class GitService extends Service { String? sha, { bool force = false, }) { + final formattedRef = ref.startsWith('refs/') ? ref.substring(5) : ref; final body = GitHubJson.encode({'sha': sha, 'force': force}); - // Somehow the reference updates PATCH request needs a valid content-length. - final headers = {'content-length': body.length.toString()}; - - return github - .request('PATCH', '/repos/${slug.fullName}/git/refs/$ref', - body: body, headers: headers) - .then((response) { - return GitReference.fromJson( - jsonDecode(response.body) as Map); - }); + return github.requestJson, GitReference>( + 'PATCH', + '/repos/${slug.fullName}/git/refs/$formattedRef', + statusCode: StatusCodes.OK, + body: body, + convert: GitReference.fromJson, + ); } /// Deletes a reference. /// /// API docs: https://developer.github.com/v3/git/refs/#delete-a-reference Future deleteReference(RepositorySlug slug, String ref) { + final formattedRef = ref.startsWith('refs/') ? ref.substring(5) : ref; return github - .request('DELETE', '/repos/${slug.fullName}/git/refs/$ref') + .request('DELETE', '/repos/${slug.fullName}/git/refs/$formattedRef', + statusCode: StatusCodes.NO_CONTENT) .then((response) => response.statusCode == StatusCodes.NO_CONTENT); } @@ -138,13 +138,12 @@ class GitService extends Service { /// and https://developer.github.com/v3/git/trees/#get-a-tree-recursively Future getTree(RepositorySlug slug, String? sha, {bool recursive = false}) { - var path = '/repos/${slug.fullName}/git/trees/$sha'; - if (recursive) { - path += '?recursive=1'; - } - - return github.getJSON(path, - convert: GitTree.fromJson, statusCode: StatusCodes.OK); + return github.getJSON( + '/repos/${slug.fullName}/git/trees/$sha', + params: recursive ? {'recursive': '1'} : const {}, + convert: GitTree.fromJson, + statusCode: StatusCodes.OK, + ); } /// Creates a new tree in a repository. diff --git a/lib/src/common/issues_service.dart b/lib/src/common/issues_service.dart index 6b6878dc..02b658a1 100644 --- a/lib/src/common/issues_service.dart +++ b/lib/src/common/issues_service.dart @@ -174,19 +174,13 @@ class IssuesService extends Service { /// Create an issue. /// /// API docs: https://developer.github.com/v3/issues/#create-an-issue - Future create(RepositorySlug slug, IssueRequest issue) async { - final response = await github.request( - 'POST', + Future create(RepositorySlug slug, IssueRequest issue) { + return github.postJSON, Issue>( '/repos/${slug.fullName}/issues', + statusCode: StatusCodes.CREATED, body: GitHubJson.encode(issue), + convert: Issue.fromJson, ); - - if (StatusCodes.isClientError(response.statusCode)) { - //TODO: throw a more friendly error – better this than silent failure - throw GitHubError(github, response.body); - } - - return Issue.fromJson(jsonDecode(response.body) as Map); } /// Lists all available assignees (owners and collaborators) to which issues @@ -201,9 +195,9 @@ class IssuesService extends Service { /// Checks if a user is an assignee for the specified repository. /// /// API docs: https://developer.github.com/v3/issues/assignees/#check-assignee - Future isAssignee(RepositorySlug slug, String repoName) { + Future isAssignee(RepositorySlug slug, String assignee) { return github - .request('GET', '/repos/${slug.fullName}/assignees/$repoName') + .request('GET', '/repos/${slug.fullName}/assignees/$assignee') .then((response) => response.statusCode == StatusCodes.NO_CONTENT); } @@ -373,12 +367,14 @@ class IssuesService extends Service { /// API docs: https://developer.github.com/v3/issues/labels/#replace-all-labels-for-an-issue Future> replaceLabelsForIssue( RepositorySlug slug, int issueNumber, List labels) { - return github - .request('PUT', '/repos/${slug.fullName}/issues/$issueNumber/labels', - body: GitHubJson.encode(labels)) - .then((response) { - return jsonDecode(response.body).map(IssueLabel.fromJson); - }); + return github.requestJson, List>( + 'PUT', + '/repos/${slug.fullName}/issues/$issueNumber/labels', + statusCode: StatusCodes.OK, + body: GitHubJson.encode(labels), + convert: (input) => + input.cast>().map(IssueLabel.fromJson).toList(), + ); } /// Removes a label for an issue. @@ -457,8 +453,12 @@ class IssuesService extends Service { } else { body = '{}'; } - await github.postJSON('/repos/${slug.fullName}/issues/$number/lock', - body: body, statusCode: 204); + await github.request( + 'PUT', + '/repos/${slug.fullName}/issues/$number/lock', + body: body, + statusCode: 204, + ); } /// Unlock an issue. diff --git a/lib/src/common/misc_service.dart b/lib/src/common/misc_service.dart index 30385a18..213dc712 100644 --- a/lib/src/common/misc_service.dart +++ b/lib/src/common/misc_service.dart @@ -14,10 +14,10 @@ class MiscService extends Service { /// /// API docs: https://developer.github.com/v3/emojis/ Future> listEmojis() { - final r = github.getJSON>( + final r = github.getJSON, Map>( '/emojis', statusCode: StatusCodes.OK, - convert: (Map json) => json.cast(), + convert: (Map json) => json.cast(), ); return r; } @@ -26,7 +26,10 @@ class MiscService extends Service { /// /// API docs: https://developer.github.com/v3/gitignore/#listing-available-templates Future> listGitignoreTemplates() { - return github.getJSON('/gitignore/templates') as Future>; + return github.getJSON, List>( + '/gitignore/templates', + convert: List.from, + ); } /// Gets a .gitignore template by [name]. @@ -63,7 +66,8 @@ class MiscService extends Service { /// API docs: https://developer.github.com/v3/rate_limit/ Future getRateLimit() { return github.request('GET', '/rate_limit').then((response) { - return RateLimit.fromRateLimitResponse(jsonDecode(response.body)); + return RateLimit.fromRateLimitResponse( + jsonDecode(response.body) as Map); }); } diff --git a/lib/src/common/orgs_service.dart b/lib/src/common/orgs_service.dart index b2ca7b26..50917a23 100644 --- a/lib/src/common/orgs_service.dart +++ b/lib/src/common/orgs_service.dart @@ -133,6 +133,8 @@ class OrganizationsService extends Service { /// Edits a Team. /// + /// Edits a team (Legacy endpoint). + /// /// API docs: https://developer.github.com/v3/orgs/teams/#edit-team Future editTeam( int teamId, @@ -146,7 +148,8 @@ class OrganizationsService extends Service { 'permission': permission, }); - return github.postJSON( + return github.requestJson, Team>( + 'PATCH', '/teams/$teamId', statusCode: StatusCodes.OK, convert: Team.fromJson, @@ -154,6 +157,35 @@ class OrganizationsService extends Service { ); } + /// Updates a team in an organization. + /// + /// API docs: https://docs.github.com/en/rest/teams/teams#update-a-team + Future editTeamInOrg( + String org, + String teamSlug, { + String? name, + String? description, + String? permission, + String? privacy, + String? notificationSetting, + }) { + final map = createNonNullMap({ + 'name': name, + 'description': description, + 'permission': permission, + 'privacy': privacy, + 'notification_setting': notificationSetting, + }); + + return github.requestJson, Team>( + 'PATCH', + '/orgs/$org/teams/$teamSlug', + statusCode: StatusCodes.OK, + convert: Team.fromJson, + body: GitHubJson.encode(map), + ); + } + /// Deletes the team specified by the [teamId] /// /// API docs: https://developer.github.com/v3/orgs/teams/#delete-team @@ -175,9 +207,10 @@ class OrganizationsService extends Service { } Future getTeamMemberStatus(int teamId, String user) { - return github.getJSON('/teams/$teamId/memberships/$user').then((json) { - return json['state']; - }); + return github.getJSON, bool>( + '/teams/$teamId/memberships/$user', + convert: (Map json) => json['state'] == 'active', + ); } /// Returns the membership status for a [user] in a team with [teamId]. @@ -190,8 +223,8 @@ class OrganizationsService extends Service { return github.getJSON( '/teams/$teamId/memberships/$user', statusCode: StatusCodes.OK, - convert: (dynamic json) => TeamMembershipState( - json['state'], + convert: (Map json) => TeamMembershipState( + json['state'] as String?, ), ); } @@ -209,8 +242,8 @@ class OrganizationsService extends Service { return github.getJSON( '/orgs/$orgName/teams/$teamName/memberships/$user', statusCode: StatusCodes.OK, - convert: (dynamic json) => TeamMembershipState( - json['state'], + convert: (Map json) => TeamMembershipState( + json['state'] as String?, ), ); } @@ -227,21 +260,24 @@ class OrganizationsService extends Service { '/teams/$teamId/memberships/$user', statusCode: StatusCodes.OK, ); - return TeamMembershipState(jsonDecode(response.body)['state']); + final map = jsonDecode(response.body) as Map; + return TeamMembershipState(map['state'] as String?); } /// Removes a user from the specified team. /// /// API docs: https://developer.github.com/v3/orgs/teams/#get-team-membership - Future removeTeamMembership( + Future removeTeamMembership( int teamId, String user, ) { - return github.request( - 'DELETE', - '/teams/$teamId/memberships/$user', - statusCode: StatusCodes.NO_CONTENT, - ); + return github + .request( + 'DELETE', + '/teams/$teamId/memberships/$user', + statusCode: StatusCodes.NO_CONTENT, + ) + .then((_) => null); } /// Lists the repositories that the specified team has access to. @@ -332,8 +368,11 @@ class OrganizationsService extends Service { /// /// API docs: https://developer.github.com/v3/orgs/hooks/#list-hooks Stream listHooks(String org) { - return PaginationHelper(github).objects('GET', '/orgs/$org/hooks', - (dynamic i) => Hook.fromJson(i)..repoName = org); + return PaginationHelper(github).objects( + 'GET', + '/orgs/$org/hooks', + (dynamic i) => + Hook.fromJson(i as Map)..repoName = org); } /// Fetches a single hook by [id]. @@ -344,7 +383,8 @@ class OrganizationsService extends Service { int id, ) => github.getJSON('/orgs/$org/hooks/$id', - convert: (dynamic i) => Hook.fromJson(i)..repoName = org); + convert: (Map i) => + Hook.fromJson(i)..repoName = org); /// Creates an organization hook based on the specified [hook]. /// diff --git a/lib/src/common/repos_service.dart b/lib/src/common/repos_service.dart index 83fa5ef0..12b7c9f4 100644 --- a/lib/src/common/repos_service.dart +++ b/lib/src/common/repos_service.dart @@ -163,20 +163,21 @@ class RepositoriesService extends Service { bool? hasWiki, bool? hasDownloads}) async { ArgumentError.checkNotNull(slug); - final data = createNonNullMap({ - 'name': name!, - 'description': description!, - 'homepage': homepage!, - 'private': private!, - 'has_issues': hasIssues!, - 'has_wiki': hasWiki!, - 'has_downloads': hasDownloads!, - 'default_branch': 'defaultBranch' + final data = createNonNullMap({ + 'name': name, + 'description': description, + 'homepage': homepage, + 'private': private, + 'has_issues': hasIssues, + 'has_wiki': hasWiki, + 'has_downloads': hasDownloads, }); - return github.postJSON( + return github.requestJson, Repository>( + 'PATCH', '/repos/${slug.fullName}', body: GitHubJson.encode(data), - statusCode: 200, + statusCode: StatusCodes.OK, + convert: Repository.fromJson, ); } @@ -594,8 +595,8 @@ class RepositoriesService extends Service { // it was likely a 404 – but we don't have the status code here // But we can guess an the JSON content if (input.containsKey('message')) { - throw GitHubError(github, input['message'], - apiUrl: input['documentation_url']); + throw GitHubError(github, input['message'] as String?, + apiUrl: input['documentation_url'] as String?); } contents.file = GitHubFile.fromJson(input as Map); } else { @@ -1180,6 +1181,7 @@ class RepositoriesService extends Service { createReleaseAsset.label, ), headers: headers, + statusCode: StatusCodes.CREATED, body: createReleaseAsset.assetData, convert: ReleaseAsset.fromJson); releaseAssets.add(releaseAsset); @@ -1323,4 +1325,50 @@ class RepositoriesService extends Service { convert: ReleaseNotes.fromJson, ); } + + /// Creates a repository dispatch event. + /// + /// API docs: https://docs.github.com/en/rest/repos/repos#create-a-repository-dispatch-event + Future createDispatchEvent( + RepositorySlug slug, + String eventType, { + Map? clientPayload, + }) async { + final body = { + 'event_type': eventType, + if (clientPayload != null) 'client_payload': clientPayload, + }; + + await github.request( + 'POST', + '/repos/${slug.fullName}/dispatches', + body: GitHubJson.encode(body), + statusCode: 204, + ); + } + + /// Lists all topics for a repository. + /// + /// API docs: https://docs.github.com/en/rest/repos/repos#get-all-repository-topics + Future> listTopics(RepositorySlug slug) async { + return github.getJSON, List>( + '/repos/${slug.fullName}/topics', + convert: (json) => List.from(json['names'] as List? ?? []), + ); + } + + /// Replaces all topics for a repository. + /// + /// API docs: https://docs.github.com/en/rest/repos/repos#replace-all-repository-topics + Future> replaceTopics( + RepositorySlug slug, + List topics, + ) async { + return github.requestJson, List>( + 'PUT', + '/repos/${slug.fullName}/topics', + body: GitHubJson.encode({'names': topics}), + convert: (json) => List.from(json['names'] as List? ?? []), + ); + } } diff --git a/lib/src/common/search_service.dart b/lib/src/common/search_service.dart index 27cf6eef..64fc8761 100644 --- a/lib/src/common/search_service.dart +++ b/lib/src/common/search_service.dart @@ -22,20 +22,14 @@ class SearchService extends Service { final controller = StreamController(); - var isFirst = true; - PaginationHelper(github) .fetchStreamed('GET', '/search/repositories', params: params, pages: pages) .listen((response) { - if (response.statusCode == 403 && - response.body.contains('rate limit') && - isFirst) { - throw RateLimitHit(github); + if (response.statusCode != 200) { + github.handleStatusCode(response); } - isFirst = false; - final input = jsonDecode(response.body); if (input['items'] == null) { @@ -120,7 +114,13 @@ class SearchService extends Service { return PaginationHelper(github) .fetchStreamed('GET', '/search/code', params: params, pages: pages) - .map((r) => CodeSearchResults.fromJson(json.decode(r.body))); + .map((r) { + if (r.statusCode != 200) { + github.handleStatusCode(r); + } + return CodeSearchResults.fromJson( + json.decode(r.body) as Map); + }); } String _searchQualifier(String key, String? value) { @@ -141,19 +141,13 @@ class SearchService extends Service { final controller = StreamController(); - var isFirst = true; - PaginationHelper(github) .fetchStreamed('GET', '/search/issues', params: params, pages: pages) .listen((response) { - if (response.statusCode == 403 && - response.body.contains('rate limit') && - isFirst) { - throw RateLimitHit(github); + if (response.statusCode != 200) { + github.handleStatusCode(response); } - isFirst = false; - final input = jsonDecode(response.body); if (input['items'] == null) { @@ -191,19 +185,13 @@ class SearchService extends Service { final controller = StreamController(); - var isFirst = true; - PaginationHelper(github) .fetchStreamed('GET', '/search/users', params: params, pages: pages) .listen((response) { - if (response.statusCode == 403 && - response.body.contains('rate limit') && - isFirst) { - throw RateLimitHit(github); + if (response.statusCode != 200) { + github.handleStatusCode(response); } - isFirst = false; - final input = jsonDecode(response.body); if (input['items'] == null) { diff --git a/lib/src/common/util/utils.dart b/lib/src/common/util/utils.dart index 5c690774..45b90f47 100644 --- a/lib/src/common/util/utils.dart +++ b/lib/src/common/util/utils.dart @@ -99,32 +99,41 @@ final RegExp githubDateRemoveRegExp = RegExp(r'\.\d*'); const v3ApiMimeType = 'application/vnd.github.v3+json'; -String buildQueryString(Map params) { - final queryString = StringBuffer(); - - if (params.isNotEmpty && !params.values.every((value) => value == null)) { - queryString.write('?'); - } - - var i = 0; - for (final key in params.keys) { - i++; - if (params[key] == null) { +String buildQueryString(Map params, + {bool prefixQuestionMark = true}) { + final queryParts = []; + for (final entry in params.entries) { + final key = entry.key; + final value = entry.value; + if (value == null) { continue; } - queryString.write('$key=${Uri.encodeComponent(params[key].toString())}'); - if (i != params.keys.length) { - queryString.write('&'); + + final encodedKey = Uri.encodeQueryComponent(key); + if (value is Iterable) { + for (final item in value) { + if (item != null) { + final itemStr = item is Enum ? item.name : item.toString(); + queryParts.add('$encodedKey=${Uri.encodeQueryComponent(itemStr)}'); + } + } + } else { + final valueStr = value is Enum ? value.name : value.toString(); + queryParts.add('$encodedKey=${Uri.encodeQueryComponent(valueStr)}'); } } - return queryString.toString(); + + if (queryParts.isEmpty) { + return ''; + } + return prefixQuestionMark ? '?${queryParts.join('&')}' : queryParts.join('&'); } dynamic copyOf(dynamic input) { if (input is Iterable) { - return List.from(input); + return List.from(input); } else if (input is Map) { - return Map.from(input); + return Map.from(input); } else { throw Exception('type could not be copied'); } diff --git a/test/contract/service_contracts_test.dart b/test/contract/service_contracts_test.dart new file mode 100644 index 00000000..dbab2869 --- /dev/null +++ b/test/contract/service_contracts_test.dart @@ -0,0 +1,339 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:github/github.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; + +void main() { + group('OpenAPI Bindings Manifest Integrity', () { + test('all bindings in bindings.yaml resolve to valid RestContracts', () { + final file = File('tool/spec/bindings.yaml'); + expect(file.existsSync(), isTrue); + + final yaml = loadYaml(file.readAsStringSync()) as Map; + final bindings = yaml['bindings'] as List; + expect(bindings, isNotEmpty); + + for (final binding in bindings) { + final opId = binding['operationId'] as String; + final contract = RestContracts.all[opId]; + expect( + contract, + isNotNull, + reason: 'Binding operationId "$opId" must exist in RestContracts.all', + ); + } + }); + }); + + group('Service Method Contract Conformance', () { + late http.Request lastRequest; + late GitHub github; + + void interceptWithResponse(http.Response response) { + github = GitHub( + client: MockClient((req) async { + lastRequest = req; + return response; + }), + ); + } + + tearDown(() { + github.dispose(); + }); + + test('activity star conforms to activity/star-repo-for-authenticated-user', + () async { + final contract = + RestContracts.all['activity/star-repo-for-authenticated-user']!; + interceptWithResponse(http.Response('', 204)); + + await github.activity.star(RepositorySlug('octocat', 'hello-world')); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/user/starred/octocat/hello-world')); + expect(contract.successStatuses, contains(204)); + }); + + test( + 'activity unstar conforms to activity/unstar-repo-for-authenticated-user', + () async { + final contract = + RestContracts.all['activity/unstar-repo-for-authenticated-user']!; + interceptWithResponse(http.Response('', 204)); + + await github.activity.unstar(RepositorySlug('octocat', 'hello-world')); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/user/starred/octocat/hello-world')); + }); + + test('activity getThread conforms to activity/get-thread', () async { + final contract = RestContracts.all['activity/get-thread']!; + interceptWithResponse(http.Response( + jsonEncode({'id': '123', 'unread': false}), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.activity.getThread('123'); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/notifications/threads/123')); + }); + + test('gists star conforms to gists/star', () async { + final contract = RestContracts.all['gists/star']!; + interceptWithResponse(http.Response('', 204)); + + final res = await github.gists.starGist('g123'); + expect(res, isTrue); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/gists/g123/star')); + }); + + test('gists update conforms to gists/update (PATCH)', () async { + final contract = RestContracts.all['gists/update']!; + interceptWithResponse(http.Response( + jsonEncode({'id': 'g123', 'description': 'Updated'}), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.gists.editGist('g123', description: 'Updated'); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PATCH')); + expect(lastRequest.url.path, equals('/gists/g123')); + }); + + test('gists createComment conforms to gists/create-comment (201)', + () async { + final contract = RestContracts.all['gists/create-comment']!; + interceptWithResponse(http.Response( + jsonEncode({'id': 1, 'body': 'test comment'}), + 201, + headers: {'content-type': 'application/json'}, + )); + + await github.gists + .createComment('g123', CreateGistComment('test comment')); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/gists/g123/comments')); + expect(contract.successStatuses, contains(201)); + }); + + test('git updateRef conforms to git/update-ref (PATCH)', () async { + final contract = RestContracts.all['git/update-ref']!; + interceptWithResponse(http.Response( + jsonEncode({ + 'ref': 'refs/heads/master', + 'object': {'sha': 'abc'} + }), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.git.editReference( + RepositorySlug('octocat', 'hello-world'), + 'refs/heads/master', + 'abc', + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PATCH')); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/git/refs/heads/master')); + }); + + test('git getTree with recursive query conforms to git/get-tree', () async { + final contract = RestContracts.all['git/get-tree']!; + interceptWithResponse(http.Response( + jsonEncode({'sha': 'sha123', 'tree': [], 'truncated': false}), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.git.getTree( + RepositorySlug('octocat', 'hello-world'), + 'sha123', + recursive: true, + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/git/trees/sha123')); + expect(lastRequest.url.queryParameters['recursive'], equals('1')); + }); + + test('issues lock conforms to issues/lock (PUT / lock reason)', () async { + final contract = RestContracts.all['issues/lock']!; + interceptWithResponse(http.Response('', 204)); + + await github.issues.lock( + RepositorySlug('octocat', 'hello-world'), + 42, + lockReason: 'resolved', + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PUT')); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/issues/42/lock')); + final body = jsonDecode(lastRequest.body) as Map; + expect(body['lock_reason'], equals('resolved')); + }); + + test('issues isAssignee conforms to issues/check-user-can-be-assigned', + () async { + final contract = RestContracts.all['issues/check-user-can-be-assigned']!; + interceptWithResponse(http.Response('', 204)); + + final res = await github.issues + .isAssignee(RepositorySlug('octocat', 'hello-world'), 'octocat'); + expect(res, isTrue); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/assignees/octocat')); + }); + + test('issues replaceLabelsForIssue returns List', () async { + final contract = RestContracts.all['issues/set-labels']!; + interceptWithResponse(http.Response( + jsonEncode([ + {'name': 'bug', 'color': 'f29513'} + ]), + 200, + headers: {'content-type': 'application/json'}, + )); + + final labels = await github.issues.replaceLabelsForIssue( + RepositorySlug('octocat', 'hello-world'), + 42, + ['bug'], + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PUT')); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/issues/42/labels')); + expect(labels, isA>()); + expect(labels.length, equals(1)); + expect(labels.first.name, equals('bug')); + }); + + test('teams editTeam conforms to teams/update-legacy (PATCH)', () async { + final contract = RestContracts.all['teams/update-legacy']!; + interceptWithResponse(http.Response( + jsonEncode({'id': 123, 'name': 'Core Team'}), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.organizations.editTeam(123, 'Core Team'); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PATCH')); + expect(lastRequest.url.path, equals('/teams/123')); + }); + + test('teams editTeamInOrg conforms to teams/update-in-org (PATCH)', + () async { + final contract = RestContracts.all['teams/update-in-org']!; + interceptWithResponse(http.Response( + jsonEncode({'id': 123, 'name': 'Core Team', 'slug': 'core-team'}), + 200, + headers: {'content-type': 'application/json'}, + )); + + await github.organizations + .editTeamInOrg('my-org', 'core-team', name: 'Core Team'); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PATCH')); + expect(lastRequest.url.path, equals('/orgs/my-org/teams/core-team')); + }); + + test( + 'repos createDispatchEvent conforms to repos/create-dispatch-event (204)', + () async { + final contract = RestContracts.all['repos/create-dispatch-event']!; + interceptWithResponse(http.Response('', 204)); + + await github.repositories.createDispatchEvent( + RepositorySlug('octocat', 'hello-world'), + 'custom_event', + clientPayload: {'run_id': 123}, + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, + equals('/repos/octocat/hello-world/dispatches')); + final body = jsonDecode(lastRequest.body) as Map; + expect(body['event_type'], equals('custom_event')); + expect(body['client_payload']['run_id'], equals(123)); + }); + + test('repos listTopics conforms to repos/get-all-topics', () async { + final contract = RestContracts.all['repos/get-all-topics']!; + interceptWithResponse(http.Response( + jsonEncode({ + 'names': ['dart', 'github-api'] + }), + 200, + headers: {'content-type': 'application/json'}, + )); + + final topics = await github.repositories + .listTopics(RepositorySlug('octocat', 'hello-world')); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/repos/octocat/hello-world/topics')); + expect(topics, equals(['dart', 'github-api'])); + }); + + test('repos replaceTopics conforms to repos/replace-all-topics', () async { + final contract = RestContracts.all['repos/replace-all-topics']!; + interceptWithResponse(http.Response( + jsonEncode({ + 'names': ['dart', 'api'] + }), + 200, + headers: {'content-type': 'application/json'}, + )); + + final topics = await github.repositories.replaceTopics( + RepositorySlug('octocat', 'hello-world'), + ['dart', 'api'], + ); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.method, equals('PUT')); + expect(lastRequest.url.path, equals('/repos/octocat/hello-world/topics')); + expect(topics, equals(['dart', 'api'])); + }); + + test('misc listGitignoreTemplates conforms to gitignore/get-all-templates', + () async { + final contract = RestContracts.all['gitignore/get-all-templates']!; + interceptWithResponse(http.Response( + jsonEncode(['Dart', 'Go', 'Rust']), + 200, + headers: {'content-type': 'application/json'}, + )); + + final templates = await github.misc.listGitignoreTemplates(); + + expect(lastRequest.method, equals(contract.method)); + expect(lastRequest.url.path, equals('/gitignore/templates')); + expect(templates, equals(['Dart', 'Go', 'Rust'])); + }); + }); +} diff --git a/test/git_test.dart b/test/git_test.dart index e95ef5d6..6f145b26 100644 --- a/test/git_test.dart +++ b/test/git_test.dart @@ -11,7 +11,8 @@ GitHub createGithub() { return GitHub( endpoint: fakeApiUrl, auth: const Authentication.withToken( - '0000000000000000000000000000000000000001')); + '0000000000000000000000000000000000000001'), + allowInsecureAuth: true); } void main() { @@ -96,8 +97,9 @@ void main() { }); test('deleteReference()', () async { - nock(fakeApiUrl).delete('/repos/o/n/git/refs/heads/b').reply(200, '{}'); - await git.deleteReference(repo, 'heads/b'); + nock(fakeApiUrl).delete('/repos/o/n/git/refs/heads/b').reply(204, ''); + final res = await git.deleteReference(repo, 'heads/b'); + expect(res, isTrue); }); test('getTag()', () async {