Optional route model binding type

[apasquini95]

Hi everyone, I'm having an issue with optional routing models and their type representation in the openapi specification. Here is a simple routing with an optional model as path param:

// api.php

\Illuminate\Support\Facades\Route::get('test/{user?}', [\App\Http\Controllers\MyController::class, 'myHandler']);

And its controller:

// MyController.php

use App\Http\Controllers\Controller;
use App\Models\User;
use Illuminate\Http\Request;

class MyController extends Controller
{
    public function myHandler(Request $request, ?User $user = null): int 
    {
        $param = $request->boolean("test_param"); 
        return (int) $param;
    }
}

When the openapi specification is generated, the model binding fallbacks to string, instead of using the model's defined attribute binding (which should have been int):

"/test/{user}": {
  "get": {
      "operationId": "myHanlder",
      "tags": [
          "My"
      ],
      "parameters": [
          {
              "name": "user",
              "in": "path",
              "required": true,
              "description": "**Optional**",
              "schema": {
                  "type": [
                      "string",  <-- Why is this string?
                      "null"
                  ],
                  "default": null
              },
              "x-optional": true
          },
          {
              "name": "test_param",
              "in": "query",
              "schema": {
                  "type": "boolean",
                  "default": false
              }
          }
      ],
      "responses": {
          "200": {
              "description": "",
              "content": {
                  "application/json": {
                      "schema": {
                          "type": "integer"
                      }
                  }
              }
          }
      }
  }
}

I thaught it could have been a sort of convention, since path params are always required in a certain sense for the openapi syntax, so I tried to divide the original endpoint into two (without touching the controller implementation). This is the new routing:

// api.php

Route::controller(\App\Http\Controllers\MyController::class)->group(function () {
    Route::get('test/{user}', 'myHandler')->name("myHandler.withUser");
    Route::get('test', 'myHandler')->name("myHandler");
});

But now the specification lacks of the test_param declaration in the endpoint without the model binding:

"/test/{user}": {
    "get": {
        "operationId": "myHandler.withUser",
        "tags": [
            "My"
        ],
        "parameters": [
            {
                "name": "user",
                "in": "path",
                "required": true,
                "schema": {
                    "type": [
                        "string",  // Still string
                        "null"
                    ],
                    "default": null
                }
            },
            {
                "name": "test_param",
                "in": "query",
                "schema": {
                    "type": "boolean",
                    "default": false
                }
            }
        ],
        "responses": {
            "200": {
                "description": "",
                "content": {
                    "application/json": {
                        "schema": {
                            "type": "integer"
                        }
                    }
                }
            }
        }
    }
},
"/test": {
    "get": {
        "operationId": "myHandler",
        "tags": [
            "My"
        ],
        // I would have expected 'test_param' to be still documented
        "responses": {
            "200": {
                "description": "",
                "content": {
                    "application/json": {
                        "schema": {
                            "type": "integer"
                        }
                    }
                }
            }
        }
    }
}

Is this intended or am I doing something wrong?

[romalytvynenko]

Hey @apasquini95
Can you create reproduction repo with this issue? I cannot reproduce

[apasquini95]

Sure, I created this public repository.

https://github.com/apasquini95/demo_scramble

[romalytvynenko]

@apasquini95 Are you sure you run DB migrations before generating the documentation?

[apasquini95]

Yes, migrations have run, but to be sure I did it again and the specification is still with string. If I remove the nullability of the user param in the controller's method, everything works great (even the 404 response is created):

Route::get('endpoint-with-optional-path-param/{user?}', [MyController::class, 'myHandler'])
    ->name('optional_path_param');

  public function myHandler(Request $request, User $user /* ?User $user = null */): int
  {
      $bool = $request->boolean('test_param');
      return $bool ? $user->id : -1;
  }

"get": {
    "operationId": "optional_path_param",
    "tags": [
        "My"
    ],
    "parameters": [
        {
            "name": "user",
            "in": "path",
            "required": true,
            "description": "**Optional**. The user ID",    // Correct
            "schema": {
                "type": "integer"
            },
            "x-optional": true
        },
        {
            "name": "test_param",
            "in": "query",
            "schema": {
                "type": "boolean",
                "default": false
            }
        }
    ],
    "responses": {
        "200": {
            "description": "",
            "content": {
                "application/json": {
                    "schema": {
                        "type": "integer"
                    }
                }
            }
        },
        "404": {
            "$ref": "#/components/responses/ModelNotFoundException"
        }
    }
}

Since it maps to the ID column I would think it depends on the inferred type of the handler's param, rather than db structure.

[apasquini95]

I think I found the issue. In PathParametersExtractor::getParameterType there's this portion of code that produces the string type:

private function getParameterType(
    string $paramName,
    string $description,
    RouteInfo $routeInfo,
    Route $route,
    ?ParamTagValueNode $phpDocParam,
    ?ReflectionParameter $reflectionParam,
    ?string $boundClass,
) {
    // ↓↓↓↓↓↓↓↓↓ Here the type is correctly retrieved as 'App\Models\User'
    $type = $boundClass ? new ObjectType($boundClass) : new UnknownType;
    if ($routeInfo->reflectionMethod()) {
        $type->setAttribute('file', $routeInfo->reflectionMethod()->getFileName());
        $type->setAttribute('line', $routeInfo->reflectionMethod()->getStartLine());
    }

    if ($phpDocParam?->type) {
        $type = PhpDocTypeHelper::toType($phpDocParam->type);
    }

    if ($reflectionParam?->hasType()) {
         // ↓↓↓↓↓↓↓↓↓ Here the type is converted to 'null | App\Models\User'
        $type = TypeHelper::createTypeFromReflectionType($reflectionParam->getType());
    }

    $simplifiedType = Union::wrap(array_map(
        fn (InferType $t) => $t instanceof ObjectType
            ? (enum_exists($t->name) ? $t : new \Dedoc\Scramble\Support\Type\StringType)
            : $t,
        $type instanceof Union ? $type->types : [$type],
    ));

    $schemaType = $this->openApiTransformer->transform($simplifiedType);

    // ↓↓↓↓↓↓↓↓↓ When it comes to map the model, '$type' is a Union object, not an ObjectType
    if ($isModelId = $type instanceof ObjectType) {
        [$schemaType, $description] = $this->getModelIdTypeAndDescription($schemaType, $type, $paramName, $description, $route->bindingFields()[$paramName] ?? null);

        $schemaType->setAttribute('isModelId', true);
    }
    // ...

Could this be a solution?

/* ==> */ if ($reflectionParamHasType = $reflectionParam?->hasType()) {
    $type = TypeHelper::createTypeFromReflectionType($reflectionParam->getType());
}

$simplifiedType = Union::wrap(array_map(
    fn (InferType $t) => $t instanceof ObjectType
        ? (enum_exists($t->name) ? $t : new \Dedoc\Scramble\Support\Type\StringType)
        : $t,
    $type instanceof Union ? $type->types : [$type],
));

$schemaType = $this->openApiTransformer->transform($simplifiedType);

// ↓↓↓↓↓ '$type' is overwritten with the ObjectType
if ($reflectionParamHasType && $reflectionParam?->allowsNull() && $type instanceof Union) {
    $type = $type->types[1];       // createTypeFromReflectionType puts Null as first type
}

// ↓↓↓↓↓ Now '$type' is instanceof ObjectType and it points to 'App\Models\User'
if ($isModelId = $type instanceof ObjectType) {
    [$schemaType, $description] = $this->getModelIdTypeAndDescription($schemaType, $type, $paramName, $description, $route->bindingFields()[$paramName] ?? null);

    $schemaType->setAttribute('isModelId', true);
}

This would fix the model binding, but not the lack of test_param in the openapi specification.