search

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
304 stars 193 forks source link

FR - Change List GetById to use Items(ID) Endpoint #1514

Open randellhodges opened 2 months ago

randellhodges commented 2 months ago

Category

Describe the feature

I've been fighting a fighting with a lookup threshold issue between 2 identically configured list (this is a configuration problem, not a problem with PnP.Core)

I had code something like the following:

list.Items.GetByIdAsync(listItemId, x => x.All)

This exposed my configuration issues. However, I noticed if I did something like this:

var listItem = await list.Items.GetByIdAsync(listItemId, x => x.Id);
await listItem.EnsurePropertiesAsync(x => x.All)

It worked.

Looking at the rest calls, the first example generated a rest call that uses a filter:

_api/web/lists(guid'<<GUID>>')/items?$select=Id%2c*&$filter=Id+eq+25&$top=1

but the second did this:

list.Items.GetByIdAsync: _api/web/lists(guid'<<GUID>>')/items?$select=Id&$filter=Id+eq+25&$top=1

listItem.EnsurePropertiesAsync: _api/web/lists/getbyid(guid'<<GUID>>')/items(25)?$select=Id%2c*

So it seems that, if you have a list with more than 12 managed metadata fields (which have hidden lookup columns), if you try to load everything with a List.GetById call, it'll fail because of whatever underlying code path SPO uses.

However, ListItem.EnsureProperties follows a different path and will happily load all the data.

Describe the solution you'd like

Use the second method /items(<ID>) instead of /items?filter=Id+eq+<ID>&$top=1 when calling the GetById methods.

Alternatively, it would be nice if we issued our own ApiRequest to project the results into the underlying internal ListItem implementation, or have a GetByRequest or something where we could pass our own ApiRequest?

jansenbe commented 1 month ago

Interesting finding @randellhodges, you confirmed that 12 managed metadata fields was the trigger?

Adding a GetByRequest accepting an ApiRequest as input is something to further investigate, we already have an internal RequestAsync (see https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core/Model/Base/BaseDataModel.cs#L927-L956) method on every model in PnP Core SDK.

randellhodges commented 1 month ago

Yes, I can confirm. I removed managed metadata fields from the list until the original call worked. Added back a single MM field and it failed again.

I would love to see both ways implemented. For this specific issue, internally use the /items(ID) endpoint when calling Items.GetById.

The alternative where I could pass an ApiRequest would be nice to have everywhere so that if I want to really (at my own peril) control the request, I can and still get the IObject (IList, IFolder, etc) back to use.