Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support in Set-GitHubContent to upload binary files #336

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions GitHubContents.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ filter Set-GitHubContent
.PARAMETER Content
The new file content.

.PARAMETER ContentPath
The local path to file content.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The local path to file content.
The local path to a file whose content should replace what is currently stored remotely in the repo at the Path location.


.PARAMETER Sha
The SHA value of the current file if present. If this parameter is not provided, and the
file currently exists in the specified branch of the repo, it will be read to obtain this
Expand Down Expand Up @@ -323,9 +326,16 @@ filter Set-GitHubContent

[Parameter(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also want to allow this to be ValueFromPipeline to enable the content to be piped directly into this method. That's certainly an optional change for this PR though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to file an issue for this, or should I?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're not addressing this in this PR, then go ahead and open it up as a suggestion once this PR lands.

Mandatory,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to remain mandatory in its own ParameterSet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a misunderstanding with how I phrased this. It does need to be in its own parameter set, but not at the exclusion of the other required information. For instance, in order to what repo we're changing the content of, we still need to have the information either in the Elements parameter set or the Uri parameter set.

Previously, Content was being used with either of those parameter sets. Now you are introducing ContentPath as an alternative to Content. That means we're now exploded from 2 possible parameter sets to 4:

  • Elements with Content (maybe named ElementsContent)
  • Elements with ContentPath(maybe namedElementsContentPath`)
  • Uri with Content (maybe named UriContent)
  • Uri with ContentPath (maybe named UriContentPath)

You should try running Get-Help -Name Set-GitHubContent and look at the Syntax section. That will show you the various parameter sets that PowerShell has understood from the authored code. If you look at that with what you've currently authored, you should see more clearly how your new Content parameter set excludes the key information that we need, as well as prevents Content and ContentPath from being used when the repo information is provided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at Get-GitHubEvent. That's an example where we had to explode the number of ParameterSets in order to support both the Elements and Uri repo scenarios while still accurately separating out the different possible inputs for the method itself.

Position = 4)]
Position = 4,
ParameterSetName = 'Content')]
[string] $Content,

[Parameter(
Mandatory,
Position = 4,
ParameterSetName = 'Content')]
[string] $ContentPath,

[Parameter(ValueFromPipelineByPropertyName)]
[string] $Sha,

Expand Down Expand Up @@ -358,7 +368,14 @@ filter Set-GitHubContent

$uriFragment = "/repos/$OwnerName/$RepositoryName/contents/$Path"

$encodedContent = [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($Content))
if ($ContentPath)
{
$encodedContent = [Convert]::ToBase64String((Get-Content -Path $ContentPath -AsByteStream -Raw))
}
else
{
$encodedContent = [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($Content))
}

$hashBody = @{
message = $CommitMessage
Expand Down
61 changes: 61 additions & 0 deletions Tests/GitHubContents.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,67 @@ try
}
}

Context 'When setting new file content by path' {
BeforeAll {
$filePath = 'notes'
$fileName = 'hello.txt'
$commitMessage = 'Commit Message'
$content = 'This is the content for test.txt'
$branchName = 'master'
$committerName = 'John Doe'
$committerEmail = '[email protected]'
$authorName = 'Jane Doe'
$authorEmail = '[email protected]'

$contentPath = New-TemporaryFile
$content | Out-File -FilePath $contentPath

$setGitHubContentParms = @{
Path = "$filePath/$fileName"
CommitMessage = $commitMessage
Branch = $branchName
ContentPath = $contentPath
Uri = $repo.svn_url
CommitterName = $committerName
CommitterEmail = $committerEmail
authorName = $authorName
authorEmail = $authorEmail
}

$result = Set-GitHubContent @setGitHubContentParms -PassThru
}

It 'Should have the expected type and additional properties' {
$result.PSObject.TypeNames[0] | Should -Be 'GitHub.Content'
$result.content.name | Should -Be $fileName
$result.content.path | Should -Be "$filePath/$fileName"
$result.content.url | Should -Be ("https://api.github.com/repos/$($script:ownerName)" +
"/$repoName/contents/$filePath/$($fileName)?ref=$BranchName")
$result.commit.author.name | Should -Be $authorName
$result.commit.author.email | Should -Be $authorEmail
$result.commit.committer.name | Should -Be $committerName
$result.commit.committer.email | Should -Be $committerEmail
$result.commit.message | Should -Be $commitMessage
}

It 'Should have written the correct content' {
$getGitHubContentParms = @{
Path = "$filePath/$fileName"
Uri = $repo.svn_url
MediaType = 'Raw'
ResultAsString = $true
}

$writtenContent = Get-GitHubContent @getGitHubContentParms

$content | Should -Be $writtenContent
}

AfterAll {
Remove-Item -Path $contentPath
}
}

Context 'When overwriting file content' {
BeforeAll {
$filePath = 'notes'
Expand Down