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

JS client: allOf mishandled, composition impossible #2161

Closed
demonfiddler opened this issue Feb 17, 2016 · 1 comment
Closed

JS client: allOf mishandled, composition impossible #2161

demonfiddler opened this issue Feb 17, 2016 · 1 comment

Comments

@demonfiddler
Copy link
Contributor

From what I can tell, when an API specification contains a type definition A that composes by reference to two or more other type definitions (allOf B, C, ...), the model code generated for A only knows about the properties of the last such composed type (C in this case). For an instance of A, a.constructFromObject() will only populate the properties defined by C; the other composed properties (of B in the case) will not be populated. There may be other issues with composition besides this.

Also, I've seen assertions in the Swagger docs that allOf entails type composition not inheritance. Yet based on the presence of a boolean io.swagger.codegen.DefaultCodegen.supportsInheritance field (default: false) there seems to be at least the intention of supporting inheritance yet the Java template does not set this to field to true but does in fact emit the appropriate extends <first-superclass> construct.

Anyhow, It would seem churlish not to exploit JavaScript's extension mechanism even if it is only single inheritance. Also, the JavaScript model.mustache emits a comment to the effect that the class extends the {{parent}} model, although right now I'm trying to understand why the declared JavaScript instance variables are sometimes those of the last listed super type yet in other cases those locally defined within the YAML for the template's current context definition. The following code at io.swagger.parser.util.SwaggerDeserializer lines 821-822 strikes me as suspect:

                    if (allComponents.size() >= 2) {
                        model.setChild(allComponents.get(allComponents.size() - 1)); // <== 822
                        List<RefModel> interfaces = new ArrayList<RefModel>();
                        int size = allComponents.size();
                        for (Model m : allComponents.subList(1, size - 1)) { // <== 825
                            if (m instanceof RefModel) {
                                RefModel ref = (RefModel) m;
                                interfaces.add(ref);
                            }
                        }

It seems to me that the above line 822 incorrectly assumes that the last component is always an anonymous local type defining the owned properties of the current type definition. If the type in question only inherits super-type properties but declares no owned properties, model.child will incorrectly be set to the last allOf super type!

Similarly, line 825 incorrectly excludes the last composed super type, based on the same erroneous assumption that it must necessarily be a local definition. Again, if the definition in question merely composes other definitions by reference, the last such definition needs to be included in the interfaces list.

The above two problems are separately reported as swagger-parser Issue #204.

Note: additionally, I'm unable to comment on the status where A extends B by the inclusion of additional locally defined properties, as I haven't found any examples of this pattern in the system I'm working on (and I don't own the schema or the server).

Finally, I'm not yet in a position to comment on the status with polymorphism (single inheritance) via allOf but I see there's a separate Issue #2041 open for that anyway.

@demonfiddler
Copy link
Contributor Author

This issue is largely addressed by swagger-parser Issue #204 and Issue #2258, both of which are now fixed. The remaining aspect of polymorphism is covered by Issue #2041.

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

2 participants