OneOf Polymorphism

[Bangertm]

One of the nice features in OpenAPI 3 is support for oneOf polymorphism. In one of our API's we are using marshmallow-oneofschema to enable serialization/deserialization/validation of multiple schemas on an endpoint or in a nested field. This merge request enables API's using marshmallow-onofofschema to be described in OpenAPI 3 format.

A couple notes;

• I'm testing to see if the schema matches the marshmallow-onofschema API as a determination of if the schema is a oneOf schema. This seemed cleaner than testing for inheritance.
• I think the most user friendly behavior when apispec encounters a marshmallow-oneofschea is to add a oneOf schema object to the spec and also add each of the referenced schemas to the spec. This way the user doesn't have to add them separately. The potential concern I encountered is that by default nested fields are not resolved right away and json.dumps serialization of the spec results in a RuntimeError: dictionary changed size during iteration. (Oddly this doesn't happen with print) I was able to work around this by using the new schema_name_resolver functionality, but I'm not 100% sure this is the right thing to do.
• I added most of the new code and tests to new modules because swagger.py was getting pretty long.
• Marshmallow-Polyfield provides similar functionality, but would require additional work to represent in a spec.

[lafrech]

Hi.

Thanks for this. I'd really like such a feature added to apispec.

I've been doing this on a project of mine. I'm also using marshmallow-oneofschema.

From what I remember, OpenAPI v2 also allows polymorphism, but it is "not well defined" (OAI/OpenAPI-Specification#403). OpenAPI v3 makes it better: you don't have to use the class name (which should be hidden implementation detail) as the discriminator. Overall, I think it makes sense to support only OpenAPI v3 polymorphisme, like you did.

I'm testing to see if the schema matches the marshmallow-onofschema API as a determination of if the schema is a oneOf schema. This seemed cleaner than testing for inheritance.

Why do you think the inheritance test wouldn't be clean? Do you just want to avoid importing marshmallow-oneofschema? Checking type_field and type_schema restricts the test to marshmallow-oneofschema anyway. Unless you want to allow another class to propose this interface?

I think this PR adds an optional dependency on marshmallow-oneofschema. It would be nice if a polymorphic field was added to Marshmallow, but last time I asked (can't find the source anymore) @sloria didn't want to integrate that into marshmallow and recommended using marshmallow-oneofschema.

If we go that way, we could add marshmallow-oneofschema as an optional dependency explicitly, try/except its import, only use the feature if import succeeds and test inheritance using isinstance. Just thoughts. I'm totally open to discussion about this.

I think the most user friendly behavior when apispec encounters a marshmallow-oneofschea is to add a oneOf schema object to the spec and also add each of the referenced schemas to the spec. This way the user doesn't have to add them separately.

Yes, this is what I wanted to achieve but I didn't go that far yet. My client code registers all schemas in a loop.

The potential concern I encountered is that by default nested fields are not resolved right away and json.dumps serialization of the spec results in a RuntimeError: dictionary changed size during iteration. (Oddly this doesn't happen with print) I was able to work around this by using the new schema_name_resolver functionality, but I'm not 100% sure this is the right thing to do.

This should be investigated. Unfortunately, I have no time to look into this right now.

I added most of the new code and tests to new modules because swagger.py was getting pretty long.

Good.

This is definitely a feature worth having. It is a recurrent request.

We'd rather get it right the first time, though.

[Bangertm]

Regarding the test, if there was a canonical way of supporting polymorphism with Marshmallow this would be easier... I don't feel strongly about it, testing for the API still seems simpler than adding an optional dependency, try/excepting the import, optioning the feature based on the import result, and testing for inheritance.

If one were to rewrite marshmallow-onofschema in light of OpenAPI 3, the attributes would probably be property_name and mapping. In which case, sure, multiple classes could have that interface.

The issue with adding nested marshmallow-oneofschema's to a spec comes down to this line where an unevaluated function is added to a LazyDict. By default the function is not evaluated until it is referenced in the LazyDict. Using the schema_name_resolver works around the issue because it adds a hook to immediately evaluate the function instead of waiting for something like json.dumps to resolve the function as part of a loop over the entire LazyDict.

[lafrech]

Regarding the test, if there was a canonical way of supporting polymorphism with Marshmallow this would be easier...

Yes, totally. I wish a polymorphic schema feature was added to Marshmallow. AFAIK, @sloria prefers to keep oneofschema as an external library. Maybe it is not deemed canonical enough to be part of the core. But this makes it more complicated for external libs like apispec (well, I use the plural "libs", but apispec is the only case I know of).

[timakro]

Until this is ready you can use my plugin apispec-oneofschema for this.

[lafrech]

Thanks @timakro.

I just noticed @timakro's plugins are GPL, not MIT. No license war intended but I thought it would be worth mentioning here because it is not that usual in this ecosystem so anyone could use them inadvertently and because this means it is still worth having in apispec (or as an MIT plugin).

[timakro]

@lafrech I moved to the less restrictive LGPL.

[sloria]

I'm still lukewarm about adding first-class support for a oneofschema. It feels like more complexity than is worth adding to both apispec and marshmallow.

This isn't to say we'll never support it, but I don't think it's a priority right now.

Closing this, since it's gone stale. Thanks anyway @Bangertm for the proposed implementation and thanks @timakro for releasing the plugin.

[deckar01]

What if this package provided OneOf as a custom field? It should be possible to implement apispec's "discriminator" strategy as a subclass of Nested. The discriminator map isn't technically required by OpenAPI, but it would be needed so that the field could figure out which schema to use.

from marshmallow import Schema, fields
from apispec.ext.marshmallow import OneOf

class PetSchema(Schema):
    animal = OneOf('species', {'turtle': TurtleSchema, 'dog': DogSchema, 'cat': CatSchema})
    name = fields.Str()

Edit: Updated the example to specify the discriminator field name.

[lafrech]

@deckar01 do you mean create a polymorphic field à la oneofschema in apispec?

The original author of oneofschema is not interested in it anymore (marshmallow-code/marshmallow-oneofschema#17 (comment)) but offered to transfer ownership so it would be nice to either have some polymorphic field implementation either in marshmallow or in apispec (marshmallow would be a better place IMHO), or if openofschema is the external lib we recommend, at least keep an eye on the status of the project.

[deckar01]

I was suggesting a OneOf field rather than a OneOf schema. It wouldn't support OneOf for the response level schema, but it could be used for nested fields and non-homogeneous lists. It would also be much easier to implement.

[lafrech]

Oh, sorry. I didn't notice the field vs. schema distinction.

Serialization would also require a class -> schema mapping.

I don't see right now how this is easier than oneof schema, but I wouldn't mind the lack of response level polymorphism and I see why you're suggesting to put it in apispec rather than marshmallow.

(Yet, a polymorphic schema in marshmallow is a recurrent request and would be great.)