Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collision between parametrized and not parametrized routes #123

Closed
dmytro-demchyna opened this issue Mar 15, 2021 · 9 comments · Fixed by #131
Closed

Collision between parametrized and not parametrized routes #123

dmytro-demchyna opened this issue Mar 15, 2021 · 9 comments · Fixed by #131
Labels
bug Something isn't working

Comments

@dmytro-demchyna
Copy link
Contributor

dmytro-demchyna commented Mar 15, 2021

For example, 2 routes:

  1. GET /apples/{apple_id}
  2. GET /apples/ready-to-eat
"apple_id": {
  "in": "path",
  "name": "apple_id",
  "schema": {
    "type": "integer"
  },
  "required": true
}
$pathFinder = new League\OpenAPIValidation\PSR7\PathFinder($schema, '/apples/ready-to-eat', 'GET');
$addresses = $pathFinder->search();
print_r($addresses);

/*
Array
(
    [0] => League\OpenAPIValidation\PSR7\OperationAddress Object
        (
            [method:protected] => get
            [path:protected] => /apples/{apple_id}
        )

    [1] => League\OpenAPIValidation\PSR7\OperationAddress Object
        (
            [method:protected] => get
            [path:protected] => /apples/ready-to-eat
        )

)

*/

PathFinder::search() must return only one route in this case, because apple_id is integer.

league/openapi-psr7-validator: v0.15.2

@scaytrase
Copy link
Member

Looks like https://github.com/thephpleague/openapi-psr7-validator/blob/master/src/PSR7/OperationAddress.php#L33-L44 this code knows nothing about intended types to match

Does it fail any kind of validation? PathFinder is a kind of internal utilities and invalid matched operations could be validated out on the later stages

@dmytro-demchyna
Copy link
Contributor Author

I want to validate a response before send it to client. I have request URI and method.
I can't use URI directly in

public function validate(OperationAddress $opAddr, ResponseInterface $response): void
because it expects OperationAddress object, so only way for me to convert URI to OperationAddress is to use PathFinder:

$pathFinder = new PathFinder($responseValidator->getSchema(), $path, $method);
$addresses = $pathFinder->search();

foreach ($addresses as $address) {
    $responseValidator->validate($address, $psrResponse);
}

I can't use OperationAddress without PathFinder:

$operation = new \League\OpenAPIValidation\PSR7\OperationAddress('/apples/12345678', 'get') ;

// Exception, because it wants '/apples/{apple_id}', not '/apples/12345678'
$validator->validate($operation, $response); 

@dmytro-demchyna
Copy link
Contributor Author

Looks like https://github.com/thephpleague/openapi-psr7-validator/blob/master/src/PSR7/OperationAddress.php#L33-L44 this code knows nothing about intended types to match

If types matching is too hard, maybe prioritize static routes over parametrized routes is better way. I mean exclude all parametrized OperationAddress from PathFinder return value, if it find one with exact match to static route.

@dmytro-demchyna
Copy link
Contributor Author

dmytro-demchyna commented Mar 15, 2021

Request validation fails too. "GET /apples/ready-to-eat".

MultipleOperationsMismatchForRequest: The given request matched these operations: [/apples/{apple_id},get],[/apples/ready-to-eat,get]. However, it matched not a single schema of theirs.

@scaytrase
Copy link
Member

I can't use OperationAddress without PathFinder:

$operation = new \League\OpenAPIValidation\PSR7\OperationAddress('/apples/12345678', 'get') ;

// Exception, because it wants '/apples/{apple_id}', not '/apples/12345678'
$validator->validate($operation, $response);

Can you provide more info on this? Because for me is the correct way to do this, but not really tested

@dmytro-demchyna
Copy link
Contributor Author

Can you provide more info on this? Because for me is the correct way to do this, but not really tested

Maybe I confuse you by my inaccurate description of behaviors, but, please, help me understand, why the code validates response content against /apples/{apple_id} instead of /apples/ready-to-eat

$schema = /** @lang JSON */ '
  {
    "openapi": "3.0.0",
    "info": {
      "description": "",
      "version": "1.0.0",
      "title": "Apples"
    },
    "servers": [
      {
        "url": "/api"
      }
    ],
    "paths": {
      "/apples/{apple_id}": {
        "get": {
          "summary": "",
          "description": "",
          "operationId": "appleGet",
          "parameters": [
            {
              "in": "path",
              "name": "apple_id",
              "schema": {
                "type": "integer"
              },
              "required": true
            }
          ],
          "responses": {
            "200": {
              "description": "Success",
              "content": {
                "application/json": {
                  "schema": {
                    "type": "object",
                    "required": [
                      "apple"
                    ],
                    "properties": {
                      "apple": {
                        "$ref": "#/components/schemas/apple"
                      }
                    }
                  }
                }
              }
            }
          }
        }
      },
      "/apples/ready-to-eat": {
        "get": {
          "summary": "",
          "description": "",
          "operationId": "applesReadyGet",
          "responses": {
            "200": {
              "description": "Success",
              "content": {
                "application/json": {
                  "schema": {
                    "properties": {
                      "apples": {
                        "type": "array",
                        "items": {
                          "$ref": "#/components/schemas/apple"
                        }
                      }
                    },
                    "required": [
                      "apples"
                    ]
                  }
                }
              }
            }
          }
        }
      }
    },
    "components": {
      "schemas": {
        "apple": {
          "type": "object",
          "required": [
            "id"
          ],
          "properties": {
            "id": {
              "type": "integer"
            }
          }
        }
      }
    }
  }
';


$validator = (new \League\OpenAPIValidation\PSR7\ValidatorBuilder)->fromJson($schema)->getResponseValidator();
$operation = new \League\OpenAPIValidation\PSR7\OperationAddress('/api/apples/ready-to-eat', 'get') ;

$responseContent = /** @lang JSON */ '
    {
      "apples": [
        {
          "id": 0
        }
      ]
    }
';

$response = new \Nyholm\Psr7\Response(200, ['Content-Type' => 'application/json'], $responseContent);

$validator->validate($operation, $response);

@scaytrase
Copy link
Member

This sound like a solid bug now. I will try to check it, thanks

@scaytrase
Copy link
Member

Sorry for late response. I've managed to fix last "test" by replacing /api/apples/ready-to-eat -> /apples/ready-to-eat. Maybe there is a problem with server substitution for static paths

@scaytrase
Copy link
Member

Yeah, doSearch in PathFinder does server magic, but matching static path doesn't. Looks like quite easy fix.

@scaytrase scaytrase added the bug Something isn't working label Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants