[PHP-Symfony] revamp the computation of the contentType

[gturri]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package || exit
  ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
  ./bin/utils/export_docs_generators.sh || exit
  

  (For Windows users, please run the script in WSL)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Why this commit

recent commit 4089438 already improved that method: before that other commit it was just impossible to query a endpoint with a response type that was something else than application/json or application/xml. With that commit it became possible to query such endpoint provided that the client declare in its Accept header that it can cope with */* (or provided that the client omitted that header altogether).

But there were still cases badly handled. For instance if an endpoint returns a response of type image/png and that it receives a query with header Accept: image/png, then it would reply with 406 (instead of just serving the image).

To avoid any other issues with type resolution, this commit revamps the getOutputFormat function more thoroughly and does it by implementing the specification (available at
https://httpwg.org/specs/rfc9110.html#field.accept ), which means in particular that the format accepted by the client are ordered by the relative weights specified (if any).

Nb: note that merging this PR would make it possible to close the PR #15560 since this one is a kind of superset of that other one.

How to test this commit

Use a simple openapi specification with an endpoint that can serve several different response type. I used:

openapi: 3.0.0
servers:
  - url: 'http://petstore.swagger.io/v2'
info:
  version: 1.0.0
  title: OpenAPI Petstore
paths:
  '/pet/{petId}':
      get:
        tags:
          - pet
        summary: Find pet by ID
        description: Returns a single pet
        operationId: getPetById
        parameters:
          - name: petId
            in: path
            description: ID of pet to return
            required: true
            schema:
              type: integer
              format: int64
        responses:
          '200':
            description: successful operation
            content:
              application/xml:
                schema:
                  $ref: '#/components/schemas/Pet'
              application/json:
                schema:
                  $ref: '#/components/schemas/Pet'
              text/plain:
                schema:
                  type: string
              text/html:
                schema:
                  type: string
components:
  schemas:
    Pet:
      title: a Pet
      description: A pet for sale in the pet store
      type: object
      required:
        - name
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
          example: doggie
      xml:
        name: Pet

then generate the symfony bundle and create a Symfony app with a placeholder implementation of the generated interface. I used:

<?php
namespace App\Controller;

use OpenAPI\Server\Api\PetApiInterface;
use OpenAPI\Server\Model\Pet;

class PetApi implements PetApiInterface // An interface is autogenerated
{
    public function getPetById(
        int $petId,
        int &$responseCode,
        array &$responseHeaders
    ): array|object|null {
            return new Pet(array('id' => $petId, 'name' => "medor"));
    }
}

then I ran against that server the following curl queries, with the -i flag to check the content type of the response (nb: we don't care at all about the body of the response: only the content type header is important for those tests):

Some basic checks:

• curl -i -H 'Accept: text/plain' http://localhost:8000/pet/1 => response should have content type text/plain (without this patch this query yields a 406)
• curl -i -H 'Accept: text/html' http://localhost:8000/pet/1 => response should have content type text/html (it receives a 406 without this patch
• curl -i -H 'Accept: application/json' http://localhost:8000/pet/1 => should be application/json

when there are not common format

• curl -i -H 'Accept: image/png' http://localhost:8000/pet/1 => should return a 406

queries with weight

• curl -i -H 'Accept: text/html, application/xml ; q=0.8' http://localhost:8000/pet/1 => when there are no explicit weight then the default value is 1. So response should have content type text/html (nb: this query also has a bunch of whitespaces in the Accept header to make sure it is correctly handled). (nb: without this commit, the response has content type application/xml, which does not conform to the spec)
• curl -i -H 'Accept: text/html;q=0.7, application/xml ; q=0.8' http://localhost:8000/pet/1 => this ensure it works when the format with higher priority is not provided first in the list
• curl -i -H 'Accept: image/png;q=0.9,text/html;q=0.8' http://localhost:8000/pet/1 => tests that when the format with higher value is not supported, we correctly fallback to another one
• curl -i -H 'Accept: text/*, application/xml ; q=0.8' http://localhost:8000/pet/1 => tests that we can match when the subtype is a wildcard
• curl -i -H 'Accept: */*' http://localhost:8000/pet/1 => tests that we can match when both the type and subtype are wildcard

Testing with parameters in the header

• curl -i -H 'Accept: text/plain;format=fixed;q=0.4' http://localhost:8000/pet/1 => response should have content type text/plain
• curl -i -H 'Accept: text/plain;format=fixed' http://localhost:8000/pet/1 => same test but without explicit weight
• curl -i -H 'Accept: text/plain;format=fixed;q=0.4,text/html;q=0.5' http://localhost:8000/pet/1 => response should be text/html
• curl -i -H 'Accept: text/plain;format=fixed;q=0.4,text/html;q=0.3' http://localhost:8000/pet/1> response should be text/plain
• curl -i -H 'Accept: text/plain;format=fixed,text/html;q=0.3' http://localhost:8000/pet/1 => response should be text/plain

Test with some badly formatted query, to ensure we can fallback to an acceptable format instead of crashing

• curl -i -H 'Accept: text;q=0.9, application/xml ; q=0.8' http://localhost:8000/pet/1 => the type with higher weigh has no subtype, which is invalid. With this commit we receive a response of type application/xml in thise case (as is the case without this commit)
• curl -i -H 'Accept: application/xml ; q=0.8,' http://localhost:8000/pet/1 => the header has a trailing comma. This ensure we can still reply a response of type application/xml

Edit: I forgot to ping the technical committee: @jebentier @dkarlovi @mandrean @jfastnacht @ybelenko @renepardon

[wing328]

thanks the PR

have you tested this change locally to confirm it works for your use cases?

[gturri]

yes, I ran locally the test I described (in particular I ran all the curl queries I described, and they each lead to the content type expected)

[gturri]

and, just to give more context: I have also tested this change locally on my little open source project which relies on OpenApiGenerator, and it's much better with that commit.

To give a little more context: I use OpenApiGenerator to generate both this Symfony bundle + an angular client (that I use to query this symfony app). The behavior I observe is:

• without this commit: I have a Symfony bundle with an endpoint that generates a plain/text response. I can query this endpoint with a client such as Bruno, since by default it does not have an Accept header. But out of the box the angular client receives a 406, because the client sends Accept: text/plain. I guess I could tweak the call because it seems this client makes it possible to override this header. but...
• ... but with this commit everything just works out of the box: I can just use my generated typescript client, and the symfony generated endpoint works just fine.

[wing328]

thanks for the details.

just merged it 👍