Proposed Feature | New DTO Experience! 🪄

[Sammyjo20]

After having a fantastic conversation with @techenby, we came up with a way of improving the DTO experience with Saloon, so this is my proposed version.

Before

Previously you'd have to define a method on your connector or request called createDtoFromResponse. This method would accept a Response object and return mixed (allowing you to return any object or type of your choosing). This worked but it doesn't have very good PHPStan / type support because the response doesn't have any knowledge of the type defined in its request.

<?php

use Saloon\Http\Request;
use Saloon\Http\Response;

class GetServerRequest extends Request
{
    // {...}
    
    public function createDtoFromResponse(Response $response): mixed
    {
        $data = $response->json();
    
        return new Server(
            id: $data['id'],
            name: $data['name'],
            ipAddress: $data['ip'],
        );
    }
}

Usage

$dunno = $connector->send($request)->dto(); // Mixed! 🤔

This would result in your application and PHPStan not knowing what the return of dto is resulting in having to do type-gymnastics or using instanceof checks to tell the application it's working correctly.

Proposed Idea

The proposed solution is a new into method on the response. This method accepts a class string.

$connector->send($request)->into(Order::class); // Returns `Order` instance

Order Class

<?php

namespace App\Data;

use Saloon\Http\Response;
use Saloon\Contracts\DataObjects\DataTransferObject;

final readonly class Order implements DataTransferObject
{
    public function __construct(
        public int $number,
        public string $name,
        public string $sender,
    ) {
        //
    }

    public static function fromResponse(Response $response): static
    {
        $data = $response->json();

        return new static($data['number'], $data['name'], $data['sender']);
    }
}

The class must implement the DataTransferObject interface provided by Saloon, which requires the class to have a fromResponse method defined. With this, the DTO definition is moved into the DTO itself, which is great when you might construct the DTO elsewhere like fromRequest (e.g from a Laravel Request) and also makes it a lot easier to do typing. It also means that you could re-use one DTO for multiple requests write definitions based on the connector/request in the response.

public static function fromResponse(Response $response): static
{
    if ($response->getRequest() instanceof GetAmazonOrderRequest) {
         // Logic A
    }

    if ($response->getRequest() instanceof GetDhlOrderRequest) {
         // Logic B
    }
}

Questions

• What do people think about this new approach?
• Please could people test this branch in their IDE to test that PHPStan / IDE support is good?
• Any other thoughts?

[Sammyjo20]

Self review

[Sammyjo20]

Howdy @techenby - following our chat on Friday, I've been thinking about the "transformer" name we came up with and I thought that maybe it'll be better to keep this functionality as DTOs and only "outgoing" rather than incoming too, I know we were trying to figure out how to do incoming too like $request->with(Order) however I think that could be achieved by just expecting the DTO in the constructor? Happy to mull on this

[Sammyjo20]

Taken a note, a couple of people don't like into. So far I've come up with return

$connector->send($request)->return(Order::class);

[Sammyjo20]

Another suggestion is to add the type support to the existing dto method. I was undecided on this as I don't like too many options but it could definitely help.

$connector->send($request)->dto(Order::class);

We tried this before and couldn't get the generics right, but we might be able to try this again.

[onlime]

Hey @Sammyjo20 very much like this idea, this whole PR! So it would be flexible and we could keep using Spatie Laravel-Data DTOs, but just flavor them with your DataTransferObject interface.

But honestly, I am too lazy to remember the DTO classes for my hundreds of requests when tinkering, so I would end up adding a helper method to all of my Requests, e.g. getDefaultDtoClass() and would need to add a macro to your Response class magicDto() that does return $this->dto($this->pendingRequest->getRequest()->getDefaultDtoClass()).

Could you be so nice and also add such an (optional) getDefaultDtoClass(): string (class-string) method to your interface? And let me know if I didn't quite explain it right or have overseen something.
(Ok, maybe that helper magicDto() would suffer from the same problem, making static analysis impossible for return type...)

In the past, I have been using dtoOrFail() in Tinker/Tinkerwell a lot and it would be a big regression, if every time I would need to look up the corresponding DTO class for each Request (which I don't even remember as class name, as I am always firing requests through a resource class from the connector).

[onlime]

about the naming: dto() would be a BC, was already used, but I quite like it. return() wouldn't be self-explanatory at all. Why not just stick with the original idea into()?

Or why not intoDto(Order::class) + intoDtoOrFail(Order::class) to be totally explicit?

So, in my whole project, I would then use intoDto(Order::class), but only when tinkering, I could use my (static-analytically "unusable") helper method intoDto() (without passing class-name param) – the magicDto thing I proposed in my previous comment. That would solve both problems. The intoDto() implementation would take care of resolving the DTO class via $this->pendingRequest->getRequest()->getDefaultDtoClass() if it was not passed.

[eclipse313]

I'm just wondering how this would work if my API returns an array (e.g. when I query orders) and the JSON is maybe not quite clean but like this:

{
   "count":7,
   "status":"OK",
   "orders":[
      {
         "id":1,
         "name":"X"
      },
      {
         "id":2,
         "name":"X"
      },
      …
   ]
}

[Sammyjo20]

@eclipse313 That's a good point I haven't thought about yet - what happens when the API returns multiple results? Do we maybe need to make this similar to Laravel's HTTP Resources/Resource collections or maybe we could have a DataTransferObjectCollection interface?

I'm not sure on that.

@onlime I agree with all your points, I don't feel super confident in a defaultDto method... Perhaps instead of replacing the old dto method, this is just a newer way. You're right I'd face the same problem with type hinting if everything lived on the request.

[dvdheiden]

Foremost, I think it's a great initiative to see if this can be improved. I don't have a specific idea how to solve it currently, but at least I can provide some information/thoughts.

For multiple resources, I tend to use a factory (as it can be used for both the GET /orders and GET /orders/{id}). What I currently do is something like (following the example of @eclipse313):

public function createDtoFromResponse(Response $response): Collection
{
    return $response
        ->collect('orders')
        ->map(function (array $order): Order {
            return OrderFactory::buildFromResponse($order);
        });
}

This works fine, but the type hint is missing. The proposed solution does solve it for a single resource, but in this case, it would be preferred to even specific the type of the item in the Collection, something like:

/**
 * @return Collection<Order>
 */

Maybe, it would be an idea to separate the functionality of building the DTO and the request? Also, because the same logic can be used for multiple requests, i.e. the GET /orders or GET /orders/{id}.

[Sammyjo20]

Hey all many thanks for your contribution, it's much appreciated 🙏

Okay so I've come with another concept/prototype for many objects. I'm not 100% sold on it, but something is better than nothing.

There's now two interfaces (IntoObject and IntoObjects) - Needs a better name. If your API response returns multiple objects then you can use IntoObjects and then use the intoMany method. This will return an array and construct an array of the objects.

Now one improvement I want to make is the ability to have the same interface on the class at the same time, but I need some help with naming. Is fromResponse a good idea? Maybe I need a fromResponseToObject and a fromResponseToArray perhaps. I also don't feel like returning an array of the same object is the right place to go in a DTO - but maybe it is.

Example

<?php

use Saloon\Http\Response;
use Saloon\Contracts\DataObjects\IntoObjects;

class Superhero implements IntoObjects
{
    public function __construct(
        public string $name,
    ) {
        //
    }

    public static function fromResponse(Response $response): array
    {
        return $response->collect('data')
            ->map(function (array $item): self {
                return new static($item['superhero']);
            })
            ->toArray();
    }
}

Usage

$superheroes = $connector->send($request)->intoMany(Superhero::class); // array<Superhero>

I do wonder if there's a way I can do this automatically, like accept a method like this:

function intoMany(string $class, string|callable $key, bool $throw = true)
{
    $iterator = is_callable($key) ? $key() : $this->json($key);

    // Iterate and call `fromResponse`?
}

Then

$superheroes = $connector->send($request)->intoMany(Superhero::class, 'items'); 

[ejntaylor]

This great. Only to say I use Saloon with Spatie Laravel-Data DTOs and if you could align with their pattern of the from() method then that would be a cleaner experience than having to add an additional fromResponse method https://spatie.be/docs/laravel-data/v3/as-a-resource/from-data-to-resource

[binaryfire]

I love the idea!

Regarding the implementation - I use Saloon with Hyperf and other non-Laravel frameworks. And I don't use Spatie's Laravel Data package in my Laravel projects. So it'd be good to go for the best generic solution rather than tying it to how a third-party Laravel package does things.

[gturpin-dev]

Hey 👋

Just a personal opinion

I love the DX here :

$superheroes = $connector->send($request)->intoMany(Superhero::class); // array<Superhero>

But I think the definition in DTO could be better named, I don't have perfect ones, but here's some ideas :

use Saloon\Http\Response;
use Saloon\Contracts\DataObjects\IntoDataObjects;

class Superhero implements IntoDataObjects
{
    public function __construct(
        public string $name,
    ) {
        //
    }

    public static function fromResponseToDataObjects(Response $response): array
    {
        //
    }
}

This is more specific, I think a fromResponse() could be too generic for a DTO class which could be used in a lot of external context

For this, I think it could be a great DX to reduce some code and goes with the rest of the package which is great !

$superheroes = $connector->send($request)->intoMany(Superhero::class, 'items'); 

[yankewei]

Hi @Sammyjo20,

I have an idea about the return type of the fromResponse method in the intoObjects class. Could we make it return not just an array, but an iterable? This would allow for clearer return types in other frameworks, such as Laravel's Collection or array.
For example:

interface IntoObjects
{
    /**
     * Handle the creation of the object from Saloon
     *
     * @return iterable<self>
     */
    public static function fromResponse(Response $response): iterable;
}

use Saloon\Http\Response;
use Saloon\Contracts\DataObjects\IntoDataObjects;
Illuminate\Support\Collection;

class Superhero implements IntoDataObjects
{
    public function __construct(
        public string $name,
    ) {
        //
    }

    public static function fromResponseToDataObjects(Response $response):Collection
    {
        return $response->collect('data')
            ->map(function (array $item): self {
                return new static($item['superhero']);
            });
    }
}

[techenby]

@Sammyjo20 Totally sorry I missed this, I'll take a look :)

[techenby]

Maybe this is just me, but I don't want the place where I send the request to have to worry about what it's getting back. I like the request class being in charge of formatting the data to send and transforming the data it receives.

My simplest suggestion would be to rename createDtoFromResponse to transformResponse or formatResponse and change dto() to transform() or format()

I don't like DTOs and would rather pass back a single value or an array. You can already do that with the DTO methods, but I feel weird about calling ->dto() when I know I'm not getting a DTO back.

[timacdonald]

into is nice as it has symmetry with Laravel's Collection::mapInto, but I agree by itself it feels a little awkward by itself.

hydrate is another that I like to reach for and feels fitting here:

$order = $connector->send($request)->hydrate(Order::class);

[harryqt]

into / dto 👍

[Sammyjo20]

@timacdonald I love the word hydrate. I'll get this PR wrapped up and merged.