Don't document many=True schemas as array

[lafrech]

Fixes #383.

This fix is based on @Bangertm's analysis in #383 (comment).

I'm gonna get pawned by black. I'll try to fix those issues at home if I get the time.

[lafrech]

BTW, I didn't understand this part of the code. It sounded related but I left it unchanged.

            # marshmallow>=2.7.0 compat
            field.metadata.pop("many", None)

Any idea what this is about?

[sloria]

Thanks! I'm off to do errands but I'll hopefully have some time later today or tomorrow to review.

I'm gonna get pawned by black.

I really hope pre-commit/pre-commit.com#211 becomes a reality...

[sloria]

Any idea what this is about?

#64 is the PR that added that. I don't recall the rationale, though.. =/

[lafrech]

I've been using your pre-commit docker container at home but AFAIU, either I need to add my user to the docker group which is not recommended because it means more or less give my user root privileges, or I need to run it as root, which is not practical (and creates root-owned files in the repo).

I didn't take the time to try pyenv.

[lafrech]

I don't see anything related in the 2.7.0 entry of the changelog.
https://github.com/marshmallow-code/marshmallow/blob/dev/CHANGELOG.rst#270-2016-04-04

Removing the line doesn't break any test.

[sloria]

My guess is it's tied to this change: marshmallow-code/marshmallow#413 , which may result in many being added to field.metadata (?).

[lafrech]

Yes, probably.

But I don't think it makes any difference as we only pull a limited list of items from metadata (VALID_PROPERTIES + x-*).

I'd be tempted to remove it...

[sloria]

Sure, it's your call. If there's no way for many to make it incorrectly into the OAS doc, then it can be removed.

[lafrech]

One one hand, I'm pretty confident. On the other hand, I don't see what changed since the change was accepted, so if there was a good reason by the time, it should still be valid. I'm still in favor of removing. Happy if @Bangertm has an opinion about this and this PR.

[Bangertm]

Looks good to me. Thanks.

[sloria]

Code looks good, and I tested it out on the code in the OP. Works as expected.

from apispec import APISpec
from apispec.ext.marshmallow import MarshmallowPlugin
from apispec_webframeworks.flask import FlaskPlugin
from flask import Flask, jsonify
from marshmallow import Schema, fields
from pprint import pprint

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)
    pprint(spec.to_dict())
# {'components': {'parameters': {},
#                 'responses': {},
#                 'schemas': {'Category': {'properties': {'id': {'format': 'int32',
#                                                                'type': 'integer'},
#                                                         'name': {'type': 'string'}},
#                                          'required': ['name'],
#                                          'type': 'object'},
#                             'Pet': {'properties': {'category': {'items': {'$ref': '#/components/schemas/Category'},
#                                                                 'type': 'array'},
#                                                    'name': {'type': 'string'}},
#                                     'type': 'object'}},
#                 'securitySchemes': {}},
#  'info': {'title': 'Swagger Petstore', 'version': '1.0.0'},
#  'openapi': '3.0.2',
#  'paths': OrderedDict([('/pet',
#                         OrderedDict([('get',
#                                       {'responses': {200: {'content': {'application/json': {'schema': {'$ref': '#/components/schemas/Pet'}}}}}})])),
#                        ('/category',
#                         OrderedDict([('get',
#                                       {'responses': {200: {'content': {'application/json': {'schema': {'$ref': '#/components/schemas/Category'}}}}}})]))]),
#  'tags': []}

[sloria]

This closes the last issue in the 1.0 milestone. Are we a go to release? @lafrech @Bangertm

[lafrech]

Thanks for finishing this.

There's still this

            # marshmallow>=2.7.0 compat
            field.metadata.pop("many", None)

but it can wait. No big deal. AFAIU, it is useless but doesn't hurt.

I don't think any of the open issues justifies postponing 1.0.0, so let's go!

[lafrech]

OK, I get it. When that change was introduced in #64, there was a line below in the code that would include all metadata unconditionally:

https://github.com/lucasrcosta/apispec/blob/d9697baebfbba67808d42ec5a90de0cebc0cae29/apispec/ext/marshmallow/swagger.py#L129

ret.update(field.metadata)

This was before VALID_PROPERTIES were introduced.

It can now be safely removed. Sending a PR.