Feature: Paging (small breaking change)

[replicaJunction]

Adds support for paging result sets using PowerShell's own paging parameters: -First and -Skip.

Addresses #19.

The -First parameter is functionally identical to the previous -Limit parameter, so I have removed that parameter. I have preserved the default behavior - if -First is not specified, 10 results are returned. However, this will be a breaking change to scripts that provide the -Limit parameter. Those scripts will just need to have that parameter renamed to -First instead. I considered leaving -Limit to avoid this situation, but in my opinion, it's bad design to have redundant parameters, and since the solution is a simple rename (no need to re-think or re-design anything), I don't see it as a huge problem so long as it's documented.

The -Skip parameter provides the ability to start at an offset. Combining this with -First allows you to page through multiple result sets.

Here is a usage example:

function Get-PagedServiceNowRequestItem {
    [CmdletBinding()]
    param(
        [Parameter(Mandatory, Position = 0)]
        [Alias('Splat')]
        [Hashtable] $ServiceNowSplat
        ,

        [Parameter()]
        [int] $PageSize = 1000
        ,

        [Parameter()]
        [int] $MaxResults
    )

    begin {
        $skip = 0
        $total = 0
        $isComplete = $false

        if ($MaxResults -and $MaxResults -lt $PageSize) {
            $PageSize = $MaxResults
        }
    }

    process {
        while (-not $isComplete) {
            $thisRecords = Get-ServiceNowRequestItem @ServiceNowSplat -First $PageSize -Skip $skip
            $thisCount = ($thisRecords | Measure-Object).Count
            $total += $thisCount
            Write-Verbose "Returned results $($skip + 1) - $total"
            $skip += $PageSize
            $thisRecords | Write-Output

            if ((-not $thisRecords) -or ($thisCount -lt $PageSize)) {
                Write-Verbose "$thisCount records were returned, compared to $PageSize expected; complete"
                $isComplete = $true
            }
            elseif ($MaxResults -and $total -ge $MaxResults) {
                Write-Verbose "-MaxResults $MaxResults was specified; complete"
                $isComplete = $true
            }
        }
    }
}

# Assume $splat contains parameters for Get-ServiceNowRequestItem (filter criteria, etc.)

# Return all results, 1000 at a time
Get-PagedServiceNowRequestItem -ServiceNowSplat $splat

# Return all results, 500 at a time
Get-PagedServiceNowRequestItem -ServiceNowSplat $splat -PageSize 500

# Return 10 results, 2 at a time
Get-PagedServiceNowRequestItem -ServiceNowSplat $splat -PageSize 2 -MaxResults 10

(Edit: fixed a minor issue in the example code.)

[replicaJunction]

I've been doing some additional thinking about this change. I think it's possible to remove the breaking change by adding the -Limit parameter back, but not providing a default value. Then we could use some logic like this:

# In Get-ServiceNowTable
    if ($PSBoundParameters.ContainsKey('Limit')) {
        Write-Warning "The -Limit parameter is deprecated, and may be removed in a future release. Use the -First parameter instead."
        $Body['sysparm_limit'] = $Limit
    }
    elseif ($PSCmdlet.PagingParameters.First -ne [uint64]::MaxValue) {
        # If the user doesn't provide the -First parameter, it defaults to [uint64]::MaxValue
        $Body['sysparm_limit'] = $PSCmdlet.PagingParameters.First
    }
    else {
        # If no paging information was provided, default to the legacy behavior,
        # which was to return 10 records
        $Body['sysparm_limit'] = 10
    }

# In functions that call it
    if ($PSBoundParameters.ContainsKey('Limit')) {
        $getServiceNowTableSplat.Add('Limit', $Limit)
    }

This would allow users to continue using the -Limit parameter for now if necessary, which would make for a more graceful transition since it's no longer a breaking change.

Would this be a change you'd like to see in this PR?

[lipkau]

First of... great to see you active again, @replicaJunction ;-)

to your PR:

1. The parameters added by SupportsPaging to not have help unless you add it yourself.
   If you plan on adding the help to the CBH, you can see copy it from JiraPS
2. -First is not the number of entries per page, but how many of the results should be fetched.
   Same as dir | Select-Object -First 5
3. you are being inconsistent in how you delcare the attirbute. Example: Get-ServiceNowRequest.ps1 vs Get-ServiceNowTable.ps1
4. you might wanna declare -PageSize as [UInt32]. that way you don't have to worry about negatives

        [Parameter()]
        [int] $PageSize = 1000
        ,

5. why would you not set SupportsPaging in your example?
   That way you would be able to skip the

        Parameter()]
        [int] $MaxResults

and

        $skip = 0
        $total = 0

[replicaJunction]

@lipkau - great to be alive again, and thanks for the feedback. :)

1. I hadn't realized there was no comment-based help for the paging parameters, so thanks for letting me know about that. I don't believe this repo uses PlatyPS right now, though - do you know if you can inline CBH for native PowerShell parameters?

2. Interesting. I'm not 100% sure I understand what you're getting at, so if you could show me a slightly longer code example, that would help. I've used this approach to page through data before, though. For example, just yesterday I needed to split a CSV file into several parts:

$csv = Import-Csv $csvPath
$csv | select -First 100 | Export-Csv $firstOutputFile -NoTypeInformation
$csv | select -First 100 -Skip 100 | Export-Csv $secondOutputFile -NoTypeInformation
# ...etc

3. Thanks for catching that. Based on the existing style, I'm going to use the SupportsPaging style without the = $true definition, since that's how DefaultParameterSetName has been defined already. I'll make those style changes.

4. Good call, but that's just for the example code I provided. I put that together pretty quickly just to demonstrate the feature. I'll make the edit to the example, but that doesn't affect the PR itself.

5. Same as above, though in hindsight, it does seem silly not to use SupportsPaging in the example code when that's the whole point of this PR. :)

Still looking for feedback above on my comment about adding back the -Limit parameter and avoiding the breaking change.

[lipkau]

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_functions_cmdletbindingattribute?view=powershell-6#supportspaging
https://github.com/Microsoft/Windows-classic-samples/blob/master/Samples/PowerShell/SupportsPaging/SupportsPaging01/cs/SupportsPaging01.ps1

1. Like this:

<#
.PARAMETER IncludeTotalCount
    Causes an extra output of the total count at the beginning.

    Note this is actually a uInt64, but with a custom string representation.

.PARAMETER Skip
    Controls how many things will be skipped before starting output.

    Defaults to 0.

.PARAMETER First
    Indicates how many items to return.
#>

2. Imagine a REST API where you can define the following things:

• how many results do you want on a single page? -PageSize
• how many entries are you not interested in (from the beginning)? -Skip
• how many results do you want? -First

With the full example:

# use pagesize of 100 because the payload of each object is small
Invoke-Something -PageSize 100 -First 50 -Skip 25

With a DB of 1000 entries, the cmdlet would: request up to 10 pages (100 entries/page *10 = 1000 entries), will ignore $result[0..24] and will stop after the 75th entry (25 (skipped) + 50 = 75). So it will actually never get to invoke the API for the 2nd page.

3. the = $true is only necessary in PSv2. I dropped it already in all of my stuff

4. I know. took me a while to get to that idea :-P

5. 👍

6. About this:

Still looking for feedback above on my comment about adding back the -Limit parameter and avoiding the breaking change.

Yes, keep it in until the next major version. with a Write-Warning "deprecated". maybe a comment in the doc of the parameter.
But you are still confusing First with -PageSize. You can keep it as -Limit (not taking my suggestion of naming it -PageSize).

[replicaJunction]

Okay, I think I see what you're getting at. The confusion comes from the translation from PowerShell to the ServiceNow API, and I think it's only in my example, not in the PR code itself.

First off, the -Skip parameter does not start at the beginning of the total result set - it skips the first X records after -First has been processed. For example:

PS> 0..100 | select-object -First 5 -Skip 5
5
6
7
8
9

PS> 0..100 | select-object -First 5 -Skip 10
10
11
12
13
14

The same example works with the Get-Numbers function you linked:

PS> Get-Numbers -First 5 -Skip 5

Skip Number IncludeTotalCount First
---- ------ ----------------- -----
   5      5 False                 5
   5      6 False                 5
   5      7 False                 5
   5      8 False                 5
   5      9 False                 5

PS> Get-Numbers -First 5 -Skip 10

Skip Number IncludeTotalCount First
---- ------ ----------------- -----
  10     10 False                 5
  10     11 False                 5
  10     12 False                 5
  10     13 False                 5
  10     14 False                 5

Next, ServiceNow doesn't define a separate "page size" parameter. It only provides two URL parameters:

1. sysparm_limit - number of records to return
2. sysparm_offset - start index of the records to be returned

To your previous example of paging through 1000 entries starting on 25, in ServiceNow, the URL parameters would need to look like this:

• sysparm_limit=100&sysparm_offset=25
• sysparm_limit=100&sysparm_offset=125
• etc.

To me, these map pretty easily to -First and to -Skip:

• -First 100 -Skip 25
• -First 100 -Skip 125
• etc.

So to return all items with this API, the -First parameter stays at a consistent value, and the -Skip parameter is what tells the API to move forward through records. That same logic works with Select-Object when paging through records from another data source (like the CSV example I listed above), and that's the logic used in the implementation of the paging parameters in Get-ServiceNowTable.

My example code above is not as well-written, and it does use a PageSize variable as a value for the First parameter. The naming is a bit weird because of that, but the logic is sound - the "page size" for each API request maps to the -First PowerShell parameter, which maps to the sysparm_limit URL parameter. If each page is a consistent size, then the -First parameter should stay consistent as well, and then it could be considered a page size since it determines how many records are returned.

To summarize - no, I don't think I'm confusing First and PageSize in the code in this PR, though my example is not really a good one.

[lipkau]

I still think you are. but I also think I don"t know the module. And this is becoming a lot of discussion for a PR.
I will shut up now :-P
If you (or someone else) wants to continue this conversation, let me know/drop me an email/find me on slack

[replicaJunction]

@Rick-2CA - If you're still interested in this change, here's what I'm thinking for a plan of action:

1. I will cancel this PR
2. I will wait for your acceptance of Feature: specify fields to return #67 (so I don't have two pending PRs at the same time that could possibly cause merge conflicts)
3. I will re-implement this functionality without the breaking change, but adding a deprecation warning when someone uses the existing -Limit parameter (as described above)
4. I will also implement some the quality changes recommended by @lipkau
5. I will provide a better usage example (and avoid use of the phrase "page size" to avoid confusion)
6. I will submit a new PR once all of that is done

How does that sound?

[Rick-2CA]

@replicaJunction - Sounds good. I like the idea of deprecating the limit param.

[replicaJunction]

Closing for the time being per my previous comment. I'll re-implement this after #67 is accepted.