pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
910 stars 318 forks source link

Bug: 'spo file add' submitting $null or blank values as 'true' #707

Closed ohthreesixtyfive closed 5 years ago

ohthreesixtyfive commented 5 years ago

Description:

When adding a file and attempting to set a field using a variable, if the variable is $null or empty the value passed through is the string 'true'.

Steps to reproduce:

$path = 'C:\MS365.jpg'

Scenario A)

Variable set to $null

$nullTitle = $null

o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path $path --FileLeafRef 'Null Title.jpg' --Title $nullTitle

Expected

New file added to destination with Title attribute of '' (empty string)

Actual

New file added to destination with Title attribute of 'true'

Scenario B)

Variable set to '' (empty string)

$emptyTitle = ''

o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path $path --FileLeafRef 'Empty String Title.jpg' --Title $emptyTitle

Expected

New file added to destination with Title attribute of '' (empty string)

Actual

New file added to destination with Title attribute of 'true'

Scenario C) [Working as Expected]

Variable set to 'not null or empty'

$stringTitle = 'not null or empty'

o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path $path --FileLeafRef 'String Title.jpg' --Title $stringTitle

Expected/Actual

New file added to destination with Title attribute of 'not null or empty'

Additional notes:

This bug is particularly nasty when trying to pass an empty or null variable into a Date/Time, Number, or Lookup field as it will result in an incorrect type error.

waldekmastykarz commented 5 years ago

Thank you for the detailed repro steps. I'll have a look at it. Am I correct to assume that you've spotted the issue in PowerShell?

ohthreesixtyfive commented 5 years ago

Thank you for the detailed repro steps. I'll have a look at it. Am I correct to assume that you've spotted the issue in PowerShell?

You're very welcome. Your assumption is correct, this issue has been spotted in PowerShell using non-immersive mode. Apologies for not including it in the original issue report.

Looking forward to your fix, in the meantime I'm working out how to dynamically build the parameters passed to 'spo file add' to only include non-null attributes.

Best regards,

Ramiro

waldekmastykarz commented 5 years ago

Looking forward to your fix, in the meantime I'm working out how to dynamically build the parameters passed to 'spo file add' to only include non-null attributes.

Just to check if I'm getting it correctly: you're passing a set of options to the spo file add command and would like the command to only submit those to SharePoint, which are not-null?

ohthreesixtyfive commented 5 years ago

Looking forward to your fix, in the meantime I'm working out how to dynamically build the parameters passed to 'spo file add' to only include non-null attributes.

Just to check if I'm getting it correctly: you're passing a set of options to the spo file add command and would like the command to only submit those to SharePoint, which are not-null?

To clarify, in my case I'm querying a SQL Database to get all the files in a given table and their information (filepath, filename, and other metadata) - from there, I'm iterating over each object and using the 'spo file add' command to send it up to SharePoint. There's about 30 different pieces of metadata, but not all of them are populated for each item.

In my initial script (which is how I stumbled upon this bug), I simply 'hardcoded' the 'spo file add' command with all of the options set to a given property of the object. So it looked something like this:

$documents = <Invoke-Sqlcmd>

foreach ($doc in $documents)
{
o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path $doc.filepath --column1 $doc.column1 --column2 $doc.column2 --column3 $doc.column3
}

As mentioned above, if any of the optional properties (column1, column2, etc.) were null or blank, 'spo file add' would try to send them over as 'true' instead of null or blank. As a result, if one of those columns was a non-string field, the file would get uploaded but no properties changed.

My workaround for the time being is to simply look at the object ahead of time and filter out any null or empty properties like so:

$documentAttributes = $doc.PSObject.Properties | ?{!([string]::IsNullOrEmpty($_.Value))} | Select Name, Value

And then build a 'options' string based off of those properties:

$optionalAttributes = ''

foreach($attr in $documentAttributes)
    {
    $commandOptions = ($commandOptions + ' --' + $attr.Name + " '" + $attr.Value + "'")
    }

Once that's built, I simply append those to the 'base' command and execute it:

$executeO365SPOFileAdd = ("o365 spo file add o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents --path $doc.filepath" + $commandOptions)

iex $executeO365SPOFileAdd

Thankfully, the column names in SharePoint have been setup to match exactly the names provided from the SQL Table, so this works well-enough.

It's probably more than you were wanting or needing to know, and I did simplify it a bit for brevity, but I hope that explains what I meant by my comment.

I'll be the first to admit that I am no expert in PowerShell (or any other scripting language for that matter), so there's probably a much more efficient way of accomplishing this task that I'm not aware of but unfortunately time is tight on this one.

waldekmastykarz commented 5 years ago

This is great info! Thanks for the details on the use case and the workaround. I bet others will find it helpful should they come across a similar problem 👏

waldekmastykarz commented 5 years ago

I had a look into it, and it seems that the problem might be beyond our reach. When you pass a PowerShell $null as an option value, it gets removed from the list of arguments passed into Node.js. So running a command like o365 spo web get -u $a (where $a is $null) passes an array of arguments such as o365, spo, web, get, -u. Nothing is passed in place of the $null argument, which makes the given option behave like a switch parameter (basically like --debug) which explains the true value you're seeing. I don't think there is much we can do here without changing how parameters are parsed and trying to change default values for parameters that aren't meant as a switch, which could be tricky. I wish I had better news.

ohthreesixtyfive commented 5 years ago

I had a look into it, and it seems that the problem might be beyond our reach. When you pass a PowerShell $null as an option value, it gets removed from the list of arguments passed into Node.js. So running a command like o365 spo web get -u $a (where $a is $null) passes an array of arguments such as o365, spo, web, get, -u. Nothing is passed in place of the $null argument, which makes the given option behave like a switch parameter (basically like --debug) which explains the true value you're seeing. I don't think there is much we can do here without changing how parameters are parsed and trying to change default values for parameters that aren't meant as a switch, which could be tricky. I wish I had better news.

That's a bummer. Thanks for looking into it all the same!

On a related note:

The workaround I described above seemed way too inefficient, as it was iterating n number of times for each n number of objects to build a unique string, so I went back to the drawing board to come up with something cleaner.

Since iex worked just fine, I simply turned my existing 'hardcoded' command from earlier and set that as a string. Using a regular expression, I was able to match and remove options that had $null or empty values. I'll post it here in case anyone could use it in the future:

### for example of this regular expression visit https://regexr.com/44vuq ###
$NullorEmptyPattern = '((-|--)\w+\s{2}|(-|--)\w+\s{1}$)'

$path = "C:\MS365.jpg"

$notNull = 'Not Null'
$Title = "$null"
$Value = "$notNull"

$execute = "o365 spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path $path --FileLeafRef 'MS365.jpg' --NotNullValue $Value --Title $Title"

Write-Host String With Null or Empty Values Included -F DarkYellow
Write-Host $execute
Write-Host

### use regular expression pattern to match and remove any null or empty options ###
$executeO365SPOFileAdd = ($execute -replace $NullorEmptyPattern, '')

Write-Host String With Null or Empty Values Excluded -F DarkGreen
Write-Host ($executeO365SPOFileAdd)

### execute o365 file add with $null or empty options removed ###
iex $executeO365SPOFileAdd

Some items to consider and known limitations of this implementation:

Feel free to let me know if you have any questions or recommendations on how to make this even cleaner!

Thank you again for taking a look at this Waldek, I really appreciate your efforts! ⭐️

waldekmastykarz commented 5 years ago

Thank you for sharing your script! I'm sure it will help others. Please, don't ever hesitate to reach out if we can clarify or help with anything.