From 39986d6296be0f75a280389b0781447e8a7df0f1 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Wed, 16 Oct 2024 15:03:38 -0400 Subject: [PATCH] [Storage] [DataMovement] Refactor Try-style methods (#46645) * Initial commit * Changed wording of doc comments --- .../src/DataTransferInternalState.cs | 12 ++++++------ .../src/DataTransferStatus.cs | 12 ++++++------ .../src/JobPartInternal.cs | 6 +++--- .../src/ServiceToServiceTransferJob.cs | 2 +- .../src/StreamToUriTransferJob.cs | 2 +- .../src/TransferJobInternal.cs | 8 ++++---- .../src/UriToStreamTransferJob.cs | 2 +- .../tests/DataTransferTests.cs | 5 +---- 8 files changed, 23 insertions(+), 26 deletions(-) diff --git a/sdk/storage/Azure.Storage.DataMovement/src/DataTransferInternalState.cs b/sdk/storage/Azure.Storage.DataMovement/src/DataTransferInternalState.cs index 2cdc83a106ad..7181589d26fb 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/DataTransferInternalState.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/DataTransferInternalState.cs @@ -86,10 +86,10 @@ public DataTransferStatus GetTransferStatus() /// Sets the completion status /// /// - /// Returns whether or not the status has been changed/set - public bool TrySetTransferState(DataTransferState state) + /// Returns whether or not the status has been changed from its original state. + public bool SetTransferState(DataTransferState state) { - if (_status.TrySetTransferStateChange(state)) + if (_status.SetTransferStateChange(state)) { if (DataTransferState.Completed == _status.State || DataTransferState.Paused == _status.State) @@ -104,9 +104,9 @@ public bool TrySetTransferState(DataTransferState state) return false; } - public bool TrySetFailedItemsState() => _status.TrySetFailedItem(); + public bool SetFailedItemsState() => _status.SetFailedItem(); - public bool TrySetSkippedItemsState() => _status.TrySetSkippedItem(); + public bool SetSkippedItemsState() => _status.SetSkippedItem(); internal bool CanPause() => DataTransferState.InProgress == _status.State; @@ -119,7 +119,7 @@ public async Task PauseIfRunningAsync(CancellationToken cancellationToken) } CancellationHelper.ThrowIfCancellationRequested(cancellationToken); // Call the inner cancellation token to stop the transfer job - TrySetTransferState(DataTransferState.Pausing); + SetTransferState(DataTransferState.Pausing); if (TriggerCancellation()) { // Wait until full pause has completed. diff --git a/sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs b/sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs index 9b7486acc968..919d10ff324d 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs @@ -75,8 +75,8 @@ protected internal DataTransferStatus(DataTransferState state, bool hasFailureIt /// /// This should only be triggered when a failed item has been seen. /// - /// True if was updated. False otherwise. - internal bool TrySetFailedItem() + /// True if this was the first time was updated. False otherwise. + internal bool SetFailedItem() { return Interlocked.Exchange(ref _hasFailedItemValue, 1) != 1; } @@ -86,8 +86,8 @@ internal bool TrySetFailedItem() /// /// This should only be triggered when a skipped item has been seen. /// - /// /// True if was updated. False otherwise. - internal bool TrySetSkippedItem() + /// /// True if this was the first time was updated. False otherwise. + internal bool SetSkippedItem() { return Interlocked.Exchange(ref _hasSkippedItemValue, 1) != 1; } @@ -98,8 +98,8 @@ internal bool TrySetSkippedItem() /// /// This should only be triggered when the state updates. /// - /// True if was updated. False otherwise. - internal bool TrySetTransferStateChange(DataTransferState state) + /// True if was changed from its original state. False otherwise. + internal bool SetTransferStateChange(DataTransferState state) { return Interlocked.Exchange(ref _stateValue, (int)state) != (int)state; } diff --git a/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs b/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs index 29d356e1c54f..bb202344615c 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs @@ -275,7 +275,7 @@ internal async Task TriggerCancellationAsync() /// internal async Task OnTransferStateChangedAsync(DataTransferState transferState) { - if (JobPartStatus.TrySetTransferStateChange(transferState)) + if (JobPartStatus.SetTransferStateChange(transferState)) { // Progress tracking, do before invoking the event below if (transferState == DataTransferState.InProgress) @@ -355,7 +355,7 @@ await TransferSkippedEventHandler.RaiseAsync( // Update the JobPartStatus. If was already updated (e.g. there was a failed item before) // then don't raise the PartTransferStatusEventHandler - if (JobPartStatus.TrySetSkippedItem()) + if (JobPartStatus.SetSkippedItem()) { await PartTransferStatusEventHandler.RaiseAsync( new TransferStatusEventArgs( @@ -409,7 +409,7 @@ await TransferFailedEventHandler.RaiseAsync( // Update the JobPartStatus. If was already updated (e.g. there was a failed item before) // then don't raise the PartTransferStatusEventHandler - if (JobPartStatus.TrySetFailedItem()) + if (JobPartStatus.SetFailedItem()) { await PartTransferStatusEventHandler.RaiseAsync( new TransferStatusEventArgs( diff --git a/sdk/storage/Azure.Storage.DataMovement/src/ServiceToServiceTransferJob.cs b/sdk/storage/Azure.Storage.DataMovement/src/ServiceToServiceTransferJob.cs index 9ad7c393a93e..1afa3db3dd13 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/ServiceToServiceTransferJob.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/ServiceToServiceTransferJob.cs @@ -105,7 +105,7 @@ public override async IAsyncEnumerable ProcessJobToJobPartAsync { if (!part.JobPartStatus.HasCompletedSuccessfully) { - part.JobPartStatus.TrySetTransferStateChange(DataTransferState.Queued); + part.JobPartStatus.SetTransferStateChange(DataTransferState.Queued); yield return part; } } diff --git a/sdk/storage/Azure.Storage.DataMovement/src/StreamToUriTransferJob.cs b/sdk/storage/Azure.Storage.DataMovement/src/StreamToUriTransferJob.cs index d50c8c540f36..4eaf4c62da0d 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/StreamToUriTransferJob.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/StreamToUriTransferJob.cs @@ -103,7 +103,7 @@ public override async IAsyncEnumerable ProcessJobToJobPartAsync { if (!part.JobPartStatus.HasCompletedSuccessfully) { - part.JobPartStatus.TrySetTransferStateChange(DataTransferState.Queued); + part.JobPartStatus.SetTransferStateChange(DataTransferState.Queued); yield return part; } } diff --git a/sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs b/sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs index 53724e47b415..f60858399372 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs @@ -150,7 +150,7 @@ private TransferJobInternal( Argument.AssertNotNull(clientDiagnostics, nameof(clientDiagnostics)); _dataTransfer = dataTransfer ?? throw Errors.ArgumentNull(nameof(dataTransfer)); - _dataTransfer.TransferStatus.TrySetTransferStateChange(DataTransferState.Queued); + _dataTransfer.TransferStatus.SetTransferStateChange(DataTransferState.Queued); _checkpointer = checkPointer; _arrayPool = arrayPool; _jobParts = new List(); @@ -344,7 +344,7 @@ public async Task JobPartEvent(TransferStatusEventArgs args) } else if (jobPartStatus.HasFailedItems) { - if (_dataTransfer._state.TrySetFailedItemsState()) + if (_dataTransfer._state.SetFailedItemsState()) { await SetCheckpointerStatus().ConfigureAwait(false); await OnJobPartStatusChangedAsync().ConfigureAwait(false); @@ -352,7 +352,7 @@ public async Task JobPartEvent(TransferStatusEventArgs args) } else if (jobPartStatus.HasSkippedItems) { - if (_dataTransfer._state.TrySetSkippedItemsState()) + if (_dataTransfer._state.SetSkippedItemsState()) { await SetCheckpointerStatus().ConfigureAwait(false); await OnJobPartStatusChangedAsync().ConfigureAwait(false); @@ -387,7 +387,7 @@ public async Task JobPartEvent(TransferStatusEventArgs args) public async Task OnJobStateChangedAsync(DataTransferState state) { - if (_dataTransfer._state.TrySetTransferState(state)) + if (_dataTransfer._state.SetTransferState(state)) { // If we are in a final state, dispose the JobPartEvent handlers if (state == DataTransferState.Completed || diff --git a/sdk/storage/Azure.Storage.DataMovement/src/UriToStreamTransferJob.cs b/sdk/storage/Azure.Storage.DataMovement/src/UriToStreamTransferJob.cs index 7a3dfdfdf2dd..d94490bdd9ad 100644 --- a/sdk/storage/Azure.Storage.DataMovement/src/UriToStreamTransferJob.cs +++ b/sdk/storage/Azure.Storage.DataMovement/src/UriToStreamTransferJob.cs @@ -103,7 +103,7 @@ public override async IAsyncEnumerable ProcessJobToJobPartAsync { if (!part.JobPartStatus.HasCompletedSuccessfully) { - part.JobPartStatus.TrySetTransferStateChange(DataTransferState.Queued); + part.JobPartStatus.SetTransferStateChange(DataTransferState.Queued); yield return part; } } diff --git a/sdk/storage/Azure.Storage.DataMovement/tests/DataTransferTests.cs b/sdk/storage/Azure.Storage.DataMovement/tests/DataTransferTests.cs index 168c37b7c345..02f434ceaf37 100644 --- a/sdk/storage/Azure.Storage.DataMovement/tests/DataTransferTests.cs +++ b/sdk/storage/Azure.Storage.DataMovement/tests/DataTransferTests.cs @@ -161,10 +161,7 @@ public async Task TryPauseAsync() Assert.AreEqual(DataTransferState.Pausing, transfer.TransferStatus.State); // Assert - if (!transfer._state.TrySetTransferState(DataTransferState.Paused)) - { - Assert.Fail("Unable to set the transfer status internally to the DataTransfer."); - } + Assert.IsTrue(transfer._state.SetTransferState(DataTransferState.Paused)); await pauseTask;