[PoC] Add option to get connector on DTO

[PhiloNL]

Hey Sam! 👋

I made a little proof of concept that will allow you to get the connector that made the request that resulted in the response.
I have no idea if this is something you would like to solve the way I did it, but I thought it would be cool if you could do some sort of chaining like this.

These changes allow you to use a dto object for additional API calls on subresources.

$product = $api
->products()
->get('977a77b5-79a6-4714-9614-bfc751b15b75')
->dto();

$policies = $product->policies()->all()->json();

class Product implements WithResponse, WithConnector
{
    use HasResponse;
    use HasConnector;

    public function __construct(
        public string $id,
        public string $name,
    ) {
    }

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

        return new static(
            id: $data['id'],
            name: $data['name'],
        );
    }

    public function policies()
    {
        return new PoliciesResource(connector: $this->getConnector(), productId: $this->id);
    }
}

One additional thing I thought about is adding some sort of trait like the one that always throws an exception on failure; is one that always returns the dto on success or throws an exception on failure, so you never have to call dto.

try {
  $productName = $api
  ->products()
  ->get('977a77b5-79a6-4714-9614-bfc751b15b75')->name
} catch (\Exception $e) {
  dd('something went wrong');
}

Both are ideas so happy to discuss and it's also fine of course if you think differently about this.
Thanks for creating Saloon! I've already used v1 but v2 is even better! 😎

[what-the-diff]

• Added a new interface WithConnector
• Added the trait HasConnector to implement this interface and provide functionality for setting/getting connector on data objects
• Modified the method dto() in HasResponseHelpers to set connector if it is implemented by data object class

[Sammyjo20]

Hey @PhiloNL, lovely to see you here and enjoying Saloon! 🤠

Thank you for the PR - I really like this idea, especially around the chaining. It's something we would also use for nested resources in our SDKs. I see you implemented it exactly the way I was thinking - however I have an idea. What if we have a "DataObject" class that implements both WithResponse and WithConnector - then all you have to do is extend the DataObject response.

This would be handy because then you can access the underlying response if you need to and the connector so you can chain methods like this without having to add an interface and trait or four things if you wanted both. Let me know your thoughts.

Regarding the DTOs throwing exceptions - that's a good point. We do currently have dtoOrFail() I believe which will throw an exception if the response wasn't successful. The AlwaysThrowOnErrors will throw an exception before your ->dto() method. Alternatively, we could add an optional argument like ->dto(throw: true)?

Let's riff on some of these ideas and build something awesome 😎

[PhiloNL]

Having a dedicated DataObject class to extend sounds like a great alternative!

Regarding the DTOs, it's more of a convenience thing. If I look at other SDK's they often return an object/DTO by default. Having a method called dto or dtoOrFail seems a bit off / confusing for an end user using the SDK (IMO).

So something like this would be great, I think.

class GetProductRequest extends Request
{
    use AlwaysReturnDtoOrFail; // Trait either on the request or connector

    protected Method $method = Method::GET;

    public function resolveEndpoint(): string
    {
        return '/products/'.$this->id;
    }

    public function __construct(protected string $id)
    {
    }

    public function createDtoFromResponse(Response $response): Product
    {
        return Product::fromResponse($response);
    }
}

Now you can interact with the object or use the exception to figure out what went wrong:

try {
  $product = $api
  ->products()
  ->get('977a77b5-79a6-4714-9614-bfc751b15b75'); 

  dd($product->name);
} catch (Saloon\Exceptions\Request\RequestException $e) {
  $e->getResponse() // Handle exceptions e.g. notify user
}

[Sammyjo20]

Great, I will add the DataObject or something similar to this PR 👍

With regards to the AlwaysReturnDtoOrFail - how would you suggest this be implemented?

Are you able to share your products resource? Could you do something like this:

public function get(string $uuid): YourDto
{
    $request = new YourRequest;

    return $this->connector->send($request)->dtoOrFail();
}

That way when your users write the following;

$sdk->products()->get('id');

It will return your DTO or throw a RequestException, and then you can carry on with the chaining as described previously. With this approach, you wouldn't need the AlwaysReturnDtoOrFail method as you're implementing the failure logic inside of your resource. Let me know if I've made the wrong assumptions here!

[Sammyjo20]

Hey @PhiloNL what are your thoughts on the above? 😇

[PhiloNL]

Sorry for the delay @Sammyjo20. I'll look into the AlwaysReturnDtoOrFail in a little bit (I'm on a quick holiday at the moment 🙈) I think what you suggest can work as well.

+1 for the DataObject 🚀

[Sammyjo20]

@PhiloNL Enjoy your trip mate! ⛵️ Here's an idea around the always throwing errors on DTOs I'd like to get your opinion on - what if instead of a trait, we ask people to add an interface to the class e.g

class ForgeConnector extends Connector implements AlwaysReturnDtoOrFail

Which then in our DTO code we could just add something like...

public function dto()
{
    if ($this->pendingRequest->getRequest() instanceof AlwaysReturnDtoOrFail || $this->pendingRequest->getConnector() instanceof AlwaysReturnDtoOrFail) {
        return $this->dtoOrFail()
    }
}

My only concern is that it's inconsistent with the traits.

[Sammyjo20]

Alternatively, this also could help with future features I'm working on for Saloon - what if there was a sort of feature-flag system built into the PendingRequest class so we can quickly enable features against the PendingRequest and check one class.

Then we could have a trait like you suggested:

trait AlwaysReturnDtoOrFail
{
     public function bootAlwaysReturnDtoOrFail($pendingRequest)
     {
           $pendingRequest->useFeature(Feature::ALWAYS_RETURN_DTO_OR_FAIL);
     }
}

I always like throwing random ideas out there just to feed the conversation, let me know if you think feature flags are a bad idea.

[Sammyjo20]

Hey @PhiloNL how was your skiing trip? It looked awesome on Twitter!

What are your thoughts on my comments above?

[Sammyjo20]

Hey @PhiloNL apologies to chase but I'm currently shaping up some changes for v3 so please let me know if you have any ideas for v3 around this or if you like my idea of feature flags in Saloon

[Sammyjo20]

I'm going to close this for now, but I will consider adding the DataObject class into v2 :)

[PhiloNL]

Hey Sam 🙌🏻 My apologies for the delay. The feature flag approach seems like an interesting way of solving this 😄