Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
### Added

- [#134](https://github.com/Shopify/shopify-php-api/pull/134) ⚠️ [Breaking] Add support for PHP 8.1 and remove 7.3 from the supported list, since it's no longer supported
- [#136](https://github.com/Shopify/shopify-php-api/pull/136) Allow full paths in REST requests

### Fixed

Expand Down
6 changes: 3 additions & 3 deletions docs/usage/rest.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ REST uses `get`, `post`, `put`, and `delete` requests to retrieve, create, updat

| Parameter | Type | Required? | Default Value | Notes |
|:----------|:----------------|:---------:|:----------------:|:-------------------------------------------------|
| path | string | Yes | | The URL path to request |
| body | string or array | No | null | Only `Post`, and `put` methods can have body |
| path | string | Yes | | The requested API endpoint path. This can be one of two formats:<ul><li>The path starting after the `/admin/api/{version}/` prefix, such as `'products'`, which executes `/admin/api/{version}/products.json`</li><li>The full path, such as `/admin/oauth/access_scopes.json`</li></ul> |
| body | string or array | No | null | Only `post`, and `put` methods can have body |
| headers | array | No | [] | Any extra headers to send along with the request |
| query | array | No | [] | Query parameters as an associative array |
| tries | int | No | null | How many times to attempt the request |
| dataType | No | No | `DATA_TYPE_JSON` | Only `Post`, and `put` methods can have body |
| dataType | No | No | `DATA_TYPE_JSON` | Only `post`, and `put` methods can have body |

In the following example we will retrieve a list of products from a shop using `Shopify\Clients\Rest` class.

Expand Down
15 changes: 10 additions & 5 deletions src/Clients/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,10 @@ protected function request(

$client = Context::$HTTP_CLIENT_FACTORY->client();

if (strpos($path, '/') !== 0) {
$path = "/$path";
}

$url = (new Uri())
->withScheme('https')
->withHost($this->domain)
->withPath($path)
->withPath($this->getRequestPath($path))
->withQuery(http_build_query($query));

$request = new Request($method, $url, $headers);
Expand Down Expand Up @@ -217,6 +213,15 @@ protected function request(
return $response;
}

protected function getRequestPath(string $path): string
{
if (strpos($path, '/') !== 0) {
$path = "/$path";
}

return $path;
}

/**
* Logs an API deprecation for the given URL to the app's logged, if one was given.
*
Expand Down
13 changes: 10 additions & 3 deletions src/Clients/Rest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected function request(
$headers[HttpHeaders::X_SHOPIFY_ACCESS_TOKEN] =
Context::$IS_PRIVATE_APP ? Context::$API_SECRET_KEY : $this->accessToken;

$response = parent::request($this->getRestPath($path), $method, $body, $headers, $query, $tries, $dataType);
$response = parent::request($path, $method, $body, $headers, $query, $tries, $dataType);

return new RestResponse(
$response->getStatusCode(),
Expand All @@ -57,10 +57,17 @@ protected function request(
);
}

private function getRestPath(string $path): string
protected function getRequestPath(string $path): string
{
$path = parent::getRequestPath($path);
$path = preg_replace("/\.json$/", "", $path) . ".json";

$apiVersion = Context::$API_VERSION;
return "admin/api/$apiVersion/$path.json";
if (strpos($path, "/admin") !== 0) {
$path = "/admin/api/$apiVersion$path";
Comment on lines +66 to +67
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of assuming $path has a / with the getRequestPath helper, can we instead:

Suggested change
if (strpos($path, "/admin") !== 0) {
$path = "/admin/api/$apiVersion$path";
if ($path[0] != '/') {
$path = "/admin/api/$apiVersion/$path";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

parent::getRequestPath is set up to always ensure the path has a / at position 0, which of course we could change here, but I wasn't too keen on using just the slash as a marker for a full path because I felt it would be too error prone.

My main concern was that folks would try things like /product.json, which I think should just be prefixed with the default.

}

return $path;
}

/**
Expand Down
23 changes: 23 additions & 0 deletions tests/Clients/RestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,29 @@ public function testCanMakeGetRequest()
$this->assertThat($response, new HttpResponseMatcher(200, [], $this->successResponse));
}

public function testAllowsFullPaths()
{
$headers = ['X-Test-Header' => 'test_value'];

$client = new Rest($this->domain, 'dummy-token');

$this->mockTransportRequests([
new MockRequest(
$this->buildMockHttpResponse(200, $this->successResponse),
"https://$this->domain/admin/custom_path.json",
'GET',
"Shopify Admin API Library for PHP v$this->version",
['X-Test-Header: test_value', 'X-Shopify-Access-Token: dummy-token'],
null,
null,
false,
),
]);

$response = $client->get('/admin/custom_path', $headers);
$this->assertThat($response, new HttpResponseMatcher(200, [], $this->successResponse));
}

public function testCanMakeGetRequestWithPathInQuery()
{
$client = new Rest($this->domain, 'dummy-token');
Expand Down