镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

JSON created by Export-CrescendoCommand can not be read by Export-CrescendoModule #155

Open

Description

Export-CrescendoCommand calls the ExportConfigurationFile method of the command object.
This generates json which looks like this

{
  "Verb": "Install",
   ...
}

Export-CrescendoModule expects to find JSON which looks like this

{
  "Commands": [
    {
      "Verb": "Install",
      ...
    }
  ]
}

Unless the command definitions are in an array the Export-CrescendoModule generates an error

InvalidOperation: You cannot call a method on a null-valued expression.

Granted anyone generating code with Export-CrescendoCommand can hack the JSON file, but (IMHO) that should not be required
edit crucial word was missing from the last sentence

Activity

  1. sdwheeler commented on May 20, 2022

    @sdwheeler
    Contributor

    This used to work. You used to be able to export individual commands to separate JSON files then use Export-CrescendoModule -command *.json to create the module. The problematic code is in Import-CommandConfiguration. It expects an array of commands. It would be nice if it could handle single command as output by Export-CrescendoCommand or a JSON file containing a collection of commands.

    The alternative is to have Export-CrescendoCommand create a JSON file with an array containing a single command.

  2. jhoneill commented on May 21, 2022

    @jhoneill
    Author

    This used to work. You used to be able to export individual commands to separate JSON files then use Export-CrescendoModule -command *.json to create the module. The problematic code is in Import-CommandConfiguration. It expects an array of commands. It would be nice if it could handle single command as output by Export-CrescendoCommand or a JSON file containing a collection of commands.

    The alternative is to have Export-CrescendoCommand create a JSON file with an array containing a single command.

    That alternative would be far neater - I'll share my reworked version later - I also spotted this

    if($PSCmdlet.ShouldProcess($crescendoCommand)) 
    

    So -verbose or -confirm output the PowerShell built for the command

  3. jhoneill commented on May 25, 2022

    @jhoneill
    Author

    Here is my new Export-command

    • I've made the default path $pwd instead of . so that the full path is displayed in a confirm prompt
    • If the path is filename all the commands will go to an array in that file. If it is a directory they will go to files as now, but the files will contain a single member array. In both cases the code built by the command object is indented by four spaces.
    • If -confirm is specified the user will be prompted for each file (unless they say yes to all / no to all) and the hash table is there to cope with the one file / many files situation
    • I've added a -PassThru switch which outputs the files
    function Export-CrescendoCommand {
        [CmdletBinding(SupportsShouldProcess=$true)]
        param (
            [Parameter(Position=0, Mandatory=$true, ValueFromPipeline=$true)]
            [Command[]]$Command,
            [Alias('TargetDirectory')]
            [string]$Path = $pwd.path,
            [alias('pt')]
            [switch]$PassThru
        )
        process {
            $filesAllowed = @{}
            foreach($crescendoCommand in $Command) {
                if (Test-Path -Path $Path -PathType Container) {
                    $exportPath = Join-Path -Path  $Path -ChildPath "$($crescendoCommand.Verb)-$($crescendoCommand.Noun).crescendo.json"
                }
                elseif (Test-Path -IsValid $Path -PathType Leaf) {$exportPath = $Path}
                else   {throw "$Path must be a direcory or a valid file path."}
    
                #if we have already sent something to this file add ",", newline, and the JSON for this command - but don't close the JSON yet.
                if ($filesAllowed.ContainsKey($exportPath)) {
                    $filesAllowed[$exportPath] +=
                        ",`n        " + ($crescendoCommand.GetCrescendoConfiguration() -replace '\n',"`n        ")
                }
                #If not, check we are allowed to output to it, and add the opening and the first command but leave the JSON open.
                elseif ($PSCmdlet.ShouldProcess($exportPath)) {
                    $filesAllowed[$exportPath] =
                         "{`n" +
                         "    `"`$schema`": `"https://aka.ms/PowerShell/Crescendo/Schemas/2021-11`",`n"+
                         "    `"Commands`": [`n        " +
                                       ($crescendoCommand.GetCrescendoConfiguration() -replace '\n',"`n        " )
                }
            }
        }
        end {
            foreach ($exportPath in $filesAllowed.Keys)  {
                #close the json that was left open when we added the command(s) and write the file
                Set-Content -Confirm:$false -Path $exportPath -Value ($filesAllowed[$exportPath] + "`n   ]`n}")
                if ($PassThru) {Get-item $exportPath}
            }
        }
    }
  4. jhoneill commented on May 25, 2022

    @jhoneill
    Author

    And my new Import-CommandConfiguration

    • In Both commands the default name for the path is -Path with an alias for the old name
    • Now import looks for an array of commands and if it doesn't find one but finds Verb and noun processes that as a command if it finds neither, it throws an error at that point.
    function Import-CommandConfiguration {
        [CmdletBinding()]
        param (
            [Parameter(Position=0,Mandatory=$true)]
            [Alias('File')]
            [string]$Path
        )
        # this dance is to support multiple configurations in a single file
        # The deserializer doesn't seem to support creating [command[]]
        $objects = Get-Content $Path -ErrorAction Stop | ConvertFrom-Json -depth 10
        if     ($objects.commands)                {$commands = $objects.commands }
        elseif ($objects.verb -and $objects.noun) {$commands = $objects}
        else   {throw "$Path does not appear to contain suitable JSON"}
        $options = [System.Text.Json.JsonSerializerOptions]::new()
        foreach ($c in $commands) {
            $jsonText      = $c | ConvertTo-Json -depth 10
            $errs          = $null
            $configuration = [System.Text.Json.JsonSerializer]::Deserialize($jsonText, [command], $options)
            if (-not (Test-Configuration -configuration $configuration -errors ([ref]$errs))) {
                    $errs  | Foreach-Object { Write-Error -ErrorRecord $_ }
            }
            # emit the configuration even if there was an error
            $configuration
        }
    }
  5. jhoneill commented on May 25, 2022

    @jhoneill
    Author

    Jason Helmick (@theJasonHelmick) what I meant on #154 was that if I could send two PRs for these two, each as a file for that function it would be a lot easier than either sending you a PSM with a ton of changes, or making a branch for each change with its own updates to original PSM1. Merging the second means git will say "two people have changed this file - you need to say which changes stay in"

  6. theJasonHelmick commented on Jul 1, 2022

    @theJasonHelmick
    Collaborator

    Thank you James O'Neill (@jhoneill) for the suggestion with code! I'm reviewing this and will report back after I investigate. Thank you!

  7. theJasonHelmick commented on Jul 20, 2022

    @theJasonHelmick
    Collaborator

    Originally, Export-CrescendoCommand was not designed to provide JSON directly to Export-CrescendoModule in the workflow. But we think this is a reasonable scenario. We are investigating the best course of action and will add this to our future plans. We also don't think the original scenario is invalid, so we don't want to create a breaking change if not necessary.

  8. JamesWTruher commented on Aug 1, 2022

    @JamesWTruher
    Contributor

    fixed by #165

  9. ethanbergstrom commented on Feb 5, 2023

    @ethanbergstrom

    This doesn't seem to be fixed with the 1.1 preview when used with -targetDirectory functionality, which still emits the old-style JSON.

  10. theJasonHelmick commented on Feb 7, 2023

    @theJasonHelmick
    Collaborator

    Thanks Ethan Bergstrom (@ethanbergstrom) -- We are investigating and will resolve along with add another test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions