Skip to content

Commit cb5e1e3

Browse files
gep13gep13
committed
(#146) Add breaking API changes
In order to fully support GitLab, we need to introduce come breaking changes to the GitReleaseManager API. GitLab has the concept of both a public and an internal number for things like a milestone. Due to this, the decision has been made to not assume the property that is required for a method. For example, when deleting a release, let's not assume that we want/need the number of the release, instead, let's pass the entire release to the method, and then internally, the implementation of the method can decide what property it needs to use to perform the operation. This approach as been implemented across all of the API surface, so we now pass in the entire Release, or Label, or Issue. This has meant quite a few changes across things like the tests to accommodate this change.
1 parent 46b7762 commit cb5e1e3

File tree

8 files changed

+108
-102
lines changed

8 files changed

+108
-102
lines changed

‎src/GitReleaseManager.Core.Tests/Provider/GitHubProviderTests.cs

Lines changed: 39 additions & 39 deletions
Large diffs are not rendered by default.

‎src/GitReleaseManager.Core.Tests/VcsServiceTests.cs

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ public class VcsServiceTests
2626
{
2727
private const string OWNER = "owner";
2828
private const string REPOSITORY = "repository";
29-
private const int MILESTONE_NUMBER = 1;
29+
private const int MILESTONE_PUBLIC_NUMBER = 1;
30+
private const int MILESTONE_INTERNAL_NUMBER = 123;
3031
private const string MILESTONE_TITLE = "0.1.0";
3132
private const string TAG_NAME = "0.1.0";
3233
private const string RELEASE_NOTES = "Release Notes";
@@ -126,7 +127,7 @@ public async Task Should_Add_Assets()
126127
await _vcsService.AddAssetsAsync(OWNER, REPOSITORY, TAG_NAME, _assets).ConfigureAwait(false);
127128

128129
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
129-
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<int>()).ConfigureAwait(false);
130+
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<ReleaseAsset>()).ConfigureAwait(false);
130131
await _vcsProvider.Received(assetsCount).UploadAssetAsync(release, Arg.Any<ReleaseAssetUpload>()).ConfigureAwait(false);
131132

132133
_logger.DidNotReceive().Warning(Arg.Any<string>(), Arg.Any<string>());
@@ -149,7 +150,7 @@ public async Task Should_Add_Assets_With_Deleting_Existing_Assets()
149150
await _vcsService.AddAssetsAsync(OWNER, REPOSITORY, TAG_NAME, _assets).ConfigureAwait(false);
150151

151152
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
152-
await _vcsProvider.Received(releaseAssetsCount).DeleteAssetAsync(OWNER, REPOSITORY, releaseAsset.Id).ConfigureAwait(false);
153+
await _vcsProvider.Received(releaseAssetsCount).DeleteAssetAsync(OWNER, REPOSITORY, releaseAsset).ConfigureAwait(false);
153154
await _vcsProvider.Received(assetsCount).UploadAssetAsync(release, Arg.Any<ReleaseAssetUpload>()).ConfigureAwait(false);
154155

155156
_logger.Received(releaseAssetsCount).Warning(Arg.Any<string>(), Arg.Any<string>());
@@ -172,7 +173,7 @@ public async Task Should_Throw_Exception_On_Adding_Assets_When_Asset_File_Not_Ex
172173
ex.Message.ShouldContain(assetFilePath);
173174

174175
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
175-
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<int>()).ConfigureAwait(false);
176+
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<ReleaseAsset>()).ConfigureAwait(false);
176177
await _vcsProvider.DidNotReceive().UploadAssetAsync(release, Arg.Any<ReleaseAssetUpload>()).ConfigureAwait(false);
177178
}
178179

@@ -182,7 +183,7 @@ public async Task Should_Do_Nothing_On_Missing_Assets(IList<string> assets)
182183
await _vcsService.AddAssetsAsync(OWNER, REPOSITORY, TAG_NAME, assets).ConfigureAwait(false);
183184

184185
await _vcsProvider.DidNotReceive().GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
185-
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<int>()).ConfigureAwait(false);
186+
await _vcsProvider.DidNotReceive().DeleteAssetAsync(OWNER, REPOSITORY, Arg.Any<ReleaseAsset>()).ConfigureAwait(false);
186187
await _vcsProvider.DidNotReceive().UploadAssetAsync(Arg.Any<Release>(), Arg.Any<ReleaseAssetUpload>()).ConfigureAwait(false);
187188
}
188189

@@ -205,7 +206,7 @@ public async Task Should_Create_Labels()
205206
_vcsProvider.GetLabelsAsync(OWNER, REPOSITORY)
206207
.Returns(Task.FromResult((IEnumerable<Label>)labels));
207208

208-
_vcsProvider.DeleteLabelAsync(OWNER, REPOSITORY, Arg.Any<string>())
209+
_vcsProvider.DeleteLabelAsync(OWNER, REPOSITORY, Arg.Any<Label>())
209210
.Returns(Task.CompletedTask);
210211

211212
_vcsProvider.CreateLabelAsync(OWNER, REPOSITORY, Arg.Any<Label>())
@@ -214,7 +215,7 @@ public async Task Should_Create_Labels()
214215
await _vcsService.CreateLabelsAsync(OWNER, REPOSITORY).ConfigureAwait(false);
215216

216217
await _vcsProvider.Received(1).GetLabelsAsync(OWNER, REPOSITORY).ConfigureAwait(false);
217-
await _vcsProvider.Received(labels.Count).DeleteLabelAsync(OWNER, REPOSITORY, Arg.Any<string>()).ConfigureAwait(false);
218+
await _vcsProvider.Received(labels.Count).DeleteLabelAsync(OWNER, REPOSITORY, Arg.Any<Label>()).ConfigureAwait(false);
218219
await _vcsProvider.Received(_configuration.Labels.Count).CreateLabelAsync(OWNER, REPOSITORY, Arg.Any<Label>()).ConfigureAwait(false);
219220

220221
_logger.Received(1).Verbose(Arg.Any<string>(), OWNER, REPOSITORY);
@@ -235,61 +236,65 @@ public async Task Should_Log_An_Warning_When_Labels_Not_Configured()
235236
[Test]
236237
public async Task Should_Close_Milestone()
237238
{
238-
var milestone = new Milestone { Number = MILESTONE_NUMBER };
239+
var milestone = new Milestone { PublicNumber = MILESTONE_PUBLIC_NUMBER, InternalNumber = MILESTONE_INTERNAL_NUMBER };
239240

240241
_vcsProvider.GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Open)
241242
.Returns(Task.FromResult(milestone));
242243

243-
_vcsProvider.SetMilestoneStateAsync(OWNER, REPOSITORY, milestone.Number, ItemState.Closed)
244+
_vcsProvider.SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Closed)
244245
.Returns(Task.CompletedTask);
245246

246247
await _vcsService.CloseMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE).ConfigureAwait(false);
247248

248249
await _vcsProvider.Received(1).GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Open).ConfigureAwait(false);
249-
await _vcsProvider.Received(1).SetMilestoneStateAsync(OWNER, REPOSITORY, milestone.Number, ItemState.Closed).ConfigureAwait(false);
250+
await _vcsProvider.Received(1).SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Closed).ConfigureAwait(false);
250251
}
251252

252253
[Test]
253254
public async Task Should_Log_An_Warning_On_Closing_When_Milestone_Cannot_Be_Found()
254255
{
256+
var milestone = new Milestone { PublicNumber = MILESTONE_PUBLIC_NUMBER, InternalNumber = MILESTONE_INTERNAL_NUMBER };
257+
255258
_vcsProvider.GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Open)
256259
.Returns(Task.FromException<Milestone>(_notFoundException));
257260

258261
await _vcsService.CloseMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE).ConfigureAwait(false);
259262

260263
await _vcsProvider.Received(1).GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Open).ConfigureAwait(false);
261-
await _vcsProvider.DidNotReceive().SetMilestoneStateAsync(OWNER, REPOSITORY, MILESTONE_NUMBER, ItemState.Closed).ConfigureAwait(false);
264+
await _vcsProvider.DidNotReceive().SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Closed).ConfigureAwait(false);
262265
_logger.Received(1).Warning(UNABLE_TO_FOUND_MILESTONE_MESSAGE, "open", MILESTONE_TITLE, OWNER, REPOSITORY);
263266
}
264267

265268
[Test]
266269
public async Task Should_Open_Milestone()
267270
{
268-
var milestone = new Milestone { Number = MILESTONE_NUMBER };
271+
var milestone = new Milestone { PublicNumber = MILESTONE_PUBLIC_NUMBER, InternalNumber = MILESTONE_INTERNAL_NUMBER };
269272

270273
_vcsProvider.GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Closed)
271274
.Returns(Task.FromResult(milestone));
272275

273-
_vcsProvider.SetMilestoneStateAsync(OWNER, REPOSITORY, milestone.Number, ItemState.Open)
276+
_vcsProvider.SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Open)
274277
.Returns(Task.CompletedTask);
275278

276279
await _vcsService.OpenMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE).ConfigureAwait(false);
277280

278281
await _vcsProvider.Received(1).GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Closed).ConfigureAwait(false);
279-
await _vcsProvider.Received(1).SetMilestoneStateAsync(OWNER, REPOSITORY, milestone.Number, ItemState.Open).ConfigureAwait(false);
282+
await _vcsProvider.Received(1).SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Open).ConfigureAwait(false);
280283
_logger.Received(2).Verbose(Arg.Any<string>(), MILESTONE_TITLE, OWNER, REPOSITORY);
281284
}
282285

283286
[Test]
284287
public async Task Should_Log_An_Warning_On_Opening_When_Milestone_Cannot_Be_Found()
285288
{
289+
var milestone = new Milestone { PublicNumber = MILESTONE_PUBLIC_NUMBER, InternalNumber = MILESTONE_INTERNAL_NUMBER };
290+
286291
_vcsProvider.GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Closed)
287292
.Returns(Task.FromException<Milestone>(_notFoundException));
288293

289294
await _vcsService.OpenMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE).ConfigureAwait(false);
290295

291296
await _vcsProvider.Received(1).GetMilestoneAsync(OWNER, REPOSITORY, MILESTONE_TITLE, ItemStateFilter.Closed).ConfigureAwait(false);
292-
await _vcsProvider.DidNotReceive().SetMilestoneStateAsync(OWNER, REPOSITORY, MILESTONE_NUMBER, ItemState.Open).ConfigureAwait(false);
297+
await _vcsProvider.DidNotReceive().SetMilestoneStateAsync(OWNER, REPOSITORY, milestone, ItemState.Open).ConfigureAwait(false);
293298
_logger.Received(1).Warning(UNABLE_TO_FOUND_MILESTONE_MESSAGE, "closed", MILESTONE_TITLE, OWNER, REPOSITORY);
294299
}
295300

@@ -556,13 +561,13 @@ public async Task Should_Delete_Draft_Release()
556561
_vcsProvider.GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME)
557562
.Returns(Task.FromResult(release));
558563

559-
_vcsProvider.DeleteReleaseAsync(OWNER, REPOSITORY, release.Id)
564+
_vcsProvider.DeleteReleaseAsync(OWNER, REPOSITORY, release)
560565
.Returns(Task.CompletedTask);
561566

562567
await _vcsService.DiscardReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
563568

564569
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
565-
await _vcsProvider.Received(1).DeleteReleaseAsync(OWNER, REPOSITORY, release.Id).ConfigureAwait(false);
570+
await _vcsProvider.Received(1).DeleteReleaseAsync(OWNER, REPOSITORY, release).ConfigureAwait(false);
566571
}
567572

568573
[Test]
@@ -576,7 +581,7 @@ public async Task Should_Not_Delete_Published_Release()
576581
await _vcsService.DiscardReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
577582

578583
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
579-
await _vcsProvider.DidNotReceive().DeleteReleaseAsync(OWNER, REPOSITORY, release.Id).ConfigureAwait(false);
584+
await _vcsProvider.DidNotReceive().DeleteReleaseAsync(OWNER, REPOSITORY, release).ConfigureAwait(false);
580585
_logger.Received(1).Warning(Arg.Any<string>(), TAG_NAME);
581586
}
582587

@@ -601,13 +606,13 @@ public async Task Should_Publish_Release()
601606
_vcsProvider.GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME)
602607
.Returns(Task.FromResult(release));
603608

604-
_vcsProvider.PublishReleaseAsync(OWNER, REPOSITORY, TAG_NAME, release.Id)
609+
_vcsProvider.PublishReleaseAsync(OWNER, REPOSITORY, TAG_NAME, release)
605610
.Returns(Task.CompletedTask);
606611

607612
await _vcsService.PublishReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
608613

609614
await _vcsProvider.Received(1).GetReleaseAsync(OWNER, REPOSITORY, TAG_NAME).ConfigureAwait(false);
610-
await _vcsProvider.Received(1).PublishReleaseAsync(OWNER, REPOSITORY, TAG_NAME, release.Id).ConfigureAwait(false);
615+
await _vcsProvider.Received(1).PublishReleaseAsync(OWNER, REPOSITORY, TAG_NAME, release).ConfigureAwait(false);
611616
_logger.Received(1).Verbose(Arg.Any<string>(), TAG_NAME, OWNER, REPOSITORY);
612617
_logger.Received(1).Debug(Arg.Any<string>(), Arg.Any<Release>());
613618
}

‎src/GitReleaseManager.Core/Provider/GitHubProvider.cs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using NotFoundException = GitReleaseManager.Core.Exceptions.NotFoundException;
1717
using RateLimit = GitReleaseManager.Core.Model.RateLimit;
1818
using Release = GitReleaseManager.Core.Model.Release;
19+
using ReleaseAsset = GitReleaseManager.Core.Model.ReleaseAsset;
1920
using ReleaseAssetUpload = GitReleaseManager.Core.Model.ReleaseAssetUpload;
2021

2122
namespace GitReleaseManager.Core.Provider
@@ -34,11 +35,11 @@ public GitHubProvider(IGitHubClient gitHubClient, IMapper mapper)
3435
_mapper = mapper;
3536
}
3637

37-
public Task DeleteAssetAsync(string owner, string repository, int id)
38+
public Task DeleteAssetAsync(string owner, string repository, ReleaseAsset asset)
3839
{
3940
return ExecuteAsync(async () =>
4041
{
41-
await _gitHubClient.Repository.Release.DeleteAsset(owner, repository, id).ConfigureAwait(false);
42+
await _gitHubClient.Repository.Release.DeleteAsset(owner, repository, asset.Id).ConfigureAwait(false);
4243
});
4344
}
4445

@@ -84,21 +85,21 @@ public string GetCommitsUrl(string owner, string repository, string head, string
8485
return url;
8586
}
8687

87-
public Task CreateIssueCommentAsync(string owner, string repository, int issueNumber, string comment)
88+
public Task CreateIssueCommentAsync(string owner, string repository, Issue issue, string comment)
8889
{
8990
return ExecuteAsync(async () =>
9091
{
91-
await _gitHubClient.Issue.Comment.Create(owner, repository, issueNumber, comment).ConfigureAwait(false);
92+
await _gitHubClient.Issue.Comment.Create(owner, repository, issue.Number, comment).ConfigureAwait(false);
9293
});
9394
}
9495

95-
public Task<IEnumerable<Issue>> GetIssuesAsync(string owner, string repository, int milestoneNumber, ItemStateFilter itemStateFilter = ItemStateFilter.All)
96+
public Task<IEnumerable<Issue>> GetIssuesAsync(string owner, string repository, Milestone milestone, ItemStateFilter itemStateFilter = ItemStateFilter.All)
9697
{
9798
return ExecuteAsync(async () =>
9899
{
99100
var openIssueRequest = new RepositoryIssueRequest
100101
{
101-
Milestone = milestoneNumber.ToString(CultureInfo.InvariantCulture),
102+
Milestone = milestone.PublicNumber.ToString(CultureInfo.InvariantCulture),
102103
State = (Octokit.ItemStateFilter)itemStateFilter,
103104
};
104105

@@ -120,7 +121,7 @@ public Task<IEnumerable<Issue>> GetIssuesAsync(string owner, string repository,
120121
});
121122
}
122123

123-
public Task<IEnumerable<IssueComment>> GetIssueCommentsAsync(string owner, string repository, int issueNumber)
124+
public Task<IEnumerable<IssueComment>> GetIssueCommentsAsync(string owner, string repository, Issue issue)
124125
{
125126
return ExecuteAsync(async () =>
126127
{
@@ -131,7 +132,7 @@ public Task<IEnumerable<IssueComment>> GetIssueCommentsAsync(string owner, strin
131132
do
132133
{
133134
var options = GetApiOptions(startPage);
134-
results = await _gitHubClient.Issue.Comment.GetAllForIssue(owner, repository, issueNumber, options).ConfigureAwait(false);
135+
results = await _gitHubClient.Issue.Comment.GetAllForIssue(owner, repository, issue.Number, options).ConfigureAwait(false);
135136

136137
comments.AddRange(results);
137138
startPage++;
@@ -152,11 +153,11 @@ public Task CreateLabelAsync(string owner, string repository, Label label)
152153
});
153154
}
154155

155-
public Task DeleteLabelAsync(string owner, string repository, string labelName)
156+
public Task DeleteLabelAsync(string owner, string repository, Label label)
156157
{
157158
return ExecuteAsync(async () =>
158159
{
159-
await _gitHubClient.Issue.Labels.Delete(owner, repository, labelName).ConfigureAwait(false);
160+
await _gitHubClient.Issue.Labels.Delete(owner, repository, label.Name).ConfigureAwait(false);
160161
});
161162
}
162163

@@ -187,14 +188,14 @@ public Task<Milestone> GetMilestoneAsync(string owner, string repository, string
187188
return ExecuteAsync(async () =>
188189
{
189190
var milestones = await GetMilestonesAsync(owner, repository, itemStateFilter).ConfigureAwait(false);
190-
var milestone = milestones.FirstOrDefault(m => m.Title == milestoneTitle);
191+
var foundMilestone = milestones.FirstOrDefault(m => m.Title == milestoneTitle);
191192

192-
if (milestone is null)
193+
if (foundMilestone is null)
193194
{
194195
throw new NotFoundException(NOT_FOUND_MESSGAE);
195196
}
196197

197-
return milestone;
198+
return foundMilestone;
198199
});
199200
}
200201

@@ -222,12 +223,12 @@ public Task<IEnumerable<Milestone>> GetMilestonesAsync(string owner, string repo
222223
});
223224
}
224225

225-
public Task SetMilestoneStateAsync(string owner, string repository, int milestoneNumber, ItemState itemState)
226+
public Task SetMilestoneStateAsync(string owner, string repository, Milestone milestone, ItemState itemState)
226227
{
227228
return ExecuteAsync(async () =>
228229
{
229230
var update = new MilestoneUpdate { State = (Octokit.ItemState)itemState };
230-
await _gitHubClient.Issue.Milestone.Update(owner, repository, milestoneNumber, update).ConfigureAwait(false);
231+
await _gitHubClient.Issue.Milestone.Update(owner, repository, milestone.PublicNumber, update).ConfigureAwait(false);
231232
});
232233
}
233234

@@ -242,11 +243,11 @@ public Task<Release> CreateReleaseAsync(string owner, string repository, Release
242243
});
243244
}
244245

245-
public Task DeleteReleaseAsync(string owner, string repository, int id)
246+
public Task DeleteReleaseAsync(string owner, string repository, Release release)
246247
{
247248
return ExecuteAsync(async () =>
248249
{
249-
await _gitHubClient.Repository.Release.Delete(owner, repository, id).ConfigureAwait(false);
250+
await _gitHubClient.Repository.Release.Delete(owner, repository, release.Id).ConfigureAwait(false);
250251
});
251252
}
252253

@@ -299,7 +300,7 @@ public Task<IEnumerable<Release>> GetReleasesAsync(string owner, string reposito
299300
});
300301
}
301302

302-
public Task PublishReleaseAsync(string owner, string repository, string tagName, int releaseId)
303+
public Task PublishReleaseAsync(string owner, string repository, string tagName, Release release)
303304
{
304305
return ExecuteAsync(async () =>
305306
{
@@ -309,7 +310,7 @@ public Task PublishReleaseAsync(string owner, string repository, string tagName,
309310
TagName = tagName,
310311
};
311312

312-
await _gitHubClient.Repository.Release.Edit(owner, repository, releaseId, update).ConfigureAwait(false);
313+
await _gitHubClient.Repository.Release.Edit(owner, repository, release.Id, update).ConfigureAwait(false);
313314
});
314315
}
315316

0 commit comments

Comments
 (0)