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

Fix Issue #1 $count is evaluated prematurely at the call queryOptions.ApplyTo #562

Merged
merged 1 commit into from Nov 18, 2015

Conversation

Projects
None yet
4 participants
@VikingsFan
Copy link
Contributor

commented Nov 16, 2015

No description provided.

@@ -146,6 +165,12 @@ public Routing.ODataPath Path
return (long)totalCount;
}

if (this.TotalCountFunc != null)
{
_request.Properties[TotalCountKey] = this.TotalCountFunc();

This comment has been minimized.

Copy link
@lewischeng-ms

lewischeng-ms Nov 16, 2015

Contributor

Cache the result of _request.Properties[TotalCountKey] to a local variable to increase performance.

public void GetEntityCountFunc_ReturnsFunc_IfValueIsTrue()
{
// Arrange
var countOption = new CountQueryOption("true", _context);

This comment has been minimized.

Copy link
@lewischeng-ms

lewischeng-ms Nov 16, 2015

Contributor

Also test the case where $count not present?

This comment has been minimized.

Copy link
@VikingsFan

VikingsFan Nov 17, 2015

Author Contributor

Added Thanks!

@lewischeng-ms

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2015

Looks good to me basically:P

@VikingsFan VikingsFan force-pushed the VikingsFan:Issue#1 branch from 2eca4cc to 0d497c1 Nov 17, 2015

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
string result = response.Content.ReadAsStringAsync().Result;
int tryParseInt;
bool tryResult = Int32.TryParse(result, out tryParseInt);

This comment has been minimized.

Copy link
@xuzhg

xuzhg Nov 18, 2015

Member

why do you try to parse the total payload? As I know, you should only to test the @odata.count.

This comment has been minimized.

Copy link
@VikingsFan

VikingsFan Nov 18, 2015

Author Contributor

Thanks, updated

@xuzhg

This comment has been minimized.

Copy link
Member

commented Nov 18, 2015

others look good to me. Thanks.

@VikingsFan VikingsFan force-pushed the VikingsFan:Issue#1 branch from 0d497c1 to 337a8cc Nov 18, 2015

@VikingsFan VikingsFan merged commit 337a8cc into OData:master Nov 18, 2015

@VikingsFan VikingsFan added the PR_Team label Dec 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.