[NancyFx] Inheritance - Githubissues

craffael commented 6 years ago

Description

Currently the Nancy Generator does not support inheritance at all. I'll briefly discuss the current status of nancy w.r.t. inheritance based on the simple Pet/Cat model found further down.

Models:

The models inherit correctly from each other:

public class Pet:  IEquatable<Pet>{...}
public sealed class Cat: Pet,  IEquatable<Cat>{...}

However, the builder of the Cat currently doesn't allow setting the Pet properties (with immutable=true):

public sealed class CatBuilder {
  public CatBuilder HuntingSkil(string value) {...}
  public Cat Build() {...} 
  ... private members....
}

Deserialization:

The /pet endpoint deserializes subtypes (such as a Cat) currently wrong, i.e. it just serializes them as a Pet and ignores all additional properties of Cat:

Post["/pet"] = parameters =>
{
    var body = this.Bind<Pet>();
    Preconditions.IsNotNull(body, "Required parameter: 'body' is missing at 'PetPost'");

    return service.PetPost(Context, body);
};

Serialization:

Serialization works out of the box. That is a Cat is serialized as a Cat. The only problem is, that the discriminator is not automatically set (and because of the problem with model described above, it can in fact not be set at all).

Swagger-codegen version

2.3-Snapshot

Swagger declaration file content or url

Here is the swagger file that I have used above to describe the current status:

swagger: "2.0"
info:
  version: "1.0.0"
  title: Demo

paths:
  /pet:
    post:
      summary: "post a bar"
      parameters:
        - name: body
          in: body
          required: true
          schema: 
            $ref: '#/definitions/Pet'
      responses:
        201:
          description: successful.
          schema:
            $ref: '#/definitions/Pet'

definitions:
  Pet:
    discriminator: type
    type: object
    required:
      - type
    properties:
      type:
        type: string
      name:
        type: string

  Cat:
    allOf:
      - $ref: '#/definitions/Pet'
      - properties: 
          huntingSkil:
            type: string

Command line used for generation

-l nancyfx --additional-properties packageContext=v1,interfacePrefix=I

Related issues/PRs

I have not found a related issue/PR.

Suggest a fix/enhancement

I would be interested very much to fix this problem but I was hoping to get a bit of feedback before I start with it (@jimschubert ?, @mandrean ?).

Especially the deserialization issue is not so easy. At the moment I'm considering two possible solutions but maybe there are even better ideas...: 1) Rely on json.net and the nancy integration. This seems to be the industry standard for c# and is also used by the Restsharp swagger client. However it would be an additional dependency. This was also suggested by the folks from the #nancyfx slack channel. 2) Build a custom ModelBinder for every type with a discriminator. This will probably be a bit slower than the json.net implementation but it would avoid the introduction of an additional dependency.

In order to solve the problem with the model builder, I suggest to use the approach suggested here.

jimschubert commented 6 years ago

@craffael It sounds like most of the issue is in the implementation of the NancyFX server generated code, which I'm not that familiar with.

I'd recommend if you take this up, sticking with the JSON.net route. This is likely going to be ~affected by~ similar to #6969 (which was merged into master about a week before you opened this PR), and #7043 (if you're targeting 3.5).

I've had some computer issues over the last few weeks, but I'm back in business and I could try to assist if you get stuck. I have a friend who is somewhat familiar with NancyFX, and I may be able to lean on him a bit for advice.