Skip to content

Commit

Permalink
🚧Fix bug with null ref exceptions on unsubscribe all (#203)
Browse files Browse the repository at this point in the history
* Start work for issue #202

* config: added create pr powershell script

* fix: check for null before trying to invoke unsubscribe method

* test: create unit test to test for fix
  • Loading branch information
CalvinWilkinson authored Apr 3, 2024
1 parent 9cac8e8 commit ef864f4
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 7 deletions.
7 changes: 7 additions & 0 deletions Carbonate/ReactableBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ public virtual void UnsubscribeAll()

foreach (TSubscription subscription in CollectionsMarshal.AsSpan(InternalSubscriptions))
{
// NOTE: This suppression is because the items could be null even with nullable reference types enabled.
// ReSharper disable ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
if (subscription is null)
{
continue;
}

subscription.OnUnsubscribe();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,54 @@ public class NonDirectionalPushReactable_IntegrationTests
public void WhenPushing_And_WithSingleEventID_And_WithSingleSubscription_And_WithSingleUnsubscribe_ReturnsCorrectResults()
{
// Arrange
var eventId = new Guid("98a879d4-e819-41da-80e4-a1b459b3e43f");
var id = new Guid("98a879d4-e819-41da-80e4-a1b459b3e43f");

IDisposable? unsubscriber = null;

var reactable = new PushReactable();
var sut = new PushReactable();

unsubscriber = reactable.Subscribe(new ReceiveSubscription(
id: eventId,
unsubscriber = sut.Subscribe(new ReceiveSubscription(
id: id,
onReceive: () => { },
onUnsubscribe: () => unsubscriber?.Dispose()));

// Act
reactable.Push(eventId);
reactable.Unsubscribe(eventId);
sut.Push(id);
sut.Unsubscribe(id);

// Assert
reactable.Subscriptions.Should().HaveCount(0);
sut.Subscriptions.Should().HaveCount(0);
}

[Fact]
public void Unsubscribing_BeforeCallingUnsubscribeAll_DoesNotThrowException()
{
// Arrange
var id = new Guid("f227d62c-1830-4a42-a5b3-3c920779bf94");
IDisposable? unsubscriberB = null;
var sut = new PushReactable();

sut.Subscribe(
new ReceiveSubscription(
id: id,
onReceive: () => { },
onUnsubscribe: () =>
{
// Unsubscribe from the second subscription so when
// it comes its turn to be unsubscribed, it will be null.
unsubscriberB.Dispose();
}));

unsubscriberB = sut.Subscribe(
new ReceiveSubscription(
id: id,
onReceive: () => { },
onUnsubscribe: () => unsubscriberB.Dispose()));

// Act
var act = () => sut.UnsubscribeAll();

// Assert
act.Should().NotThrow();
}
}
50 changes: 50 additions & 0 deletions create-pr.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Clear-Host;

Write-Host -NoNewline "Please enter an issue number: " -ForegroundColor Cyan;
$issueNumber = Read-Host;

if ($issueNumber -notmatch '^\d+$') {
Write-Error "User input is not a number";
exit 1;
}

Write-Host -NoNewline "Please enter a branch name: " -ForegroundColor Cyan;
$branchDescrip = Read-Host;

$branchDescrip = $branchDescrip.ToLower();
$branchDescrip = $branchDescrip.Replace(" ", "-").Replace("_", "-");
$branchDescrip = $branchDescrip.TrimStart("-");
$branchDescrip = $branchDescrip.TrimEnd("-");

$headBranch = "feature/$issueNumber-$branchDescrip";
$commitMsg = "Start owrk for issue #$issueNumber";

$destBranch = "not-set";

$baseBranches = @("main", "preview");
Write-Host -NoNewline "Please choose a base branch from the list [$($baseBranches -join ', ')]: " -ForegroundColor Cyan;
$chosenBaseBranch = Read-Host;

if ($baseBranches -contains $chosenBaseBranch) {
$destBranch = $chosenBaseBranch;
} else {
Write-Error "Invalid base branch.";
exit 1;
}

Write-Host "`n--------------------------------`n";

Write-Host "Creating branch. . ." -ForegroundColor Yellow;
git checkout -B "$headBranch";
Write-Host "";

Write-Host "Creating empty commit. . ." -ForegroundColor Yellow;
git commit --allow-empty -m $commitMsg;
Write-Host "";

Write-Host "Pushing branch to remote. . ." -ForegroundColor Yellow;
git push --set-upstream origin "$headBranch";
Write-Host "";

Write-Host "Creating PR. . ." -ForegroundColor Yellow;
gh pr create -B $destBranch -b "" -t "new pr" -d;

0 comments on commit ef864f4

Please sign in to comment.