Avoid inlined responses in schema for paginated results in AutoSchema

[l0rn]

Checklist

• I have verified that that issue exists against the master branch of Django REST framework.
• I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• This is not a usage question. (Those should be directed to the discussion group instead.)
• This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• I have reduced the issue to the simplest possible case.
• I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Context

Currently AutoSchema will call get_paginated_response_schema from any configured Paginator. Not having access to the components section of the schema the behavior of the included paginators is just to create an inline response like so:

django-rest-framework/rest_framework/pagination.py

Lines 228 to 252 in 05a5909

	def get_paginated_response_schema(self, schema):
	return {
	'type': 'object',
	'properties': {
	'count': {
	'type': 'integer',
	'example': 123,
	},
	'next': {
	'type': 'string',
	'nullable': True,
	'format': 'uri',
	'example': 'http://api.example.org/accounts/?{page_query_param}=4'.format(
	page_query_param=self.page_query_param)
	},
	'previous': {
	'type': 'string',
	'nullable': True,
	'format': 'uri',
	'example': 'http://api.example.org/accounts/?{page_query_param}=2'.format(
	page_query_param=self.page_query_param)
	},
	'results': schema,
	},
	}

Inlined responses cause bad developer experiences if you plan to use automatic client type generators like this. It'd create types like InlinedResponse000. So a better solution would be to a new component of your new paginated component response and reference that.

I have a local solution for this below, generating the paginated component in AutoSchema.get_components and referencing it in AutoSchema.get_responses, but i am not sure whether this holds as a generalized solution. I'd like to hear a word from maintainers if this is something they'd accept as PR before I will make the effort.

My solution:
https://github.com/l0rn/django-minutes/blob/9cf2b85bb48f7f4af7d7f9a07753dc4bde95b8d3/minutes/schema.py#L6-L61

Steps to reproduce

• Create a paginated ListView
• generate schema using AutoSchema

Expected behavior

Paginated responses are also references to components:

        "responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/MeetingSeriesResponse"
                }
              }
            },
            "description": ""
          }
        },
[...]
	"components": {
		"schemas": {

			"MeetingSeriesResponse": {
				"type": "object",
				"properties": {
					"count": {
						"type": "integer",
						"example": 123
					},
					"next": {
						"type": "string",
						"nullable": true,
						"format": "uri",
						"example": "http://api.example.org/accounts/?offset=400&limit=100"
					},
					"previous": {
						"type": "string",
						"nullable": true,
						"format": "uri",
						"example": "http://api.example.org/accounts/?offset=200&limit=100"
					},
					"results": {
						"type": "array",
						"items": {
							"$ref": "#/components/schemas/MeetingSeries"
						}
					}
				}
			}
		}
	}

Actual behavior

Paginated responses look like this (inlined):

"responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "type": "object",
                  "properties": {
                    "count": {
                      "type": "integer",
                      "example": 123
                    },
                    "next": {
                      "type": "string",
                      "nullable": true,
                      "format": "uri",
                      "example": "http://api.example.org/accounts/?offset=400&limit=100"
                    },
                    "previous": {
                      "type": "string",
                      "nullable": true,
                      "format": "uri",
                      "example": "http://api.example.org/accounts/?offset=200&limit=100"
                    },
                    "results": {
                      "type": "array",
                      "items": {
                        "$ref": "#/components/schemas/User"
                      }
                    }
                  }
                }
              }
            },
            "description": ""
          }
        },

[carltongibson]

Hi @l0rn. Interesting.

Not initially sure what to say here.

We started with everything inline. We're moving the body elements (the obvious candidates) to components. That's expected.

Having the response structures as components seems a little like overkill. Q: Why is OpenAPIGenerator so keen on having them there? The spec is pretty flexible, surely it's the tools job to take valid input and handle it sensibly...?

[l0rn]

When generating the interfaces you need to name it, best case meaningful. If the response is a component you naturally can take the component name, in inlined responses there is no way to reliably deduct a meaningful name, therefore the fallback to "InlineResponse#####"

[carltongibson]

When generating the interfaces...

You mean like for Java classes? Is this not purely internal? I'm expecting the user facing parts to be OperationIDs...

[l0rn]

When generating the interfaces...

You mean like for Java classes? Is this not purely internal? I'm expecting the user facing parts to be OperationIDs...

Whatever output language you chose, could be java, could be typescript or any other supported language.
It creates types from the openAPI specification, and if it's missing a component name the fallback takes place.

Look at this output generated by a inlined response from openAPI specs:

/* tslint:disable */
/* eslint-disable */
/**
 * Minutes API V1
 * django-minutes API specifications
 *
 * The version of the OpenAPI document: 1.0
 * 
 *
 * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
 * https://openapi-generator.tech
 * Do not edit the class manually.
 */

import { exists, mapValues } from '../runtime';
import {
    User,
    UserFromJSON,
    UserFromJSONTyped,
    UserToJSON,
} from './';

/**
 * 
 * @export
 * @interface InlineResponse200
 */
export interface InlineResponse200 {
    /**
     * 
     * @type {number}
     * @memberof InlineResponse200
     */
    count?: number;
    /**
     * 
     * @type {string}
     * @memberof InlineResponse200
     */
    next?: string | null;
    /**
     * 
     * @type {string}
     * @memberof InlineResponse200
     */
    previous?: string | null;
    /**
     * 
     * @type {Array<User>}
     * @memberof InlineResponse200
     */
    results?: Array<User>;
}

export function InlineResponse200FromJSON(json: any): InlineResponse200 {
    return InlineResponse200FromJSONTyped(json, false);
}

export function InlineResponse200FromJSONTyped(json: any, ignoreDiscriminator: boolean): InlineResponse200 {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    return {
        
        'count': !exists(json, 'count') ? undefined : json['count'],
        'next': !exists(json, 'next') ? undefined : json['next'],
        'previous': !exists(json, 'previous') ? undefined : json['previous'],
        'results': !exists(json, 'results') ? undefined : ((json['results'] as Array<any>).map(UserFromJSON)),
    };
}

export function InlineResponse200ToJSON(value?: InlineResponse200 | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        
        'count': value.count,
        'next': value.next,
        'previous': value.previous,
        'results': value.results === undefined ? undefined : ((value.results as Array<any>).map(UserToJSON)),
    };
}

In comparision the generated type if the response is referenced in components (as done like in my code example):

/* tslint:disable */
/* eslint-disable */
/**
 * Minutes API V1
 * django-minutes API specifications
 *
 * The version of the OpenAPI document: 1.0
 * 
 *
 * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
 * https://openapi-generator.tech
 * Do not edit the class manually.
 */

import { exists, mapValues } from '../runtime';
import {
    User,
    UserFromJSON,
    UserFromJSONTyped,
    UserToJSON,
} from './';

/**
 * 
 * @export
 * @interface UserResponse
 */
export interface UserResponse {
    /**
     * 
     * @type {number}
     * @memberof UserResponse
     */
    count?: number;
    /**
     * 
     * @type {string}
     * @memberof UserResponse
     */
    next?: string | null;
    /**
     * 
     * @type {string}
     * @memberof UserResponse
     */
    previous?: string | null;
    /**
     * 
     * @type {Array<User>}
     * @memberof UserResponse
     */
    results?: Array<User>;
}

export function UserResponseFromJSON(json: any): UserResponse {
    return UserResponseFromJSONTyped(json, false);
}

export function UserResponseFromJSONTyped(json: any, ignoreDiscriminator: boolean): UserResponse {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    return {
        
        'count': !exists(json, 'count') ? undefined : json['count'],
        'next': !exists(json, 'next') ? undefined : json['next'],
        'previous': !exists(json, 'previous') ? undefined : json['previous'],
        'results': !exists(json, 'results') ? undefined : ((json['results'] as Array<any>).map(UserFromJSON)),
    };
}

export function UserResponseToJSON(value?: UserResponse | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        
        'count': value.count,
        'next': value.next,
        'previous': value.previous,
        'results': value.results === undefined ? undefined : ((value.results as Array<any>).map(UserToJSON)),
    };
}

It doesn't really change anything functionwise, but improves readability and quality of the resulting types

[carltongibson]

OK, good. That is much clearer to read.

Is there a benefit to making all responses components (or just where their are additional fields, i.e. when we paginate)?

[l0rn]

Apart from naming consistency i think not as in case of an unpaginated response equals the item schema...

But I can imagine that creating a component from a response as a general rule would avoid this issue with potential other fields that become part of the response (i.e. enveloping, HATEOAS or whatever). So in many cases we'd get duplicates like User == UserDetailResponse but in other cases those might not be the same therefore benitfiting

[carltongibson]

OK... so, yes, happy to think about this. If you'd like to work on a PR, that would be great.

Thanks for the report!

[carltongibson]

So in many cases we'd get duplicates like User == UserDetailResponse but in other cases those might not be the same therefore benitfiting

Not sure I followed you 100% but using operationID as part of the component name might be sufficient, so operationIDResponse and such. Anyhow... to the codes. :)

[l0rn]

So in many cases we'd get duplicates like User == UserDetailResponse but in other cases those might not be the same therefore benitfiting

Not sure I followed you 100% but using operationID as part of the component name might be sufficient, so operationIDResponse and such. Anyhow... to the codes. :)

Ye sure, it's straight forward to just use the operationID - I can look into making a PR later this day