JSON Schema incompatible with Slim Framework

[sander-bol]

The JSON Schema middleware does not work with Slim Framework. Reason is that it relies on the Payload middleware to have getParsedBody to yield a stdClass object, using the newly added forceArray=false option on Middleware\Payload.

Slim's Request object has body parsing built-in. Unfortunately they force the JSON payload into an associative array (as documented in their manual) Our Payload MW currently does the following check to decide whether or not we should parse the body:

if (*!$request->getParsedBody()* && in_array($request->getMethod(), [...]

Since getParsedBody will hit Slim's version, we're basically stuck with whatever parsed body they provide. I've come up with three ways to deal with this:

Option 1: "Not our problem", have the developer deal with it in their Framework. In effect this would mean registering a custom "body parser" in Slim using registerMediaTypeParser to overwrite the default parser for application/json media types.

Option 2: Instead of bailing out in JSON Schema if the parsed body is not an object, do an if-array-then-cast-to-object, which will yield the stdClass we need.

Option 3: Fix it in Middleware::Payload by (optionally?) having it overwrite previously parsed bodies. Slight loss of efficiency, but ultimately it does give us more control over the request.

Would like your opinion on this before forging ahead with a PR. My preference would be option 3, under the following rationale: By adding the Payload middleware to your stack you're explicitly handing us responsibility to handle the request body, since apparently your current stack is not capable of dealing with the provided payload... it would make sense then not to rely on any previous parsed body contents.

I'll also raise this issue over at Slim to get their opinion on this.

[akrabat]

FWIW, the specification for getParsedBody() states that it may return an array, object or null. Therefore, I think that to be interoperable, any middleware needs to take this into account.

Personally, I would modify JsonSchema to read the body as a string and json_decode it itself if getParsedBody() doesn't return an object.

However, dropping a piece of middleware in that sets the parsed body to an object if it isn't would also work.

[oscarotero]

I see some problems in read the body and parse it again:

• Loss of control of the parsed body. For example, may be other middleware components that modify the parsed body (or replace it entirely). I consider the parsedBody as the reliable value that all components should use. For me, it makes no sense that some components use the body string and other the parsed body.
• Duplication. json_decode has other config values like depth or JSON_BIGINT_AS_STRING that can affect to the validation, so they should be added to this middleware just in case. All this things are out of scope of this middleware, that should only validate a given value, not generate the value.
• Error handling. If there's an error parsing the json, it must returns a 400 error code, a different code than what is supposed to return (422).

To me, the best option is try to fix this out of this component:

• In my opinion, the option 3 (Allow to override previously parsed body by Payload middleware) is the best option because is easier to implement (just a new option ->override(true) and that's all).
• Another option is create a new component to convert arrays to object (and viceversa), using something like this: https://github.com/oscarotero/fly-crud/blob/master/src/Document.php#L79. This avoid to lose information if the parsed body has ben modified previously, but maybe it's a edge case.

[sander-bol]

I agree.. it seems that we'd be adding a lot of responsibility to Middleware::JsonSchema which it shouldn't be concerned with.

I've experimented with the recursive arrayToObject() conversion suggested, but immediately found a bug in that it doesn't deal with empty objects correctly - they get turned into arrays, thus failing schema validation. Apart from that, they do some magical assumptions in that if keys are strings, it must be an object and otherwise it must be an array. I can easily see usecases where this simply does not hold, for example: {10101: {"title": "Hello"}, 10102: {"title", "Goodbye"}}.

Tomorrow I'll send a PR for the override on Payload. Seems the best way forward for now. Thanks for thinking along!

[sander-bol]

While working on the PR, this is what my middleware stack now needs to look like:

$app
    ->add(MW::payload(['forceArray' => false])->override(true))
    ->add(
        MW::jsonSchema(
            ['/search' => $schemataDir . '/search/searchQuery.schema.json']
        )
    )
    ->add(MW::payload(['forceArray' => true])->override(true))

You can see that it's... suboptimal. Reason for forcing it back to an array at the end of the ride is because the rest of the codebase relies on the data being an associative array. Relying on Payload is starting to seem like a worse idea by the minute. It would either require developers to rewrite all parts of their application that deal with the parsedResponseBody when they would want to introduce the JSON Schema Validation middleware, or resort to the double-conversion as demonstrated above to ensure the parsed body can be handled by the schema validator.

@akrabat 's suggestion to contain this issue within JSONSchema is quickly becoming my preferred solution, even if it means dragging responsibilities into JSONSchema.

[oscarotero]

What about creating a payloadToArray() component? converting an object to array is easier and has not the issues of converting array to object. For example:

$app
    ->add(MW::payload(['forceArray' => false])->override(true))
    ->add(MW::jsonSchema($schemas))
    ->add(MW::payloadToArray());

A better component name is desirable :)