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

Support for OData $count operations as per v4.01 spec section 11.2.10 #1298

Closed
wants to merge 24 commits into from

Conversation

joshcomley
Copy link

@joshcomley joshcomley commented Oct 21, 2018

Issues

I am unable to find an open issue relating to this.

Description

As per the title, this is to implement support for $count operations such as:

GET http://host/service/Products/$count
GET http://host/service/Products/$count?$filter=Price lt 10.00
GET http://host/service/Customers?$filter=Interests/$count gt 5
GET http://host/service/Categories?$filter=Products/$count($filter=Price gt 5.0) gt 1

With this change, the following is still unsupported:

GET http://host/service/Products/$filter=@foo/$count?@foo=Price lt 10.00

Specification reference:
http://docs.oasis-open.org/odata/odata/v4.01/cs01/part1-protocol/odata-v4.01-cs01-part1-protocol.html#sec_RequestingtheNumberofItemsinaCollect

Checklist (Uncheck if it is not completed)

  • Test cases added (although only URI builder tests)
  • Build and test with one-click build and test script passed

Additional work necessary

Functional tests need to be written.

Unsure if docs need to be updated.

@msftclas
Copy link

msftclas commented Oct 21, 2018

CLA assistant check
All CLA requirements met.

joshcomley added a commit to joshcomley/WebApi that referenced this pull request Oct 21, 2018
@xuzhg
Copy link
Member

xuzhg commented Oct 23, 2018

@joshcomley Would you please file two issues:

  1. One issue is to describe the functionality/requirement/problem/etc implemented in this PR.
  2. The other is for the remaining unsupported scenario

@xuzhg xuzhg added this to the 7.6 milestone Oct 24, 2018
@xuzhg
Copy link
Member

xuzhg commented Oct 31, 2018

@joshcomley I see there are some build errors:

src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,69): Error CS1026: ) expected
src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,84): Error CS1002: ; expected
src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,84): Error CS1513: } expected
src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,86): Error CS1525: Invalid expression term '||'
src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,115): Error CS1002: ; expected
src\Microsoft.OData.Core\JsonLight\ODataJsonLightWriter.cs(562,115): Error CS1513: } expected
Process 'msbuild.exe' exited with code '1'.

Would you please help take a look?

Assert.Equal(new Uri("http://gobbledygook/People?$expand=MyPaintings%2F%24count"), actualUri);
}

// This is not supported, yet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support parameter alias. :)

}

public ExpandedCountSelectItem(ODataExpandPath pathToNavigationProperty, IEdmNavigationSource navigationSource) : base(pathToNavigationProperty, navigationSource) { }
public ExpandedCountSelectItem(ODataExpandPath pathToNavigationProperty, IEdmNavigationSource navigationSource, FilterClause filterOption, OrderByClause orderByOption, long? topOption, long? skipOption, bool? countOption, SearchClause searchOption) : base(pathToNavigationProperty, navigationSource, filterOption, orderByOption, topOption, skipOption, countOption, searchOption) { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshcomley The spec says " The $count segment can be appended to a navigation property name or type-cast segment following a navigation property name to return just the count of the related entities. The $filter and $search system query options can be used to limit the number or related entities included in the count. ",

So, it seems it only accepts "filterOption" and "SearchOption" in "ExpandedCountSelectItem" ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuzhg It should only accept filter and search, yes

@xuzhg xuzhg force-pushed the master branch 2 times, most recently from f9519fd to 14a4b12 Compare September 22, 2019 21:12
@joshcomley
Copy link
Author

I've just realised this issue is still outstanding (have been using my own patched version of OData for a while). What do I need to do to get this merged?

@xuzhg
Copy link
Member

xuzhg commented Jan 8, 2020

@joshcomley Thanks for tracking on this. I have fixed some failure test cases and recreate a PR at: #1336.

We do like this feature to be implemented. However, for your changes, i'd like to change some design and implementation. Owing to other high priority task, this change is postpone. It seems you are interested it again, maybe we can find some time to sync it.

@joshcomley
Copy link
Author

Hi @xuzhg, thanks for getting back to me. As it's been a year since these changes, some things have changed, but I have re-applied my changes to the latest version of the code-base. Should I delete this PR and create a new one with the new amendments?

@joshcomley
Copy link
Author

joshcomley commented Mar 4, 2021

This PR has today been rebased on master, the changes added in #1336 have been merged in and I have added support for $select=Collection/$count (it already supported $expand=Collection/$count)

@KenitoInc
Copy link
Contributor

@joshcomley
We merged these 2 PRs #2051 and #2069
They will be out in v7.9 release.
Check if they resolve your issue

@KenitoInc KenitoInc closed this Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants