pnp / pnpcore

The PnP Core SDK is a modern .NET SDK designed to work for Microsoft 365. It provides a unified object model for working with SharePoint Online and Teams which is agnostic to the underlying API's being called
https://aka.ms/pnp/coresdk/docs
MIT License
307 stars 192 forks source link

LoadListDataAsStream with Location column: System.FormatException: One of the identified items was in an invalid format. #1369

Closed wizofaus closed 8 months ago

wizofaus commented 9 months ago

Category

Describe the bug

Exception thrown calling LoadListDataAsStream(Async) when the document library has location columns and rows with data for such columns.

System.FormatException: One of the identified items was in an invalid format. at System.Text.Json.ThrowHelper.ThrowFormatException() at System.Text.Json.JsonElement.GetInt32() at PnP.Core.Model.SharePoint.ListDataAsStreamHandler.GetJsonPropertyValueAsString(JsonElement propertyValue) at PnP.Core.Model.SharePoint.ListDataAsStreamHandler.TransformRowData(JsonElement row, IFieldCollection fields, Dictionary`2 fieldLookupCache) at PnP.Core.Model.SharePoint.ListDataAsStreamHandler.Deserialize(String json, List list) at PnP.Core.Model.SharePoint.List.LoadListDataAsStreamAsync(RenderListDataOptions renderOptions) at TDI.DocumentService.SPLibraryConnector.GetItems(ItemsParameters itemsParameters, Boolean asUser) in

Repro steps

Create a document library with a location column and add at least one document with a valid location for said column.

Call as per:

                var output = await lib.LoadListDataAsStreamAsync(new RenderListDataOptions()
                {
                    RenderOptions = RenderListDataOptionsFlags.None,
                    MergeDefaultView = true,
                    AddRequiredFields = true,
                }).ConfigureAwait(false);

Expected behavior

No exception should be thrown, and location data available in item values.

Environment details (development & target environment)

Additional context

Thanks for your contribution! Sharing is caring.

wizofaus commented 9 months ago

Due to attempting to parse decimal numbers (lat/long) as integers. GetJsonPropertyValueAsString( ) shouldn't assume ValueKind == Number means integer (I tried GetDecimal(), but GetDouble() should work for 99.99% of cases and IFieldLocationValue uses double for Latitude/Longitude anyway).

BTW this problem doesn't occur when just using Lists.GetBy...(..., l => l.Items); - in fact case the FieldLocationValue properties are populated correctly (it uses GetDouble). Not sure why the code is different.

wizofaus commented 9 months ago

@jansenbe do you have any preference as to how Image fields are handled (as noted elsewhere, the CSOM library returns them as a string, as in fact the data returned by the underlying api is a JSON string embedded within the JSON response. But note that same is true for Location columns, however PnP.Core supports parsing those)? I already have a fix ready to contribute but it just needs a decision on that, plus the unit test I have doesn't do the list creation/populating part.

jansenbe commented 9 months ago

@wizofaus : ideally we have them as a FieldImageValue similar to the location field FieldLocationValue. I can finalize the test cases if needed. Also, we plan to release a new version by end of the week (or I can push this out to next week), would be good to get the PR committed before so it can be included.

jansenbe commented 9 months ago

@wizofaus : can I assign this one to you?

wizofaus commented 8 months ago

Sure, though I'm not sure when I'll get to it, hopefully in the next week.

jansenbe commented 8 months ago

@wizofaus : your PR was merged, closing this now