CSharp: add support for excluding models from a proxy by referencing …

[jhancock93]

…external assembly

• Change CustomSettings to be dictionary of object to allow for some settings to be arrays (this is used for referencing one or more external assemblies)
• Allow CodeGenSettings to be passed in as a json file on the commandline rather than the settings in the original swagger document (better for this to be a decision by the person generating the proxy than the service provider)
• Change CSharpGenerator to be able to reference external assemblies and
  namespaces when assembly is passed in on commandline (like WCF svcutil). Namespaces of the matching types are automatically referenced in the proxy so the proxy does not need to be hand-edited.

[azuresdkci]

Can one of the admins verify this patch?

[azurecla]

Hi @jhancock93, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, AZPRBOT;

[jhancock93]

This is some what related to issue #642 but is a different way of dealing with external types, more along the lines of how the WCF svcutil allowed types to be referenced from external assemblies.

[azuresdkci]

Can one of the admins verify this patch?

[jhancock93]

Rebased pull request to more recent version of master to avoid merge conflicts.

[jhancock93]

I have one test that I added that passes on Windows but is failing when run by Travis. Any suggestions on how to reproduce the issue locally?

[tbombach]

@azuresdkci test this please!

[jhancock93]

I see there are now changes that require conflicts to be merged. I can rebase this branch on top of master again and fix the conflicts. Will one of the admins please respond as to whether I should go ahead and do that? This PR has been sitting idle for 2 weeks...

[tbombach]

@jhancock93 I'll take a look at this today and leave any comments if there are any necessary changes before you have to rebase.

[tbombach]

I don't think you need this suppression anymore, since you removed the try/catch from an earlier commit

[jhancock93]

Good catch! I have removed this line.

[jhancock93]

Rebased on latest master and addressed code review comments.

[hovsepm]

@azuresdkci retest this please

[jhancock93]

I don't understand what might cause the "No test results found" error in the "default" check. Is there something I need to do from my end to fix the problem?

[fearthecowboy]

Howdy, @jhancock93 -- We've been discussing this PR in our internal standup.

I really like the parts about using a response-file on the command line for pulling in the settings. If you give me a PR with that separately, we'll take that right away.

We're mulling over how we want to handle skipping code-gen of referenced classes. Currently you can mark a type as x-ms-external ( see https://github.com/Azure/autorest/blob/master/Documentation/swagger-extensions.md#x-ms-external ) .

I've also got some ideas swirling around on how to do some declarations a bit more generically so we can apply the same pattern against multiple language generators.

Is there a something that x-ms-external doesn't do that you'd need in order to use that?

[jhancock93]

Thanks for taking a look at this PR. [UPDATE]: I have separated out the piece for using a JSON file to supply code gen settings on the command line and created PR #1147.

Referencing external types is a complex issue, so I'm glad to have a discussion about it. I was aiming for functionality along the lines of what SvcUtil provides for WCF services. It provides similar functionality to exclude types from a proxy based on what it finds in a externally referenced DLL. I have a team that is transitioning a bunch of services from SOAP to WebAPI, so it was convenient to be able to reference the same contract assemblies for SOAP proxies generated by SvcUtil and WebAPI proxies generated by AutoRest. Before I implemented my changes, I looked at the x-ms-external tag and there were a few issues with it that I could see:

1. As I recall, x-ms-external only works with the Azure generator right now.
2. The x-ms-external tag doesn't say anything about what namespace the type is in. So unless the type is in a namespace that would already be referenced by the client, the client won't compile without manual modifications. In contrast, my PR finds the namespace associated with types with matching names in the external DLL and adds them to a namespaces property that then creates using statements in the client so it compiles with no manual code changes.
3. Because the x-ms-external tag is part of the swagger json, the decision whether to share types or not must be made by the author of the service API, not the client owner that consumes the API. In constrast, my change allows both of kinds of clients to be generated, one that generates the types as part of the client and one that references the external DLL depending on the needs of the client.
4. For this particular use case of sharing a C# contracts assembly, we would only want to exclude types when doing CSharp proxy generation and would want to still generate model classes for other languages. That is something I can do with this methodology but not with x-ms-external because it is part of the swagger definition instead of a client generation option.

[fearthecowboy]

As I recall, x-ms-external only works with the Azure generator right now.

IIRC, yeah, that's on my list of things to fix.

The x-ms-external tag doesn't say anything about what namespace the type is in. So unless the type is in a namespace that would already be referenced by the client, the client won't compile without manual modifications. In contrast, my PR finds the namespace associated with types with matching names in the external DLL and adds them to a namespaces property that then creates using statements in the client so it compiles with no manual code changes.

That was one of my assumptions, glad to see it validated.

Because the x-ms-external tag is part of the swagger json, the decision whether to share types or not must be made by the author of the service API, not the client owner that consumes the API. In constrast, my change allows both of kinds of clients to be generated, one that generates the types as part of the client and one that references the external DLL depending on the needs of the client.

Currently true, but I'm thinking of a way to allow you to specify that tangential to the swagger itself.

For this particular use case of sharing a C# contracts assembly, we would only want to exclude types when doing CSharp proxy generation and would want to still generate model classes for other languages. That is something I can do with this methodology but not with x-ms-external because it is part of the swagger definition instead of a client generation option.

And that's my other assumption validated.

Again, I am thinking about a different workaround that would get us past the need to embed it in the swagger, but ... you're right, if the namespace isn't the same as the expected namespace, it's not going to find it...

Lemme get back to you.

[jhancock93]

Any more discussion on this topic? I see that the team is currently working to reorganize the folder structure in the repo...

[devigned]

@fearthecowboy and @tbombach can we get a thumbs up or down on this functionality prior to pulling in #1220?

[fearthecowboy]

This will need to be refactored to work in the new code base (going into a branch RefactoredCodeModel today)

Fixing externals is something that's definitely in my short term planning.