Skip to content

NH-3987 - Re-implement NhQueryable options #601

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

Merged
merged 2 commits into from
Jul 28, 2017

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Apr 16, 2017

NH-3987 Re-implement NhQueryable options.

At least that way, adding more options should be way less intrusive.

Instead of having one extension method per option, we now would have a single SetOptions extension method, taking a delegate for setting options with a IQueryableOptions object.

query = query.SetOptions(o => o.SetTimeout(10).SetCacheable(true));

I could have directly exposed the IQuery instead but well, it would have exposed undesirable features in a Linq context I think.

@fredericDelaporte
Copy link
Member Author

The SetOptions parameter does not end up in cache parameters key, so it does not cause an issue. and keeping the delegate will allow more code simplification if we mutualise later the options interface on all query API.

@fredericDelaporte
Copy link
Member Author

Updated the reference documentation too. And fixed the SetOptions xml comment.

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented May 2, 2017

Rebased for resolving conflicts x2.

@hazzik
Copy link
Member

hazzik commented May 2, 2017

I was thinking. Do we need to have the separate method of options? Why not include it into .Query(options)?

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented May 2, 2017

Well I have tried at first something similar. But there is a roadblock: each Linq call creates a new NhQueryable, which only gets the expression and the provider. I have not found better ways for transmitting the options than keeping them in the expression. (The provider creates new NhQueryable by only having the expression to use for that. We do not have any reference to the original NhQueryable from which the expression was derived. See IQueryProvider.CreateQuery methods.)

Then, only allowing setting the options when obtaining the queryable from the session may be limiting: I frequently build different queries from the same queryable base on which I have already applied some Linq transformations, and those resulting different queries may not have the same options requirements.

@hazzik hazzik removed the to review label Jul 25, 2017
@fredericDelaporte
Copy link
Member Author

Thanks Alexander,
I will merge that after some more PR on fixing tests.

@fredericDelaporte fredericDelaporte merged commit 79e65dd into nhibernate:master Jul 28, 2017
@fredericDelaporte fredericDelaporte deleted the NH-3987 branch July 28, 2017 07:32
@hazzik hazzik added this to the 5.0 milestone Aug 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants