Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

discriminator generated in both base and child #3829

Closed
AlexNolasco opened this issue Sep 19, 2016 · 12 comments
Closed

discriminator generated in both base and child #3829

AlexNolasco opened this issue Sep 19, 2016 · 12 comments

Comments

@AlexNolasco
Copy link
Contributor

AlexNolasco commented Sep 19, 2016

Description

Should the discriminator property be also a property of the child class even if it is already in the base class?

Swagger-codegen version

2.2

Swagger declaration file content or url

ticket.yaml.txt

Command line used for generation

Objective-C and C# client generators generate the petType in the base and child class. Thus generating compilation warnings. OTOH, the Java generator does not duplicate the property.

Steps to reproduce

Simply generate C# or Objective-C client code for the YAML aforementioned

Related issues
Suggest a Fix

Child classes should not duplicate properties found in the base class. The work around is to only define the discriminator in the base class and then copy/paste the property on each child class.

@wing328
Copy link
Contributor

wing328 commented Sep 21, 2016

@AlexanderN am I understanding correctly that that the Java client does not have the issue and only ObjC, Swift and C# client has the issue?

@AlexNolasco
Copy link
Contributor Author

AlexNolasco commented Sep 21, 2016

@wing328

am I understanding correctly that that the Java client does not have the issue and only ObjC and C# client has the issue?

Correct,

I've attached the Java client generated code for the yaml in question which does what you will expect. Base properties are not copied to the child class.

Dog.java.txt
Pet.java.txt

On the other hand, the Objective-C does duplicate the properties
SWGPet.h.txt
SWGCat.h.txt

Possible workaround would be to mark the child properties as dynamic to avoid the compilation warnings produced by the current output. Not having the parent properties would probably be best.

The C# client generation also copies the properties found in the parent class
Pet.cs.txt
Dog.cs.txt

@AlexNolasco
Copy link
Contributor Author

@wing328

I've tested Swift Client and it does not use inherit the Pet class, it uses composition instead. So, it should not be part of this issue.

@cbornet
Copy link
Contributor

cbornet commented Sep 21, 2016

languages that support inheritance should have useInheritance=true. I guess it should be set for Obj-C and C# codegens.

@AlexNolasco
Copy link
Contributor Author

AlexNolasco commented Sep 21, 2016

@cbornet

As you mentioned, I see some references to useInheritance
https://github.com/swagger-api/swagger-codegen/search?utf8=%E2%9C%93&q=useInheritance

One would assume to see this in the Java client though.

@cbornet
Copy link
Contributor

cbornet commented Sep 21, 2016

Oh, my mistake ! It's supportsInheritance I was talking about !

@AlexNolasco
Copy link
Contributor Author

@cbornet

In that case, I wonder if the fix is as simple to add the code below to the clients in question.

 if (additionalProperties.containsKey(USE_INHERITANCE)) {
              setUseInheritance(Boolean.parseBoolean((String)additionalProperties.get(USE_INHERITANCE)));
        } else {
            supportsInheritance = true;
}

@cbornet
Copy link
Contributor

cbornet commented Sep 21, 2016

I would say just supportsInheritance = true;. The useInheritance property is only in javascript to have the choice between inheritance and composition.

@AlexNolasco
Copy link
Contributor Author

@cbornet
Makes perfect sense, I've submitted a commit to my fork. I still need to figure out if it may break any unit tests.

https://github.com/alexandern/swagger-codegen/commit/585f23185688aad7d46a6faa402abc0cbbddc20a

@cbornet
Copy link
Contributor

cbornet commented Sep 22, 2016

You should regenerate the samples. There are scripts for this in the ./bin directory.

@AlexNolasco
Copy link
Contributor Author

AlexNolasco commented Sep 28, 2016

@cbornet
Ok, I ran the ./bin/objc-petstore.sh against petstore-with-fake-endpoints-models-for-testing.yaml instead of petstore.json and I seem to generate what one would expect.

@interface SWGAnimal : SWGObject
@property(nonatomic) NSString* className;
@property(nonatomic) NSString* color;
@end

Child class no longer copies the className property


#import "SWGAnimal.h"
@protocol SWGCat
@end
@interface SWGCat : SWGAnimal
@property(nonatomic) NSNumber* declawed;
@end

All in all, changing ObjcClientCodegen.java appears to do the work which makes me wonder why it it was false. It's my understanding that using allOf along with a discriminator is an indication of inheritance for code-gen.

    public ObjcClientCodegen() {
        super();
        supportsInheritance = true;
        ... 
    }

@AlexNolasco
Copy link
Contributor Author

@cbornet
Submitted a pull request
#3903

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants