[MetricsAdvisor] Fixed UpdateHook methods

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/api/Azure.AI.MetricsAdvisor.netstandard2.0.cs

@@ -299,8 +299,8 @@ public MetricsAdvisorAdministrationClient(System.Uri endpoint, Azure.Core.TokenC
         public virtual System.Threading.Tasks.Task<Azure.Response> UpdateDataFeedAsync(string dataFeedId, Azure.AI.MetricsAdvisor.Models.DataFeed dataFeed, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
         public virtual Azure.Response UpdateDetectionConfiguration(string detectionConfigurationId, Azure.AI.MetricsAdvisor.Models.AnomalyDetectionConfiguration detectionConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
         public virtual System.Threading.Tasks.Task<Azure.Response> UpdateDetectionConfigurationAsync(string detectionConfigurationId, Azure.AI.MetricsAdvisor.Models.AnomalyDetectionConfiguration detectionConfiguration, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
-        public virtual Azure.Response UpdateHook(string hookId, Azure.AI.MetricsAdvisor.Models.NotificationHook hook, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
-        public virtual System.Threading.Tasks.Task<Azure.Response> UpdateHookAsync(string hookId, Azure.AI.MetricsAdvisor.Models.NotificationHook hook, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
+        public virtual Azure.Response<Azure.AI.MetricsAdvisor.Models.NotificationHook> UpdateHook(Azure.AI.MetricsAdvisor.Models.NotificationHook hook, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
+        public virtual System.Threading.Tasks.Task<Azure.Response<Azure.AI.MetricsAdvisor.Models.NotificationHook>> UpdateHookAsync(Azure.AI.MetricsAdvisor.Models.NotificationHook hook, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
     }
 }
 namespace Azure.AI.MetricsAdvisor.Models

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/src/MetricsAdvisorAdministrationClient.cs

@@ -1588,22 +1588,27 @@ public virtual Response<NotificationHook> CreateHook(NotificationHook hook, Canc
         }
 
         /// <summary>
-        /// Updates an existing <see cref="NotificationHook"/>.
+        /// Updates an existing <see cref="NotificationHook"/>. In order to update your hook, you cannot create a <see cref="NotificationHook"/>
+        /// directly from its constructor. You need to obtain an instance via <see cref="GetHookAsync"/> or another CRUD operation and update it
+        /// before calling this method.
         /// </summary>
-        /// <param name="hookId">The ID of the existing <see cref="NotificationHook"/> to update.</param>
         /// <param name="hook">The <see cref="NotificationHook"/> model containing the updates to be applied.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
         /// <returns>
         /// A <see cref="Response"/> containing the result of the operation.
         /// </returns>
-        /// <exception cref="ArgumentNullException"><paramref name="hookId"/> or <paramref name="hook"/> is null.</exception>
-        /// <exception cref="ArgumentException"><paramref name="hookId"/> is empty or not a valid GUID.</exception>
-        public virtual async Task<Response> UpdateHookAsync(string hookId, NotificationHook hook, CancellationToken cancellationToken = default)
+        /// <exception cref="ArgumentNullException"><paramref name="hook"/> or <paramref name="hook"/>.Id is null.</exception>
+        public virtual async Task<Response<NotificationHook>> UpdateHookAsync(NotificationHook hook, CancellationToken cancellationToken = default)
         {
-            /*
-            Guid hookGuid = ClientCommon.ValidateGuid(hookId, nameof(hookId));
             Argument.AssertNotNull(hook, nameof(hook));
 
+            if (hook.Id == null)
+            {
+                throw new ArgumentNullException(nameof(hook), $"{nameof(hook)}.Id not available. Call {nameof(GetHookAsync)} and update the returned model before calling this method.");
+            }
+
+            Guid hookGuid = new Guid(hook.Id);
+
             using DiagnosticScope scope = _clientDiagnostics.CreateScope($"{nameof(MetricsAdvisorAdministrationClient)}.{nameof(UpdateHook)}");
             scope.Start();
 
@@ -1618,29 +1623,30 @@ public virtual async Task<Response> UpdateHookAsync(string hookId, NotificationH
                 scope.Failed(e);
                 throw;
             }
-            */
-
-            await Task.CompletedTask.ConfigureAwait(false);
-            return default;
         }
 
         /// <summary>
-        /// Updates an existing <see cref="NotificationHook"/>.
+        /// Updates an existing <see cref="NotificationHook"/>. In order to update your hook, you cannot create a <see cref="NotificationHook"/>
+        /// directly from its constructor. You need to obtain an instance via <see cref="GetHook"/> or another CRUD operation and update it
+        /// before calling this method.
         /// </summary>
-        /// <param name="hookId">The ID of the existing <see cref="NotificationHook"/> to update.</param>
         /// <param name="hook">The <see cref="NotificationHook"/> model containing the updates to be applied.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
         /// <returns>
         /// A <see cref="Response"/> containing the result of the operation.
         /// </returns>
-        /// <exception cref="ArgumentNullException"><paramref name="hookId"/> or <paramref name="hook"/> is null.</exception>
-        /// <exception cref="ArgumentException"><paramref name="hookId"/> is empty or not a valid GUID.</exception>
-        public virtual Response UpdateHook(string hookId, NotificationHook hook, CancellationToken cancellationToken = default)
+        /// <exception cref="ArgumentNullException"><paramref name="hook"/> or <paramref name="hook"/>.Id is null.</exception>
+        public virtual Response<NotificationHook> UpdateHook(NotificationHook hook, CancellationToken cancellationToken = default)
         {
-            /*
-            Guid hookGuid = ClientCommon.ValidateGuid(hookId, nameof(hookId));
             Argument.AssertNotNull(hook, nameof(hook));
 
+            if (hook.Id == null)
+            {
+                throw new ArgumentNullException(nameof(hook), $"{nameof(hook)}.Id not available. Call {nameof(GetHook)} and update the returned model before calling this method.");
+            }
+
+            Guid hookGuid = new Guid(hook.Id);
+
             using DiagnosticScope scope = _clientDiagnostics.CreateScope($"{nameof(MetricsAdvisorAdministrationClient)}.{nameof(UpdateHook)}");
             scope.Start();
 
@@ -1655,9 +1661,6 @@ public virtual Response UpdateHook(string hookId, NotificationHook hook, Cancell
                 scope.Failed(e);
                 throw;
             }
-            */
-
-            return default;
         }
 
         /// <summary>

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/src/Models/CodeGenInternal/EmailHookParameterPatch.cs

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+
+namespace Azure.AI.MetricsAdvisor.Models
+{
+    internal partial class EmailHookParameterPatch
+    {
+        public IList<string> ToList { get; internal set; }
+    }
+}

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/src/Models/CodeGenInternal/WebhookHookParameterPatch.cs

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+
+namespace Azure.AI.MetricsAdvisor.Models
+{
+    internal partial class WebhookHookParameterPatch
+    {
+        public IDictionary<string, string> Headers { get; internal set; }
+    }
+}

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/src/Models/NotificationHook/NotificationHook.cs

@@ -64,12 +64,33 @@ internal NotificationHook(HookType hookType, string id, string name, string desc
 
         internal static HookInfoPatch GetPatchModel(NotificationHook hook)
         {
-            return hook switch
+            HookInfoPatch patch = hook switch
             {
-                EmailNotificationHook h => new EmailHookInfoPatch() { HookName = h.Name, Description = h.Description, ExternalLink = h.ExternalLink?.AbsoluteUri, /*HookParameter = h.HookParameter,*/ Admins = h.Administrators },
-                WebNotificationHook h => new WebhookHookInfoPatch() { HookName = h.Name, Description = h.Description, ExternalLink = h.ExternalLink?.AbsoluteUri, /*HookParameter = h.HookParameter,*/ Admins = h.Administrators },
-                _ => throw new InvalidOperationException("Unknown AlertingHook type.")
+                EmailNotificationHook h => new EmailHookInfoPatch()
+                {
+                    HookParameter = new() { ToList = h.EmailsToAlert }
+                },
+                WebNotificationHook h => new WebhookHookInfoPatch()
+                {
+                    HookParameter = new()
+                    {
+                        Endpoint = h.Endpoint?.AbsoluteUri,
+                        Username = h.Username,
+                        Password = h.Password,
+                        CertificateKey = h.CertificateKey,
+                        CertificatePassword = h.CertificatePassword,
+                        Headers = h.Headers
+                    }
+                },
+                _ => throw new InvalidOperationException("Unknown hook type.")
             };
+
+            patch.HookName = hook.Name;
+            patch.Description = hook.Description;
+            patch.ExternalLink = hook.ExternalLink?.AbsoluteUri;
+            patch.Admins = hook.Administrators;
+
+            return patch;
         }
     }
 }

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/tests/MetricsAdvisorAdministrationClient/NotificationHookLiveTests.cs

@@ -175,7 +175,6 @@ public async Task CreateAndGetWebNotificationHookWithOptionalMembers()
         [RecordedTest]
         [TestCase(true)]
         [TestCase(false)]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
         public async Task UpdateEmailNotificationHookWithMinimumSetupAndGetInstance(bool useTokenCredential)
         {
             // Create a hook.
@@ -198,7 +197,7 @@ public async Task UpdateEmailNotificationHookWithMinimumSetupAndGetInstance(bool
 
             hookToUpdate.EmailsToAlert.Add("fake3@email.com");
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -239,7 +238,7 @@ public async Task UpdateEmailNotificationHookWithMinimumSetupAndNewInstance()
 
             hookToUpdate.EmailsToAlert.Add("fake3@email.com");
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -255,7 +254,6 @@ public async Task UpdateEmailNotificationHookWithMinimumSetupAndNewInstance()
         }
 
         [RecordedTest]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
         public async Task UpdateEmailNotificationHookWithEveryMemberAndGetInstance()
         {
             // Create a hook.
@@ -281,7 +279,7 @@ public async Task UpdateEmailNotificationHookWithEveryMemberAndGetInstance()
             hookToUpdate.ExternalLink = new Uri("http://fake.endpoint.com/");
             hookToUpdate.EmailsToAlert.Add("fake3@email.com");
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -327,7 +325,7 @@ public async Task UpdateEmailNotificationHookWithEveryMemberAndNewInstance()
 
             hookToUpdate.EmailsToAlert.Add("fake3@email.com");
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -343,7 +341,7 @@ public async Task UpdateEmailNotificationHookWithEveryMemberAndNewInstance()
         }
 
         [RecordedTest]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21504")]
         public async Task UpdateWebNotificationHookWithMinimumSetupAndGetInstance()
         {
             // Create a hook.
@@ -362,7 +360,7 @@ public async Task UpdateWebNotificationHookWithMinimumSetupAndGetInstance()
 
             hookToUpdate.Username = "fakeUsername";
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -384,7 +382,7 @@ public async Task UpdateWebNotificationHookWithMinimumSetupAndGetInstance()
         }
 
         [RecordedTest]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21504")]
         public async Task UpdateWebNotificationHookWithMinimumSetupAndNewInstance()
         {
             // Create a hook.
@@ -402,7 +400,7 @@ public async Task UpdateWebNotificationHookWithMinimumSetupAndNewInstance()
 
             var hookToUpdate = new WebNotificationHook() { Endpoint = endpoint, Username = "fakeUsername" };
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -424,7 +422,7 @@ public async Task UpdateWebNotificationHookWithMinimumSetupAndNewInstance()
         }
 
         [RecordedTest]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21504")]
         public async Task UpdateWebNotificationHookWithEveryMemberAndGetInstance()
         {
             // Create a hook.
@@ -460,7 +458,7 @@ public async Task UpdateWebNotificationHookWithEveryMemberAndGetInstance()
                 hookToUpdate.Headers.Add(header);
             }
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 
@@ -482,7 +480,7 @@ public async Task UpdateWebNotificationHookWithEveryMemberAndGetInstance()
         }
 
         [RecordedTest]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21504")]
         public async Task UpdateWebNotificationHookWithEveryMemberAndNewInstance()
         {
             // Create a hook.
@@ -520,7 +518,7 @@ public async Task UpdateWebNotificationHookWithEveryMemberAndNewInstance()
                 hookToUpdate.Headers.Add(header);
             }
 
-            await adminClient.UpdateHookAsync(disposableHook.Id, hookToUpdate);
+            await adminClient.UpdateHookAsync(hookToUpdate);
 
             // Get the hook and check if updates are in place.
 

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/tests/MetricsAdvisorAdministrationClient/NotificationHookTests.cs

@@ -81,37 +81,33 @@ public void CreateHookRespectsTheCancellationToken()
         }
 
         [Test]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
         public void UpdateHookValidatesArguments()
         {
             MetricsAdvisorAdministrationClient adminClient = GetMetricsAdvisorAdministrationClient();
 
             var hook = new EmailNotificationHook();
 
-            Assert.That(() => adminClient.UpdateHookAsync(null, hook), Throws.InstanceOf<ArgumentNullException>());
-            Assert.That(() => adminClient.UpdateHookAsync("", hook), Throws.InstanceOf<ArgumentException>());
-            Assert.That(() => adminClient.UpdateHookAsync("hookId", hook), Throws.InstanceOf<ArgumentException>().With.InnerException.TypeOf(typeof(FormatException)));
-            Assert.That(() => adminClient.UpdateHookAsync(FakeGuid, null), Throws.InstanceOf<ArgumentNullException>());
+            Assert.That(() => adminClient.UpdateHookAsync(null), Throws.InstanceOf<ArgumentNullException>());
+            Assert.That(() => adminClient.UpdateHook(null), Throws.InstanceOf<ArgumentNullException>());
 
-            Assert.That(() => adminClient.UpdateHook(null, hook), Throws.InstanceOf<ArgumentNullException>());
-            Assert.That(() => adminClient.UpdateHook("", hook), Throws.InstanceOf<ArgumentException>());
-            Assert.That(() => adminClient.UpdateHook("hookId", hook), Throws.InstanceOf<ArgumentException>().With.InnerException.TypeOf(typeof(FormatException)));
-            Assert.That(() => adminClient.UpdateHook(FakeGuid, null), Throws.InstanceOf<ArgumentNullException>());
+            var hookWithNullId = new EmailNotificationHook();
+
+            Assert.That(() => adminClient.UpdateHookAsync(hookWithNullId), Throws.InstanceOf<ArgumentNullException>());
+            Assert.That(() => adminClient.UpdateHook(hookWithNullId), Throws.InstanceOf<ArgumentNullException>());
         }
 
         [Test]
-        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/21177")]
         public void UpdateHookRespectsTheCancellationToken()
         {
             MetricsAdvisorAdministrationClient adminClient = GetMetricsAdvisorAdministrationClient();
 
-            var hook = new EmailNotificationHook();
+            var hook = new EmailNotificationHook(default, FakeGuid, default, default, default, new List<string>(), new EmailHookParameter(new List<string>()));
 
             using var cancellationSource = new CancellationTokenSource();
             cancellationSource.Cancel();
 
-            Assert.That(() => adminClient.UpdateHookAsync(FakeGuid, hook, cancellationSource.Token), Throws.InstanceOf<OperationCanceledException>());
-            Assert.That(() => adminClient.UpdateHook(FakeGuid, hook, cancellationSource.Token), Throws.InstanceOf<OperationCanceledException>());
+            Assert.That(() => adminClient.UpdateHookAsync(hook, cancellationSource.Token), Throws.InstanceOf<OperationCanceledException>());
+            Assert.That(() => adminClient.UpdateHook(hook, cancellationSource.Token), Throws.InstanceOf<OperationCanceledException>());
         }
 
         [Test]

sdk/metricsadvisor/Azure.AI.MetricsAdvisor/tests/Samples/Sample04_HookCrudOperations.cs

@@ -112,13 +112,13 @@ public async Task UpdateHookAsync()
             string originalDescription = hook.Description;
             hook.Description = "This description was generated by a sample.";
 
-            await adminClient.UpdateHookAsync(hookId, hook);
+            await adminClient.UpdateHookAsync(hook);
 
             // Undo the changes to leave the hook unaltered. Skip this step if you intend to keep
             // the changes.
 
             hook.Description = originalDescription;
-            await adminClient.UpdateHookAsync(hookId, hook);
+            await adminClient.UpdateHookAsync(hook);
         }
 
         [Test]