[Storage] [DataMovement] Refactor Try-style methods (#46645)

nickliu-msft · web-flow · commit 39986d6296be · 2024-10-16T15:03:38.000-04:00
* Initial commit

* Changed wording of doc comments
diff --git 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
         /// </summary>
         /// <param name="state"></param>
-        /// <returns>Returns whether or not the status has been changed/set</returns>
-        public bool TrySetTransferState(DataTransferState state)
+        /// <returns>Returns whether or not the status has been changed from its original state.</returns>
+        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
@@ -75,8 +75,8 @@ protected internal DataTransferStatus(DataTransferState state, bool hasFailureIt
         ///
         /// This should only be triggered when a failed item has been seen.
         /// </summary>
-        /// <returns>True if <see cref="HasFailedItems"/> was updated. False otherwise.</returns>
-        internal bool TrySetFailedItem()
+        /// <returns>True if this was the first time <see cref="HasFailedItems"/> was updated. False otherwise.</returns>
+        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.
         /// </summary>
-        /// /// <returns>True if <see cref="HasSkippedItems"/> was updated. False otherwise.</returns>
-        internal bool TrySetSkippedItem()
+        /// /// <returns>True if this was the first time <see cref="HasSkippedItems"/> was updated. False otherwise.</returns>
+        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.
         /// </summary>
-        /// <returns>True if <see cref="State"/> was updated. False otherwise.</returns>
-        internal bool TrySetTransferStateChange(DataTransferState state)
+        /// <returns>True if <see cref="State"/> was changed from its original state. False otherwise.</returns>
+        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
@@ -275,7 +275,7 @@ internal async Task TriggerCancellationAsync()
         /// <param name="transferState"></param>
         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
@@ -105,7 +105,7 @@ public override async IAsyncEnumerable<JobPartInternal> 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
@@ -103,7 +103,7 @@ public override async IAsyncEnumerable<JobPartInternal> 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
@@ -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<JobPartInternal>();
@@ -344,15 +344,15 @@ 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);
                 }
             }
             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
@@ -103,7 +103,7 @@ public override async IAsyncEnumerable<JobPartInternal> 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
@@ -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;
 

Original file line number	Diff line number	Diff line change
`@@ -75,8 +75,8 @@ protected internal DataTransferStatus(DataTransferState state, bool hasFailureIt`
`75`	`75`	`///`
`76`	`76`	`/// This should only be triggered when a failed item has been seen.`
`77`	`77`	`/// </summary>`
`78`		`- /// <returns>True if <see cref="HasFailedItems"/> was updated. False otherwise.</returns>`
`79`		`- internal bool TrySetFailedItem()`
	`78`	`+ /// <returns>True if this was the first time <see cref="HasFailedItems"/> was updated. False otherwise.</returns>`
	`79`	`+ internal bool SetFailedItem()`
`80`	`80`	`{`
`81`	`81`	`return Interlocked.Exchange(ref _hasFailedItemValue, 1) != 1;`
`82`	`82`	`}`
`@@ -86,8 +86,8 @@ internal bool TrySetFailedItem()`
`86`	`86`	`///`
`87`	`87`	`/// This should only be triggered when a skipped item has been seen.`
`88`	`88`	`/// </summary>`
`89`		`- /// /// <returns>True if <see cref="HasSkippedItems"/> was updated. False otherwise.</returns>`
`90`		`- internal bool TrySetSkippedItem()`
	`89`	`+ /// /// <returns>True if this was the first time <see cref="HasSkippedItems"/> was updated. False otherwise.</returns>`
	`90`	`+ internal bool SetSkippedItem()`
`91`	`91`	`{`
`92`	`92`	`return Interlocked.Exchange(ref _hasSkippedItemValue, 1) != 1;`
`93`	`93`	`}`
`@@ -98,8 +98,8 @@ internal bool TrySetSkippedItem()`
`98`	`98`	`///`
`99`	`99`	`/// This should only be triggered when the state updates.`
`100`	`100`	`/// </summary>`
`101`		`- /// <returns>True if <see cref="State"/> was updated. False otherwise.</returns>`
`102`		`- internal bool TrySetTransferStateChange(DataTransferState state)`
	`101`	`+ /// <returns>True if <see cref="State"/> was changed from its original state. False otherwise.</returns>`
	`102`	`+ internal bool SetTransferStateChange(DataTransferState state)`
`103`	`103`	`{`
`104`	`104`	`return Interlocked.Exchange(ref _stateValue, (int)state) != (int)state;`
`105`	`105`	`}`
Original file line number	Diff line number	Diff line change
`@@ -275,7 +275,7 @@ internal async Task TriggerCancellationAsync()`
`275`	`275`	`/// <param name="transferState"></param>`
`276`	`276`	`internal async Task OnTransferStateChangedAsync(DataTransferState transferState)`
`277`	`277`	`{`
`278`		`- if (JobPartStatus.TrySetTransferStateChange(transferState))`
	`278`	`+ if (JobPartStatus.SetTransferStateChange(transferState))`
`279`	`279`	`{`
`280`	`280`	`// Progress tracking, do before invoking the event below`
`281`	`281`	`if (transferState == DataTransferState.InProgress)`
`@@ -355,7 +355,7 @@ await TransferSkippedEventHandler.RaiseAsync(`
`355`	`355`
`356`	`356`	`// Update the JobPartStatus. If was already updated (e.g. there was a failed item before)`
`357`	`357`	`// then don't raise the PartTransferStatusEventHandler`
`358`		`- if (JobPartStatus.TrySetSkippedItem())`
	`358`	`+ if (JobPartStatus.SetSkippedItem())`
`359`	`359`	`{`
`360`	`360`	`await PartTransferStatusEventHandler.RaiseAsync(`
`361`	`361`	`new TransferStatusEventArgs(`
`@@ -409,7 +409,7 @@ await TransferFailedEventHandler.RaiseAsync(`
`409`	`409`
`410`	`410`	`// Update the JobPartStatus. If was already updated (e.g. there was a failed item before)`
`411`	`411`	`// then don't raise the PartTransferStatusEventHandler`
`412`		`- if (JobPartStatus.TrySetFailedItem())`
	`412`	`+ if (JobPartStatus.SetFailedItem())`
`413`	`413`	`{`
`414`	`414`	`await PartTransferStatusEventHandler.RaiseAsync(`
`415`	`415`	`new TransferStatusEventArgs(`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ public override async IAsyncEnumerable<JobPartInternal> ProcessJobToJobPartAsync`
`105`	`105`	`{`
`106`	`106`	`if (!part.JobPartStatus.HasCompletedSuccessfully)`
`107`	`107`	`{`
`108`		`- part.JobPartStatus.TrySetTransferStateChange(DataTransferState.Queued);`
	`108`	`+ part.JobPartStatus.SetTransferStateChange(DataTransferState.Queued);`
`109`	`109`	`yield return part;`
`110`	`110`	`}`
`111`	`111`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ public override async IAsyncEnumerable<JobPartInternal> ProcessJobToJobPartAsync`
`103`	`103`	`{`
`104`	`104`	`if (!part.JobPartStatus.HasCompletedSuccessfully)`
`105`	`105`	`{`
`106`		`- part.JobPartStatus.TrySetTransferStateChange(DataTransferState.Queued);`
	`106`	`+ part.JobPartStatus.SetTransferStateChange(DataTransferState.Queued);`
`107`	`107`	`yield return part;`
`108`	`108`	`}`
`109`	`109`	`}`
Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ private TransferJobInternal(`
`150`	`150`	`Argument.AssertNotNull(clientDiagnostics, nameof(clientDiagnostics));`
`151`	`151`
`152`	`152`	`_dataTransfer = dataTransfer ?? throw Errors.ArgumentNull(nameof(dataTransfer));`
`153`		`- _dataTransfer.TransferStatus.TrySetTransferStateChange(DataTransferState.Queued);`
	`153`	`+ _dataTransfer.TransferStatus.SetTransferStateChange(DataTransferState.Queued);`
`154`	`154`	`_checkpointer = checkPointer;`
`155`	`155`	`_arrayPool = arrayPool;`
`156`	`156`	`_jobParts = new List<JobPartInternal>();`
`@@ -344,15 +344,15 @@ public async Task JobPartEvent(TransferStatusEventArgs args)`
`344`	`344`	`}`
`345`	`345`	`else if (jobPartStatus.HasFailedItems)`
`346`	`346`	`{`
`347`		`- if (_dataTransfer._state.TrySetFailedItemsState())`
	`347`	`+ if (_dataTransfer._state.SetFailedItemsState())`
`348`	`348`	`{`
`349`	`349`	`await SetCheckpointerStatus().ConfigureAwait(false);`
`350`	`350`	`await OnJobPartStatusChangedAsync().ConfigureAwait(false);`
`351`	`351`	`}`
`352`	`352`	`}`
`353`	`353`	`else if (jobPartStatus.HasSkippedItems)`
`354`	`354`	`{`
`355`		`- if (_dataTransfer._state.TrySetSkippedItemsState())`
	`355`	`+ if (_dataTransfer._state.SetSkippedItemsState())`
`356`	`356`	`{`
`357`	`357`	`await SetCheckpointerStatus().ConfigureAwait(false);`
`358`	`358`	`await OnJobPartStatusChangedAsync().ConfigureAwait(false);`
`@@ -387,7 +387,7 @@ public async Task JobPartEvent(TransferStatusEventArgs args)`
`387`	`387`
`388`	`388`	`public async Task OnJobStateChangedAsync(DataTransferState state)`
`389`	`389`	`{`
`390`		`- if (_dataTransfer._state.TrySetTransferState(state))`
	`390`	`+ if (_dataTransfer._state.SetTransferState(state))`
`391`	`391`	`{`
`392`	`392`	`// If we are in a final state, dispose the JobPartEvent handlers`
`393`	`393`	`if (state == DataTransferState.Completed \|\|`