java support inheritance

[ahgittin]

as per #1015 (and #946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).

(this is a resubmission of #1094 targetting master branch -- see comments there)

[webron]

Just a reminder, before merging this we need to clarify the inheritance with regards to the spec. Can't merge it for now.

[ahgittin]

To expand on @webron -- this does not fix the underlying problem which I think is that what should be treated as extension is implemented as inheritance in the current Java implementation. However this expands the (already very large) set of cases where the theoretically-wrong implementation works totally fine. Partial work towards a proper implementation (see #1094 for discussion) has caused a regression in the cases where java codegen worked, including the model we use (of course) and the following cut-down model:

swagger: '2.0'
info:
  title: buggy inheritance for java
  version: "0.0.1"
definitions:
  Super:
    properties:
      my_enum:
        type: string
        enum: [ A, B ]

  Sub:
    allOf:
    - $ref: '#/definitions/Super'
    - type: object
    properties:
      another:
        type: string

paths:
  /home:
    get:
      summary: java generation for this fails with `develop_2.0` branch as at 21 Aug 2015, fixed with #1120

[cbornet]

Note that if you have multiple allOf with enums, the class extension trick will only work for one of the allOf (since we can only extend one class) so I think this is not an optimal solution.

[erikvanzijst]

What's the status of this PR?

[ahgittin]

No idea. This was a useful band-aid but not a cure-all. Maybe someone has improved the underlying problems in which case this could be closed?

[nickcmaynard]

Right now we have a situtation where:

• The JavaClientCodegen implementations/extensions specify supportsInheritance = false, so we end up with redundant properties.
• Nonetheless, the Java model.mustache definitions all specify "X extends Y" in the class definition.

So, the child models specify that they extend another model, but then copy in their properties anyway. This is Bad Java.

I'd like to see a single decision here - the models in Java either extend, or they don't. Right now, they're straddling that boundary.

[nickcmaynard]

@webron Please can you chip in? Right now, for a bunch of previously "working" cases, Java codegen doesn't work properly. It's also somewhat schizophrenic, where generated model classes declare extension, but nonetheless redefines the properties. See comment above.

If we can't enable supportsInheritance on a theoretical blocker, then I'd suggest exposing this parameter through the CLI and allowing users to override it.

[webron]

We have a clearer idea now, but I'm not really sure what's implemented right. I'd rather have @wing328 chip in and look at it.

[wing328]

Please refer to #2096 for the latest discussion on allOf

[jeff9finger]

@wing328 What is the status of this?

[wing328]

Please refer to #2096 for the latest update on supporting inheritance in Java.

[cbornet]

This solution doesn't work when a definition imports multiple models through allOf : only the first model is taken for inheritance (obviously) and the attributes of the other models are not copied.

[yaelah]

This is a philosophical question - does "allOf" indicate inheritance?
If yes, then you can inherit only from one class in Java.
If not, then there should be something else to clearly indicate inheritance.

I was using the supportsInheritance flag also to suppress copying of all the fields from the parent class to the subclass. The subclasses should not have an extra copy of all the fields of the parent class.

[cbornet]

The spec is quite clear about that : allOf indicates inheritance only if the referenced model holds a discriminator field. Otherwise it is composition.
The next release of the swagger-parser will allow to set supportInheritance with Java but until then we can't set it.

[cbornet]

Can be closed.

[FireZenk]

When this is planned to integrate? Can I help on anything related?

[cbornet]

@FireZenk It's been integrated long ago. This issue can be closed.