We store the CSDL file as an embedded resource. When we want to load the and parse the CSDL in memory, we find the resource that matches the target CSDL file name. We use resourceName.EndsWith(filePath) to find the match. However, this causes a conflict when one CSDL name is a substring of the other, e.g.:
OData ServiceCsdl.xml
Another OData ServiceCsdl.xml
Currently an exception is thrown when such a conflict is detected. This PR fixes this issue by resolving the conflict by selecting the shortest resource name that ends with the target filename.
This solves the problem for simple cases, and possibly all practical common scenarios. However, it would be problematic in the following cases:
The user manually creates a file called OData ServiceCsdl.xml in the same project and adds it as an embedded resource.
The user renames the embedded resources or folders that were generated by the connected service or code generation tool.
I did not address the concerns because:
Our code generation tool has the convention of adding the Csdl.xml suffix the service name set in the connected service configuration when generating the CSDL file. We do not expect the user to create files with this exact naming convention manually.
We do not expect users to manually rename any files generated by the code generation tool. Doing so should be considered undefined behaviour.
It would complicate the code.
The main issue here is that we EndsWith to match the files and resources. Why don't we just check the full resource name? The full resource name does not match the file name, it's generated based on the assembly namespace and connected service folder. It as the format: <Namespace>.Connected_services.OData_Services.<filename>. I am reluctant to generate a match based on the full resource name because I have not seen an official documentation of this resource naming convention. I am concerned about taking a dependency on a format that's not documented when there other easier alternatives. I don't know if there's a guarantee that this naming convention would not be changed in some future version of Visual Studio or .NET without our knowledge.
About the tests
Frist, you many notice that while I have updated a lot of generated C# client code in the tests, I have not updated any test generated VB client code. This is because all of the tests in ODataConnectedServiceTests test against code that includes the CSDL in the reference code as a string, instead of as a separate file that's compiled as an embedded resource. That code path is not affected by the change in this PR. Only that OData.Cli.Tests test this code path. But we don't have any CLI tests for Visual Basic.
That made me realize that the OData CLI does not actually support Visual Basic, despite the fact the documentation says it does. I've created a separate issue to track that: https://github.com/OData/ODataConnectedService/issues/408
Secondly, while we do have tests to verify that the generate code looks as expected and compiles without errors, we don't have tests to verify that the code we generate runs as expected. For example, testing that when you run the generated code with two schemas that have overlapping names does not throw an exception or that the correct schema is selected. We achieve that using manual testing, which is tedious and not enforced consistently. I've created a separate issue to track an E2E testing environment: https://github.com/OData/ODataConnectedService/issues/409
Fixes #405
We store the CSDL file as an embedded resource. When we want to load the and parse the CSDL in memory, we find the resource that matches the target CSDL file name. We use
resourceName.EndsWith(filePath)to find the match. However, this causes a conflict when one CSDL name is a substring of the other, e.g.:OData ServiceCsdl.xmlAnother OData ServiceCsdl.xmlCurrently an exception is thrown when such a conflict is detected. This PR fixes this issue by resolving the conflict by selecting the shortest resource name that ends with the target filename.
This solves the problem for simple cases, and possibly all practical common scenarios. However, it would be problematic in the following cases:
OData ServiceCsdl.xmlin the same project and adds it as an embedded resource.I did not address the concerns because:
Csdl.xmlsuffix the service name set in the connected service configuration when generating the CSDL file. We do not expect the user to create files with this exact naming convention manually.The main issue here is that we
EndsWithto match the files and resources. Why don't we just check the full resource name? The full resource name does not match the file name, it's generated based on the assembly namespace and connected service folder. It as the format:<Namespace>.Connected_services.OData_Services.<filename>. I am reluctant to generate a match based on the full resource name because I have not seen an official documentation of this resource naming convention. I am concerned about taking a dependency on a format that's not documented when there other easier alternatives. I don't know if there's a guarantee that this naming convention would not be changed in some future version of Visual Studio or .NET without our knowledge.About the tests
Frist, you many notice that while I have updated a lot of generated C# client code in the tests, I have not updated any test generated VB client code. This is because all of the tests in
ODataConnectedServiceTeststest against code that includes the CSDL in the reference code as a string, instead of as a separate file that's compiled as an embedded resource. That code path is not affected by the change in this PR. Only thatOData.Cli.Teststest this code path. But we don't have any CLI tests for Visual Basic.That made me realize that the OData CLI does not actually support Visual Basic, despite the fact the documentation says it does. I've created a separate issue to track that: https://github.com/OData/ODataConnectedService/issues/408
Secondly, while we do have tests to verify that the generate code looks as expected and compiles without errors, we don't have tests to verify that the code we generate runs as expected. For example, testing that when you run the generated code with two schemas that have overlapping names does not throw an exception or that the correct schema is selected. We achieve that using manual testing, which is tedious and not enforced consistently. I've created a separate issue to track an E2E testing environment: https://github.com/OData/ODataConnectedService/issues/409