[5.5] [WIP] Transformable Responses #18502

Aferz · 2017-03-26T18:49:05Z

Why ?

The purpose of this pull request is to give Laravel better flexibility when responding to API requests.

I have created a new TransformableResponse class that allows to define a few rulesets for transform the response in multiple forms that will be passed to the client.

Its design is highly inspired in FormRequests classes. If we can define Requests individually, why don't extend this behaviour to responses?

What is included in this PR ?

First of all, I suggest to store our Response classes inside app/Http/Responses folder(A nice artisan command will be included out of the box). Once said this ...

Class structure

class GetTestsResponse extends TransformableResponse
{
    /**
     * Set visibility rules to apply to current response.
     *
     * @return array
     */
    public function visibilityRules()
    {
        return [];
    }
    
   /**
     * Set casting rules to apply to current response.
     *
     * @return array
     */
    public function castingRules()
    {
        return [];
    }

    /**
     * Set mutation rules to apply to current response.
     *
     * @return array
     */
    public function mutationRules()
    {
        return [];
    }
    
    /**
     * Set renaming rules to apply to current response.
     *
     * @return array
     */
    public function renamingRules()
    {
        return [];
    }
}

Dummy Data

We will use this dummy data for demo purposes:

[
  'id' => '1',
  'name' => 'Name #1',
  'surname' => 'Surname #1',
  'posts' => [
      [
          'id' => '1',
          'title' => 'Post #1',
          'comments' => [
              [
                  'id' => '1',
                  'title' => 'Comment #1-1'
              ]
          ]
      ],
      [
          'id' => '2',
          'title' => 'Post #2',
          'comments' => [
              [
                  'id' => '1',
                  'title' => 'Comment #2-1'
              ]
          ]
      ]
  ]
]

Defining visibility rules

This ruleset define what fields will be present once the response is emitted. It's the first ruleset to be executed.

   /**
     * Set visibility rules to apply to current response.
     *
     * @return array
     */
    public function visibilityRules()
    {
        return [
            'id' => true,
            'name' => true,
            'posts.*.id' => true,
            'posts.*.comments.*.id' => true
        ];
    }

This will be the resultant response:

{
   "id":"1",
   "name":"Name #1",
   "posts":[
      {
         "id":"1",
         "comments":[
            {
               "id":"1"
            }
         ]
      },
      {
         "id":"2",
         "comments":[
            {
               "id":"1"
            }
         ]
      }
   ]
}

How does this work ?

As you can see in the rules section you define which attributes should be visible and which attributes should be hidden. So:

Fields checked as true will be evaluated first. If none, all will be visible by default.
Fields checked as false will be hidden using the data passed from the previous process.

Easy.

Defining casting rules

This ruleset define which fields should be casted. Valid casting values are: int, integer, real, float, double, string, bool, boolean. It's the second ruleset to be executed.

    /**
     * Set casting rules to apply to current response.
     *
     * @return array
     */
    public function castingRules()
    {
        return [
            'id' => 'int',
            'posts.*.id' => 'int',
            'posts.*.comments.*.id' => 'int'
        ];
    }

This will be the resultant response (chained from first ruleset):

{
   "id":1,
   "name":"Name #1",
   "posts":[
      {
         "id":1,
         "comments":[
            {
               "id":1
            }
         ]
      },
      {
         "id":2,
         "comments":[
            {
               "id":1
            }
         ]
      }
   ]
}

Defining mutation rules

This ruleset define how the data should be mutated. You can define your own mutators in the own class, just prefix the name of your method with the word mutator. If you want to reuse mutators you always can inherit from a parent class 👍 . It's the third ruleset to be executed.

    /**
     * Set mutation rules to apply to current response.
     *
     * @return array
     */
    public function mutationRules()
    {
        return [
            'id' => 'double|uppercase',
            'name' => 'uppercase',
            'posts.*.id' => 'double',
            'posts.*.comments.*.id' => 'double'
        ];
    }

    public function mutatorDouble($value)
    {
        return $value * 2;
    }
    
    public function mutatorUppercase($value)
    {
        return strtoupper($value);
    }

This will be the resultant response (chained from first and second ruleset):

{
   "id":"2",
   "name":"NAME #1",
   "posts":[
      {
         "id":2,
         "comments":[
            {
               "id":2
            }
         ]
      },
      {
         "id":4,
         "comments":[
            {
               "id":2
            }
         ]
      }
   ]
}

Defining renaming rules

This ruleset define how the keys of your data should be renamed. It's the forth ruleset to be executed.

    /**
     * Set renaming rules to apply to current response.
     *
     * @return array
     */
    public function renamingRules()
    {
        return [
            'id' => 'user_id',
            'posts.*.id' => 'post_id',
            'posts.*.comments.*.id' => 'comment_id'
        ];
    }

This will be the resultant response (chained from first, second and third ruleset):

{
   "user_id":"2",
   "name":"NAME #1",
   "posts":[
      {
         "post_id":2,
         "comments":[
            {
               "comment_id":2
            }
         ]
      },
      {
         "post_id":4,
         "comments":[
            {
               "comment_id":2
            }
         ]
      }
   ]
}

How this feature will be used ?

Simple. Look at this controller method:

    public function index()
    {
        $users = User::all(); // Our dummy data
        
        return new GetTestsResponse($users, 200, []); // status and headers are optional
    }

Why is a WIP ?

There are a few things to be covered before merge this PR, but first I want to check if this is useful for the Laravel community.

TODO:

php artisan make:response MyCustomResponse command.
Tests to cover this PR.

Any idea ? Any feedback ? Let me know what you all think!

Thanks !

feryardiant · 2017-03-26T19:55:46Z

Woa, really nice to see someone come to this idea.

I've done something similar before, using @thephpleague/fractal on Laravel 5.4, the implementation was pretty strike forward, nothing's comlicated & it support Paginators well.

IMO I'm go with fractal.

Aferz · 2017-03-26T21:40:34Z

Yeah, before make this PR I've reviewed Fractal, but IMO I think that It doesn't really fit with a nice Laravel approach. For me It was too verbose.

I thought that filter through data needed to be easier, that's why I used validation rules style w\ custom classes to represent a specific response for a specific request instead of injecting multiple classes.

Implement pagination is easy, I'm just waiting to an oficial response to see if this is useful.

Thanks for your feedback @feryardiant !!

konigbach · 2017-03-27T08:49:39Z

Nice and elegant approach! Fractal is more confuse and verbose and this gives us the simplicity to transform the data the way it should be. Great job!

ggcasuso · 2017-03-27T09:18:00Z

Definitely more useful than Fractal! Good work!

devcircus · 2017-03-27T14:15:06Z

It may be overkill, but did you consider adding the ability to mutate the data? We can already define mutators and casts, but it would be nice to be able to define all of this in one custom response class. With this, I would drop Fractal completely. However, you probably don't want to add a bunch of unneeded complexity.

Either way, nice work! If not accepted, I'll probably use in some of my api projects anyway.

Aferz · 2017-03-27T14:23:20Z

@devcircus What do you mean with mutate the data ?

Do you mean mutate with something like posts.*.commentsCount + 1 ? If yes, I won't because in my opinion, response object is not responsable for mutate the data this way.

If you mean mutate the schema (replace A key for B key when responding), yes, that's something that I have in mind if this PR goes forward.

I'm glad you like it. Thanks!

JairoRC · 2017-03-27T14:45:37Z

+1! very useful

devcircus · 2017-03-27T15:26:36Z

By mutate, I mean simply altering the presentation, but yes, (A) changing the reference(key) and/or (B) changing how the data is presented, similar to how Fractal does it.

This is something that can already be handled by presenters, accessors, etc. I just thought, notwithstanding SRP, that it would be nice to be able to fully define the response inside a custom response class.

deleugpn · 2017-03-27T15:41:37Z

Maybe a decorator outside of core would be better for that?

Aferz · 2017-03-27T15:46:50Z

Ok, now I see what you mean.

I don't really mind to try to figure out how we could encapsulate that presentation logic if that does not reject this PR (because that's not the main purpose of this feature).

Let's see what @taylorotwell or @GrahamCampbell think about this.

GrahamCampbell · 2017-03-27T19:44:34Z

src/Illuminate/Http/TransformableResponse.php

    /**
     * Transforms the response data.
     *
-     * @param  array  $data
+     * @param  array $data


please revert

Aferz · 2017-03-27T19:45:11Z

Hi guys,

I've updated the main message to reflect some updates I made in this package:

Casting Rules
Mutation Rules
Renaming Rules

Thanks

GrahamCampbell · 2017-03-27T19:44:56Z

src/Illuminate/Http/TransformableResponse.php

+     * @param  int    $status
+     * @param  array  $headers
+     * @param  int    $options
+     */


missing return void annotation

I copied the constructor from JsonResponse, should I apply equally?

Looks like a mistake there then. Laravel always includes return annotations, on all methods, without exception.

GrahamCampbell · 2017-03-27T19:45:02Z

src/Illuminate/Http/TransformableResponse.php

+    /**
+     * Transforms the response data.
+     *
+     * @param  array $data


GrahamCampbell · 2017-03-27T19:45:05Z

src/Illuminate/Http/TransformableResponse.php

+     *
+     * Example: posts.*.comments.*.title => posts.0.comments.0.title
+     *
+     * @param  array $data


GrahamCampbell · 2017-03-27T19:45:12Z

src/Illuminate/Http/TransformableResponse.php

+     * Sanitize gathered array rules removing those that don't appear into
+     * orginal rules.
+     *
+     * @param  string $rule


GrahamCampbell · 2017-03-27T19:45:31Z

src/Illuminate/Http/TransformableResponse.php

+
+    /**
+     * Sanitize gathered array rules removing those that don't appear into
+     * orginal rules.


please shorten to 80 chars including 5 spaces, star, and space

80 chars length only applies to DocBlocks ?

It's something we apply (ish) to the short descriptions to stop them getting too long. One rule that is strict is that they cannot overflow onto the next line.

GrahamCampbell · 2017-03-27T19:45:55Z

src/Illuminate/Http/TransformableResponse.php

+    {
+        switch ($this->getCastType($attribute)) {
+            case 'int':
+            case 'integer':


lots of code duplication here with eloquent

Yep, thats something I wanted to review here.

I didn't want to refactor Eloquent to use the switch that it uses to define the casting. What should I do here ?

kavinsky · 2017-03-27T20:12:49Z

👍 I like the second approach, but casting and mutating should be unified.

GrahamCampbell · 2017-03-27T21:21:38Z

src/Illuminate/Http/TransformableResponse.php

+     * Constructor.
+     *
+     * @param  mixed  $data
+     * @param  int    $status


Please use exactly 2 spaces before the dollar for param annotations on new code, rather than aligning.

GrahamCampbell · 2017-03-27T21:22:07Z

src/Illuminate/Http/TransformableResponse.php

+    protected function getCastType($attribute)
+    {
+        if (! $castings = $this->castingRules()) {
+            return null;


just return; gives the same semantics

GrahamCampbell · 2017-03-27T21:22:12Z

src/Illuminate/Http/TransformableResponse.php

+     * Get casting rule for given attribute.
+     *
+     * @param  string  $attribute
+     * @return bool


bool|null

GrahamCampbell · 2017-03-27T21:22:49Z

src/Illuminate/Http/TransformableResponse.php

+
+    /**
+     * Sanitize gathered array rules removing those that
+     * don't appear into orginal rules.


these descriptions cannot span more than one line.

GrahamCampbell · 2017-03-27T21:22:53Z

src/Illuminate/Http/TransformableResponse.php

+
+    /**
+     * Resolve array rules generating a new rule for every
+     * item in the array specified with the '*' symbol.


fernandobandeira · 2017-03-27T22:25:20Z

I think you should remove abstract from those methods since most of the times we won't use the 4 methods on our child class... Making them optional would be better IMO.

Also this seems pretty good so far... 👍

Aferz · 2017-03-28T06:11:51Z

It seems reasonable. I will modify It.

Thanks

lucasmichot · 2017-03-28T06:10:52Z

src/Illuminate/Http/TransformableResponse.php

+     */
+    public function __construct($data = null, $status = 200, $headers = [], $options = 0)
+    {
+        if (is_array($data)) {


What if data implements \Illuminate\Contracts\Support\Jsonable?

I would also allow any object to implement a \Illuminate\Contracts\Support\Transformable interface, so that the object itself can always control the way it's transformed.

How we should treat \Illuminate\Contracts\Support\Jsonable ?? I mean, I have no way to determine how to transform the data if I can't get it. Any suggestion ?

I like the idea of \Illuminate\Contracts\Support\Transformable, but how do we should treat a Collection of Transformables ? Should Transformable prevail against Arrayable ?

How we should treat \Illuminate\Contracts\Support\Jsonable ??

Have a look at \Illuminate\Routing\Router::prepareResponse

How do we should treat a Collection of Transformables

If data is a Collection, map all its items

Should Transformable prevail against Arrayable

I believe so

lucasmichot · 2017-03-28T06:16:15Z

src/Illuminate/Http/TransformableResponse.php

+
+        return array_reduce(array_keys($applicableRules),
+            function ($transformedData, $rule) {
+                if (Arr::has($transformedData, $rule)) {


\Illuminate\Support\Arr::forget always make sure that the array key exist, so using \Illuminate\Support\Arr::has is useless.

lucasmichot · 2017-03-28T06:28:40Z

src/Illuminate/Http/TransformableResponse.php

+     * @param  mixed  $value
+     * @param  string  $mutators
+     * @return mixed
+     */


Missing @throws \Exception

lucasmichot · 2017-03-28T06:29:03Z

src/Illuminate/Http/TransformableResponse.php

+            if (! method_exists($this, $method)) {
+                $classname = static::class;
+
+                throw new \Exception("There is no mutator [$method] declared in [$classname].");


Please import \Exception

lucasmichot · 2017-03-28T06:37:11Z

src/Illuminate/Http/TransformableResponse.php

+
+        return array_reduce($gatheredRules,
+            function ($validRules, $rule) use ($pattern, $valueForValidOnes) {
+                preg_match($pattern, $rule, $matches);


This is enough as we only want to re-use the $subject argument.

if (preg_match($pattern, $rule)) { $validRules[$rule] = $valueForValidOnes; }

lucasmichot · 2017-03-28T06:50:47Z

src/Illuminate/Http/TransformableResponse.php

+    }
+
+    /**
+     * Sanitize rules removing those that don't appear into orginal rules.


Typo: original

lucasmichot · 2017-03-28T06:51:04Z

src/Illuminate/Http/TransformableResponse.php

+    }
+
+    /**
+     * Apply visbility rules to given data.


Typo: visibility

lucasmichot · 2017-03-28T14:25:33Z

@Aferz - how do you handle $original variable ?
If I pass a a Collection as data in my custom Response, I want to be sure to actually retrieve it

So $this->getOriginal() should return my Collection
And $this->getData() should return my transformed content...

Aferz · 2017-03-28T15:09:37Z

@lucasmichot

In constructor:

if (is_array($data)) {
    $transformedData = $this->transform($data);
} elseif ($data instanceof Arrayable) {
    $transformedData = $this->transform($data->toArray());
} else {
   $transformedData = $data;
}

parent::__construct($transformedData, $status, $headers, $options);

$this->original = $data;

should do the trick.

$this->getOriginalContent() will return given Collection or data.
$this->getData() will return transformed content.

I would like to think it a bit more because $this->original = $data will be set in parent setData method and it will be reasigned again once the JsonResponse constructor ends. Anyway, is not a big problem...

I will update this PR with your suggestions tomorrow.

Thanks for the feedback!

jaketoolson · 2017-03-29T16:42:18Z

If this gets approved yet #14357 (simple Transformer) PR was not... that would be a shame.

Aferz · 2017-03-30T00:16:56Z

I've updated this PR with following changes:

Now you can access original data passed to custom response with getOriginalData() parent method.
Removed abstract methods to cherry pick what ruleset should be applied to custom response.
Added Illuminate\Support\Contracts\Transformable to allow object to define how it should be mapped.
If data is a Illuminate\Support\Collection class, its items are mapped checking (In order):
- $item instanceof Transformable
- or $item instanceof Arrayable
- or returning $item itself.
Improved renaming feature.
Paginator/LengthAwarePaginator works right out the box

TODO:

Command for generate custom responses.
Tests.

…lection handling.

GrahamCampbell · 2017-03-30T09:39:21Z

src/Illuminate/Http/TransformableResponse.php

+    /**
+     * Handle how a collection will be transformed.
+     *
+     * @param  Collection  $data


fqnc required please

Oops! Fixed.

GrahamCampbell · 2017-03-30T12:34:23Z

src/Illuminate/Http/TransformableResponse.php

+     *
+     * @param  string  $mutators
+     * @param  mixed  $value
+     * @throws Exception


Please follow our cs regarding throws docs. :)

Sorry but what do you eaxctly mean ??

framework/src/Illuminate/Auth/Access/HandlesAuthorization.php

Line 24 in 7212b1e

* @throws \Illuminate\Auth\Access\AuthorizationException

GrahamCampbell · 2017-03-30T12:35:00Z

I still wonder if this whole thing would be best suited as a laravel package, rather than shipping in the core.

Aferz · 2017-03-30T14:16:05Z

@GrahamCampbell - That something that we all should talk about, but I'm gonna give my 2 cents.

So far, Laravel offers support/utilities for build an API (Auth & JWT, separated API routes file, Throttling, etc...) but one of the most difficult things is how you display your data and what you show and what you don't for your multiple requests. Yes, you have $hidden attribute it your models, but when you try to display, for example, different model data depending on the Request, you don't have enough with a single attribute. That could be the main reason why there are several pull requests about transforming data.

This could be a package? Of course. But IMO I think Laravel should provide 'something' to filter your response before sending to a client (There is no a easy/clean way so far to make this). This way you gain full control from the way your request is validated to how your response is displayed.

So far, Laravel treats every Request as unique (If you want, of course), but not the Response. Conceptually, that's something that could be improved to gain granular control about your code. And that's what this PR wants to achieve.

Anyways, this is my opinion. I would like to know what you all think, if you share my concept about 'unique Response' and if this PR should be accepted or rejected.

I would like to know what @taylorotwell thinks about this to know if this worth it.

taylorotwell · 2017-03-30T14:48:24Z

I'm not ready to commit to merging this into the core at this time (or any other fractal API type package). However, I will definitely be revisiting all of this before the 5.5 release and will add this PR to my list of PRs to look over again when I get to that point. Thanks.

Aferz · 2017-03-30T15:07:37Z

Mmmh ok then.

joshmanders · 2017-03-30T15:11:26Z

@Aferz

There is no a easy/clean way so far to make this

That's not true at all.. Whatever you return from your controller is formatted as a response. You can return anything, and easily intercept and format it however you like. I'm doing it right now. I return a class that extends a certain class, and I typecheck and format. It's pretty easy AND straight forward.

Aferz · 2017-06-05T19:04:22Z

Hi @taylorotwell , sorry for bothering you again.

5.5 release is near so... Will this PR take in consideration for 5.5 or higher versions??.

Just for keep updating this PR with useful stuff and improvements or think of it as a possible package.

Thanks

fernandobandeira · 2017-06-05T21:36:33Z

@Aferz take a look on #19449

Aferz added 2 commits March 26, 2017 20:03

Added transformable Response

7b20e01

Applied CI Styles Fix

0f9ea29

WIP

33ce58f

GrahamCampbell requested changes Mar 27, 2017

View reviewed changes

Applied CI Styles Fix

753424c

lucasmichot reviewed Mar 28, 2017

View reviewed changes

Fix typos and useless codes.

bf31967

lucasmichot mentioned this pull request Mar 28, 2017

[proposal] Provide a RFC-5988 compliant paginator laravel/ideas#192

Closed

Added Transformable Contract. Improved renaming feature. Improved Col…

a993cb1

…lection handling.

GrahamCampbell requested changes Mar 30, 2017

View reviewed changes

Fix DockBlock Full Qualified Domain Name.

53ff032

GrahamCampbell requested changes Mar 30, 2017

View reviewed changes

Fixed cs @throws in DocBlock.

fd49983

taylorotwell closed this Mar 30, 2017

Aferz mentioned this pull request Jun 5, 2017

[5.5] API Responses #19449

Closed

[5.5] [WIP] Transformable Responses #18502

[5.5] [WIP] Transformable Responses #18502

Conversation

Aferz commented Mar 26, 2017 • edited

Why ?

What is included in this PR ?

Class structure

Dummy Data

Defining visibility rules

How does this work ?

Defining casting rules

Defining mutation rules

Defining renaming rules

How this feature will be used ?

Why is a WIP ?

feryardiant commented Mar 26, 2017

Aferz commented Mar 26, 2017 • edited

konigbach commented Mar 27, 2017

ggcasuso commented Mar 27, 2017

devcircus commented Mar 27, 2017 • edited

Aferz commented Mar 27, 2017

JairoRC commented Mar 27, 2017

devcircus commented Mar 27, 2017

deleugpn commented Mar 27, 2017

Aferz commented Mar 27, 2017 • edited

Choose a reason for hiding this comment

Aferz commented Mar 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aferz Mar 27, 2017 • edited

Choose a reason for hiding this comment

kavinsky commented Mar 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fernandobandeira commented Mar 27, 2017

Aferz commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aferz Mar 28, 2017 • edited

Choose a reason for hiding this comment

lucasmichot Mar 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmichot Mar 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmichot commented Mar 28, 2017

Aferz commented Mar 28, 2017 • edited

jaketoolson commented Mar 29, 2017

Aferz commented Mar 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrahamCampbell commented Mar 30, 2017

Aferz commented Mar 30, 2017

taylorotwell commented Mar 30, 2017

Aferz commented Mar 30, 2017

joshmanders commented Mar 30, 2017

Aferz commented Jun 5, 2017

fernandobandeira commented Jun 5, 2017

Aferz commented Mar 26, 2017 •

edited

Aferz commented Mar 26, 2017 •

edited

devcircus commented Mar 27, 2017 •

edited

Aferz commented Mar 27, 2017 •

edited

Aferz Mar 27, 2017 •

edited

Aferz Mar 28, 2017 •

edited

lucasmichot Mar 28, 2017 •

edited

lucasmichot Mar 28, 2017 •

edited

Aferz commented Mar 28, 2017 •

edited