Version 2 is in beta 🤠

Sammyjo20 commented 2 years ago

Hey everyone, thanks so much for all the support that you have given for Saloon. I can't believe it's almost at 500 stars on GitHub and receiving over 150 installs a day. I just wanted to take a moment to thank you all! 😀🙌

That being said, there are some good things coming to Saloon. I'm working on version 2, which will help create a road for the future of Saloon, as well as improving developer experience and making your life easier.

Here is a summary of the changes that are going to happen.

New Flow

Currently, Saloon will run your request through the "Request Manager" which merges all of the headers, config, and everything else from the connector and request into one. With Version 2, I am introducing the "PendingSaloonRequest". Inside of here, this is where all of the logic like merging properties together and running your plugins will happen. This is so the building of a request is entirely separate from the sending of requests. After that, it will be sent to the "Guzzle Sender", and the class will receive the full PendingSaloonRequest with all the final configuration and headers before sending.

I am changing to this new flow because eventually, I want Saloon to be HTTP client agnostic and allow you to use any client you like, so you don't have to use Guzzle if you don't want - and if Guzzle decides to be abandoned, Saloon won't be left in the dark.

Here's the new flow in detail.

The only exception to the flow in the image above is that it will not create a PSR-7 request just yet, I am still working that one out.

Other Changes

Brand new middleware pipeline so you don't have to rely on Guzzle's handler pipeline
Requests don’t go straight to the request manager anymore, they can be built within a PendingSaloonRequest.
Better way to send data, by implementing interfaces that are processed by the PendingSaloonRequest.
All properties of a request like headers, config, data, handlers etc. Will all be standardised and now in separate buckets like $request→headers()→push().
Interfaces to be used a lot more
PHP 8.1 minimum
Real asynchronous request
Request pooling support

Middleware Pipeline

To help move the dependency on Guzzle, Saloon has also implanted its own middleware pipeline for requests and responses. This will replace the old Guzzle Handler support and response interceptors. You will be able to define request pipes and response pipes to modify the request/response before it is sent or given back to the user.

$request = new GetForgeServersRequest;

$request->middleware()
    ->addRequestPipe(function (PendingSaloonRequest $request) {
       //
    })
    ->addRequestPipe(new MyInvokableClass)
     ->addResponsePipe(function (SaloonResponse $response) {
       //
    })

Saloon's middleware pipeline will also be supported for asynchronous requests, so even if you have a pool of requests being sent simultaneously, they can each have their own middleware pipeline, which is something that Guzzle does not support with their existing handler stack logic, since you can only have one handler stack per client.

Middleware pipes can be added anywhere. Inside the request/connector, added by plugins, or even applied right before a request is sent. It will really allow you to tap into Saloon.

External API

Saloon’s external API will still remain the same, like the following:

<?php

$request = new GetForgeServersRequest;
$response = $request->send($mockClient);

$connnector = new ForgeConnector;
$request = $connector->request(new GetForgeServersRequest)->send();
$response = $request->send();

// Or

$connector->send($request);

There will be some additions to the external API, like interacting with request properties

<?php

// Before

$request = new GetForgeServersRequest;
$request->addHeader('X-User-Name', 'Sammy');
$request->addConfig('debug', true);
$config = $request->getConfig(); // Array

// Now

$request->headers()->push('X-User-Name', 'Sammy');
$request->config()->push('debug', true);

$config = $request->config()->all();

// Same with config, handlers, response interceptors, etc.

There will also be some new features, like the ability to set a mock client on the connector or a request, so it doesn’t have to be passed into the send of every request.


<?php

$request = new GetForgeServersRequest;
$request->withMockClient($mockClient);
$request->send(); // Won't need to set it here!

// Even more useful

$connector = new ForgeConnector;
$connector->withMockClient($mockClient);

// All requests using the connector will use the same mock client!

$connector->request($requestA)->send();
$connector->request($requestB)->send();

juse-less commented 2 years ago

Hey, @Sammyjo20. First off - obviously no stress, just wanted to check in. 🙂

How far would you say you've come on this? Do you happen to have some documentation and/or notes started, that one could possibly look at?

I've just started looking into both Saloon v2 and Spatie's Laravel Data v2. I'm not really in a hurry right now (waiting for releases before upgrading), but would like to start experimenting and possibly start making the way for an upgrade, if possible, so I have some new features already using these packages.

As always, thanks for this excellent package, and the superb support. ❤️

Sammyjo20 commented 2 years ago

Hey @juse-less,

Thanks for the message - at the moment v2 is in a bit of a funny state, I’ve got a basic request working from start to finish, using the brand new PendingSaloonRequest. That’s pretty much done. The GuzzleSender is about 90% complete and I’m confident that async requests are working properly. However a lot of the tests (pretty much all) are borked at the moment, so I wouldn’t trust all of it.

Overall I’d say I’m about 60% of the way through. The core is in there you can add everything like headers, config, plugins and the brand new middleware pipeline is working very very well.

I’m currently working on the mocking. Since I no longer use Guzzle’s handler stack, I need to build in “early responses” into the middleware/sending pipeline. I’ve got a prototype working.

After I’ve sorted the mocking, it will just be plumbing in all the extra bits like laravel support and OAuth2, but that’s all pretty easy since a lot of it is backwards compatible (except the new headers/config syntax)

I know you are probably anxious about using the async requests, and I can say with confidence that request pooling will work, and you’ll be able to maintain custom config and middleware pipelines for every request. It’s really going to be the dream. I’m also thinking of building pools like this:

$connector = new MyConnector;

$connector->pool([ new RequestOne, new RequestTwo ])->then()->catch()

and of course you’ll be able to use a generator or maybe an invokable class for the requests.

The way it works is once a request is sent once, we store the request sender on the connector, keeping the current guzzle client open and ready for more requests.

Hope this helps!

P.S I am about to go on holiday for 11 days and I won’t have my laptop, but I can’t wait to get back to coding v2 when I am back!

Sammyjo20 commented 2 years ago

As soon as I have a stable-ish API I’ll try to public v2 alphas / betas for you

juse-less commented 2 years ago

Thanks! That sounds amazing.

The pooling should solve a weird issue we're having with libcurl starting to fail to resolve hosts after ~10 minutes, depending on how quickly we send requests. The only solution is to restart our Laravel queue workers, or wait for like.. 15-30 minutes before it self-heals (after hundreds of failed Jobs we need to requeue). I've tried all sorts of settings and configs in Guzzle (even for libcurl directly), to no avail. I haven't tested PHP's cURL directly, though, but definitely a weird issue.

Nonetheless. Can't wait for it. 😊

Have a great holiday!

Sammyjo20 commented 2 years ago

Very strange! Hopefully the pooling will fix the issue. Did pooling fix the problem when you used Guzzle alone?

It’s my pleasure, thank you!

juse-less commented 2 years ago

I never tried pooling in Guzzle, actually. I just used Laravel's HTTP Client at the time, and quickly tried sending requests with Guzzle directly. This was also at the time while I was also mid-converting to Saloon to actually use async requests (we used sync with Laravel). So I did try both sync and async with Guzzle. As you might've guessed, it became even more apparent when we moved to using async requests, since we send 3-5 requests concurrently. Our solution for now is to simply restart all Queue workers after 5 minutes, to ensure we don't have a long-running job making it surpass that 10 minute marker.

The problem seems to be that keep-alive requests are being sent (or at least connection handles not being closed correctly), and the handles remain open, causing the whole process (Laravel's Queue worker) to run out of file handles. I tried disabling keep-alive, enforce HTTP 1.0, etc. But, for some reason, none of seems to do it. I guess I could just try increasing file handles and see if it can cope with more requests, but.. doesn't feel like an adequate solution. Since we're doing so many requests to begin with, the correct option would really be to use pooling. So hopefully it's solved once we move to pooling to keep the connection open, instead of initiating thousands of requests.

I did quickly try both HTTP/2 and HTTP/3, actually, but couldn't get it to work, and didn't have the headroom to test if all different external systems supports either to begin with.

Sammyjo20 commented 2 years ago

Hey @juse-less - other than the proper support for Async requests, is there anything else in v2 that you need? I may branch out and build request pooling support into v1, with the assumption that Guzzle middleware won't work on the request level.

I'm saying this just because I want to take my time a bit more with v2 and there's going to be quite a lot of work with rewriting the documentation, upgrade guides etc, so I'm wondering if I can help out with the mini feature in v1.

Otherwise, I could suggest that you go back to using your Guzzle approach for the parts of the app that were causing problems because of Saloon's async requests?

juse-less commented 2 years ago

@Sammyjo20 I think it's ok for now. I appreciate you asked, but I think it's better if you focus on v2. 🙇‍♀️

I apologise for the confusion - what I meant is that I tried with Guzzle as well, but to no avail. The problem is seemingly that it's opening a new 'file' for each request, and, despite setting libcurl constants (or Guzzle settings) directly in Guzzle, it still didn't fix the underlying issue. So, ultimately, it would appear that the only option is to support pooling. But I never tried pooling itself, just tried getting Guzzle/libcurl to close the handles in-between async requests.

I think the temporary solution I have in place also helps a bit preventing us from being banned from external systems and we push a lot of requests back and forth. Other than.. not sending that many requests (and frequently), obviously. 😛

So, that said, I really appreciate you asked and trying to help me with my situation. But I definitely think it's worth just focusing on v2. 🙂

Sammyjo20 commented 2 years ago

Thanks for the clarification @juse-less ! 🙌

I have some other work commitments I also need to spend my time on so v2 might be a bit longer than my initial 4-6 week estimation, but I'll keep updating this when I have more progress to mention. I'm really happy with how v1 works and I think v1 will be fine for most people for the next few months.

If you have any suggestions for v2 please don't hesitate to say!

juse-less commented 2 years ago

I have a few ideas, that I can write down next week, or so, as I have some work commitments myself (started a tight 2 week sprint today even). v1 is definitely more than enough. Since I can set Guzzle settings through Saloon already, I could possibly solve my issue in v1 as well, if I can just figure out the Guzzle/libcurl settings to apply.

One thing that directly comes to mind, however, when I quickly tried the parts of v2 last week (but behaves the same in v1), is the usage of interfaces. I actually register my various connectors, requests, and responses inside the Laravel container. This is currently not possible, the way that Saloon instantiates the defined class strings from the connector/request/response class string properties. So, I think there are 2 possibilities.

Have the Laravel Saloon package overwrite certain parts to resolve things from the Laravel service container, or
Easily let us override the sort of.. resolving mechanics, so we can easily call the container with our interface, to resolve the implementation. I think this one would be more versatile, as Saloon isn't Laravel-specific. So, letting developers resolve it, they could use other service containers, or other complex logic to resolve them.

It's probably only connectors and responses I'm thinking of, since requests are created directly, but mentioned all 3, just in case.

If you'd like to try something out later on (performance, resource usage, or otherwise), I can definitely try it in our internal tool I'm building.

bilfeldt commented 2 years ago

Version 1 is awesome - version 2 will be even better I can see 😉 Thanks @Sammyjo20.

The idea about interchangeable HTTP client using PSR-7, PSR-17 and PSR-18 sounds awesome! A good add-on might be adding a compatible driver for the Laravel HTTP Client. I know that under the hood this is using Guzzle, but so many things in the Laravel community is build around this, so you get easy integration with almost anything using that - examples could be:

Ray
Laravel Telescope
Logging (yes my own package)
Events

Sammyjo20 commented 2 years ago

Thanks for the message @bilfeldt that means a lot!

I agree it would be amazing to use the existing tools that wrap around Laravel's HTTP client. I will likely release V2 with just Guzzle support initially since that will make it on-par with version one, but the way it's designed, it would be super easy to write a custom adapter for the HTTP client.

Thanks for the feedback and for enlightening me on the idea!

niladam commented 2 years ago

Hello @Sammyjo20 - and thank you for your package! I've started using it and i'm really happy with it.

I was wondering if you need any help on getting v2 out the door ?

Sammyjo20 commented 2 years ago

Hey @niladam

Thank you for the offer but I'm all good, I am getting there but had to stop because work is so busy at the moment and I think I'd fall apart if I worked extra in the evening 😂

I think V2 is definitely going to be here before the end of the year so hang tight!

I will let you know if I need someone to peer review/help me because that would be very helpful.

Sammyjo20 commented 2 years ago

Just a little update, I've been starting back up my work for v2! 🙌

I've gone back through my todo list and created a fresh one with all the tasks I want to complete. I will keep the thread updated as progress comes along. My goal is to have it ready before the end of the year, but it may be January when it is released. January would mark Saloon's 1 year anniversary so that would be relevant :D

Sammyjo20 commented 2 years ago

Hello again everyone! I wanted to share with you an update on the progress of v2 as I feel it’s approaching the time where I am not introducing any more breaking changes, but I wanted to ask your opinion on the developer experience and how you will feel with a few breaking changes.

Just wanted to note that V2 is NOT ready to be used, even in an alpha state. I need to rewrite a lot of tests and battle-test it. The following is not an upgrade guide either, I am just looking for some feedback before I continue.

Goal

The Goal with v2 is to simplify Saloon’s codebase, make it more future proof and reduce its dependancy on Guzzle allowing it to be used with any HTTP client in the future. V2 also uses a lot more of PSR-7 and full PSR-7 support will likely be released in v3. The gap between v2 and v3 will be much smaller since less breaking changes will be required.

(New, Breaking) PendingSaloonRequest

Previously when you sent a request, Saloon would pass your request instance through the “request manager”. This class would be very closely tied to the HTTP Client (Guzzle) and the entire class was responsible for merging together query parameters, configuration, data and triggering things like authenticators.

Saloon now has a PendingSaloonRequest. This class when created will merge everything together into the one PendingSaloonRequest instance. This prevents the original SaloonRequest class from being polluted with mutations, and also allows Saloon to have a separate class that is built before passing it onto the HTTP Client.

Inside the PendingSaloonRequest’s constructor, it runs various methods to “build up” the pending request.

// PendingSaloonRequest.php / Constructor

$this
  ->registerDefaultMiddleware()
  ->mergeRequestProperties()
  ->mergeData()
  ->bootConnectorAndRequest()
  ->bootPlugins()
  ->authenticateRequest();

This also allows Saloon in the future to convert this class into a PSR-7 request with minimal effort, since it’s already separate.

[Click here to see an example of PendingSaloonRequest](https://github.com/Sammyjo20/Saloon/blob/v2/src/Http/PendingSaloonRequest.php)

(New) Senders

Once a PendingSaloonRequest is created, it will check if a MockResponse has been set. If one hasn’t been set it will pass the PendingSaloonRequest into a “sender”. This sender class is a wrapper around a HTTP Client. The default sender that will ship with Saloon v2 will be the GuzzleSender. Inside of this class, you are required to specify a “sendRequest” method.

The sender instance is created once on the connector and then will be re-used for every request. This allows us to keep the HTTP Client open and allows Saloon to finally support true asynchronous requests. More on that later.

This new sender class will allow Saloon to easily support other HTTP clients in the future without breaking Saloon’s internal logic. This is super exciting because Saloon no longer needs to depend on Guzzle to work. You can customise the sender inside of your own application too if you choose to make your own sender logic.

Another benefit of the senders logic over v1 is that the request and the HTTP client logic is now separated which reduces code complexity.

[Click here to see an example of the GuzzleSender](https://github.com/Sammyjo20/Saloon/blob/v2/src/Http/Senders/GuzzleSender.php)

(Updated, Breaking) Headers, Query Parameters, Config & Data

Saloon v2 also improves the way headers, query parameters, config and data is interacted with. Previously, each bucket of information lived in its own trait. They were inconsistent and sometimes didn’t make too much sense. I’ve now built a standardised “ContentBag” class inspired by Laravel’s MessageBag and ErrorBag classes. These are standardised repository classes that allow you to interact with them each in the same way.

Let’s look at some examples of managing headers.

Old Way

$request = new UserRequest;

$request->addHeader('X-Name', 'Sam');
$request->mergeHeaders([])
$request->setHeaders(['X-Foo' => 'Bar'])
$request->getHeaders(); // array
$request->getHeader('X-Name') // string

New Way

$request = new UserRequest;

$request->headers()->add('X-Name', 'Sam')
$request->headers()->merge([])
$request->headers()->set(['X-Foo' => 'Bar'])
$request->headers()->all()
$request->headers()->get('X-Name')

The same is true for query, data and config. This is a breaking change, but it massively reduces the complexity of code and makes it easier to test.

Another benefit of using the method access is that default headers will show up! Previously, if you had set an array of default headers in your request and then tried to access the headers by using $request->getHeaders it wouldn’t have shown you the default. Now it will show you the default headers, but it won’t show the default headers on the connector.

The same logic has been shared for query, data and config

$request = new UserRequest;

$request->query()->add()

$request->config()->add()

$request->data()->add()

(Breaking) Data Interfaces Replacing Traits

Previously when you wanted to tell Saloon that a request or connector will have data, you would have to use a plugin like this:

class CreateForgeSiteRequest extends SaloonRequest
{
    use HasJsonBody;

Saloon now has switched to interfaces for data.

class CreateForgeSiteRequest extends SaloonRequest implements SendsJsonBody
{

This is because previously the plugin would add a Guzzle-specific configuration option that adds a JSON, form or multipart body. Saloon will now handle this for you.

(New) Middleware Pipeline

To help move the dependency on Guzzle, Saloon has also implanted its own middleware pipeline for requests and responses. This will replace the old Guzzle Handler support and response interceptors. You will be able to define request middleware and response middleware to modify the request/response before it is sent or given back to the user.

It will support closures and accept a PendingSaloonRequest or an invokable class.

$request = new GetForgeServersRequest;

$request->middleware()
    ->onRequest(function (PendingSaloonRequest $request) {
       //
    })
    ->onRequest(new MyInvokableClass)
    ->onResponse(function (SaloonResponse $response) {
       //
    });

Saloon's middleware pipeline will also be supported for asynchronous requests, so even if you have a pool of requests being sent simultaneously, they can each have their own middleware pipeline, which is something that Guzzle does not support with their existing handler stack logic, since you can only have one handler stack per client.

Middleware pipes can be added anywhere. Inside the request/connector, added by plugins, or even applied right before a request is sent. It will really allow you to tap into Saloon.

It also will across any HTTP Client so in the future if Guzzle is not used, this middleware functionality is Saloon feature.

(Updated, New) Concurrent Requests & Pooling

Saloon’s old design meant that asynchronous requests just didn’t work. This was because every request would create a new Guzzle client, send the request and destruct the object. With Saloon’s new sender logic, Saloon v2 will keep the HTTP Client in memory on the connector. With some caveats.

The following example will not work because a new connector instance will be defined on every request. You must instantiate the connector and use the same connector or use the pool method.

// Will not work

$requestA = new CreateForgeSiteRequest;
$requestB = new CreateForgeSiteRequest;
$requestC = new CreateForgeSiteRequest;

$requestA->sendAsync();
$requestB->sendAsync();
$requestC->sendAsync();

The following examples will work

$conector = new ForgeConnector;

$requestA = new CreateForgeSiteRequest;
$requestB = new CreateForgeSiteRequest;
$requestC = new CreateForgeSiteRequest;

$connector->sendAsync($requestA)
$connector->sendAsync($requestB)
$connector->sendAsync($requestC)

$conector = new ForgeConnector;

$promises = $connector->pool([
    new CreateForgeSiteRequest,
    new CreateForgeSiteRequest,
    new CreateForgeSiteRequest,
]);

Asynchronous requests will return a PromiseInterface instead of SaloonResponse, but it will contain a response inside.

$conector = new ForgeConnector;

$requestA = new CreateForgeSiteRequest;

$promise = $connector->sendAsync($requestA);

$promise
    ->then(fn (PsrResponse $response) => ...) // PsrResponse is extension of SaloonResponse
    ->catch(fn (Exception) => ...)

(New) PSR Responses

Saloon’s response has also been updated to make it abstract, and you can now make your own response classes that accept different data. This makes it useful if in the future senders need to pass in a different object to responses.

For responses you will receive an instance of PsrResponse. This instance can be created with any class that implement PSR-7’s ResponseInterface class.

The benefit of this is that it makes Saloon almost PSR-7 ready.

(New) Simulated Response

There will be a new SimulatedResponse class that will be sent back if you are using the Saloon cache plugin or a mock response. The API will be the same as the existing SaloonResponse, it will just make it easier to know if a response is real or not.

juse-less commented 2 years ago

Note: I haven't looked through it all just yet, and haven't looked at the progress of the v2 branch.

I have to say, though - I'm liking many of the things I've seen, so far. The middleware pipeline is gonna be pretty sweet, as I work fairly extensively with first-class callables and invocable classes.

At the top of my head, some things I'd love to see is

More strongly typed code. Preferably using generics, where applicable. I've had to ignore some stuff in PHPStan, that I believe could be resolved through, say, generics. Although, I should've been able to suggest DocBlock changes, or write stubs (this is all new territory for me, though).
On top of this, I'd love for Saloon to primarily use interfaces, but supply default implementations (possibly splitting into traits for composability, where applicable). That would make it possible to still extend the base class (same as today), but also entirely custom implementations, that could reuse some implementation details through composability. The situation with Saloon v1 has made some parts tricky for me, but might be because I've been doing things incorrectly. (I have a tendency to think wildly differently than others, and is not directly related to Saloon, but still wanted to mention.)
Allow for the SaloonConnector::pool() method to accept arguments along the lines of
1. $requests - basically either an array, as per the example in your latest message above, a \Generator generating them, or a callable that Saloon has to call in order to retrieve the aforementioned array of requests or \Generator. Possibly give the callable the SaloonConnector, or its client as argument (to more easily create async requests for the client/connector). As an inspiration, you might look into how Guzzle handles pools, as they support callable|\Generator. This is probably a bit extreme and wide typing, though. But one could always wish. 🙂
```
array<array-key, PendingSaloonRequest>|\Generator<PendingSaloonRequest>|(callable(): array<array-key, PendingSaloonRequest>|\Generator(PendingSaloonRequest))` $requests
```
2. $concurrency - maximum simultaneous requests to 'keep open' at once, in case you need to send many.
```
int<0, max> $concurrency
```

Regarding Promises, it might be good to research around PSR for Promises, but it appears it's kinda died out the past several years. I know Guzzle is following the Promises/A+ standard, but I've also seen other PHP HTTP libraries follow it.

I haven't been able to make the move into collaborating- or contributing to open source yet, but would give it a shot if you'd want some help (and think it'd be useful).

Sammyjo20 commented 2 years ago

Hey @bilfeldt

I'm pleased to announce that Saloon v2 will ship with full support for Laravel's HTTP Client out of the box. By default it will detect that you are using Laravel and it will use send via the HTTP client instead of using Guzzle directly. This is great because you not only get all the power of Guzzle like you did before, but now everything that uses the events for the HTTP client will work through Saloon without any extra configuration.

I think this is going to be huge for Laravel developers!

bilfeldt commented 2 years ago

I'm pleased to announce that Saloon v2 will ship with full support for Laravel's HTTP Client out of the box. By default it will detect that you are using Laravel and it will use send via the HTTP client instead of using Guzzle directly.

Christmas came early this year 🥳

Sammyjo20 commented 2 years ago

Just an update everyone - I'm going to be tagging a beta release very soon - I want to fix all the broken tests now that the codebase is stable and settling down. I also want to write the docs for v2 with the first beta so you feel comfortable trying it out. This will come within the next 2-3 weeks, it would be good to have some feedback on

The upgrade guide
How v2 feels
Any new ideas

I'm aiming to have v2 pretty much done before the end of the year. I released v1 of Saloon last year on the 14th January so I think that would be an awesome time to release v2 👀 I'll let you know when it's ready to test!

Gummibeer commented 1 year ago

@Sammyjo20 seems like #101 hasn't found it's way to v2 yet!?

it returns all countries

<html>
<head>
<title>404 Not Found</title>
</head>
<body>
<h1>Not Found</h1>
</body>
</html>

Gummibeer commented 1 year ago

Here's a public upgrade PR: https://github.com/Astrotomic/steam-sdk/pull/1 Besides the missing #101 feature everything seems to work.

Was a bit of "useless" renaming and importing because of changed namespaces, method names and so on. But was still a pretty fast upgrade - would say around ~15-30min without any upgrade guide but simply running pest until everything was back green. 🙈

~And it seems like the whole MockClient thing changed or is broken - I'm on the way to debug it. But the mock client defined on my connector doesn't reach the pending request right now.~ Edit: my bad, had to drop the Laravel Saloon wrapper and because of that missed to change the container binding.

Sammyjo20 commented 1 year ago

This is awesome, thanks for sharing @Gummibeer - I am still working on the Laravel side at the moment, v2 branch will currently be very broken since the changing of namespaces.

Glad it was fairly easy to upgrade - most of my steps in the upgrade guide will explain detailed find-and-replace instructions which should fix 90% of the issues.

The other major breaking changes is the use of headers(), queryParameters(), config() and the request "data" being replaced with body(). I'm going to be working on some Saloon tonight, I'm just so excited to get v2 out there as I feel it's a real improvement while keeping all the good parts of v1, plus I've learned so much from you guys about building decent packages I really hope v2 is the one people really start using.

Gummibeer commented 1 year ago

Accidentally I was still on the v2 branch and fixed a bug in the SDK logic. While doing so I've seen that no fixture file was persisted for a 401 Unauthorized failing request. 🤔 But this is absolutely right and I would like to have this fixture as well. In v1 even failed requests created a fixture file. Seems like there's a problem with fixtures if an exception is thrown.

Sammyjo20 commented 1 year ago

@Gummibeer I can't prove it right now, but I'm guessing you're using the AlwaysThrowErrors trait? If you are, that is probably the issue; in v2 - plugins are loaded first, and that plugin adds a response middleware to throw an exception - that will be added before the middleware records the response, so it's throwing the error before the fixture.

I believe this may be a v1 issue, too - would you mind testing when you get a free moment, please?

By the way I updated v2 so the bug with the URLs is now fixed 🥳

Edit: Perhaps we need a method to add a middleware to the top instead of adding to the bottom so internal Saloon logic could process first?

Sammyjo20 commented 1 year ago

Just to let everyone know, beta is coming very soon - I had some additional final changes that I really wanted to make, I've also been focusing on writing v2's docs - as soon as I've written all the basic docs, I will publish a beta. I'd really love to get some feedback from you guys when I do publish the first one as I want to be able to make any breaking changes now before v2 is actually tagged.

Many thanks for the support so far!

Here's a sneak peak of the docs!

Sammyjo20 commented 1 year ago

You may have noticed from the new preview image that Saloon has moved to a connector-driven design.

This was something that I can thank @Gummibeer for helping me shape up.

From version two, Saloon is going to aim to be less "magical" and also introduce less friction for the developer. One of the points I found frustrating was defining a connector class on every request that you make. This was solely so you could make a request directly without instantiating the connector, for example

$request = new UserRequest;
$response = $request->send();

This approach was very minimalist, but it introduced complexity and friction for the developer.

From version two, the connector property is being dropped entirely from the request. This means that you must send your requests through the connector like this:

$connector = new TwitterConnector;
$response = $connector->send(new UserRequest);

This allows you to have constructor arguments on the connector, perfect for API tokens or configuration. Similar to before, the request can have its own headers, config, query parameters and body but the connector will provide the very top-level defaults.

Although this is being taken out of the request, you may still add the functionality back with the HasConnector trait on the request. Although, if you add it back - you need to be aware of the downsides like not being able to have constructor arguments on your connector.

I am also introducing a new SoloRequest class which will be perfect for making just one request for API integration. With SoloRequests, you don't need a connector at all - you can define everything in the request and send it above like you used to do.

I hope these changes are great for y'all and I'm so excited to get this out!

Sammyjo20 commented 1 year ago

Hey folks, Happy Holidays! ⛄️

Just wanted to keep everyone updated as it’s been a little while - I’m still working on Saloon v2 and it’s just working through a few final bits of polish before I’m happy to tag the first beta.

For me, the biggest thing is documenting the upgrade guide correctly, but recently I’ve found a few fundamental changes I wanted to make, so I’m going to make sure there aren’t any massive things stopping beta.

The great news is I think it’s so close and I’m looking forward to hearing your feedback, just in case there is anything obvious I’ve missed.

I will be taking some much needed time off over the holiday period to play video games and eat food 😂

Thank you for a wonderful year with all the support on Saloon and I can’t wait to release v2, v3, v4+ in the future!

Sammyjo20 commented 1 year ago

Hey @bilfeldt I have decided that instead of making the HttpSender the default sender when installing the Laravel package, you will need to enable it by overwriting the config file and changing the default sender. I feel this is better as it doesn't introduce unexpected behaviour, e.g a developer installs the package and it swaps the sender. I also have tested the GuzzleSender at great lengths and I would rather everyone using the Laravel package has a great experience and doesn't have any issues if there was a bug with just the HttpSender.

francoisauclair911 commented 1 year ago

Hi, Great Job on that package!

Do you have an ETA on the beta for V2 ?

No stress though 📦 Thank you

Sammyjo20 commented 1 year ago

Hey @francoisauclair911

I want to get it out ASAP, but I’ve got a client project that I don’t want hanging over me, so I’m going to be doing a lot of coding for that in the evenings - but I’ll work through Saloon when I can, my actual list of bugs/changes is really small but it’s just docs.

Do people want me to release a Beta with WIP docs? Happy to do so if people really want their hands on it

Sammyjo20 commented 1 year ago

I don't think it's far away from releasing at all come to think of it... I'll review this week and keep you updated!

georgeboot commented 1 year ago

Do people want me to release a Beta with WIP docs? Happy to do so if people really want their hands on it

Yes sure, would love it!

Sammyjo20 commented 1 year ago

I will get that sorted this week then! @juse-less has very kindly offered to help me with some documentation and potentially some of the last few things on my list like getting it PHPStan level 5/6 ready!

Sammyjo20 commented 1 year ago

Drumroll please 🥁🥁🥁

Saloon v2 is finally in beta! Please let me know what you all think, the documentation is still a work in progress, I wanted to just release the beta for you all as all the "basics" are covered in the docs, just the more advanced things are yet to be filled out. With the release of the beta it will motivate me to keep me updating the docs :D

Docs: https://docs.saloon.dev/v/2/ Release: https://github.com/Sammyjo20/Saloon/releases/tag/v2.0.0-beta1

I'm still working on a number of things like

Getting it PHPStan complete
Making sure the Laravel HTTP Sender is in a decent state
Updating the caching plugin to work with v2
Adding generics
General polish

Please provide feedback here, that'll be awesome!

Sammyjo20 commented 1 year ago

I would just like to say a huge thank you for everyone who has helped reshape Saloon, I'm so excited to be offering this to the community and I really feel it's a true "upgrade" and matures it massively.

Big thanks to @juse-less and @Gummibeer too for helping shape Saloon v2, you two have been awesome!

bilfeldt commented 1 year ago

Congratulations on the beta @Sammyjo20, this is truly nice work 🥇

I could not help noticing that your upgrade guide mentions Request Groups (previously called Request Collections - I personally use the term Resource which I cannot remember where I picked up). I could not find anything when source diving - can you point me in the right direction @Sammyjo20?

I also tried looking at the response to DTO conversion and was wondering if this is actually implemented when one has access to custom response classes 🤔

Look forward to playing around with this package.

Sammyjo20 commented 1 year ago

Hey @bilfeldt

Apologies for the docs, I know their still a work in progress and some parts are still waiting to be completed. With regards to request collections / request groups, I have actually removed support for them entirely in v2. I felt that by having them, Saloon had a lot of "magic" logic which was cool, but tricky for IDEs to support. As request collections were just classes that passed in the connector, I recommend that you create your own classes that support this, and then add methods into your connector. For example:

Properties Example

e.g $forge->servers->get();

class Forge extends Connector
{
    public Resource $servers;

    public function __construct()
    {
          $this->servers = new ServersResource($this);
    }
}

Method example

e.g $forge->servers()->get();

class Forge extends Connector
{
    public function servers(): ServersResource
    {
         return new ServersResource($this);
    }
}

Resource Class

use Saloon\Contracts\Connector;

class Resource 
{
    public function __construct(
           protected Connector $connector;
    }{}
}

Hope this helps, I actually really like your name "resource" for them, and I do feel like it would be cool to have a really simple class like this in Saloon, just without the magic methods. What are your thoughts @bilfeldt? If it's something that you feel you would copy from project to project it could be something I invest into. I'm personally a big fan of wrapping requests up however my other favourite way to make requests is to just do this:

$forge = new Forge; // Connector
$forge->send(new GetServersRequest);

bilfeldt commented 1 year ago

@Sammyjo20 that is exactly the implementation I am doing myself.

I have made these resource classes basically for three reasons:

I like the fluent syntax it allows: $forge->servers()->list()
This is the class I use for the following logic
- I can decide to have method parameters that are then used to create the request (see below).
- Casting the generic Response to a specific response like ListServersResponse with return type declaration
- Throwing custom exceptions (specific to that request). I usually throw the more generic HTTP exceptions in the connector, catches these in the resource and re-throw them as more detailed content specific exceptions. Like for example catching a NotFound exception (404) and re-throwing it as UnknownServer (specific for that request)

Here are two different ways I could implement your Forge example above:

use Saloon\Http\Response;

class ServersResource
{
    public function __construct(
           protected Connector $connector;
    }{}

    public function list(int $limit = 10): ServersListResponse
    {
        return $this->mapResponseToServersListResponse(
            $connector->send(new ServersListRequest(limit: $limit))
        );
    }

    protected function mapResponseToServersListResponse(Response $response): ServersListResponse
    {
        return new ServersListResponse($response);
    }
}

which could then be used like so:

$forge = new Forge(...);
$forge->servers()->list(15)->data;

It seems logical to me to put logic like this because:

Controller: How a generic Request is send and returns a generic Response or throws a generic Exception
Request: What are the headers, body, ....
ExampleRequest: You cannot freely set the headers and body, so here are public methods to set only the stuff you should be able to modify.
Response: What is the headers, body and http status we got back
ExampleResponse: The response is converted to DTO and has convenient methods that might be relevant.
ExampleResourse: Has knowledge about converting an ExampleRequest to an ExampleResponse

But - this is just my setup without having converted it to a package, not sure how it fits into that :) Just wanted to share.

Maybe an ResourceInterface and an option to register those on a Connector could be helpful, but again you are free to do that as you see fit already now 🤔

bnzo commented 1 year ago

@Sammyjo20 thanks for the great work!

It will be nice to have those kind of examples in the documentation as a collections transition from V1 or maybe an SDK project with those real life implementations.

Resource name is great!

Sammyjo20 commented 1 year ago

@bnzo I absolutely agree, I will add these examples to the documentation. I'll spend some time tonight doing some writing 🤠

juse-less commented 1 year ago

@Sammyjo20 What @bilfeldt describes is pretty much exactly the way I'm writing my SDK PoC we've chatted about on Twitter.
I got a bit stalled because of work, but hoping to have all parts in a repo so you can pick-and-choose from the automagic paging, the ResourceRepository, etc.

I was trying to help push through the todo list, so you could hit that goal of releasing on January 14th (the Saloon anniversary). But I guess the universe has other ideas. 🙁

Sammyjo20 commented 1 year ago

I love how you do it, @bilfeldt and I think your exception handling is going to be event easier with v2 because of the new exceptions that match the status e.g (NotFoundException or ServerErrorException).

Don't worry @juse-less in an ideal world I would have loved to have it out sooner but I know I don't want to rush a good project :D

Sammyjo20 commented 1 year ago

Hey @bilfeldt @bnzo I've updated Saloon's "Building SDK" docs with the example of the resource you suggested, thank you for helping! Let me know if there's anything more I should add to this page.

https://docs.saloon.dev/v/2/digging-deepeer/building-sdks

bilfeldt commented 1 year ago

https://docs.saloon.dev/v/2/digging-deepeer/building-sdks

Good place to put it 👍

Sammyjo20 commented 1 year ago

How have people found version two so far? Documentation on my side is coming along really nicely but I don't think we're far off release!

Sammyjo20 commented 1 year ago

Hey folks, as I'm writing the last docs I'm making small tweaks to the code and I was just reviewing the DTO conversion logic. This code is on the response ($response->dto()) and I was wondering, should I return null if the response failed, or should I leave it up to the developer to handle an error DTO?

public function dto(): mixed
{
    if ($this->failed()) {
        return null;
    }

    $dataObject = $this->pendingRequest->createDtoFromResponse($this);

    if ($dataObject instanceof WithResponse) {
        $dataObject->setResponse($this);
    }

    return $dataObject;
}

bilfeldt commented 1 year ago

Hey folks, as I'm writing the last docs I'm making small tweaks to the code and I was just reviewing the DTO conversion logic. This code is on the response ($response->dto()) and I was wondering, should I return null if the response failed, or should I leave it up to the developer to handle an error DTO?
public function dto(): mixed

{

    if ($this->failed()) {

        return null;

    }

    $dataObject = $this->pendingRequest->createDtoFromResponse($this);

    if ($dataObject instanceof WithResponse) {

        $dataObject->setResponse($this);

    }

    return $dataObject;

}

I would say throw a LogicException. Trying to convert an error response to a DTO should not happen 🤷‍♂️

Sammyjo20 commented 1 year ago

I would say throw a LogicException.

That's a good way of handling it, but do you think people might have a ServiceError DTO or anything like that?

bilfeldt commented 1 year ago

I would say throw a LogicException.

That's a good way of handling it, but do you think people might have a ServiceError DTO or anything like that?

Hmmm. It might be that the api errors always comes back in a certain format for a given API. In that case I would implement a custom exception which adds this format (as a dto perhaps). Much like the laravel VilidationException does.

So somewhere it should be possible to take any 4xx error and throw a custom exception.

But IF the user does not throw an exception and does not conditionally check for a success before casting to a DTO, then they are doing something wrong (missing either of those two points), and a LogicalException should be thrown in my opinion 🤷‍♂️

saloonphp / saloon