[Bug] - Incorrect usage of InputObject

[andreasjordan]

As this is the case in a lot of commands, I will use a general example:

function Test-DbaInputObject {
    [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'Low')]
    param (
        [string[]]$SqlInstance,
        [PSCredential]$SqlCredential,
        [parameter(ValueFromPipeline)]
        [string[]]$InputObject,
        [switch]$EnableException
    )
    process {
        Write-Verbose -Message "starting process"

        if (($null -eq $SqlInstance) -and ($null -eq $InputObject)) {
            Write-Warning -Message "You must supply either -SqlInstance or an Input Object"
            return
        }
        if ($null -ne $SqlInstance) {
            $InputObject += $SqlInstance
        }
        Write-Verbose -Message "InputObject.Count = '$($InputObject.Count)'"

        foreach ($instance in $InputObject) {
            Write-Verbose -Message "instance = '$instance'"
        }
    }
}

The basic idea is to add the values from the named parameter to the objects that come down the pipeline.

And for some scenarios, that works:

Test-DbaInputObject -SqlInstance a -Verbose
'a' | Test-DbaInputObject -Verbose

Test-DbaInputObject -SqlInstance a, b -Verbose
'a', 'b' | Test-DbaInputObject -Verbose

'a' | Test-DbaInputObject -SqlInstance b, c, d -Verbose

But when more than one object is piped and the named parameter is used as well, the named parameter is processed more than once:

'a', 'b' | Test-DbaInputObject -SqlInstance c, d -Verbose

[andreasjordan]

I thinks that would be a correct way to handle this:

function Test-DbaInputObject {
    [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'Low')]
    param (
        [string[]]$SqlInstance,
        [PSCredential]$SqlCredential,
        [parameter(ValueFromPipeline)]
        [string[]]$InputObject,
        [switch]$EnableException
    )
    begin {
        Write-Verbose -Message "starting begin"
        $objects = @()
        if ($null -ne $SqlInstance) {
            $objects += $SqlInstance
        }
        Write-Verbose -Message "begin: objects.Count = '$($objects.Count)'"
    }
    process {
        Write-Verbose -Message "starting process"
        foreach ($object in $InputObject) {
            $objects += $InputObject
            Write-Verbose -Message "foreach: objects.Count = '$($objects.Count)'"
        }
        Write-Verbose -Message "process: objects.Count = '$($objects.Count)'"
    }
    end {
        Write-Verbose -Message "starting end"
        if (($null -eq $SqlInstance) -and ($objects.Count -eq 0)) {
            Write-Warning -Message "You must supply either -SqlInstance or an Input Object"
            return
        }
        foreach ($instance in $objects) {
            Write-Verbose -Message "instance = '$instance'"
        }
    }
}

[wsmelton]

What commands have this issue currently?

[wsmelton]

The input object as a whole is not handled the same way in all of our commands. There are various patterns being used to validate types or to just accept what is provided by PowerShell to that internal parameter.

Are you suggesting an internal function is used to handle it all across every command?

[andreasjordan]

What commands have this issue currently?

Let me have a quick look and see what I find:

• Add-DbaAgDatabase
• Add-DbaAgListener
• ...

I think there are a lot of commands, maybe we have a look at all commands that use piped input.

But we can simply change some of them if we only except $InputObject XOR the parameter. When you have a look at Add-DbaAgDatabase:

if (Test-Bound -Not SqlInstance, InputObject) {
    Stop-Function -Message "You must supply either -SqlInstance or an Input Object"
    return
}

The message is already talking about XOR (because of the "either", right?), but the test does not.

[andreasjordan]

The input object as a whole is not handled the same way in all of our commands. There are various patterns being used to validate types or to just accept what is provided by PowerShell to that internal parameter.

Are you suggesting an internal function is used to handle it all across every command?

I don't yet know. I would try to find out if other projects had a similar problem and how they've solved this. So not reinventing the wheel. As the situations and types are so different, I would rather think of some guidelines, examples and templates.

Maybe start with a handful of commands and implement a solution and add a lot of documentation inside the code. Then try to get the active contributors to analyse and fix their commands. And then we have to take care of all the rest...

[niphlod]

not sure if it's useful or not, but check https://github.com/sqlcollaborative/dbatools/blob/b99253a618b703c7dddf3ca10db90e3792ec9f85/functions/Get-DbaBuildReference.ps1#L116 as a way of knowing if you have something in the pipeline ^_^

[github-actions[bot]]

🚧🚨 This issue is being marked as stale due to 90 days of inactivity. If you would like this issue to remain open:

• Verify the issue/bug is reproduced in the latest version of the module
• Verify the environmental info provided is still accurate
• Add any additional steps you followed to reproduce if necessary
  🚨🚧
  ⌛️ This issue will be closed in 30 days ⌛️

[andreasjordan]

I have just seen the solution in Watch-DbaXESession:
https://github.com/sqlcollaborative/dbatools/blob/048a3d8dd9d343125e6c474ce095924c5bbd5f39/functions/Watch-DbaXESession.ps1#L63-L72

We have to use the two different ParameterSetName 'instance' and 'piped'. That prevents the use of both ways to start the command.

[andreasjordan]

I will convert this into a discussion and also close the related pull request, but leave the branch still active.
Maybe there is a new discussion about this topic in the near future.
But as we don't see bug reports related to this, this is probably not a big issue for the users and so we don't have to take of that.