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

[REQ][PHP] Bump guzzle to version 7 #7869

Closed
toymachiner62 opened this issue Nov 3, 2020 · 15 comments · Fixed by #10585
Closed

[REQ][PHP] Bump guzzle to version 7 #7869

toymachiner62 opened this issue Nov 3, 2020 · 15 comments · Fixed by #10585

Comments

@toymachiner62
Copy link

toymachiner62 commented Nov 3, 2020

I have built a lib for others to use and it's using guzzle version 6.x b/c that's what openapi-generator uses. When other devs are consuming my project they are unable to use guzzle 7 since version 6 is what's being used in my lib generated from openapi-generator.

I come from node.js for the 8 years so i'm used to all of a projects dependencies using their own versions, but I guess php land is not like this.

Can you bump guzzle to version to 7? Or is there another way to handle this dependency of a dependency issue that i'm unaware of?

@wing328
Copy link
Member

wing328 commented Nov 4, 2020

Can you bump guzzle to version to 7? Or is there another way to handle this dependency of a dependency issue that i'm unaware of?

One way is to use customized templates (CLI options: -t)

@atanasiuk-hubspot
Copy link
Contributor

Hi, same question: Would the generated client be compatible with guzzle 7?

@jeffski
Copy link
Contributor

jeffski commented Nov 14, 2020

+1, have users affected by this issue.

@jaap
Copy link

jaap commented Dec 8, 2020

PR already exists but fails 1 test: #7950

@jason-o-matic
Copy link

+1 here as well, with our users being affected.

@christianl-das
Copy link

+1 as one of the users @jason-o-matic is referring to :)

@christianl-das
Copy link

Is there any update on this?

@krzaczek
Copy link

krzaczek commented May 8, 2021

Hi, it can be easly fixied by adding a custom composer template

Just create a resources dir and place a composer.mustache file in it.

{
    "name": "{{gitUserId}}/{{gitRepoId}}",
    {{#artifactVersion}}
    "version": "{{artifactVersion}}",
    {{/artifactVersion}}
    "description": "{{{appDescription}}}",
    "keywords": [
        "openapitools",
        "openapi-generator",
        "openapi",
        "php",
        "sdk",
        "rest",
        "api"
    ],
    "homepage": "https://openapi-generator.tech",
    "license": "unlicense",
    "authors": [
        {
            "name": "OpenAPI-Generator contributors",
            "homepage": "https://openapi-generator.tech"
        }
    ],
    "require": {
        "php": ">=7.3",
        "ext-curl": "*",
        "ext-json": "*",
        "ext-mbstring": "*",
        "guzzlehttp/guzzle": "^6.2 || ^7.0"
    },
    "require-dev": {
        "phpunit/phpunit": "^7.4",
        "friendsofphp/php-cs-fixer": "^2.12"
    },
    "autoload": {
        "psr-4": { "{{escapedInvokerPackage}}\\" : "{{srcBasePath}}/" }
    },
    "autoload-dev": {
        "psr-4": { "{{escapedInvokerPackage}}\\Test\\" : "{{testBasePath}}/" }
    }
}

And then use this template when building the API by adding -t ./resources to CLI command:

java -jar openapi-generator-cli-5.1.0.jar generate -t ./resources -i openapi.yml -g php`

@grizzm0
Copy link

grizzm0 commented May 17, 2021

If you do this, don't forget to add "guzzlehttp/psr7": "^1.8" as the generated code uses functions that has been deprecated in 2.0.

@SamMousa
Copy link

Shouldn't we make it guzzle agnostic?
Nowadays we have PSR17 (Request factories) and PSR18 (HTTP Client), should make it a lot easier to plug in any client library version.

christiaanderidder added a commit to channelengine/merchant-api-client-php that referenced this issue Jun 21, 2021
christiaanderidder added a commit to channelengine/channel-api-client-php that referenced this issue Jun 21, 2021
@stevenbuehner
Copy link

stevenbuehner commented Jul 21, 2021

If you do this, don't forget to add "guzzlehttp/psr7": "^1.8" as the generated code uses functions that has been deprecated in 2.0.

As far as I understand, the deprecated status functions from guzzle/psr7 ^1.8 have been listed here with their guzzle/psr7 ^2.0 equivalents: https://github.com/guzzle/psr7/tree/2.0.0#upgrading-from-function-api
Wouldn't it make sense to upgrade right away to 2.0 ?

This is needed anyways to fix another bug handling ObjectSerialization of boolean query vars #2204.

@jeffski
Copy link
Contributor

jeffski commented Sep 22, 2021

Worth noting that with Guzzle 7.3, you will get an error:

PHP Fatal error:  Uncaught Error: Call to undefined function GuzzleHttp\Psr7\build_query()

build_query() has been deprecated in favor of Query::build

So https://github.com/OpenAPITools/openapi-generator/blob/5d68bd6a03f0c48e838b4fe3b98b7e30858c0373/modules/openapi-generator/src/main/resources/php/api.mustache will need to be updated to support Guzzle 7.3 (Line 610 and 663)

@wing328
Copy link
Member

wing328 commented Sep 22, 2021

@jeffski can you please submit a PR with the suggested fix when you've time? Thank you.

@heissb2342
Copy link
Contributor

Hi there,
I can supply a PR shortly as I was already looking into the guzzle 7 update a few weeks ago for swagger-codegen.

Also another side note to deprecated function being in the code, there is also a reference in https://github.com/OpenAPITools/openapi-generator/blob/5d68bd6a03f0c48e838b4fe3b98b7e30858c0373/modules/openapi-generator/src/main/resources/php/api.mustache line 563 to try_fopen(), which needs to be replaced by Utils::tryFopen.

As I said PR should be coming in a few days so @jeffski should be off the hook ;)

@wing328
Copy link
Member

wing328 commented Oct 16, 2021

Please pull the latest master to give it a try.

Thank you @heissb2342 for his contributions 👍

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

Successfully merging a pull request may close this issue.