[csharp] Use virtual keyword for hooks

[sbward]

In C# a partial method can only be overridden from the same assembly where it's defined, which means all of the partial void hooks in csharp templates are actually unusable. To achieve the original intention of defining hooks that can be overridden by clients (edit: I was mistaken, this is not the original intention of the hooks, but it is this PR's intention), this change updates the methods to be marked as public virtual with an empty body. This is not a breaking change because the partial void methods have always been unusable (edit: by end-users).

CC: C# technical committee (@mandrean @shibayan @Blackclaws @lucamazzanti @iBicha)

Fixes #14609

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[devhl-labs]

I don't think we want this change. The intent of the virtual methods is to enable library owners to provide the body of the virtual method if desired in a manually written file, not for downstream end users to inherit the class. You're also adding the virtual keyword to sealed classes. If you want to use this approach to fix the issue, please narrow the scope of the change.

[sbward]

You're also adding the virtual keyword to sealed classes.

The classes are public and are extensible by the end user, unless I'm missing something?

The intent of the virtual methods is to enable library owners to provide the body of the virtual method if desired in a manually written file, not for downstream end users to inherit the class.

That's fair, but why not allow both library owners and end users to extend the class? The library owners should not be expected to provide custom tailored solutions for every end user's need. For example if the HttpClient must be configured a certain way for a given app, this change must be possible for the end user to make.

Due to the way RestSharp works (it creates its own HttpClient instances), we cannot even provide a constructor for the end user to pass in their custom HttpClient. Instead we must provide this hook so that the HttpClient can be modified after RestSharp creates it.

[devhl-labs]

Since your fixing a RestSharp specific problem, I suggest you limit the scope of this change to RestSharp.

[sbward]

Since your fixing a RestSharp specific problem, I suggest you limit the scope of this change to RestSharp.

Thank you for your suggestion. I looked into it, but it is not possible to access the RestSharp instance inside the generated ApiClient class. Since OpenAPI does not provide sufficient access to RestSharp the end user cannot customize its behavior.

As a workaround, I am attempting to extend the client library I'm using by adding a manually implemented partial implementation of InterceptRequest like you suggested. However since I'm just an end-user of this library and it's presumably a very common use-case to need to customize the HttpClient instance inside various SDKs, I would suggest that there is still an unfulfilled need here, as demonstrated by the existence of #14609.

[sbward]

Updating InterceptRequest/InterceptResponse to be virtual would still allow library authors to customize their behavior while also allowing end users to do so. Please reconsider adopting this change. Thank you.

[devhl-labs]

Perhaps there was a misunderstanding. This change as first written is changing the csharp genertator for three libraries: restsharp, httpclient, and generichost. The issue your concerned with only impacts one of those three libraries.

[wing328]

@sbward thanks for the PR

can you please PM me via Slack when you've time?

https://join.slack.com/t/openapi-generator/shared_invite/zt-2wmkn4s8g-n19PJ99Y6Vei74WMUIehQA