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

Feature: Add support for querying a IQueryable<dynamic> #137

Merged
merged 6 commits into from
Jan 14, 2018
Merged

Feature: Add support for querying a IQueryable<dynamic> #137

merged 6 commits into from
Jan 14, 2018

Conversation

NickDarvey
Copy link
Contributor

Updates the ParseMemberAccess method so that if it can't get the property or field, it tries getting the member by a dynamic operation or by an indexer operation.

After a lot of searching I ended up on this bit of code which seems to do the trick. The ExpressionTests_Select_ExpandoObjects test passes now.

Some notes:

  • It will throw an exception only during evaluation if it can't find the specified member (and it is an object or has an indexer), currently a InvalidOperationException but perhaps introducing a new exception type is warranted?
  • I'd still like to get ExpressionTests_Select_DynamicObjects but I'm not sure how I'll go.
  • @StefH: Are you able to pull this and run all the tests on your machine? I was only able to get the solution building with a lot of hacking of projects and stripping out all platforms except net461.

If it can't get the property or field, try getting the member by a dynamic operation or by an indexer operation.
@NickDarvey
Copy link
Contributor Author

NickDarvey commented Jan 9, 2018

Sweet CI! Okay, will take a look at which platform is dying...

@NickDarvey
Copy link
Contributor Author

Oh, also tagging #136

@NickDarvey
Copy link
Contributor Author

NickDarvey commented Jan 9, 2018

Ah, okay, it's failing to build System.Linq.Dynamic.Core.Tests.csproj because System.Linq.Expressions.Expression.Dynamic Method (CallSiteBinder, Type, IEnumerable<Expression>) is not available in netcoreapp1.1

What do you think about changing the test project to target netcoreapp2.0 (which has the method is available) and make this feature conditional and only available for netstandard2.0 platforms?
(Here's the method in the .NET API Browser.)

@StefH
Copy link
Collaborator

StefH commented Jan 9, 2018

I'll take your PR and take a look.

@codecov
Copy link

codecov bot commented Jan 9, 2018

Codecov Report

Merging #137 into master will decrease coverage by 0.4%.
The diff coverage is 38.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
- Coverage   82.95%   82.54%   -0.41%     
==========================================
  Files          35       36       +1     
  Lines        3484     3512      +28     
  Branches      486      487       +1     
==========================================
+ Hits         2890     2899       +9     
- Misses        455      481      +26     
+ Partials      139      132       -7
Impacted Files Coverage Δ
src/System.Linq.Dynamic.Core/DynamicClass.cs 7.69% <0%> (-1.24%) ⬇️
...System.Linq.Dynamic.Core/DynamicGetMemberBinder.cs 0% <0%> (ø)
...em.Linq.Dynamic.Core/DynamicQueryableExtensions.cs 98.3% <100%> (+0.01%) ⬆️
...ystem.Linq.Dynamic.Core/Parser/ExpressionParser.cs 77.43% <52.17%> (-0.27%) ⬇️
src/System.Linq.Dynamic.Core/Parser/TypeHelper.cs 88.2% <0%> (+0.47%) ⬆️
...c/System.Linq.Dynamic.Core/Tokenizer/TextParser.cs 98.03% <0%> (+1.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0c358a...879ed96. Read the comment docs.

@NickDarvey
Copy link
Contributor Author

Alright, ExpressionTests_Select_DynamicObjects might need to come in another PR... Can this get merged in without it?

@StefH
Copy link
Collaborator

StefH commented Jan 12, 2018

I will merge this code (at this point) to main and create a new NuGet. The discussion on other possible solutions will be done into the issue #136.

@StefH StefH changed the title [WIP] Adds support for querying a IQueryable<dynamic> Feature: Add support for querying a IQueryable<dynamic> Jan 14, 2018
@StefH StefH merged commit 864f75f into zzzprojects:master Jan 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants