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

[jhoneill]

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

[sdwheeler]

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.

[jhoneill]

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

[jhoneill]

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}
        }
    }
}

[jhoneill]

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
    }
}

[jhoneill]

@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"

[theJasonHelmick]

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

[theJasonHelmick]

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.

[JamesWTruher]

fixed by https://github.com/PowerShell/Crescendo/pull/165

[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.

[theJasonHelmick]

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