Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable overriding of HTTP methods #126

Merged
merged 3 commits into from May 5, 2023
Merged

Enable overriding of HTTP methods #126

merged 3 commits into from May 5, 2023

Conversation

nathan-de-pachtere
Copy link
Contributor

@nathan-de-pachtere nathan-de-pachtere commented May 5, 2023

Hello @kbond,

I'm using your bundle in my test with ApiPlatform and would love to be able to override HTTP methods.

For example, for the PATCH method only I need to change the Content-Type to application/merge-patch+json, so right now I'm doing it by creating a new function in my custom browser class (but with name patchV) :

<?php

namespace App\Tests\...;

use Zenstruck\Browser\HttpOptions;
use Zenstruck\Browser\KernelBrowser;

class TestBrowser extends KernelBrowser
{
    ...

    public function patchV(string $url, $options = [
        'json' => [],
    ])
    {
        return $this->setDefaultHttpOptions(
            HttpOptions::create()->withHeader('Content-Type', 'application/merge-patch+json')
        )->patch($url, $options);
    }

   ...
}

Then in my test I'm using it like that :

$this->browser()
->patchV(...)

So the goal of this merge request is to give developer the power of overriding these methods properly with the base HTTP method name if they need to.

<?php

namespace App\Tests\...;

use Zenstruck\Browser\HttpOptions;
use Zenstruck\Browser\KernelBrowser;

class TestBrowser extends KernelBrowser
{
    ...

    public function patch(string $url, $options = [
        'json' => [],
    ])
    {
        $this->setDefaultHttpOptions(
            HttpOptions::create()->withHeader('Content-Type', 'application/merge-patch+json')
        );
        return parent::patch($url, $options);
    }

   ...
}

Then in my test I could use it like that :

$this->browser()
->patch(...)

Example of use cases:

  • Put some default $options structure if needed
  • Setup default headers by method
  • Setup default options
  • In the case of ApiPlatform and Iri, can be usefull to findIriBy($object ... ) from $object argument instead of $url.
  • ...

Let me know if this stuff doesn't make any sens

@nathan-de-pachtere nathan-de-pachtere changed the title Enbale overriding of HTTP methods Enable overriding of HTTP methods May 5, 2023
Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

.gitignore Outdated Show resolved Hide resolved
@kbond kbond merged commit 48bb602 into zenstruck:1.x May 5, 2023
4 of 13 checks passed
@kbond
Copy link
Member

kbond commented May 5, 2023

Thanks @nathan-de-pachtere.

Do you think it could make sense to add an ApiPlatformBrowser to this package with some of the features you describe above?

@nathan-de-pachtere
Copy link
Contributor Author

@kbond
I was preparing another merge request for 2 other methods to add to browser

public function assertJsonMatchesSchema(string $jsonSchema, string $selector = null): self
    {
        $selectedJson = $this->json();
        if ($selector) {
            $selectedJson = new Json(json_encode($this->json()->search($selector)));
        }
        $selectedJson->assertMatchesSchema($jsonSchema);

        return $this;
    }

    public function assertJsonMatchesSchemaFromFile(string $filename, string $selector = null): self
    {
        return $this->assertJsonMatchesSchema(file_get_contents($filename), $selector);
    }

In order to be able to evaluate JSON schema like this one :

{
  "type": "object",
  "properties": {
    "@id": "string",
    "@type": "string",
    "title": "string",
    "score": "number",
    "content": "string",
    "answeredAt": "string",
    "askedAt": "string",
    "fromExternalSource": "boolean",
    "uuid": "string",
    "jobTitle": "string",
    "denomination": "string"
  },
  "required": ["@id", "@type", "uuid", "fromExternalSource", "content", "answeredAt", "askedAt", "score"]
}

Do you think it could make sense to add an ApiPlatformBrowser to this package with some of the features you describe above?

The idea of making an ApiPlatformBrowser … I don't know … it's not necessary related to ApiPlatform, I mean they are the most famous for Symfony API stuff but not the only one. And those features are for APIs in general, regardless of the bundle used. I think it could be great to keep that flexibility. And ApiPlatform got their test helpers too, it's just not as convenient as yours.

I think first it's just about making your bundle extensible that developers can make their own test suit and helper. After any specific ApiPlatform stuff should leave in the ApiPlatform repo directly.

Just my thought about your idea and the fact that your code is not targeted to specific bundles.

And maybe I will tell you in 2 months that we should cause I have some stuff to generalize but for now it's like that in my brain ;)

@kbond
Copy link
Member

kbond commented May 5, 2023

Ok, I've never used api-platform myself but after watching the symfonycasts.com tutorial on it, I saw the customizations necessary so it gave me that idea.

KernelBrowser is currently geared towards html requests.

Maybe a generic ApiBrowser or JsonApiBrowser where we add these type of methods directly to the browser class?

@nathan-de-pachtere
Copy link
Contributor Author

nathan-de-pachtere commented May 5, 2023

Yep, making an ApiBrowser on top of KernelBrowser with extra specific methods.

I would rather go for the ApiBrowser than JsonApiBrowser because with ApiPlatform for example you can choose another format than JSON or create your own. And one API can dynamically return many formats de pends on the config so ...

        jsonld:   ['application/ld+json']
        jsonhal:  ['application/hal+json']
        jsonapi:  ['application/vnd.api+json']
        json:     ['application/json']
        xml:      ['application/xml', 'text/xml']
        yaml:     ['application/x-yaml']
        csv:      ['text/csv']
        html:     ['text/html']
        myformat: ['application/vnd.myformat']

Maybe could be something like :

  • an ApiBrowser for API related stuff regardless of the format (HTTP methods for instance)
  • an JsonBrowserTrait to add JSON functionalities
  • an JsonLdBrowserTrait to add JSON-LD functionalities
  • an XmlBrowserTrait to add XML functionalities
  • ...

As Trait can be extended, it gives full control for the dev.

trait CustomJsonBrowserTrait 
{
    use JsonBrowserTrait  {
        JsonBrowserTrait ::assertJsonSchemas as parentAssertJsonSchemas;
    }

    public function assertJsonSchemas() {
        //your stuff

        $this->parentAssertJsonSchemas();

        //your stuff
    }
}

And then when you create your own browser, you can flavour it as you want.

class TestBrowser extends KernelBrowser
{
    use CustomJsonBrowserTrait;
    //or use JsonBrowserTrait;
}

What do you think ?

@kbond
Copy link
Member

kbond commented May 8, 2023

That could work, yes. Not sure about the traits though. I'd like users to be able to work with the different json browsers without needing to create a custom browser. Here's sort of what I'm thinking a 2.0 could look like:

Browser (abstract)
    KernelBrowser (trait with the request methods)
    DomBrowser (abstract with dom assertions)
        HtmlBrowser (abstract with form interactions)
            PantherBrowser
            KernelHtmlBrowser (uses KernelBrowser trait)
        XmlBrowser (for xml apis? uses KernelBrowser trait)
    JsonBrowser (uses KernelBrowser trait)
        JsonLdBrowser
        ...

Not sure how common xml apis are anymore so maybe the XmlBrowser isn't needed? Don't know how many custom methods we'd need for the different json api types? If none, maybe just a mode we specify when constructing the JsonBrowser?

$this->jsonBrowser(); // normal json browser
$this->jsonBrowser(JsonBrowser:JSON_LD);
$this->jsonBrowser(JsonBrowser:JSON_HAL);
$this->jsonBrowser(JsonBrowser:JSON_API);

@nathan-de-pachtere
Copy link
Contributor Author

Not sure how common xml apis are anymore so maybe the XmlBrowser isn't needed?

Not common, but still existing in legacy projects 😅

I'd like users to be able to work with the different json browsers without needing to create a custom browser.

Sounds good.

As I'm working on an SaaS using your libs for testing and ApiPlatform with JSON-LD format, I can start to regroup useful functions that I used to test. Gonna share with you, maybe you will better see if it's makes sens to create format specific browsers (LD HL, ...) or JsonBrowser is enough.

ApiPlatform test helpers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants