Proper Success Response Handling

src/Authentication/Authentication/Extensions/PSCmdletExtensions.cs

@@ -1,25 +1,42 @@
 // ------------------------------------------------------------------------------
 //  Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the MIT License.  See License in the project root for license information.
 // ------------------------------------------------------------------------------
-
-namespace Microsoft.Graph.PowerShell.Authentication.Extensions
+namespace Microsoft.Graph.PowerShell
 {
-    using Microsoft.Graph.PowerShell.Authentication.Helpers;
+    using Microsoft.Graph.PowerShell.Authentication.Extensions;
     using System;
     using System.Collections.Generic;
     using System.Collections.ObjectModel;
     using System.Linq.Expressions;
     using System.Management.Automation;
-    internal static class PSCmdletExtensions
+    public static class PSCmdletExtensions
     {
+        /// <summary>
+        /// Overrides OnDefault method in the generated cmdlets.
+        /// </summary>
+        /// <param name="cmdlet">The calling <see cref="PSCmdlet"/></param>
+        /// <param name="responseMessage">The HTTP response message from the service.</param>
+        /// <param name="returnNow">Determines whether the caller should return after OverrideOnDefault is called, or not. </param>
+        public static void OverrideOnDefault(this PSCmdlet cmdlet, global::System.Net.Http.HttpResponseMessage responseMessage, ref global::System.Threading.Tasks.Task<bool> returnNow)
+        {
+            if (responseMessage.IsSuccessStatusCode)
+            {
+                if (cmdlet.MyInvocation?.BoundParameters?.ContainsKey("PassThru") == true)
+                {
+                    cmdlet.WriteObject(true);
+                }
+                returnNow = global::System.Threading.Tasks.Task<bool>.FromResult(true);
+            }
+        }
+
         /// <summary>
         /// Executes a PowerShell script.
         /// </summary>
         /// <typeparam name="T">The output type to return.</typeparam>
         /// <param name="cmdlet">The executing cmdlet.</param>
         /// <param name="contents">The PowerShell script to execute.</param>
         /// <returns>The result for the executed script.</returns>
-        public static List<T> ExecuteScript<T>(this PSCmdlet cmdlet, string contents)
+        internal static List<T> ExecuteScript<T>(this PSCmdlet cmdlet, string contents)
         {
             List<T> output = new List<T>();
 
@@ -50,7 +67,7 @@ public static List<T> ExecuteScript<T>(this PSCmdlet cmdlet, string contents)
         /// <param name="cmdlet">The executing cmdlet.</param>
         /// <param name="parameterName">The name of the parameter to check.</param>
         /// <returns>True is the parameter was set by the user, otherwise false.</returns>
-        public static bool IsParameterBound(this PSCmdlet cmdlet, string parameterName)
+        internal static bool IsParameterBound(this PSCmdlet cmdlet, string parameterName)
         {
             return cmdlet.MyInvocation?.BoundParameters.ContainsKey(parameterName) ?? false;
         }
@@ -63,7 +80,7 @@ public static bool IsParameterBound(this PSCmdlet cmdlet, string parameterName)
         /// <param name="cmdlet">The executing cmdlet.</param>
         /// <param name="propertySelector">The parameter to check</param>
         /// <returns>True is the parameter was set by the user, otherwise false.</returns>
-        public static bool IsParameterBound<TPSCmdlet, TProp>(this TPSCmdlet cmdlet, Expression<Func<TPSCmdlet, TProp>> propertySelector) where TPSCmdlet : PSCmdlet
+        internal static bool IsParameterBound<TPSCmdlet, TProp>(this TPSCmdlet cmdlet, Expression<Func<TPSCmdlet, TProp>> propertySelector) where TPSCmdlet : PSCmdlet
         {
             var propName = ((MemberExpression)propertySelector.Body).Member.Name;
             return cmdlet.IsParameterBound(propName);

src/readme.graph.md

@@ -89,9 +89,7 @@ directive:
   - where:
       parameter-name: Top
     set:
-      parameter-name: PageSize
       alias:
-        - Top
         - Limit
   - where:
       parameter-name: Select
@@ -389,22 +387,38 @@ directive:
         }
         return $;
       }
-# Add custom -All parameter to *_List cmdlets that support Odata next link.
+# Override OnDefault to handle all success, 2xx responses, as success and not error.
   - from: source-file-csharp
     where: $
     transform: >
-      if (!$documentPath.match(/generated%2Fcmdlets%2FGet\w*_List.cs/gm))
+      if (!$documentPath.match(/generated%2Fcmdlets%2F\w*.cs/gm))
+      {
+        return $;
+      } else {
+        let overrideOnDefaultRegex = /(\s*)(partial\s*void\s*overrideOnDefault)/gmi
+        let overrideOnDefaultImplementation = "$1partial void overrideOnDefault(global::System.Net.Http.HttpResponseMessage responseMessage, global::System.Threading.Tasks.Task<Microsoft.Graph.PowerShell.Models.IOdataError> response, ref global::System.Threading.Tasks.Task<bool> returnNow) => this.OverrideOnDefault(responseMessage,ref returnNow);$1$2"
+        $ = $.replace(overrideOnDefaultRegex, overrideOnDefaultImplementation);
+
+        return $;
+      }
+
+# Add custom -PageSize parameter to *_List cmdlets that support Odata next link.
+  - from: source-file-csharp
+    where: $
+    transform: >
+      if (!$documentPath.match(/generated%2Fcmdlets%2FGet\w*_List\d*.cs/gm))
       {
         return $;
       } else {
         let odataNextLinkRegex = /(^\s*)(if\s*\(\s*result.OdataNextLink\s*!=\s*null\s*\))/gmi
         if($.match(odataNextLinkRegex)) {
           $ = $.replace(odataNextLinkRegex, '$1if (result.OdataNextLink != null && this.ShouldIteratePages(this.InvocationInformation.BoundParameters, result.Value.Length))\n$1');
+
           let psBaseClassImplementationRegex = /(\s*:\s*)(global::System.Management.Automation.PSCmdlet)/gmi
           $ = $.replace(psBaseClassImplementationRegex, '$1Microsoft.Graph.PowerShell.Cmdlets.Custom.ListCmdlet');
 
           let beginProcessingRegex = /(^\s*)(protected\s*override\s*void\s*BeginProcessing\(\)\s*{)/gmi
-          $ = $.replace(beginProcessingRegex, '$1$2\n$1$1if (this.InvocationInformation.BoundParameters.ContainsKey("PageSize")){ InitializePaging(ref this.__invocationInfo, ref this._pageSize); }\n$1');
+          $ = $.replace(beginProcessingRegex, '$1$2\n$1  if (this.InvocationInformation?.BoundParameters != null){ InitializePaging(ref this.__invocationInfo, ref this._top); }\n$1');
 
           let odataNextLinkCallRegex = /(^\s*)(await\s*this\.Client\.UsersUserListUser_Call\(requestMessage\,\s*onOk\,\s*onDefault\,\s*this\,\s*Pipeline\)\;)/gmi
           $ = $.replace(odataNextLinkCallRegex, '$1requestMessage.RequestUri = GetOverflowItemsNextLinkUri(requestMessage.RequestUri);\n$1$2');

tools/Custom/ListCmdlet.cs

@@ -3,19 +3,33 @@
 // ------------------------------------------------------------------------------
 namespace Microsoft.Graph.PowerShell.Cmdlets.Custom
 {
+    using System;
+    using System.Management.Automation;
 
     public partial class ListCmdlet : global::System.Management.Automation.PSCmdlet
     {
+        /// <summary>Backing field for <see cref="PageSize" /> property.</summary>
+        private int _pageSize;
+
+        /// <summary>Sets the page size of results.</summary>
+        [global::System.Management.Automation.Parameter(Mandatory = false, HelpMessage = "Sets the page size of results.")]
+        [Microsoft.Graph.PowerShell.Runtime.Info(
+        Required = false,
+        ReadOnly = false,
+        Description = @"The page size of results.",
+        PossibleTypes = new[] { typeof(int) })]
+        [global::Microsoft.Graph.PowerShell.Category(global::Microsoft.Graph.PowerShell.ParameterCategory.Runtime)]
+        public int PageSize { get => this._pageSize; set => this._pageSize = value; }
+
         /// <summary>Backing field for <see cref="All" /> property.</summary>
         private global::System.Management.Automation.SwitchParameter _all;
 
         /// <summary>List All pages</summary>
-        [global::System.Management.Automation.Parameter(Mandatory = false, HelpMessage = "List all pages")]
+        [global::System.Management.Automation.Parameter(Mandatory = false, HelpMessage = "List all pages.")]
         [Microsoft.Graph.PowerShell.Runtime.Info(
         Required = false,
         ReadOnly = false,
-        Description = @"List all pages",
-        SerializedName = @"$all",
+        Description = @"List all pages.",
         PossibleTypes = new[] { typeof(global::System.Management.Automation.SwitchParameter) })]
         [global::Microsoft.Graph.PowerShell.Category(global::Microsoft.Graph.PowerShell.ParameterCategory.Runtime)]
         public global::System.Management.Automation.SwitchParameter All { get => this._all; set => this._all = value; }
@@ -30,11 +44,6 @@ public partial class ListCmdlet : global::System.Management.Automation.PSCmdlet
         /// </summary>
         internal const int MaxPageSize = 999;
 
-        /// <summary>
-        /// Original page size/top/limit passed to Cmdlet via parameter.
-        /// </summary>
-        internal int originalPageSize = 0;
-
         /// <summary>
         /// Total number of pages required to iterate page collections excluding overflow items.
         /// </summary>
@@ -56,39 +65,59 @@ public partial class ListCmdlet : global::System.Management.Automation.PSCmdlet
         /// </summary>
         internal int totalFetchedItems = 0;
 
+        /// <summary>
+        /// Total number of items to be fetched.
+        /// </summary>
+        internal int limit = default;
+
         /// <summary>
         /// Initializes paging values.
         /// </summary>
         /// <param name="invocationInfo">A reference to <see cref="System.Management.Automation.InvocationInfo"/> object.</param>
-        /// <param name="PageSize">A reference to page size parameter.</param>
-        public void InitializePaging(ref global::System.Management.Automation.InvocationInfo invocationInfo, ref int PageSize)
+        /// <param name="top">A reference to top parameter.</param>
+        public void InitializePaging(ref global::System.Management.Automation.InvocationInfo invocationInfo, ref int top)
         {
-            if (PageSize > MaxPageSize)
+            if (invocationInfo.BoundParameters.ContainsKey("PageSize") && (PageSize > MaxPageSize || PageSize == default))
             {
-                invocationInfo.BoundParameters["All"] = true;
-                originalPageSize = PageSize;
-                requiredPages = PageSize / DefaultPageSize;
-                overflowItemsCount = PageSize % DefaultPageSize;
-                invocationInfo.BoundParameters["PageSize"] = DefaultPageSize;
-                PageSize = DefaultPageSize;
+                ThrowTerminatingError(
+                new ErrorRecord(
+                    new ArgumentException($"Invalid page size specified `{PageSize}`. {nameof(PageSize)} must be between 1 and {MaxPageSize} inclusive."),
+                    Guid.NewGuid().ToString(),
+                    ErrorCategory.InvalidArgument,
+                    null));
+            }
+
+            // Move `-Top` parameter to `limit`.
+            if (invocationInfo.BoundParameters.ContainsKey("Top"))
+            {
+                limit = top;
+            }
+
+            int currentPageSize = invocationInfo.BoundParameters.ContainsKey("PageSize") ? PageSize : DefaultPageSize;
+            // Explicitly set `-Top` parameter to currentPageSize in order for the generated cmdlets to construct a URL with a `$top` query parameter.
+            invocationInfo.BoundParameters["Top"] = currentPageSize;
+            top = currentPageSize;
+
+            if (limit != default)
+            {
+                requiredPages = limit / currentPageSize;
+                overflowItemsCount = limit % currentPageSize;
             }
         }
 
         /// <summary>
-        /// Determines whether the cmdlet should follow the OData next link url.
+        /// Determines whether the cmdlet should follow the OData next link URL.
+        /// Iteration will only occur when limit/top is not set, or if there a more items to fetch.
         /// </summary>
         /// <param name="boundParameters">The bound parameters of the cmdlet.</param>
         /// <param name="itemsCount">Current page items count.</param>
-        /// <returns>True if it can iterate pages; False if it can't.</returns>
+        /// <returns>True if it can iterate pages; otherwise False.</returns>
         public bool ShouldIteratePages(global::System.Collections.Generic.Dictionary<string, object> boundParameters, int itemsCount)
         {
             iteratedPages++;
             totalFetchedItems += itemsCount;
-            if ((boundParameters.ContainsKey("All") && !boundParameters.ContainsKey("PageSize")) ||
-                (boundParameters.ContainsKey("PageSize") && totalFetchedItems < originalPageSize))
-                return true;
-            else
-                return false;
+
+            return (boundParameters.ContainsKey("All") && limit == default) || totalFetchedItems < limit;
         }
 
         /// <summary>