Skip to content

Commit 3bdac86

Browse files
tobyzernertobyzerner
committed
Improve error sources
- Make BadRequestException easier to configure with a source - Specify source in some instances where it was missing - Prefix source paths in Atomic Operations, relationships, etc - Fix $sourceType must not be accessed before initialization
1 parent cce7809 commit 3bdac86

File tree

12 files changed

+115
-48
lines changed

12 files changed

+115
-48
lines changed

‎src/Endpoint/Concerns/IncludesData.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ private function validateInclude(
6969
continue 2;
7070
}
7171

72-
throw (new BadRequestException("Invalid include [{$path}{$name}]"))->setSourceParameter(
73-
'include',
74-
);
72+
throw new BadRequestException("Invalid include [{$path}{$name}]", [
73+
'parameter' => 'include',
74+
]);
7575
}
7676
}
7777
}

‎src/Endpoint/Concerns/SavesData.php

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,22 @@ private function parseData(Context $context): array
2727
$body = (array) $context->body();
2828

2929
if (!isset($body['data']) || !is_array($body['data'])) {
30-
throw new BadRequestException('data must be an object');
30+
throw new BadRequestException('data must be an object', ['pointer' => '/data']);
3131
}
3232

3333
if (!isset($body['data']['type'])) {
34-
throw new BadRequestException('data.type must be present');
34+
throw new BadRequestException('data.type must be present', ['pointer' => '/data/type']);
3535
}
3636

3737
if (isset($context->model)) {
3838
if (!isset($body['data']['id'])) {
39-
throw new BadRequestException('data.id must be present');
39+
throw new BadRequestException('data.id must be present', ['pointer' => '/data/id']);
4040
}
4141

4242
if ($body['data']['id'] !== $context->resource->getId($context->model, $context)) {
43-
throw new ConflictException('data.id does not match the resource ID');
43+
throw new ConflictException('data.id does not match the resource ID', [
44+
'pointer' => '/data/id',
45+
]);
4446
}
4547
} elseif (isset($body['data']['id'])) {
4648
throw new ForbiddenException('Client-generated IDs are not supported');
@@ -51,11 +53,15 @@ private function parseData(Context $context): array
5153
}
5254

5355
if (isset($body['data']['attributes']) && !is_array($body['data']['attributes'])) {
54-
throw new BadRequestException('data.attributes must be an object');
56+
throw new BadRequestException('data.attributes must be an object', [
57+
'pointer' => '/data/attributes',
58+
]);
5559
}
5660

5761
if (isset($body['data']['relationships']) && !is_array($body['data']['relationships'])) {
58-
throw new BadRequestException('data.relationships must be an object');
62+
throw new BadRequestException('data.relationships must be an object', [
63+
'pointer' => '/data/relationships',
64+
]);
5965
}
6066

6167
return array_merge(['attributes' => [], 'relationships' => []], $body['data']);
@@ -82,7 +88,9 @@ private function assertFieldsExist(Context $context, array $data): void
8288
foreach (['attributes', 'relationships'] as $location) {
8389
foreach ($data[$location] as $name => $value) {
8490
if (!isset($fields[$name]) || $location !== location($fields[$name])) {
85-
throw new BadRequestException("Unknown field [$name]");
91+
throw new BadRequestException("Unknown field [$name]", [
92+
'pointer' => "/data/$location/$name",
93+
]);
8694
}
8795
}
8896
}
@@ -118,7 +126,13 @@ private function deserializeValues(Context $context, array &$data): void
118126

119127
$value = get_value($data, $field);
120128

121-
set_value($data, $field, $field->deserializeValue($value, $context));
129+
try {
130+
set_value($data, $field, $field->deserializeValue($value, $context));
131+
} catch (BadRequestException $e) {
132+
throw $e->prependSource([
133+
'pointer' => '/data/' . location($field) . '/' . $field->name,
134+
]);
135+
}
122136
}
123137
}
124138

‎src/Endpoint/Index.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ private function applySorts($query, Context $context): void
135135
}
136136
}
137137

138-
throw (new BadRequestException("Invalid sort: $name"))->setSourceParameter('sort');
138+
throw new BadRequestException("Invalid sort: $name", ['parameter' => 'sort']);
139139
}
140140
}
141141

@@ -146,15 +146,13 @@ private function applyFilters($query, Context $context): void
146146
}
147147

148148
if (!is_array($filters)) {
149-
throw (new BadRequestException('filter must be an array'))->setSourceParameter(
150-
'filter',
151-
);
149+
throw new BadRequestException('filter must be an array', ['parameter' => 'filter']);
152150
}
153151

154152
try {
155153
apply_filters($query, $filters, $context->resource, $context);
156154
} catch (BadRequestException $e) {
157-
throw $e->setSourceParameter("filter[$e->source]");
155+
throw $e->prependSource(['parameter' => 'filter']);
158156
}
159157
}
160158
}

‎src/Exception/BadRequestException.php

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,32 @@
33
namespace Tobyz\JsonApiServer\Exception;
44

55
use DomainException;
6+
use Throwable;
67
use Tobyz\JsonApiServer\ErrorProviderInterface;
78

89
class BadRequestException extends DomainException implements ErrorProviderInterface
910
{
10-
public string $sourceType;
11-
public string $source;
11+
public function __construct(
12+
string $message = '',
13+
public ?array $source = null,
14+
int $code = 0,
15+
?Throwable $previous = null,
16+
) {
17+
parent::__construct($message, $code, $previous);
18+
}
1219

13-
public function setSourceParameter(string $parameter): static
20+
public function setSource(?array $source): static
1421
{
15-
$this->sourceType = 'parameter';
16-
$this->source = $parameter;
22+
$this->source = $source;
1723

1824
return $this;
1925
}
2026

21-
public function setSourcePointer(string $pointer): static
27+
public function prependSource(array $source): static
2228
{
23-
$this->sourceType = 'pointer';
24-
$this->source = $pointer;
29+
foreach ($source as $k => $v) {
30+
$this->source = [$k => $v . ($this->source[$k] ?? '')];
31+
}
2532

2633
return $this;
2734
}
@@ -34,10 +41,8 @@ public function getJsonApiErrors(): array
3441
$members['detail'] = $this->message;
3542
}
3643

37-
if ($this->sourceType === 'parameter') {
38-
$members['source']['parameter'] = $this->source;
39-
} elseif ($this->sourceType === 'pointer') {
40-
$members['source']['pointer'] = $this->source;
44+
if ($this->source) {
45+
$members['source'] = $this->source;
4146
}
4247

4348
return [

‎src/Extension/Atomic.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public function handle(Context $context): ?Response
3939
if (!is_array($operations)) {
4040
throw new BadRequestException(
4141
'atomic:operations must be an array of operation objects',
42+
['pointer' => '/atomic:operations'],
4243
);
4344
}
4445

@@ -58,7 +59,12 @@ public function handle(Context $context): ?Response
5859
default => throw new BadRequestException('Invalid operation'),
5960
};
6061
} catch (BadRequestException $e) {
61-
throw $e->setSourcePointer("/atomic:operations/$i");
62+
if (!$e->source || isset($e->source['pointer'])) {
63+
$e->setSource([
64+
'pointer' => "/atomic:operations/$i" . ($e->source['pointer'] ?? ''),
65+
]);
66+
}
67+
throw $e;
6268
}
6369

6470
$results[] = json_decode($response->getBody(), true);
@@ -70,7 +76,9 @@ public function handle(Context $context): ?Response
7076
private function add(Context $context, array $operation, array &$lids): Response
7177
{
7278
if (isset($operation['ref'])) {
73-
throw new BadRequestException('ref is not supported for add operations');
79+
throw new BadRequestException('ref is not supported for add operations', [
80+
'source' => '/ref',
81+
]);
7482
}
7583

7684
$request = $context->request

‎src/JsonApi.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,9 @@ private function validateQueryParameters(Request $request): void
140140
!preg_match('/[^a-z]/', $key) &&
141141
!in_array($key, ['include', 'fields', 'filter', 'page', 'sort'])
142142
) {
143-
throw (new BadRequestException(
144-
"Invalid query parameter: $key",
145-
))->setSourceParameter($key);
143+
throw new BadRequestException("Invalid query parameter: $key", [
144+
'parameter' => $key,
145+
]);
146146
}
147147
}
148148
}

‎src/Pagination/OffsetPagination.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ private function getOffset(Context $context): int
103103
{
104104
if ($offset = $context->queryParam('page')['offset'] ?? null) {
105105
if (preg_match('/\D+/', $offset) || $offset < 0) {
106-
throw (new BadRequestException(
107-
'page[offset] must be a non-negative integer',
108-
))->setSourceParameter('page[offset]');
106+
throw new BadRequestException('page[offset] must be a non-negative integer', [
107+
'parameter' => 'page[offset]',
108+
]);
109109
}
110110

111111
return $offset;

‎src/Schema/Field/Relationship.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ protected function findResourceForIdentifier(array $identifier, Context $context
6767
}
6868

6969
if (!in_array($identifier['type'], $this->types)) {
70-
throw new BadRequestException("type [{$identifier['type']}] not allowed");
70+
throw new BadRequestException("type [{$identifier['type']}] not allowed", [
71+
'pointer' => '/type',
72+
]);
7173
}
7274

7375
$resource = $context->api->getResource($identifier['type']);

‎src/Schema/Field/ToMany.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,21 @@ public function deserializeValue(mixed $value, Context $context): mixed
4545
}
4646

4747
if (!array_is_list($value['data'])) {
48-
throw new BadRequestException('relationship data must be a list of identifier objects');
48+
throw new BadRequestException(
49+
'relationship data must be a list of identifier objects',
50+
['pointer' => '/data'],
51+
);
4952
}
5053

51-
$models = array_map(
52-
fn($identifier) => $this->findResourceForIdentifier($identifier, $context),
53-
$value['data'],
54-
);
54+
$models = [];
55+
56+
foreach ($value['data'] as $i => $identifier) {
57+
try {
58+
$models[] = $this->findResourceForIdentifier($identifier, $context);
59+
} catch (BadRequestException $e) {
60+
throw $e->prependSource(['pointer' => "/data/$i"]);
61+
}
62+
}
5563

5664
if ($this->deserializer) {
5765
return ($this->deserializer)($models, $context);

‎src/Schema/Field/ToOne.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ public function deserializeValue(mixed $value, Context $context): mixed
5656
return null;
5757
}
5858

59-
return $this->findResourceForIdentifier($value['data'], $context);
59+
try {
60+
return $this->findResourceForIdentifier($value['data'], $context);
61+
} catch (BadRequestException $e) {
62+
throw $e->prependSource(['pointer' => '/data']);
63+
}
6064
}
6165
}

‎src/functions.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,6 @@ function apply_filters(
8787
}
8888
}
8989

90-
throw (new BadRequestException("Invalid filter: $name"))->setSourceParameter($name);
90+
throw new BadRequestException("Invalid filter: $name", ['parameter' => "[$name]"]);
9191
}
9292
}

‎tests/specification/AtomicOperationsTest.php

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Tobyz\JsonApiServer\Endpoint\Create;
66
use Tobyz\JsonApiServer\Endpoint\Delete;
77
use Tobyz\JsonApiServer\Endpoint\Update;
8+
use Tobyz\JsonApiServer\Exception\BadRequestException;
89
use Tobyz\JsonApiServer\Extension\Atomic;
910
use Tobyz\JsonApiServer\JsonApi;
1011
use Tobyz\JsonApiServer\Schema\Field\Str;
@@ -16,6 +17,8 @@
1617
*/
1718
class AtomicOperationsTest extends AbstractTestCase
1819
{
20+
private const MEDIA_TYPE = JsonApi::MEDIA_TYPE . '; ext=' . Atomic::URI;
21+
1922
private JsonApi $api;
2023

2124
public function setUp(): void
@@ -36,12 +39,10 @@ public function setUp(): void
3639

3740
public function test_atomic_operations()
3841
{
39-
$mediaType = JsonApi::MEDIA_TYPE . '; ext=' . Atomic::URI;
40-
4142
$response = $this->api->handle(
4243
$this->buildRequest('POST', '/operations')
43-
->withHeader('Accept', $mediaType)
44-
->withHeader('Content-Type', $mediaType)
44+
->withHeader('Accept', static::MEDIA_TYPE)
45+
->withHeader('Content-Type', static::MEDIA_TYPE)
4546
->withParsedBody([
4647
'atomic:operations' => [
4748
[
@@ -98,4 +99,31 @@ public function test_atomic_operations()
9899
$response->getBody(),
99100
);
100101
}
102+
103+
public function test_atomic_operations_error_prefix()
104+
{
105+
try {
106+
$this->api->handle(
107+
$this->buildRequest('POST', '/operations')
108+
->withHeader('Accept', static::MEDIA_TYPE)
109+
->withHeader('Content-Type', static::MEDIA_TYPE)
110+
->withParsedBody([
111+
'atomic:operations' => [
112+
[
113+
'op' => 'update',
114+
'ref' => ['type' => 'users', 'id' => '1'],
115+
'data' => [],
116+
],
117+
],
118+
]),
119+
);
120+
121+
$this->fail('BadRequestException was not thrown');
122+
} catch (BadRequestException $e) {
123+
$this->assertStringStartsWith(
124+
'/atomic:operations/0/data',
125+
$e->getJsonApiErrors()[0]['source']['pointer'],
126+
);
127+
}
128+
}
101129
}

0 commit comments

Comments
 (0)