Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
randellhodges opened this issue Aug 27, 2024 · 2 comments
Open
1 task done

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

randellhodges opened this issue Aug 27, 2024 · 2 comments
Assignees
Labels
area: model 📐 Related to the core SDK models question Further information is requested

Comments

@randellhodges
Copy link

randellhodges commented Aug 27, 2024

Category

  • Feature request

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
Copy link
Contributor

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.

@jansenbe jansenbe self-assigned this Sep 17, 2024
@jansenbe jansenbe added question Further information is requested area: model 📐 Related to the core SDK models labels Sep 17, 2024
@randellhodges
Copy link
Author

randellhodges commented Sep 17, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: model 📐 Related to the core SDK models question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants