search

SocalNick / orchestrate-php-client

A PHP client for Orchestrate.io
BSD 3-Clause "New" or "Revised" License
32 stars 5 forks source link

Changed headers and data for KvPatchMergeOperation #15

Closed Parent5446 closed 9 years ago

Parent5446 commented 9 years ago

As per the Orchestrate API, changed the Content-Type header for the JSON partial merge operation to be application/merge-patch+json as opposed to the standard application/json.

Also added manual JSON encoding of the body, since Guzzle will not do it for you unless the Content-Type header is standard.

Fixes #14

Parent5446 commented 9 years ago

@SocalNick this failure seems to be an issue with the API key used for Travis CI.

sportarc commented 9 years ago

Has this been enforced recently on the Orchestrate side? We are already using the Patch method in couple places and it was working fine.

On Wed, May 6, 2015 at 12:19 AM, Tyler Romeo notifications@github.com wrote:

@SocalNick https://github.com/SocalNick this failure seems to be an issue with the API key used for Travis CI.

— Reply to this email directly or view it on GitHub https://github.com/SocalNick/orchestrate-php-client/pull/15#issuecomment-99316789 .

Nicolas Menciere Founder and CTO Sport Archive Inc.

Parent5446 commented 9 years ago

@sportarc Possibly? All I really know is that without this patch, doing a merge operation gives a 400 Bad Request, but with the patch it succeeds. I will look into it a little more just in case.

sportarc commented 9 years ago

I think we're using PATCH (PARTIAL UPDATE - OPERATIONS) and not the PAtch merge

On Wed, May 6, 2015 at 1:12 PM, Tyler Romeo notifications@github.com wrote:

@sportarc https://github.com/sportarc Possibly? All I really know is that without this patch, doing a merge operation gives a 400 Bad Request, but with the patch it succeeds. I will look into it a little more just in case.

— Reply to this email directly or view it on GitHub https://github.com/SocalNick/orchestrate-php-client/pull/15#issuecomment-99541011 .

Nicolas Menciere Co-Founder and CTO Sport Archive Inc.

Parent5446 commented 9 years ago

Mhm, the Operations one works just fine. It's just the merge one that was not working.

SocalNick commented 9 years ago

@Parent5446 - unfortunately, travis does not export secure environment variables when building pull requests. I downloaded your patch and confirmed that it breaks the SocalNick\Orchestrate\Tests\KvTest::testPatchMerge as shown below. Unless the test is bad, I think it is working as is. One improvement that would be gladly accepted is getting the test suite to run with your own API key. I think my account has a couple of collections that need to exist otherwise the tests don't work.

 |2.0.0-p481| Nicholass-MacBook-Air in ~/workspace/orchestrate-php-client
±  |Parent5446-issue14 ✓| → ORCHESTRATE_API_KEY=********** phpunit
PHPUnit 3.7.38 by Sebastian Bergmann.

Configuration read from /Users/ncalugar/workspace/orchestrate-php-client/phpunit.xml.dist

.................................E...

Time: 11.39 seconds, Memory: 6.00Mb

There was 1 error:

1) SocalNick\Orchestrate\Tests\KvTest::testPatchMerge
SocalNick\Orchestrate\Exception\ClientException: ClientException occurred in request to Orchestrate: Client error response [url] https://api.orchestrate.io/v0/collection-554ad4ef8b4a3/554ad4f2e579a [status code] 400 [reason phrase] Bad Request

/Users/ncalugar/workspace/orchestrate-php-client/src/Client.php:85
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Subscriber/HttpError.php:33
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Event/Emitter.php:109
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:203
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:92
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RingBridge.php:121
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:147
/Users/ncalugar/workspace/orchestrate-php-client/vendor/react/promise/src/FulfilledPromise.php:24
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/ringphp/src/Future/CompletedFutureValue.php:55
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Message/FutureResponse.php:43
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:156
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:92
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Client.php:245
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Client.php:212
/Users/ncalugar/workspace/orchestrate-php-client/src/Client.php:75
/Users/ncalugar/workspace/orchestrate-php-client/tests/KvTest.php:228

Caused by
GuzzleHttp\Exception\ClientException: Client error response [url] https://api.orchestrate.io/v0/collection-554ad4ef8b4a3/554ad4f2e579a [status code] 400 [reason phrase] Bad Request

/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:89
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Subscriber/HttpError.php:33
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Event/Emitter.php:109
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:203
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:92
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RingBridge.php:121
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:147
/Users/ncalugar/workspace/orchestrate-php-client/vendor/react/promise/src/FulfilledPromise.php:24
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/ringphp/src/Future/CompletedFutureValue.php:55
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Message/FutureResponse.php:43
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:156
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/RequestFsm.php:92
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Client.php:245
/Users/ncalugar/workspace/orchestrate-php-client/vendor/guzzlehttp/guzzle/src/Client.php:212
/Users/ncalugar/workspace/orchestrate-php-client/src/Client.php:75
/Users/ncalugar/workspace/orchestrate-php-client/tests/KvTest.php:228

FAILURES!
Tests: 37, Assertions: 122, Errors: 1.