Address CR comments

Author: ogail

src/ResourceManager/Common/Commands.ResourceManager.Common/Properties/Resources.Designer.cs

@@ -151,6 +151,15 @@ Select Y to enable data collection [Y/N]:</value>
   <data name="NoSubscriptionFound" xml:space="preserve">
     <value>The provided account {0} does not have access to any subscriptions. Please try logging in with different credentials.</value>
   </data>
+  <data name="ResourceProviderRegisterAttempt" xml:space="preserve">
+    <value>Attempting to register resource provider '{0}'</value>
+  </data>
+  <data name="ResourceProviderRegisterFailure" xml:space="preserve">
+    <value>Failed to register resource provider '{0}'.Details: '{1}'</value>
+  </data>
+  <data name="ResourceProviderRegisterSuccessful" xml:space="preserve">
+    <value>Succeeded to register resource provider '{0}'</value>
+  </data>
   <data name="SubscriptionIdNotFound" xml:space="preserve">
     <value>The provided account {0} does not have access to subscription ID "{1}". Please try logging in with different credentials not a different subscription ID.</value>
   </data>
@@ -160,4 +169,4 @@ Select Y to enable data collection [Y/N]:</value>
   <data name="TenantNotFound" xml:space="preserve">
     <value>Tenant '{0}' was not found. Please verify that your account has access to this tenant.</value>
   </data>
-</root>
+</root>

src/ResourceManager/Common/Commands.ResourceManager.Common/Properties/Resources.resx

@@ -19,31 +19,34 @@
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Azure.Commands.ResourceManager.Common.Properties;
 using Microsoft.Azure.Common.Authentication;
 using Microsoft.Azure.Common.Authentication.Models;
 using Microsoft.Azure.Management.Resources;
 using Microsoft.Azure.Management.Resources.Models;
+using Microsoft.WindowsAzure.Commands.Utilities.Common;
 
 namespace Microsoft.Azure.Common.Authentication.Models
 {
-    public class RPRegistrationDelegatingHandler : DelegatingHandler
+    public class RPRegistrationDelegatingHandler : DelegatingHandler, ICloneable
     {
+        private const short RetryCount = 3;
+
         /// <summary>
         /// Contains all providers we have attempted to register 
         /// </summary>
         private HashSet<string> registeredProviders;
 
         private Func<ResourceManagementClient> createClient;
 
-        private Action<string> writeVerbose;
-
+        private Action<string> writeDebug;
 
         public ResourceManagementClient ResourceManagementClient { get; set; }
 
-        public RPRegistrationDelegatingHandler(Func<ResourceManagementClient> createClient, Action<string> writeVerbose)
+        public RPRegistrationDelegatingHandler(Func<ResourceManagementClient> createClient, Action<string> writeDebug)
         {
             registeredProviders = new HashSet<string>();
-            this.writeVerbose = writeVerbose;
+            this.writeDebug = writeDebug;
             this.createClient = createClient;
         }
 
@@ -57,21 +60,29 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                 var providerName = GetProviderName(request.RequestUri);
                 if (!string.IsNullOrEmpty(providerName) && !registeredProviders.Contains(providerName))
                 {
+                    // Force dispose for response messages to reclaim the used socket connection.
+                    responseMessage.Dispose();
                     registeredProviders.Add(providerName);
                     try
                     {
-                        writeVerbose(string.Format("Attempting to register resource provider '{0}'", providerName));
+                        writeDebug(string.Format(Resources.ResourceProviderRegisterAttempt, providerName));
                         ResourceManagementClient.Providers.Register(providerName);
                         Provider provider = null;
+                        short retryCount = 0;
                         do
                         {
+                            if (retryCount++ > RetryCount)
+                            {
+                                throw new TimeoutException();
+                            }
                             provider = ResourceManagementClient.Providers.Get(providerName).Provider;
+                            TestMockSupport.Delay(1000);
                         } while (provider.RegistrationState != RegistrationState.Registered.ToString());
-                        writeVerbose(string.Format("Succeeded to register resource provider '{0}'", providerName));
+                        writeDebug(string.Format(Resources.ResourceProviderRegisterSuccessful, providerName));
                     }
-                    catch
+                    catch (Exception e)
                     {
-                        writeVerbose(string.Format("Failed to register resource provider '{0}'", providerName));
+                        writeDebug(string.Format(Resources.ResourceProviderRegisterFailure, providerName, e.Message));
                         // Ignore RP registration errors.
                     }
 
@@ -99,5 +110,10 @@ private string GetProviderName(Uri requestUri)
             return (requestUri.Segments.Length > 7 && requestUri.Segments[5].ToLower() == "providers/") ?
                 requestUri.Segments[6].ToLower().Trim('/') : null;
         }
+
+        public object Clone()
+        {
+            return new RPRegistrationDelegatingHandler(createClient, writeDebug);
+        }
     }
 }

src/ResourceManager/Common/Commands.ResourceManager.Common/RPRegistrationDelegatingHandler.cs

@@ -19,6 +19,7 @@
 using Microsoft.Azure.Common.Authentication;
 using Microsoft.Azure.Commands.ResourceManager.Common;
 using Microsoft.WindowsAzure.Commands.Common;
+using Microsoft.WindowsAzure.Commands.Utilities.Common;
 
 namespace Microsoft.WindowsAzure.Commands.Test.Utilities.Common
 {
@@ -64,6 +65,7 @@ public void BaseSetup()
             }
 
             AzureSession.AuthenticationFactory = new MockTokenAuthenticationFactory();
+            TestMockSupport.RunningMocked = true;
         }
     }
 }

src/ResourceManager/Common/Commands.ScenarioTests.ResourceManager.Common/RMTestBase.cs

@@ -181,15 +181,67 @@ public void DoesNotInvokeRegistrationForIncompatibleUri()
             Assert.Equal(response.StatusCode, HttpStatusCode.Conflict);
             Assert.Equal(response.Content.ReadAsStringAsync().Result, "registered to use namespace");
         }
+
+        [Fact]
+        public void DoesNotHangForLongRegistrationCalls()
+        {
+            // Setup
+            Mock<ResourceManagementClient> mockClient = new Mock<ResourceManagementClient>();
+            Mock<IProviderOperations> mockProvidersOperations = new Mock<IProviderOperations>();
+            mockClient.Setup(f => f.Providers).Returns(mockProvidersOperations.Object);
+            mockProvidersOperations.Setup(f => f.GetAsync(It.IsAny<string>(), It.IsAny<CancellationToken>())).Returns(
+                (string rp, CancellationToken token) =>
+                {
+                    ProviderGetResult r = new ProviderGetResult
+                    {
+                        Provider = new Provider
+                        {
+                            RegistrationState = RegistrationState.Pending.ToString()
+                        }
+                    };
+
+                    return Task.FromResult(r);
+                }
+                );
+            HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, compatibleUri);
+            Dictionary<HttpRequestMessage, List<HttpResponseMessage>> mapping = new Dictionary<HttpRequestMessage, List<HttpResponseMessage>>
+            {
+                {
+                    request, new List<HttpResponseMessage>
+                    {
+                        new HttpResponseMessage(HttpStatusCode.Conflict) { Content = new StringContent("registered to use namespace") },
+                        new HttpResponseMessage(HttpStatusCode.Conflict) { Content = new StringContent("registered to use namespace") }
+                    }
+                }
+            };
+            List<string> msgs = new List<string>();
+            RPRegistrationDelegatingHandler rpHandler = new RPRegistrationDelegatingHandler(() => mockClient.Object, s => msgs.Add(s))
+            {
+                InnerHandler = new MockResponseDelegatingHandler(mapping)
+            };
+            HttpClient httpClient = new HttpClient(rpHandler);
+
+            // Test
+            HttpResponseMessage response = httpClient.SendAsync(request).Result;
+
+            // Assert
+            Assert.True(msgs.Any(s => s.Equals("Failed to register resource provider 'microsoft.compute'.Details: 'The operation has timed out.'")));
+            Assert.Equal(response.StatusCode, HttpStatusCode.Conflict);
+            Assert.Equal(response.Content.ReadAsStringAsync().Result, "registered to use namespace");
+            mockProvidersOperations.Verify(f => f.RegisterAsync("microsoft.compute", It.IsAny<CancellationToken>()), Times.AtMost(4));
+        }
     }
 
     public class MockResponseDelegatingHandler : DelegatingHandler
     {
         Dictionary<HttpRequestMessage, List<HttpResponseMessage>> mapping;
 
+        public int RequestsCount { get; set; }
+
         public MockResponseDelegatingHandler(Dictionary<HttpRequestMessage, List<HttpResponseMessage>> mapping)
         {
             this.mapping = mapping;
+            this.RequestsCount = 0;
         }
 
         protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
@@ -198,9 +250,9 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             {
                 var response = mapping[request].First();
                 mapping[request].Remove(response);
+                RequestsCount++;
                 return response;
             });
         }
-
     }
-}
+}