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

JsonValidation #2

Open
oscarotero opened this issue Jan 18, 2017 · 5 comments
Open

JsonValidation #2

oscarotero opened this issue Jan 18, 2017 · 5 comments

Comments

@oscarotero
Copy link
Member

This could be a port (or improvement) of jsonvalidator: existing currently in psr7-middlewares: https://github.com/oscarotero/psr7-middlewares#jsonvalidator
Here's other similar package: https://github.com/moffe42/json-schema-middleware

@schnittstabil
Copy link
Member

Maybe worth to consider:

@oscarotero
Copy link
Member Author

I think we're talking about different things. I meant a middleware for json schema validation (using for example https://github.com/justinrainbow/json-schema), and you're talking about validate the body content to parse to json. Maybe the name is confusing, and JsonSchemaValidation is better.

Anyway, this libraries are interesting to include in middlewares/payload or do you prefer a new specific package for json that could include the utilities for parsing and schema validation?.

@schnittstabil
Copy link
Member

I was aware of that, middlewares/payload is another project where these points could be considered.

In my opinion we should consider to validate the raw body against the json schema and should replace the parsed body:

$parsedBody = $this->parse($request->getBody());
$this->validate($parsedBody);
$request = $request->withParsedBody($parsedBody);

This could provide some benefits:

  1. better exceptions
  2. stripping BOMs
  3. easy support for vendor-mimetypes

Anyway, this libraries are interesting to include in middlewares/payload or do you prefer a new specific package for json that could include the utilities for parsing and schema validation?.

Sorry, I can't follow you on this.

@oscarotero
Copy link
Member Author

Ok, my english is a bit 💩

  1. I don't want to mix schema validation and json parsing. They have different responsabilities and may be another middlewares relying in the parsed body. For example:
$middleware = [
    new Middlewares\JsonPayload(), //parse the json
    new modifyParsedBody(), //modify the parsed body in somehow
    new Middlewares\JsonSchemaValidation(), //validate the json schema
];

To me, is a bad idea to have some middlewares using the raw string body and others the parsed body because we remove the interoperation between. This is something I commented here: oscarotero/psr7-middlewares#50 (comment)

  1. One solution could be improve the jsonPayload with the libraries you're suggesting but this add dependencies for a package than a user may not need (maybe just want to use the urlEncodePayload).

  2. Another solution could be create a specific package to parse json, I mean, move the JsonPayload to, for example middlewares/json. This package could include also the json schema validation, so on installed it, you can use both JsonPayload and JsonSchemaValidation.

@schnittstabil
Copy link
Member

You are right, implementing these responsibilities within one single project is a bad idea. But we can also create a meta-middleware project which encapsulates the cumbersome setup above. For example:

$middleware = [
    …,
    new Middlewares\JsonValidation('path/to/json-schemas', $modifyParsedBody),
    …,
];
class JsonValidation extends MiddlewarePipe
{
    public function __construct($path, MiddlewareInterface $modifyParsedBody = null)
    {
        $this->append(new Middlewares\JsonPayload()); //parse the json
        if ($modifyParsedBody !== null) {
            $this->append(new modifyParsedBody()); //modify the parsed body in somehow
        }
        $this->append(new Middlewares\JsonSchemaValidation($path)); //validate the json schema
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants