Small refactor to HttpPipelineBuilder.BuildInternal for clarity (Azure#27234)

* Small refactor to HttpPipelineBuilder.BuildInternal for clarity

Author: christothes

sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs

@@ -88,14 +88,29 @@ void AddCustomerPolicies(HttpPipelinePosition position)
                 {
                     foreach (var policy in options.Policies)
                     {
-                        if (policy.Position == position)
+                        // skip null policies to ensure that calculations for perCallIndex and perRetryIndex are accurate
+                        if (policy.Position == position && policy.Policy != null)
                         {
                             policies.Add(policy.Policy);
                         }
                     }
                 }
             }
 
+            // A helper to ensure that we only add non-null policies to the policies list
+            // This ensures that calculations for perCallIndex and perRetryIndex are accurate
+            void AddNonNullPolicies(HttpPipelinePolicy[] policiesToAdd)
+            {
+                for (int i = 0; i < policiesToAdd.Length; i++)
+                {
+                    var policy = policiesToAdd[i];
+                    if (policy != null)
+                    {
+                        policies.Add(policy);
+                    }
+                }
+            }
+
             DiagnosticsOptions diagnostics = options.Diagnostics;
 
             var sanitizer = new HttpMessageSanitizer(diagnostics.LoggedQueryParameters.ToArray(), diagnostics.LoggedHeaderNames.ToArray());
@@ -104,11 +119,10 @@ void AddCustomerPolicies(HttpPipelinePosition position)
 
             policies.Add(ReadClientRequestIdPolicy.Shared);
 
-            policies.AddRange(perCallPolicies);
+            AddNonNullPolicies(perCallPolicies);
 
             AddCustomerPolicies(HttpPipelinePosition.PerCall);
 
-            policies.RemoveAll(static policy => policy == null);
             var perCallIndex = policies.Count;
 
             policies.Add(ClientRequestIdPolicy.Shared);
@@ -123,12 +137,10 @@ void AddCustomerPolicies(HttpPipelinePosition position)
 
             policies.Add(RedirectPolicy.Shared);
 
-            policies.AddRange(perRetryPolicies);
+            AddNonNullPolicies(perRetryPolicies);
 
             AddCustomerPolicies(HttpPipelinePosition.PerRetry);
 
-            policies.RemoveAll(static policy => policy == null);
-
             var perRetryIndex = policies.Count;
 
             if (diagnostics.IsLoggingEnabled)
@@ -143,7 +155,6 @@ void AddCustomerPolicies(HttpPipelinePosition position)
             policies.Add(new RequestActivityPolicy(isDistributedTracingEnabled, ClientDiagnostics.GetResourceProviderNamespace(options.GetType().Assembly), sanitizer));
 
             AddCustomerPolicies(HttpPipelinePosition.BeforeTransport);
-            policies.RemoveAll(static policy => policy == null);
 
             // Override the provided Transport with the provided transport options if the transport has not been set after default construction and options are not null.
             HttpPipelineTransport transport = options.Transport;

sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs

@@ -70,6 +70,11 @@ public async Task CustomPolicyOrdering()
                 Assert.False(beforeTransportRan);
             }), HttpPipelinePosition.PerRetry);
 
+            // Intentionally add some null policies to ensure it does not break indexing
+            options.AddPolicy(null, HttpPipelinePosition.PerCall);
+            options.AddPolicy(null, HttpPipelinePosition.PerRetry);
+            options.AddPolicy(null, HttpPipelinePosition.BeforeTransport);
+
             options.AddPolicy(new CallbackPolicy(m =>
             {
                 beforeTransportRan = true;