explode and style are hardcoded in OpenAPIConverter.property2parameter

[lafrech]

I've been trying to document webargs's DelimitedList (child class of List) and I'm stuck because I'd need to override hardcoded stuff.

    def field2parameter(self, field, name, default_in):
        location = field.metadata.get("location", None)
        prop = self.field2property(field)
        return self.property2parameter(
            prop,
            name=name,
            required=field.required,
            multiple=isinstance(field, marshmallow.fields.List),   # <--- List
            location=location,
            default_in=default_in,
        )

    def property2parameter(self, prop, name, required, multiple, location, default_in):
        openapi_default_in = __location_map__.get(default_in, default_in)
        openapi_location = __location_map__.get(location, openapi_default_in)
        ret = {"in": openapi_location, "name": name}

        if openapi_location == "body":
            ret["required"] = False
            ret["name"] = "body"
            ret["schema"] = {"type": "object", "properties": {name: prop}}
            if required:
                ret["schema"]["required"] = [name]
        else:
            ret["required"] = required
            if self.openapi_version.major < 3:
                if multiple:
                    ret["collectionFormat"] = "multi"
                ret.update(prop)
            else:
                if multiple:                    # <--- when List, set explode and style
                    ret["explode"] = True
                    ret["style"] = "form"
                if prop.get("description", None):
                    ret["description"] = prop.pop("description")
                ret["schema"] = prop
        return ret

To document DelimitedList, we'd need to set explode to False. That's assuming we use default , as delimiter. Using space or pipe would require to also modify style.

To make this more generic, we could remove multiple and create an extensible mechanism to allow custom post-processings after property2parameter. And add a post-processor for List to do what's currently done with multiple.

Kinda like what's been done with attribute_functions in FieldConverter.

(Maybe we could separate the property2bodyparameter case as right now it does not involve a specific List case.)

[Bangertm]

Separating this out like with the attribute_functions makes sense to me.

Note that based on the comment in #499 about not parsing location from metadata the path for openapi_location == "body" won't be used going forward because that logic is captured in schema2parameters here:

apispec/src/apispec/ext/marshmallow/openapi.py

Lines 122 to 135 in 15a06eb

	if self.openapi_version.major < 3 and openapi_default_in == "body":
	prop = self.resolve_schema_dict(schema)
	param = {
	"in": openapi_default_in,
	"required": required,
	"name": name,
	"schema": prop,
	}
	if description:
	param["description"] = description
	return [param]

.

[lafrech]

Good. Looks like that change (location meta removal) will allow us to dump some code here.

[Bangertm]

This part of fields2parameters won't be needed either:

apispec/src/apispec/ext/marshmallow/openapi.py

Lines 169 to 182 in 15a06eb

	if (
	self.openapi_version.major < 3
	and param["in"] == "body"
	and body_param is not None
	):
	body_param["schema"]["properties"].update(param["schema"]["properties"])
	required_fields = param["schema"].get("required", [])
	if required_fields:
	body_param["schema"].setdefault("required", []).extend(
	required_fields
	)
	else:
	if self.openapi_version.major < 3 and param["in"] == "body":
	body_param = param

[lafrech]

Let's wait for webargs 6 and tackle this (remove location + this issue) in apispec 4.

[lafrech]

@Bangertm, your comments assume that fields2parameter is reached only through schema2parameters.

Should we make it private API?

The tests currently call it directly. We'd have to rework them.

[Bangertm]

The one reason to leave it public is that currently apispec does not handle webargs arguments defined as a dictionary of name, field pairs. The tests calling fields2parameter are basically exactly what you would see from a webargs dictionary. If we wanted to handle this case we would likely put in some logic to call fields2parameter directly from SchemaResolver.resolve_schema.

Other than that there isn't a good reason for leaving it public.

[lafrech]

I see what you mean.

I'm leaning towards making all those methods private.

We could support the dict case by copying dict2schema from webargs. In the MA3 case, it just calls Schema.from_dict so ultimately, when dropping MA2, it will be removed.

[pmdarrow]

@lafrech did you ever find an easy way to work around this limitation? I'm trying to do the exact same thing, document a DelimitedList field. I'm looking at subclassing or monkeypatching OpenAPIConverter to do it at this point.

[lafrech]

Nope, this is still a TODO.

The path is relatively clear. I just didn't get the time to do it.

I'd like to finish #526 then address this. Once this is done, user code can provide custom behaviour for DelimitedList. Not sure that custom code should be in apispec, but I was thinking of adding it to flask-smorest, for instance.

[pmdarrow]

Ok, thanks again for your work @lafrech! In the meantime, I monkeypatched field2parameter to manually manipulate the explode param. Here's how to do it if anyone else runs into this:

from apispec.ext.marshmallow import openapi
from marshmallow import fields
from webargs.fields import DelimitedList


def patched_field2parameter(self, field, *, name, default_in):
    """
    Monkeypatch apispec to add proper support for webargs `DelimitedList` field.

    A fix may be coming in a future version of the library. More details here:
    https://github.com/marshmallow-code/apispec/issues/500
    """
    # This section is copied directly from
    # https://github.com/marshmallow-code/apispec/blob/dev/src/apispec/ext/marshmallow/openapi.py#L193
    location = field.metadata.get('location', None)
    prop = self.field2property(field)
    param = self.property2parameter(
        prop,
        name=name,
        required=field.required,
        multiple=isinstance(field, fields.List),
        location=location,
        default_in=default_in,
    )

    # This section has been introduced by us
    if isinstance(field, DelimitedList):
        # Force apispec to allow DelimitedList query params in the format 'ids=123,456,789'
        # instead of the default which is 'id=123&id=456&id=789'. Read more about this setting
        # here: https://swagger.io/docs/specification/serialization/.
        param['explode'] = False

    return param


openapi.OpenAPIConverter.field2parameter = patched_field2parameter

[lafrech]

Nice.

I wasn't that clever in my own app: I patched the generated documentation dict...

[lafrech]

The refactor is achieved in 4.0. This issue should now be addressable in a non-breaking way in a 4.x version.

I can't do it right now so let's not block 4.0.

[lafrech]

It should be easier now.

It happens in _field2parameter. Currently, the List case is hardcoded. We shall make this more generic with a mechanism allowing to register field class specific functions.

[lafrech]

@pmdarrow Here's a draft solving this: #778. It allows a user to add a function to manage DelimitedList.