OData / lab

This repository is for exploring new ideas and developing early prototypes of various OData stacks.
Other
48 stars 59 forks source link

Allow making generated types internal to hide them outside the assembly #100

Closed habbes closed 4 years ago

habbes commented 4 years ago

Addresses issue #93, #97, #101

This PR handles the "internal" option for both C# and VB generated code. However it appears the connected service extension is not enabled for VB projects.

PS: I did not test this against OData V3 services.

KanishManuja-MS commented 4 years ago

@habbes The changes look fine to me. The only thing I would ask is to add one or more test cases for this new configuration. I will do a more in-depth code review and approve.

I think this would be supported for v3 for your changes as well, so just do a quick check. However, if it does not work, I would be fine with deprecating support for v3 (will need wider consensus) for new features but I would probably want to create a new extension (and rename old one) on the market place, so extension for v3 is still available.

habbes commented 4 years ago

@KanishManuja-MS is there a sample V3 service endpoint I can test with?

I had created an issue (#101) to add tests to the project on a separate PR because it does not have any testing at all. But let me just go ahead and create a test project for this PR as you have advised.

habbes commented 4 years ago

@paulodero

Since we already have tests for OData Code generator and connected service is simply an extension of OData code Generator, I don't think it is a good idea to add a whole new test solution. I would rather figure out how to reuse the OData code generator tests and build on top of it. Since we may deprecate one of the tools, this may leave us in a good place when that happens.

I think we would still need to create a test solution for this. There are things that are ODataConnectedService specifc that would still need to be tested that we can't just copy from the code generator (i.e. the VS Connected Services related stuff). And for the parts that are common, it is possible that some functionality has diverged over time. I think manually copying the tests from the Client code gen project to the code generator portion of this project could help save time, and it might present opportunities for us to refactor and make the code gen tests easier to work with instead of copying them as is. And if this library does not support VB, then we would be adding extra weight if we just used the code gen tests as-is.

habbes commented 4 years ago

@KanishManuja-MS

I've found a V3 endpoint to test with (https://services.odata.org/V3/(S(j4ofwcgyinw2fuqmqz4jgmuy))/OData/OData.svc/). This new configuration does not support V3. V3 uses a different code generator, namely the System.Data.Services.Design.EntityClassGenerator from the Microsoft.Data.Services.Design assembly, which is not part of the source code. So we can't add new configuration to it to change how it generates code. I've gone ahead and removed that option from setup wizard when a V3 endpoint is provided.

I've added a couple of test cases to test the code generation as well as general test cases for some of the main components of the connected service. I think this could do for now but we should try to improve overall test coverage in future PR's.

Furthermore, the tests should be added to the build pipeline, is it something I can do from my end, and if so, could you point me in the right direction?

habbes commented 4 years ago

A branch with the same changes as the ones from this PR were already merged to the repo (see PR https://github.com/OData/lab/pull/107), therefore we can close this PR.