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

Get list view by title throws an exception #627

Closed
1 task done
s-KaiNet opened this issue Nov 15, 2021 · 2 comments
Closed
1 task done

Get list view by title throws an exception #627

s-KaiNet opened this issue Nov 15, 2021 · 2 comments
Assignees
Labels
area: model 📐 Related to the core SDK models bug Something isn't working

Comments

@s-KaiNet
Copy link
Collaborator

Category

  • Bug

Describe the bug

When I try to get a listview by its title, I got an exception.

Steps to reproduce

The code is fairly simple:

using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
{
	var list = context.Web.Lists.GetByServerRelativeUrl($"{context.Uri.LocalPath}/Shared Documents");
	var wh = await list.Views.Where(s => s.Title == "All Documents").FirstOrDefaultAsync();
}

From developer perspective it's a perfectly valid code, it compiles, however it throws

System.InvalidCastException: Unable to cast object of type 'System.Linq.EmptyPartition`1[PnP.Core.Model.SharePoint.IView]' to type 'PnP.Core.Model.SharePoint.IView'.

at this line as a result of this. View does not implement IQueryableDataModel, as a result a new Enumerable.Empty<TModel>(); is returned, but the cast fails later on.

I used the View object as an example, we might see the similar issues for other objects as well. The problem is, that we actually don't know what should be the return type. If I use FirstOrDefault, then the return type should be default(TModel), if ToList, then IEnumerable<TModel> etc. Even if we return a valid type, it still might be confusing for a library consumer why it doesn't return value? (I know, that there is a view called `All Documents), but the library return null.

Would it be more natural to just throw an exception in this case? I.e. if a class does not implement IQueryableDataModel, then here:

- return Enumerable.Empty<TModel>();
+ throw new Exception($"LINQ queries are not supported for {typeof(TModel).Name} type.");
@jansenbe jansenbe self-assigned this Nov 15, 2021
@jansenbe
Copy link
Contributor

jansenbe commented Nov 15, 2021

Thanks for reporting this @s-KaiNet. The fact that some models that are populated via REST and having their LinqGet property set do not have the IQueryableDataModel interface set is wrong. I'll fix those.

I also do like the idea of throwing an exception instead of returning an empty collection, think this way things will be better for the developer as there's clarity on why things do not work.

@jansenbe jansenbe added bug Something isn't working area: model 📐 Related to the core SDK models labels Nov 15, 2021
@jansenbe
Copy link
Contributor

@s-KaiNet : implemented.

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 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants