Fix the behavior of Get-GitHubRepository (microsoft#179)

HowardWolosky · web-flow · commit c4c1ec344a35 · 2020-05-25T15:34:32.000-07:00
There were a few problems with this function: * -GetAllPublicRepositories didn't actually work. That's now been fixed, and support for the optional Since paramater has been added as well. * Fixed the ParameterSet handling for all of the parameters to make sure that users can only specify parameters relevant with the appropriate parameter set given that there are five different use cases for this method: * Getting repos for the current authenticated user * Getting repos for any GitHub user * Getting repos for an organization * Getting all public repos) * Getting a specific GitHub repo Resolves microsoft#178
diff --git a/GitHubCore.ps1 b/GitHubCore.ps1
@@ -6,6 +6,12 @@
     mediaTypeVersion = 'v3'
     squirrelAcceptHeader = 'application/vnd.github.squirrel-girl-preview'
     symmetraAcceptHeader = 'application/vnd.github.symmetra-preview+json'
+    mercyAcceptHeader = 'application/vnd.github.mercy-preview+json'
+    nebulaAcceptHeader = 'application/vnd.github.nebula-preview+json'
+    baptisteAcceptHeader = 'application/vnd.github.baptiste-preview+json'
+    scarletWitchAcceptHeader = 'application/vnd.github.scarlet-witch-preview+json'
+    dorianAcceptHeader = 'application/vnd.github.dorian-preview+json'
+    londonAcceptHeader = 'application/vnd.github.london-preview+json'
 
  }.GetEnumerator() | ForEach-Object {
      Set-Variable -Scope Script -Option ReadOnly -Name $_.Key -Value $_.Value
diff --git a/GitHubRepositories.ps1 b/GitHubRepositories.ps1
@@ -317,7 +317,12 @@ function Get-GitHubRepository
     .PARAMETER GetAllPublicRepositories
         If this is specified with no other parameter, then instead of returning back all
         repositories for the current authenticated user, it will instead return back all
-        public repositories on GitHub.
+        public repositories on GitHub in the order in which they were created.
+
+    .PARAMETER Since
+        The ID of the last public repository that you have seen.  If specified with
+        -GetAllPublicRepositories, will only return back public repositories created _after_ this
+        one.
 
     .PARAMETER AccessToken
         If provided, this will be used as the AccessToken for authentication with the
@@ -340,24 +345,30 @@ function Get-GitHubRepository
         Gets all public repositories on GitHub.
 
     .EXAMPLE
-        Get-GitHubRepository -OctoCat OctoCat
+        Get-GitHubRepository -OwnerName octocat
+
+        Gets all of the repositories for the user octocat
 
     .EXAMPLE
-        Get-GitHubRepository -Uri https://github.com/PowerShell/PowerShellForGitHub
+        Get-GitHubRepository -Uri https://github.com/microsoft/PowerShellForGitHub
+
+        Gets information about the microsoft/PowerShellForGitHub repository.
 
     .EXAMPLE
         Get-GitHubRepository -OrganizationName PowerShell
 
+        Gets all of the repositories in the PowerShell organization.
 #>
     [CmdletBinding(
         SupportsShouldProcess,
-        DefaultParameterSetName='Elements')]
+        DefaultParameterSetName='AuthenticatedUser')]
     [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")]
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")]
     param(
-        [Parameter(ParameterSetName='Elements')]
+        [Parameter(ParameterSetName='ElementsOrUser')]
         [string] $OwnerName,
 
-        [Parameter(ParameterSetName='Elements')]
+        [Parameter(ParameterSetName='ElementsOrUser')]
         [string] $RepositoryName,
 
         [Parameter(
@@ -369,21 +380,36 @@ function Get-GitHubRepository
         [string] $OrganizationName,
 
         [ValidateSet('All', 'Public', 'Private')]
+        [Parameter(ParameterSetName='AuthenticatedUser')]
         [string] $Visibility,
 
+        [Parameter(ParameterSetName='AuthenticatedUser')]
         [string[]] $Affiliation,
 
+        [Parameter(ParameterSetName='AuthenticatedUser')]
+        [Parameter(ParameterSetName='ElementsOrUser')]
+        [Parameter(ParameterSetName='Organization')]
         [ValidateSet('All', 'Owner', 'Public', 'Private', 'Member', 'Forks', 'Sources')]
         [string] $Type,
 
+        [Parameter(ParameterSetName='AuthenticatedUser')]
+        [Parameter(ParameterSetName='ElementsOrUser')]
+        [Parameter(ParameterSetName='Organization')]
         [ValidateSet('Created', 'Updated', 'Pushed', 'FullName')]
         [string] $Sort,
 
+        [Parameter(ParameterSetName='AuthenticatedUser')]
+        [Parameter(ParameterSetName='ElementsOrUser')]
+        [Parameter(ParameterSetName='Organization')]
         [ValidateSet('Ascending', 'Descending')]
         [string] $Direction,
 
+        [Parameter(ParameterSetName='PublicRepos')]
         [switch] $GetAllPublicRepositories,
 
+        [Parameter(ParameterSetName='PublicRepos')]
+        [int64] $Since,
+
         [string] $AccessToken,
 
         [switch] $NoStatus
@@ -395,36 +421,115 @@ function Get-GitHubRepository
     $OwnerName = $elements.ownerName
     $RepositoryName = $elements.repositoryName
 
-    $telemetryProperties = @{}
+    $telemetryProperties = @{
+        'UsageType' = $PSCmdlet.ParameterSetName
+    }
 
     $uriFragment = [String]::Empty
     $description = [String]::Empty
-    if ((-not [String]::IsNullOrEmpty($OwnerName)) -and (-not [String]::IsNullOrEmpty($RepositoryName)))
+    switch ($PSCmdlet.ParameterSetName)
     {
-        $telemetryProperties['OwnerName'] = Get-PiiSafeString -PlainText $OwnerName
-        $telemetryProperties['RepositoryName'] = Get-PiiSafeString -PlainText $RepositoryName
+        { ('ElementsOrUser', 'Uri') -contains $_ } {
+            # This is a little tricky.  Ideally we'd have two separate ParameterSets (Elements, User),
+            # however PowerShell would be unable to disambiguate between the two, so unfortunately
+            # we need to do some additional work here.  And because fallthru doesn't appear to be
+            # working right, we're combining both of those, along with Uri.
+
+            if ([String]::IsNullOrWhiteSpace($OwnerName))
+            {
+                $message = 'OwnerName could not be determined.'
+                Write-Log -Message $message -Level Error
+                throw $message
+            }
+            elseif ([String]::IsNullOrWhiteSpace($RepositoryName))
+            {
+                if ($PSCmdlet.ParameterSetName -eq 'ElementsOrUser')
+                {
+                    $telemetryProperties['UsageType'] = 'User'
+                    $telemetryProperties['OwnerName'] = Get-PiiSafeString -PlainText $OwnerName
+
+                    $uriFragment = "users/$OwnerName/repos"
+                    $description = "Getting repos for $OwnerName"
+                }
+                else
+                {
+                    $message = 'RepositoryName could not be determined.'
+                    Write-Log -Message $message -Level Error
+                    throw $message
+                }
+            }
+            else
+            {
+                if ($PSCmdlet.ParameterSetName -eq 'ElementsOrUser')
+                {
+                    $telemetryProperties['UsageType'] = 'Elements'
+
+                    if ($PSBoundParameters.ContainsKey('Type') -or
+                        $PSBoundParameters.ContainsKey('Sort') -or
+                        $PSBoundParameters.ContainsKey('Direction'))
+                    {
+                        $message = 'Unable to specify -Type, -Sort and/or -Direction when retrieving a specific repository.'
+                        Write-Log -Message $message -Level Error
+                        throw $message
+                    }
+                }
+
+                $telemetryProperties['OwnerName'] = Get-PiiSafeString -PlainText $OwnerName
+                $telemetryProperties['RepositoryName'] = Get-PiiSafeString -PlainText $RepositoryName
+
+                $uriFragment = "repos/$OwnerName/$RepositoryName"
+                $description = "Getting $OwnerName/$RepositoryName"
+            }
 
-        $uriFragment = "repos/$OwnerName/$RepositoryName"
-        $description = "Getting repo $RepositoryName"
-    }
-    elseif ([String]::IsNullOrEmpty($OwnerName) -and [String]::IsNullOrEmpty($OrganizationName))
-    {
-        $uriFragment = 'user/repos'
-        $description = 'Getting repos for current authenticated user'
-    }
-    elseif ([String]::IsNullOrEmpty($OwnerName))
-    {
-        $telemetryProperties['OrganizationName'] = Get-PiiSafeString -PlainText $OrganizationName
+            break
+        }
 
-        $uriFragment = "orgs/$OrganizationName/repos"
-        $description = "Getting repos for $OrganizationName"
-    }
-    else
-    {
-        $telemetryProperties['OwnerName'] = Get-PiiSafeString -PlainText $OwnerName
+        'Organization' {
+
+            $telemetryProperties['OrganizationName'] = Get-PiiSafeString -PlainText $OrganizationName
+
+            $uriFragment = "orgs/$OrganizationName/repos"
+            $description = "Getting repos for $OrganizationName"
+
+            break
+        }
+
+        'User' {
+            $telemetryProperties['OwnerName'] = Get-PiiSafeString -PlainText $OwnerName
+
+            $uriFragment = "users/$OwnerName/repos"
+            $description = "Getting repos for $OwnerName"
+
+            break
+        }
+
+        'AuthenticatedUser' {
+            if ($PSBoundParameters.ContainsKey('Type') -and
+                ($PSBoundParameters.ContainsKey('Visibility') -or
+                $PSBoundParameters.ContainsKey('Affiliation')))
+            {
+                $message = 'Unable to specify -Type when using -Visibility and/or -Affiliation.'
+                Write-Log -Message $message -Level Error
+                throw $message
+            }
+
+            $uriFragment = 'user/repos'
+            $description = 'Getting repos for current authenticated user'
+
+            break
+        }
+
+        'PublicRepos' {
+            $uriFragment = 'repositories'
+            $description = "Getting all public repositories"
 
-        $uriFragment = "users/$OwnerName/repos"
-        $description = "Getting repos for $OwnerName"
+            if ($PSBoundParameters.ContainsKey('Since'))
+            {
+                $description += " since $Since"
+            }
+
+            break
+        }
     }
 
     $sortConverter = @{
@@ -448,10 +553,12 @@ function Get-GitHubRepository
     {
         $getParams += "affiliation=$($Affiliation -join ',')"
     }
+    if ($PSBoundParameters.ContainsKey('Since')) { $getParams += "since=$Since" }
 
     $params = @{
         'UriFragment' = $uriFragment + '?' +  ($getParams -join '&')
         'Description' =  $description
+        'AcceptHeader' = "$script:nebulaAcceptHeader,$script:baptisteAcceptHeader,$script:mercyAcceptHeader"
         'AccessToken' = $AccessToken
         'TelemetryEventName' = $MyInvocation.MyCommand.Name
         'TelemetryProperties' = $telemetryProperties
@@ -810,7 +917,7 @@ function Get-GitHubRepositoryTopic
         'UriFragment' = "repos/$OwnerName/$RepositoryName/topics"
         'Method' = 'Get'
         'Description' =  "Getting topics for $RepositoryName"
-        'AcceptHeader' = 'application/vnd.github.mercy-preview+json'
+        'AcceptHeader' = $script:mercyAcceptHeader
         'AccessToken' = $AccessToken
         'TelemetryEventName' = $MyInvocation.MyCommand.Name
         'TelemetryProperties' = $telemetryProperties
@@ -933,7 +1040,7 @@ function Set-GitHubRepositoryTopic
         'Body' = (ConvertTo-Json -InputObject $hashBody)
         'Method' = 'Put'
         'Description' =  $description
-        'AcceptHeader' = 'application/vnd.github.mercy-preview+json'
+        'AcceptHeader' = $script:mercyAcceptHeader
         'AccessToken' = $AccessToken
         'TelemetryEventName' = $MyInvocation.MyCommand.Name
         'TelemetryProperties' = $telemetryProperties
diff --git a/Tests/GitHubRepositories.tests.ps1 b/Tests/GitHubRepositories.tests.ps1
@@ -14,8 +14,107 @@ $moduleRootPath = Split-Path -Path $PSScriptRoot -Parent
 
 try
 {
-    Describe 'Modifying repositories' {
+    Describe 'Getting repositories' {
+        Context 'For authenticated user' {
+            BeforeAll -Scriptblock {
+                $publicRepo = New-GitHubRepository -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit
+                $privateRepo = New-GitHubRepository -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit -Private
+
+                # Avoid PSScriptAnalyzer PSUseDeclaredVarsMoreThanAssignments
+                $publicRepo = $publicRepo
+                $privateRepo = $privateRepo
+            }
+
+            $publicRepos = Get-GitHubRepository -Visibility Public
+            $privateRepos = Get-GitHubRepository -Visibility Private
+
+            It "Should have the public repo" {
+                $publicRepo.Name | Should BeIn $publicRepos.Name
+                $publicRepo.Name | Should Not BeIn $privateRepos.Name
+            }
+
+            It "Should have the private repo" {
+                $privateRepo.Name | Should BeIn $privateRepos.Name
+                $privateRepo.Name | Should Not BeIn $publicRepos.Name
+            }
+
+            It 'Should not permit bad combination of parameters' {
+                { Get-GitHubRepository -Type All -Visibility All } | Should Throw
+                { Get-GitHubRepository -Type All -Affiliation Owner } | Should Throw
+            }
+
+            AfterAll -ScriptBlock {
+                Remove-GitHubRepository -Uri $publicRepo.svn_url
+                Remove-GitHubRepository -Uri $privateRepo.svn_url
+            }
+        }
+
+        Context 'For any user' {
+            $repos = Get-GitHubRepository -OwnerName 'octocat' -Type Public
+
+            It "Should have results for The Octocat" {
+                $repos.Count | Should -BeGreaterThan 0
+                $repos[0].owner.login | Should Be 'octocat'
+            }
+        }
+
+        Context 'For organizations' {
+            BeforeAll -Scriptblock {
+                $repo = New-GitHubRepository -OrganizationName $script:organizationName -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit
+
+                # Avoid PSScriptAnalyzer PSUseDeclaredVarsMoreThanAssignments
+                $repo = $repo
+            }
+
+            $repos = Get-GitHubRepository -OrganizationName $script:organizationName -Type All
+            It "Should have results for the organization" {
+                $repo.name | Should BeIn $repos.name
+            }
 
+            AfterAll -ScriptBlock {
+                Remove-GitHubRepository -Uri $repo.svn_url
+            }
+        }
+
+        Context 'For public repos' {
+            # Skipping these tests for now, as it would run for a _very_ long time.
+            # No obviously good way to verify this.
+        }
+
+        Context 'For a specific repo' {
+            BeforeAll -Scriptblock {
+                $repo = New-GitHubRepository -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit
+
+                # Avoid PSScriptAnalyzer PSUseDeclaredVarsMoreThanAssignments
+                $repo = $repo
+            }
+
+            $returned = Get-GitHubRepository -Uri $repo.svn_url
+            It "Should be a single result using Uri ParameterSet" {
+                $returned | Should -BeOfType PSCustomObject
+            }
+
+            $returned = Get-GitHubRepository -OwnerName $repo.owner.login -RepositoryName $repo.Name
+            It "Should be a single result using Elements ParameterSet" {
+                $returned | Should -BeOfType PSCustomObject
+            }
+
+            It 'Should not permit additional parameters' {
+                { Get-GitHubRepository -OwnerName $repo.owner.login -RepositoryName $repo.Name -Type All } | Should Throw
+            }
+
+            It 'Should require both OwnerName and RepositoryName' {
+                { Get-GitHubRepository -RepositoryName $repo.Name } | Should Throw
+                { Get-GitHubRepository -Uri "https://github.com/$script:ownerName" } | Should Throw
+            }
+
+            AfterAll -ScriptBlock {
+                Remove-GitHubRepository -Uri $repo.svn_url
+            }
+        }
+    }
+
+    Describe 'Modifying repositories' {
         Context -Name 'For renaming a repository' -Fixture {
             BeforeEach -Scriptblock {
                 $repo = New-GitHubRepository -RepositoryName ([Guid]::NewGuid().Guid) -AutoInit
@@ -35,7 +134,7 @@ try
             ## cleanup temp testing repository
             AfterEach -Scriptblock {
                 ## variables from BeforeEach scriptblock are accessible here, but not variables from It scriptblocks, so need to make URI (instead of being able to use $renamedRepo variable from It scriptblock)
-                Remove-GitHubRepository -Uri "$($repo.svn_url)$suffixToAddToRepo" -Verbose
+                Remove-GitHubRepository -Uri "$($repo.svn_url)$suffixToAddToRepo"
             }
         }
     }
