Skip to content

[csharp] Fix ToJson to work with composition and polymorphism#7399

Merged
wing328 merged 4 commits intoswagger-api:masterfrom
jimschubert:csharp-inheritance-7358
Jan 22, 2018
Merged

[csharp] Fix ToJson to work with composition and polymorphism#7399
wing328 merged 4 commits intoswagger-api:masterfrom
jimschubert:csharp-inheritance-7358

Conversation

@jimschubert
Copy link
Copy Markdown
Contributor

@jimschubert jimschubert commented Jan 13, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Previous implementation assumed specification only supports polymorphic
associations (via discriminator), although the code didn't seem to be
setup correctly for that in the first place. That is, the parent object
must define the discriminator (see
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#models-with-polymorphism-support),
so NOT HAS parent AND HAS discriminator doesn't make sense.

From a C# perspective, base classes should have the method marked
virtual and derived classes should override the method. This supports
both composition and polymorphic definitions.

This will resolve #7358

Previous implementation assumed specification only supports polymorphic
associations (via discrimator), although the code didn't seem to be
setup correctly for that in the first place. That is, the parent object
must define the discriminator (see
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#models-with-polymorphism-support),
so NOT HAS parent AND HAS discriminator doesn't make sense.

From a C# perspective, base classes should have the method marked
virtual and derived classes should override the method. This supports
both composition and polymorphic definitions.
@jimschubert
Copy link
Copy Markdown
Contributor Author

/cc #@mandrean

@wing328 wing328 added this to the v2.4.0 milestone Jan 22, 2018
@wing328 wing328 merged commit 1c4e6b7 into swagger-api:master Jan 22, 2018
jimschubert added a commit to jimschubert/swagger-codegen that referenced this pull request Jan 22, 2018
* master: (133 commits)
  add a link to ebook (polish version)
  Add R namespace file (swagger-api#7467)
  Add Spring Petstore samples (async, java8-localdatetime) to CircleCI (swagger-api#7468)
  Revised core team members
  Ada code generator corrected: "=>" instead of "->". Fixes swagger-api#7450 (swagger-api#7456)
  Fixes issue swagger-api#7177 (SpringCodeGen dateLibrary "java8-localdatetime" option is ignored). (swagger-api#7178)
  Issue-7438 Fix that prevents generating interfaces when interfaceOnly is false. (swagger-api#7439)
  swagger-api#7093 - Add maven wrapper (swagger-api#7356)
  [csharp] Support arrays of arrays for properties and models (swagger-api#7400)
  [csharp] Fix ToJson to work with composition and polymorphism (swagger-api#7399)
  [csharp] Reference this.Configuration in client api template (swagger-api#7394)
  [JAX-RS][Spec] Removes throws Exception. (swagger-api#7437)
  Use supportsES6 flag in ts compilation for language typescript-angular (swagger-api#7408)
  Fix 7457: [Ada] wrong order for generated structures in *-models.ads (swagger-api#7462)
  Fix 7459: [Ada] wrong JSON in POST operations (swagger-api#7460)
  [erlang-client] Erlang request utils (swagger-api#7257)
  reenable pushing snapshot to maven repo
  Create CODE_OF_CONDUCT.md
  comment out update to docker image
  deleted unnecessary notes (swagger-api#7454)
  ...
viclovsky pushed a commit to viclovsky/swagger-codegen that referenced this pull request Jan 23, 2018
…r-api#7399)

* [csharp] Support composition on toJson

Previous implementation assumed specification only supports polymorphic
associations (via discrimator), although the code didn't seem to be
setup correctly for that in the first place. That is, the parent object
must define the discriminator (see
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#models-with-polymorphism-support),
so NOT HAS parent AND HAS discriminator doesn't make sense.

From a C# perspective, base classes should have the method marked
virtual and derived classes should override the method. This supports
both composition and polymorphic definitions.

* [csharp] Regenerate integration test files

* [csharp] Regenerate samples

* [csharp] Regenerate security sample
@jimschubert jimschubert deleted the csharp-inheritance-7358 branch February 3, 2018 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C#] Error CS0506 'Account.ToJson()': cannot override inherited member 'BaseAccount.ToJson()' because it is not marked virtual, abstract, or override

2 participants