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 build uri from groupby navigation property with many child structural properties #1143

Merged
merged 1 commit into from Apr 6, 2018

Conversation

Projects
None yet
2 participants
@voronov-maxim
Contributor

voronov-maxim commented Apr 4, 2018

Description

[InlineData("http://gobbledygook/People?$apply=groupby((MyDog/Color,MyDog/Breed))")]
public void BuildUrlWithApply(string query)
{
    var parser = new ODataUriParser(HardCodedTestModel.TestModel, ServiceRoot, new Uri(query));
    ODataUri odataUri = parser.ParseUri();

    Uri result = odataUri.BuildUri(ODataUrlKeyDelimiter.Parentheses);
    Assert.Equal(query, Uri.UnescapeDataString(result.OriginalString));
}

Expected result http://gobbledygook/People?$apply=groupby((MyDog/Color,MyDog/Breed))
Actual result http://gobbledygook/People?$apply=groupby((MyDog/Color/MyDog/Breed))

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed
@@ -242,6 +242,7 @@
<Compile Include="..\ScenarioTests\Roundtrip\JsonLight\JsonBigBatchRoundTripTests.cs" />
<Compile Include="..\ScenarioTests\Roundtrip\JsonLight\MultipartMixedBatchDependsOnIdsTests.cs" />
<Compile Include="..\ScenarioTests\Roundtrip\JsonLight\Utils.cs" />
<Compile Include="..\ScenarioTests\UriBuilder\ApplyBuilderTest.cs" />

This comment has been minimized.

@AlanWong-MS

AlanWong-MS Apr 5, 2018

Contributor

Can you also please add the test file to test\FunctionalTests\Microsoft.OData.Core.Tests\Microsoft.OData.Core.Tests.NetCore.csproj?

This comment has been minimized.

@voronov-maxim

voronov-maxim Apr 6, 2018

Contributor

ApplyBuilderTest.cs default compilation includes in Microsoft.OData.Core.Tests.NetCore.csproj
image

This comment has been minimized.

@AlanWong-MS

AlanWong-MS Apr 6, 2018

Contributor

My apologies, you're absolutely right. I forgot that the new VS2017 csproj adds files from its directory by default.

@AlanWong-MS

Looks good overall, thanks for finding the bug! Just a quick request to add the test case to our NetCore version of tests as well.

@AlanWong-MS AlanWong-MS merged commit bf1f8c2 into OData:master Apr 6, 2018

2 checks passed

continuous-integration/vsts The build trigger has fired.
Details
license/cla All CLA requirements met.
Details

biaol-odata added a commit to biaol-odata/odata.net that referenced this pull request Jun 26, 2018

Fix build uri from groupby navigation property with many child struct…
…ural properties (OData#1143)

- Fix build uri from groupby navigation property with many child structural properties
- Add corresponding test case

biaol-odata added a commit that referenced this pull request Jun 26, 2018

Fix build uri from groupby navigation property with many child struct…
…ural properties (#1143)

- Fix build uri from groupby navigation property with many child structural properties
- Add corresponding test case

@biaol-odata biaol-odata changed the title from fix build uri from groupby navigation property with many child structural proeprties to fix build uri from groupby navigation property with many child structural properties Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment