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

allow (json-)strings for Mage_HTTP_Client_Curl::makeRequest $params Parameter #913

Merged
merged 3 commits into from
Apr 21, 2020

Conversation

viable-hartman
Copy link
Contributor

@viable-hartman viable-hartman commented Mar 27, 2020

What does everybody think about adding this method to allow the Mage Curl client to do proper POSTs of JSON directly. I know of a few REST apis that require this.

This would allow doing this in code to POST json data directly:

$curl = new Mage_HTTP_Client_Curl();
$curl->post_json($url, $params);

@spinsch
Copy link
Contributor

spinsch commented Mar 27, 2020

I have looked at the customized version of Magento 2 and discovered the following adaptation:

$this->curlOption(CURLOPT_POSTFIELDS, is_array($params) ? http_build_query($params) : $params);

This would solve your problem elegantly and offer the possibility of other types such as XML.

$uri = '';
$params = array();
$client = new Mage_HTTP_Client_Curl();

$client->addHeader('content-type', 'application/json');
$client->makeRequest('POST', $uri, json_encode($params));

@viable-hartman
Copy link
Contributor Author

Oh yes, that is better, I'll change the request to make that change instead 👍

spinsch
spinsch previously approved these changes Mar 27, 2020
@viable-hartman
Copy link
Contributor Author

viable-hartman commented Mar 27, 2020

Per @spinsch just submitted the Magento 2 fix for JSON POST utilized as he indicated:
Just one edit since makeRequest is protected calling post is necessary, but doesn't change the solution!

$uri = '';
$params = array();
$client = new Mage_HTTP_Client_Curl();

$client->addHeader('content-type', 'application/json');
$client->post($uri, json_encode($params));

Great idea to look at the Magento 2 repo first, should have thought of that :-) , and will in the future!

Copy link
Contributor

@tmotyl tmotyl left a comment

Choose a reason for hiding this comment

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

doc comment needs to be tuned array|string
But it's a very useful change, had similar challenge with magento EE recently too.

@viable-hartman
Copy link
Contributor Author

doc comment needs to be tuned array|string
But it's a very useful change, had similar challenge with magento EE recently too.

Updated! 👍

@colinmollenhour colinmollenhour merged commit a2218b5 into OpenMage:1.9.4.x Apr 21, 2020
@Flyingmana Flyingmana changed the title Json POST suggestion allow (json-)strings for Mage_HTTP_Client_Curl::makeRequest $params Parameter May 10, 2020
@sreichel sreichel added the Component: lib/Mage Relates to lib/Mage label Jun 4, 2020
@sreichel sreichel added this to the Release 19.4.2 milestone Jun 27, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants