Giuliov/issue 142 (#143)

* first attempt

* fix test

* Check if mapping was deleted (#142)

* review exit codes

Author: giuliov

src/aggregator-cli/CommandBase.cs

@@ -43,15 +43,18 @@ internal int Run(CancellationToken cancellationToken)
                 t.Wait(cancellationToken);
                 cancellationToken.ThrowIfCancellationRequested();
                 int rc = t.Result;
-                if (rc != 0)
+                if (rc == ExitCodes.Success)
                 {
-                    Logger.WriteError("Failed!");
+                    Logger.WriteSuccess("Succeeded");
+                }
+                else if (rc == ExitCodes.NotFound)
+                {
+                    Logger.WriteWarning("Not found");
                 }
                 else
                 {
-                    Logger.WriteSuccess("Succeeded");
+                    Logger.WriteError("Failed!");
                 }
-
                 return rc;
             }
             catch (Exception ex)
@@ -61,7 +64,7 @@ internal int Run(CancellationToken cancellationToken)
                     ? ex.Message
                     : ex.InnerException.Message
                     );
-                return 99;
+                return ExitCodes.Unexpected;
             }
         }
 

src/aggregator-cli/ExitCodes.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace aggregator.cli
+{
+    internal static class ExitCodes
+    {
+        public const int Success = 0;
+        public const int Failure = 1;
+        public const int InvalidArguments = 2;
+        public const int NotFound = 3;
+        public const int Unexpected = 99;
+    }
+}

src/aggregator-cli/Instances/ConfigureInstanceCommand.cs

@@ -36,16 +36,16 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
                 .BuildAsync(cancellationToken);
             var instances = new AggregatorInstances(context.Azure, context.Logger, context.Naming);
             var instance = context.Naming.Instance(Name, ResourceGroup);
-            bool ok = false;
             if (Authentication)
             {
-                ok = await instances.ChangeAppSettingsAsync(instance, Location, SaveMode, cancellationToken);
-            } else
+                bool ok = await instances.ChangeAppSettingsAsync(instance, Location, SaveMode, cancellationToken);
+                return ok ? ExitCodes.Success : ExitCodes.Failure;
+            }
+            else
             {
                 context.Logger.WriteError($"Unsupported command option(s)");
+                return ExitCodes.InvalidArguments;
             }
-
-            return ok ? 0 : 1;
         }
     }
-}
+}

src/aggregator-cli/Instances/InstallInstanceCommand.cs

@@ -44,12 +44,12 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (!validHostingPlanSkus.Contains(HostingPlanSku))
             {
                 Logger.WriteError($"Invalid value for hostingPlanSku: must be one of {String.Join(",", validHostingPlanSkus)}");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
             if (!validHostingPlanTiers.Contains(HostingPlanTier))
             {
                 Logger.WriteError($"Invalid value for hostingPlanTier: must be one of {String.Join(",", validHostingPlanTiers)}");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
             var tuning = new AggregatorInstances.InstanceFineTuning
             {
@@ -65,7 +65,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var instances = new AggregatorInstances(context.Azure, context.Logger, context.Naming);
             var instance = context.Naming.GetInstanceCreateNames(Name, ResourceGroup);
             bool ok = await instances.AddAsync(instance, Location, RequiredVersion, SourceUrl, tuning, cancellationToken);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
     }
 }

src/aggregator-cli/Instances/ListInstancesCommand.cs

@@ -50,7 +50,7 @@ private async Task<int> ListByLocationAsync(CommandContext context, AggregatorIn
             {
                 context.Logger.WriteInfo($"No aggregator instances found in {Location} Region.");
             }
-            return 0;
+            return ExitCodes.Success;
         }
 
         private async Task<int> ListInResourceGroupAsync(CommandContext context, AggregatorInstances instances, CancellationToken cancellationToken)
@@ -68,7 +68,7 @@ private async Task<int> ListInResourceGroupAsync(CommandContext context, Aggrega
                 context.Logger.WriteInfo($"No aggregator instances found in {ResourceGroup} Resource Group.");
             }
 
-            return 0;
+            return ExitCodes.Success;
         }
 
         private static async Task<int> ListAllAsync(CommandContext context, AggregatorInstances instances, CancellationToken cancellationToken)
@@ -80,13 +80,15 @@ private static async Task<int> ListAllAsync(CommandContext context, AggregatorIn
                 context.Logger.WriteOutput(dataObject);
                 any = true;
             }
-
             if (!any)
             {
                 context.Logger.WriteInfo("No aggregator instances found.");
+                return ExitCodes.NotFound;
+            }
+            else
+            {
+                return ExitCodes.Success;
             }
-
-            return 0;
         }
     }
 }

src/aggregator-cli/Instances/StreamLogsCommand.cs

@@ -24,7 +24,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var instance = context.Naming.Instance(Instance, ResourceGroup);
             var instances = new AggregatorInstances(context.Azure, context.Logger, context.Naming);
             bool ok = await instances.StreamLogsAsync(instance, cancellationToken);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
     }
 }

src/aggregator-cli/Instances/UninstallInstanceCommand.cs

@@ -39,7 +39,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
 
             var instances = new AggregatorInstances(context.Azure, context.Logger, context.Naming);
             var ok = await instances.RemoveAsync(instance, Location);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
     }
 }

src/aggregator-cli/Instances/UpdateInstance.cs

@@ -32,7 +32,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var instance = context.Naming.Instance(Instance, ResourceGroup);
 
             bool ok = await instances.UpdateAsync(instance, RequiredVersion, SourceUrl, cancellationToken);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
     }
 }

src/aggregator-cli/Logon/LogonAzureCommand.cs

@@ -36,7 +36,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (azure == null)
             {
                 context.Logger.WriteError("Invalid azure credentials");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
             // FIX #60: call some read API to validate parameters
             try
@@ -48,9 +48,9 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
                 int nl = ex.Message.IndexOf(Environment.NewLine);
                 string m = nl != -1 ? ex.Message.Remove(nl) : ex.Message;
                 context.Logger.WriteError("Invalid azure credentials: " + m);
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
-            return 0;
+            return ExitCodes.Success;
         }
     }
 }

src/aggregator-cli/Logon/LogonDevOpsCommand.cs

@@ -36,10 +36,10 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (devops == null)
             {
                 context.Logger.WriteError("Invalid Azure DevOps credentials");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
 
-            return 0;
+            return ExitCodes.Success;
         }
     }
 }

src/aggregator-cli/Mappings/AggregatorMappings.cs

@@ -12,6 +12,13 @@
 
 namespace aggregator.cli
 {
+    internal enum RemoveOutcome
+    {
+        Succeeded   = 0,
+        NotFound    = 2,
+        Failed      = 1
+    }
+
     internal class AggregatorMappings
     {
         private readonly VssConnection devops;
@@ -157,17 +164,17 @@ internal async Task<Guid> AddAsync(string projectName, string @event, EventFilte
             return newSubscription.Id;
         }
 
-        internal async Task<bool> RemoveInstanceAsync(InstanceName instance)
+        internal async Task<RemoveOutcome> RemoveInstanceAsync(InstanceName instance)
         {
             return await RemoveRuleEventAsync("*", instance, "*", "*");
         }
 
-        internal async Task<bool> RemoveRuleAsync(InstanceName instance, string rule)
+        internal async Task<RemoveOutcome> RemoveRuleAsync(InstanceName instance, string rule)
         {
             return await RemoveRuleEventAsync("*", instance, "*", rule);
         }
 
-        internal async Task<bool> RemoveRuleEventAsync(string @event, InstanceName instance, string projectName, string rule)
+        internal async Task<RemoveOutcome> RemoveRuleEventAsync(string @event, InstanceName instance, string projectName, string rule)
         {
             logger.WriteInfo($"Querying the Azure DevOps subscriptions for rule(s) {instance.PlainName}/{rule}");
             var serviceHooksClient = devops.GetClient<ServiceHooksPublisherHttpClient>();
@@ -200,14 +207,16 @@ internal async Task<bool> RemoveRuleEventAsync(string @event, InstanceName insta
                                                 .StartsWith(invocationUrl, StringComparison.OrdinalIgnoreCase));
             }
 
+            uint count = 0;
             foreach (var ruleSub in ruleSubs)
             {
                 logger.WriteVerbose($"Deleting subscription {ruleSub.EventDescription} {ruleSub.EventType}...");
                 await serviceHooksClient.DeleteSubscriptionAsync(ruleSub.Id);
                 logger.WriteInfo($"Subscription {ruleSub.EventDescription} {ruleSub.EventType} deleted.");
+                count++;
             }
 
-            return true;
+            return count > 0 ? RemoveOutcome.Succeeded : RemoveOutcome.NotFound;
         }
     }
 
@@ -261,4 +270,4 @@ public static IEnumerable<InputFilterCondition> ToFilterConditions(this EventFil
                                            });
         }
     }
-}
+}

src/aggregator-cli/Mappings/ListMappingsCommand.cs

@@ -29,7 +29,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
                 && string.IsNullOrEmpty(Project))
             {
                 context.Logger.WriteError("Specify at least one filtering parameter.");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
             var instance = string.IsNullOrEmpty(Instance) ? null : context.Naming.Instance(Instance, ResourceGroup);
             // HACK we pass null as the next calls do not use the Azure connection
@@ -44,8 +44,12 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (!any)
             {
                 context.Logger.WriteInfo("No rule mappings found.");
+                return ExitCodes.NotFound;
+            }
+            else
+            {
+                return ExitCodes.Success;
             }
-            return 0;
         }
     }
 }

src/aggregator-cli/Mappings/MapRuleCommand.cs

@@ -49,7 +49,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (!ok)
             {
                 context.Logger.WriteError($"Invalid event type.");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
 
             var filters = new EventFilters
@@ -62,7 +62,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
 
             var instance = context.Naming.Instance(Instance, ResourceGroup);
             var id = await mappings.AddAsync(Project, Event, filters, instance, Rule, ImpersonateExecution, cancellationToken);
-            return id.Equals(Guid.Empty) ? 1 : 0;
+            return id.Equals(Guid.Empty) ? ExitCodes.Failure : ExitCodes.Success;
         }
     }
 }

src/aggregator-cli/Mappings/UnmapRuleCommand.cs

@@ -31,16 +31,27 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
                 .WithAzureLogon()
                 .WithDevOpsLogon()
                 .BuildAsync(cancellationToken);
-            bool ok = DevOpsEvents.IsValidEvent(Event);
+            bool ok = DevOpsEvents.IsValidEvent(Event) || Event == "*";
             if (!ok)
             {
                 context.Logger.WriteError($"Invalid event type.");
-                return 2;
+                return ExitCodes.InvalidArguments;
             }
             var instance = context.Naming.Instance(Instance, ResourceGroup);
             var mappings = new AggregatorMappings(context.Devops, context.Azure, context.Logger, context.Naming);
-            ok = await mappings.RemoveRuleEventAsync(Event, instance, Project, Rule);
-            return ok ? 0 : 1;
+            var outcome = await mappings.RemoveRuleEventAsync(Event, instance, Project, Rule);
+            switch (outcome)
+            {
+                case RemoveOutcome.Succeeded:
+                    return ExitCodes.Success;
+                case RemoveOutcome.NotFound:
+                    context.Logger.WriteWarning($"No mapping(s) found for rule(s) {instance.PlainName}/{Rule}");
+                    return ExitCodes.NotFound;
+                case RemoveOutcome.Failed:
+                    return ExitCodes.Failure;
+                default:
+                    return ExitCodes.Unexpected;
+            }
         }
     }
 }

src/aggregator-cli/Program.cs

@@ -70,7 +70,7 @@ void cancelEventHandler(object sender, ConsoleCancelEventArgs e)
                     typeof(ListMappingsCommand), typeof(MapRuleCommand), typeof(UnmapRuleCommand)
                 };
                 var parserResult = parser.ParseArguments(args, types);
-                int rc = -1;
+                int rc = ExitCodes.Unexpected;
                 var cancellationToken = cancellationTokenSource.Token;
                 parserResult
                     .WithParsed<CreateTestCommand>(cmd => rc = cmd.Run(cancellationToken))
@@ -96,7 +96,7 @@ void cancelEventHandler(object sender, ConsoleCancelEventArgs e)
                     {
                         var helpText = HelpText.AutoBuild(parserResult);
                         Console.Error.Write(helpText);
-                        rc = 1;
+                        rc = ExitCodes.InvalidArguments;
                     });
 
                 Console.ForegroundColor = save;

src/aggregator-cli/Rules/AddRuleCommand.cs

@@ -30,7 +30,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var instance = context.Naming.Instance(Instance, ResourceGroup);
             var rules = new AggregatorRules(context.Azure, context.Logger);
             bool ok = await rules.AddAsync(instance, Name, File, cancellationToken);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
     }
 }

src/aggregator-cli/Rules/ConfigureRuleCommand.cs

@@ -42,7 +42,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var impersonate = GetEnableStatus(DisableImpersonateExecution, EnableImpersonateExecution);
 
             var ok = await rules.ConfigureAsync(instance, Name, disable, impersonate, cancellationToken);
-            return ok ? 0 : 1;
+            return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
 
 

src/aggregator-cli/Rules/InvokeRuleCommand.cs

@@ -58,14 +58,14 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (Local)
             {
                 bool ok = await rules.InvokeLocalAsync(Project, Event, WorkItemId, Source, DryRun, SaveMode, ImpersonateExecution, cancellationToken);
-                return ok ? 0 : 1;
+                return ok ? ExitCodes.Success : ExitCodes.Failure;
             }
             else
             {
                 var instance = context.Naming.Instance(Instance, ResourceGroup);
                 context.Logger.WriteWarning("Untested feature!");
                 bool ok = await rules.InvokeRemoteAsync(Account, Project, Event, WorkItemId, instance, Name, DryRun, SaveMode, ImpersonateExecution, cancellationToken);
-                return ok ? 0 : 1;
+                return ok ? ExitCodes.Success : ExitCodes.Failure;
             }
         }
     }

src/aggregator-cli/Rules/ListRulesCommand.cs

@@ -34,9 +34,12 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             if (!any)
             {
                 context.Logger.WriteInfo($"No rules found in aggregator instance {instance.PlainName}.");
+                return ExitCodes.NotFound;
+            }
+            else
+            {
+                return ExitCodes.Success;
             }
-
-            return 0;
         }
     }
 }