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

Docstring-only schemas incorrectly referenced as arrays? #383

Closed
whoiswes opened this issue Feb 4, 2019 · 4 comments

Comments

@whoiswes
Copy link

commented Feb 4, 2019

Tested with Marshmallow 3.0.0rc3 and APISpec 1.0.0rc1.

Took a copy of the APISpec quickstart code and removed the defintions, so that schemas were only being referenced from the docstrings. I'm seeing that the nested schema is being picked up as an array (incorrect) but then is also referenced as an array from the parent schema (correct).

Example (as short as I can make it, ignoring imports):

spec = APISpec(
    title="Swagger Petstore",
    version="1.0.0",
    openapi_version="3.0.2",
    plugins=[FlaskPlugin(), MarshmallowPlugin()],
)

app = Flask(__name__)

class CategorySchema(Schema):
    id = fields.Int()
    name = fields.Str(required=True)

class PetSchema(Schema):
    category = fields.Nested(CategorySchema, many=True)
    name = fields.Str()

@app.route("/pet")
def random_pet():
    """
    ---
    get:
      responses:
        200:
          content:
            application/json:
              schema: PetSchema
    """
    return jsonify({'foo':'bar'})

@app.route("/category")
def random_category():
    """
    ---
    get:
      responses:
        200:
          content:
            application/json:
              schema: CategorySchema
    """
    return jsonify({'foo':'bar'})

with app.test_request_context():
    spec.path(view=random_pet)
    spec.path(view=random_category)
    print(spec.to_dict()

Here's what gets output (note that both the nested reference and the child are both array types):

"schemas": {                                                  
  "Category": {                                               
    "type": "array",                                          
    "items": {                                                
      "type": "object",                                       
      "properties": {                                         
        "id": {"type": "integer", "format": "int32"}                                                     
      }                                                       
    }                                                         
  },                                                          
  "Pet": {                                                    
    "type": "object",                                         
    "properties": {                                           
      "category": {                                           
        "type": "array",                                      
        "items": {                                            
          "$ref": "#/components/schemas/Category"             
        }                                                     
      }                                                       
    }                                                         
  }                                                           
} 

I think the issue stems from fields2jsonschema method here where the schema attributes are checked for many=True. I would expect the schemas themselves would always be referenced as singletons, then references to those schemas would be checked for many. I certainly might be missing something though, but at least in my lab just commenting out those 2 lines fixed everything.

Alternately, the schemas could be registered as they are in the example, but we try to do everything in docstrings if possible. So if this is a non-issue feel free to close.

Many thanks for some awesome libraries!

@sloria sloria added the bug label Feb 5, 2019

@sloria sloria added this to the 1.0 milestone Feb 5, 2019

@sloria

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

Thanks for reporting @whoiswes . This looks like a legitimate bug.

I probably won't have time to look into this today. @lafrech or @Bangertm Can you take a look, since you're more familiar with the auto-referencing code?

@Bangertm

This comment has been minimized.

Copy link
Collaborator

commented Feb 5, 2019

I'm taking a look. Agree that this is a bug, but not sure the implications of making the change @whoiswes suggests.

@Bangertm

This comment has been minimized.

Copy link
Collaborator

commented Feb 6, 2019

It looks like there are three cases that need to be considered here:

  1. The case @whoiswes reported with a nested field with many=True. This case is not currently working properly because the nested schema is being added to the spec as an array instead of as an object.
  2. The case with a nested field with many=True and the user passes a schema_name_resolver that returns None for the nested function. This case will define the nested schema in line as an array and is currently working properly.
  3. The case where a user passes a top level schema with many=True to spec.component.schema. This case currently adds the schema to the spec as an array, which is the intended behavior. It may make more sense to ignore many=True in this case and just add the schema to spec as a singleton. The user would then just have to reference the schema from within an array if they had routes that returned arrays of objects. We are currently ignoring many as an instance parameter specifically because of the nested case.
@whoiswes

This comment has been minimized.

Copy link
Author

commented Feb 8, 2019

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.