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

Types of request param in KernelBrowser are now string instead of integer #38591

Closed
alexander-schranz opened this issue Oct 15, 2020 · 4 comments

Comments

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Oct 15, 2020

Symfony version(s) affected: 5.2.0-beta2

Description

In the request stack does now only contain strings instead of integer like before:

How to reproduce

I did test sulu against the newest 5.2.0-dev and some tests are failing now a basic example looks like this:

$this->client->request(
    'PUT',
    '/api/contacts/' . $contact->getId(),
    [
        'account' => [
            'id' => 5, // a integer type
        ],
    ]
);

If we then debug the value is now a string, in 5.1.7 its correctly a integer:

$request->request->all()['account']['id'];

Before this was a integer with 5 and now its an string with "5" and so this make in our case some test fail which I think should not happen.

Possible Solution

Not sure yet where and why this behaviour was changed.

@fabpot
Copy link
Member

fabpot commented Oct 16, 2020

IMHO, you should fix your tests as everything coming from a request is a string (that's what we fixed in 5.2).

@alexander-schranz
Copy link
Contributor Author

@fabpot In our case we are testing a JSON Api. Is there an option on the client that its able to set the client send the requestBody as json so its handled like before? Else I think it will be a lot of refactoring for some projects changing this.

@alexander-schranz
Copy link
Contributor Author

Some background we are using the FOSRestBundle which converts the json into the Request::request object with its BodyListener. I think it would help many devs to introduce some kind of jsonRequest into the BrowserKit then. I created a PR for it #38596.

@ro0NL
Copy link
Contributor

ro0NL commented Oct 16, 2020

I suggested to support json payloads in $request->request consistently: #38224 (comment) but it was ignored :)

I agree with @fabpot, the only supported usecase is x-form-www-urlencoded data e.g. $_POST.

chalasr added a commit that referenced this issue Nov 11, 2020
…it client (alexander-schranz)

This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[BrowserKit] Add jsonRequest function to the browser-kit client

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix -
| License       | MIT
| Doc PR        | symfony/symfony-docs#...

If you use the FOSRestBundle for your Api's you have maybe many tests using just:

```php
$client->request('POST', '/api/contacts', ['param' => 1]);
```

To test your JSON api as in the real browser request FOSRestBundle converts the json body into the `$request->request` object. I think something similar is done by ApiPlatform. If you have tests like above they will now fail as the integer is converted to string see also #38591.

This PR add a new `jsonRequest` which will look like the following and will so fix the above problem:

```php
$client->jsonRequest('POST', '/api/contacts', ['param' => 1]);
```

Commits
-------

c2fa2cb [BrowserKit] Add jsonRequest function to the browser-kit client
@fabpot fabpot closed this as completed Nov 14, 2020
fabpot added a commit that referenced this issue Nov 14, 2020
…t parameters string cast (chalasr)

This PR was merged into the 5.2 branch.

Discussion
----------

[BrowserKit] [Browserkit] Add changelog entry for request parameters string cast

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fixes #38651, Fixes #38591
| License       | MIT
| Doc PR        | -

Commits
-------

ec80507 [Browserkit] Add changelog entry for request parameters string cast
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants