Response code attachment?
DCzajkowski opened this issue · 3 comments
Hello. I have a proposition regarding Fractal and response codes. If you feel like it is more Laravel connected, I'll make sure to move this issue there.
The problem
Fractal's main responsibility is to change given array into given format. But. By many (or almost everyone) it is (ab)used to create JSON responses in our APIs.
return fractal()
->collection($users)
->transformWith(new UserTransformer)
->toArray();
And this is awesome. There is just one problem though. It does everything except adding a status code into our response. And that is understandable, considering Fractal's main goal, but it makes creating responses with status codes pretty hairy:
return response()->json(
fractal()
->item('These credentials do not match our records.')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->toArray()
, 401);
Proposition
My proposal is to add a public method to attach status codes like so:
return fractal()
->item('These credentials do not match our records.')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->withStatusCode(403)
->respond();
// and/or
return fractal()
->item('These credentials do not match our records.')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->respond(403);
This would generate a JSON response attaching provided status code.
Concerns
Whilst making responses in Laravel is easy (it is opinionated, the method's/function's/class' names are known. The method would be just a wrapper for response()->json()
) in a generic package as this the response would need to be generated, either by a passed-in ResponseFactory, or by our own ResponseManager.
Proposition
My proposition is to make this functionality only into Laravel's package, while it would be too messy to include it in this generic package.
TL;DR
I suggest creating a helper method ->respond($statusCode)
that would generate a HTTP JSON Response. I suggest doing that in Laravel's variant of this package, but I throw it here as it may be a good idea to include it in a more generic package.
P.S. I really appreciate you making Fractal even better than it already is.
Disclaimer: I'll make a PR, but need a positive response.
Ok, so I have made the code for Laravel only. What do you think?
In src/Fractal.php
/** @var int The status code for (if used) creating an HTTP response */
protected $statusCode;
/** @var array The array of headers for (if used) creating an HTTP response */
protected $headers = [];
/**
* Set the status code.
*
* @param int $statusCode
*
* @return $this
*/
public function statusCode($statusCode)
{
$this->statusCode = $statusCode;
return $this;
}
/**
* Add an HTTP header.
*
* @param string $key Header name
* @param string $value Header value
*
* @return $this
*/
public function header($key, $value)
{
$this->withHeaders([$key => $value]);
return $this;
}
/**
* Set HTTP headers.
*
* @param array $headers
*
* @return $this
*/
public function withHeaders(array $headers)
{
foreach ($headers as $key => $value) {
$this->headers[$key] = $value;
}
return $this;
}
/**
* Respond with an HTTP response
*
* @param int|null $statusCode
* @param array|null $headers
*
* @return \Symfony\Component\HttpFoundation\Response|\Illuminate\Contracts\Routing\ResponseFactory
*/
public function respond($statusCode = null, array $headers = null)
{
$statusCode = $statusCode ?: $this->statusCode ?: 200;
$headers = $headers ?: $this->headers ?: [];
return response()->json($this->createData()->toArray(), $statusCode, $headers);
}
Usage:
return fractal()
->item('This is an error message')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->statusCode(422)
->withHeaders([
'Test' => 'test',
'Test2' => 'test2',
])
->header('Test3', 'test3')
->respond();
// or (preferred by myself)
return fractal()
->item('This is an error message')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->respond(422, [
'Test' => 'test',
'Test2' => 'test2',
'Test3' => 'test3',
]);
Hi,
thank you for this well detailed proposal! 👍
I very much like this idea. Like you already mentioned, it's probably best to keep Fractalistic as generic as possible, so send your PR to laravel-fractal.
Looking at the API I think it could be improved. What I don't like about your suggestion is that there are methods like statusCode
and withHeaders
in the function chain that are belonging more to the respond
function than to fractal
function. I suggest the following API to make that relation more clear:
return fractal()
->item('This is an error message')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->respond(function($response) {
$response
->addHeader('name' => 'value')
->addHeaders(['name' => 'value', 'name2' => 'value2'])
->responseCode(422)
Your preferred version should also work:
return fractal()
->item('This is an error message')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->respond(422, [
'Test' => 'test',
'Test2' => 'test2',
'Test3' => 'test3',
]);
And a mix of both is also useful:
return fractal()
->item('This is an error message')
->transformWith(new ErrorTransformer)
->serializeWith(new ArraySerializer)
->respond(422, function($response) {
$response
->addHeader('name' => 'value')
->addHeaders(['name' => 'value', 'name2' => 'value2'])
});
A fully tested PR that adds all this to laravel-fractal would be highly appreciated.
I'll close this for now, we'll continue the discussion on your PR on laravel-fractal.