Skip to content

Commit c918202

Browse files
OcramiusOcramius
committed
Use issueId instead of issue when querying Tempo API worklogs
Note: to do this, we have to work around limitations in how PHP parses query strings containing array values. Ref: thephpleague/openapi-psr7-validator#181 Ref: thephpleague/openapi-psr7-validator#182 Ref: https://swagger.io/docs/specification/v3_0/serialization/
1 parent 62a7a7f commit c918202

4 files changed

Lines changed: 149 additions & 25 deletions

File tree

‎src/Tempo/Infrastructure/GetWorkLogEntriesViaTempoV4Api.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ public function __invoke(TimeEntry $timeEntry): array
4343
$query = implode(
4444
'&',
4545
array_unique(array_map(
46-
// @TODO this will fail?
47-
static fn (LogEntry $entry): string => 'issue=' . $entry->issue->key->key,
46+
static fn (LogEntry $entry): string => 'issueId=' . $entry->issue->id->id,
4847
LogEntry::splitTimeEntry($this->getId, $timeEntry, $this->fallbackJiraIssue),
4948
)),
5049
)

‎test/OpenAPI/WrapResponseCallbackInValidationCallback.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Symfony\Component\Yaml\Yaml;
1212

1313
use function Psl\File\read;
14+
use function str_replace;
1415

1516
final class WrapResponseCallbackInValidationCallback
1617
{
@@ -29,7 +30,7 @@ public static function wrap(string $openApiSpecFile, callable $callback): callab
2930
$requestValidator = $validatorBuilder->getRequestValidator();
3031
$responseValidator = $validatorBuilder->getResponseValidator();
3132

32-
$operation = $requestValidator->validate($request);
33+
$operation = $requestValidator->validate(self::fixUpKnownQueryArrayParameters($request));
3334

3435
$response = $callback($request);
3536

@@ -38,4 +39,20 @@ public static function wrap(string $openApiSpecFile, callable $callback): callab
3839
return $response;
3940
};
4041
}
42+
43+
/**
44+
* Converts known array query parameters to PHP-array-alike syntax
45+
*
46+
* @see https://github.com/thephpleague/openapi-psr7-validator/issues/181
47+
* @see https://github.com/thephpleague/openapi-psr7-validator/pull/182
48+
* @see https://swagger.io/docs/specification/v3_0/serialization/
49+
*/
50+
private static function fixUpKnownQueryArrayParameters(RequestInterface $request): RequestInterface
51+
{
52+
$uri = $request->getUri();
53+
54+
return $request->withUri(
55+
$uri->withQuery(str_replace('issueId=', 'issueId[]=', $uri->getQuery())),
56+
);
57+
}
4158
}

‎test/Tempo/Infrastructure/AddWorkLogEntriesViaTempoV4ApiTest.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function testWillAddGivenWorkEntry(): void
5151
{
5252
$response = $this->responseFactory->createResponse()
5353
->withHeader('Content-Type', 'application/json');
54-
54+
5555
$response->getBody()->write(<<<'JSON'
5656
{
5757
"attributes": {
@@ -75,8 +75,7 @@ public function testWillAddGivenWorkEntry(): void
7575
"timeSpentSeconds": 3600,
7676
"updatedAt": "2017-02-06T16:41:41Z"
7777
}
78-
JSON
79-
);
78+
JSON,);
8079

8180
$this->httpClient->expects(self::once())
8281
->method('sendRequest')
@@ -125,7 +124,7 @@ public function testWillAddGivenWorkEntry(): void
125124
}))
126125
->willReturnCallback(WrapResponseCallbackInValidationCallback::wrap(
127126
__DIR__ . '/tempo-core.yaml',
128-
static fn () : ResponseInterface => $response,
127+
static fn (): ResponseInterface => $response,
129128
));
130129

131130
($this->addEntry)(new LogEntry(

‎test/Tempo/Infrastructure/GetWorkLogEntriesViaTempoV4ApiTest.php‎

Lines changed: 127 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Psr\Http\Client\ClientInterface;
1616
use Psr\Http\Message\RequestInterface;
1717
use Psr\Http\Message\ResponseFactoryInterface;
18+
use Psr\Http\Message\ResponseInterface;
1819
use TimeSync\Harvest\Domain\SpentDate;
1920
use TimeSync\Harvest\Domain\TimeEntry;
2021
use TimeSync\Jira\Domain\GetIssueIdForKey;
@@ -23,6 +24,7 @@
2324
use TimeSync\Tempo\Domain\JiraIssueId;
2425
use TimeSync\Tempo\Domain\LogEntry;
2526
use TimeSync\Tempo\Infrastructure\GetWorkLogEntriesViaTempoV4Api;
27+
use TimeSyncTest\OpenAPI\WrapResponseCallbackInValidationCallback;
2628

2729
use function base_convert;
2830
use function sha1;
@@ -60,12 +62,21 @@ protected function setUp(): void
6062

6163
public function testWillFetchLogEntriesForGivenIssues(): void
6264
{
63-
$response = $this->responseFactory->createResponse();
65+
$response = $this->responseFactory->createResponse()
66+
->withAddedHeader('Content-Type', 'application/json');
6467

6568
$response->getBody()
6669
->write(
6770
<<<'JSON'
6871
{
72+
"self": "https://example.com/page",
73+
"metadata": {
74+
"count": 1,
75+
"limit": 50,
76+
"offset": 0,
77+
"next": "https://example.com/page/next",
78+
"previous": "https://example.com/page/previous"
79+
},
6980
"results": [
7081
{
7182
"tempoWorklogId": 123,
@@ -77,7 +88,17 @@ public function testWillFetchLogEntriesForGivenIssues(): void
7788
"billableSeconds": 60,
7889
"startDate": "2022-08-09",
7990
"startTime": "00:00:00",
80-
"description": "Working on issue foo harvest:11111"
91+
"description": "Working on issue foo harvest:11111",
92+
"createdAt": "2017-02-06T16:41:41Z",
93+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
94+
"self": "https://example.com/this",
95+
"updatedAt": "2017-02-06T16:41:41Z",
96+
"attributes": {
97+
"self": "https://example.com/this/attributes"
98+
},
99+
"author": {
100+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
101+
}
81102
},
82103
{
83104
"tempoWorklogId": 456,
@@ -89,7 +110,17 @@ public function testWillFetchLogEntriesForGivenIssues(): void
89110
"billableSeconds": 63,
90111
"startDate": "2022-08-09",
91112
"startTime": "00:00:00",
92-
"description": "Working on issue bar harvest:11111"
113+
"description": "Working on issue bar harvest:11111",
114+
"createdAt": "2017-02-06T16:41:41Z",
115+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
116+
"self": "https://example.com/this",
117+
"updatedAt": "2017-02-06T16:41:41Z",
118+
"attributes": {
119+
"self": "https://example.com/this/attributes"
120+
},
121+
"author": {
122+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
123+
}
93124
},
94125
{
95126
"tempoWorklogId": 456,
@@ -101,7 +132,17 @@ public function testWillFetchLogEntriesForGivenIssues(): void
101132
"billableSeconds": 63,
102133
"startDate": "2022-08-09",
103134
"startTime": "00:00:00",
104-
"description": "This log will be ignored, because it doesn't match the input time entry harvest:12345"
135+
"description": "This log will be ignored, because it doesn't match the input time entry harvest:12345",
136+
"createdAt": "2017-02-06T16:41:41Z",
137+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
138+
"self": "https://example.com/this",
139+
"updatedAt": "2017-02-06T16:41:41Z",
140+
"attributes": {
141+
"self": "https://example.com/this/attributes"
142+
},
143+
"author": {
144+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
145+
}
105146
}
106147
]
107148
}
@@ -113,7 +154,7 @@ public function testWillFetchLogEntriesForGivenIssues(): void
113154
->with(self::callback(static function (RequestInterface $request): bool {
114155
self::assertSame('GET', $request->getMethod());
115156
self::assertSame(
116-
'https://api.tempo.io/4/worklogs?issue=AB1-2&issue=AB1-3&issue=FALLBACK-123&from=2022-08-09&to=2022-08-09&limit=1000',
157+
'https://api.tempo.io/4/worklogs?issueId=285638&issueId=413713&issueId=265303&from=2022-08-09&to=2022-08-09&limit=1000',
117158
$request->getUri()->__toString(),
118159
);
119160
self::assertSame(
@@ -126,7 +167,10 @@ public function testWillFetchLogEntriesForGivenIssues(): void
126167

127168
return true;
128169
}))
129-
->willReturn($response);
170+
->willReturnCallback(WrapResponseCallbackInValidationCallback::wrap(
171+
__DIR__ . '/tempo-core.yaml',
172+
static fn (): ResponseInterface => $response,
173+
));
130174

131175
self::assertEquals(
132176
[
@@ -139,12 +183,21 @@ public function testWillFetchLogEntriesForGivenIssues(): void
139183

140184
public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotContainingTheIssueId(): void
141185
{
142-
$response = $this->responseFactory->createResponse();
186+
$response = $this->responseFactory->createResponse()
187+
->withAddedHeader('Content-Type', 'application/json');
143188

144189
$response->getBody()
145190
->write(
146191
<<<'JSON'
147192
{
193+
"self": "https://example.com/page",
194+
"metadata": {
195+
"count": 1,
196+
"limit": 50,
197+
"offset": 0,
198+
"next": "https://example.com/page/next",
199+
"previous": "https://example.com/page/previous"
200+
},
148201
"results": [
149202
{
150203
"tempoWorklogId": 123,
@@ -156,7 +209,17 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
156209
"billableSeconds": 60,
157210
"startDate": "2022-08-09",
158211
"startTime": "00:00:00",
159-
"description": "Working on issue AB-12 harvest:11111"
212+
"description": "Working on issue AB-12 harvest:11111",
213+
"createdAt": "2017-02-06T16:41:41Z",
214+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
215+
"self": "https://example.com/this",
216+
"updatedAt": "2017-02-06T16:41:41Z",
217+
"attributes": {
218+
"self": "https://example.com/this/attributes"
219+
},
220+
"author": {
221+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
222+
}
160223
},
161224
{
162225
"tempoWorklogId": 456,
@@ -168,7 +231,17 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
168231
"billableSeconds": 63,
169232
"startDate": "2022-08-09",
170233
"startTime": "00:00:00",
171-
"description": "Working on issue AB-13 harvest:11111"
234+
"description": "Working on issue AB-13 harvest:11111",
235+
"createdAt": "2017-02-06T16:41:41Z",
236+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
237+
"self": "https://example.com/this",
238+
"updatedAt": "2017-02-06T16:41:41Z",
239+
"attributes": {
240+
"self": "https://example.com/this/attributes"
241+
},
242+
"author": {
243+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
244+
}
172245
},
173246
{
174247
"tempoWorklogId": 789,
@@ -180,7 +253,17 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
180253
"billableSeconds": 63,
181254
"startDate": "2022-08-09",
182255
"startTime": "00:00:00",
183-
"description": "This log will be ignored, because it doesn't match the input time entry harvest:12345"
256+
"description": "This log will be ignored, because it doesn't match the input time entry harvest:12345",
257+
"createdAt": "2017-02-06T16:41:41Z",
258+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
259+
"self": "https://example.com/this",
260+
"updatedAt": "2017-02-06T16:41:41Z",
261+
"attributes": {
262+
"self": "https://example.com/this/attributes"
263+
},
264+
"author": {
265+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
266+
}
184267
},
185268
{
186269
"tempoWorklogId": 101112,
@@ -192,7 +275,17 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
192275
"billableSeconds": 63,
193276
"startDate": "2022-08-09",
194277
"startTime": "00:00:00",
195-
"description": "This log will be ignored, because it doesn't have an associated harvest id"
278+
"description": "This log will be ignored, because it doesn't have an associated harvest id",
279+
"createdAt": "2017-02-06T16:41:41Z",
280+
"startDateTimeUtc": "2017-02-05T16:06:00Z",
281+
"self": "https://example.com/this",
282+
"updatedAt": "2017-02-06T16:41:41Z",
283+
"attributes": {
284+
"self": "https://example.com/this/attributes"
285+
},
286+
"author": {
287+
"accountId": "123456:01234567-89ab-cdef-0123-456789abcdef"
288+
}
196289
}
197290
]
198291
}
@@ -201,7 +294,10 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
201294

202295
$this->httpClient->expects(self::once())
203296
->method('sendRequest')
204-
->willReturn($response);
297+
->willReturnCallback(WrapResponseCallbackInValidationCallback::wrap(
298+
__DIR__ . '/tempo-core.yaml',
299+
static fn (): ResponseInterface => $response,
300+
));
205301

206302
self::assertEquals(
207303
[
@@ -214,7 +310,8 @@ public function testWillFetchLogEntriesForGivenIssuesEvenIfIssueSelfUrlIsNotCont
214310

215311
public function testWillRejectNon200HttpResponses(): void
216312
{
217-
$response = $this->responseFactory->createResponse(201);
313+
$response = $this->responseFactory->createResponse(201)
314+
->withAddedHeader('Content-Type', 'application/json');
218315

219316
$response->getBody()
220317
->write('HEHE!');
@@ -224,13 +321,13 @@ public function testWillRejectNon200HttpResponses(): void
224321
->willReturn($response);
225322

226323
$this->expectException(InvariantViolationException::class);
227-
$this->expectExceptionMessage("Request https://api.tempo.io/4/worklogs?issue=AB1-2&issue=AB1-3&issue=FALLBACK-123&from=2022-08-09&to=2022-08-09&limit=1000 not successful: 201\nHEHE!");
324+
$this->expectExceptionMessage("Request https://api.tempo.io/4/worklogs?issueId=285638&issueId=413713&issueId=265303&from=2022-08-09&to=2022-08-09&limit=1000 not successful: 201\nHEHE!");
228325

229326
($this->getEntries)(new TimeEntry('123', 10.0, 'AB1-2, AB1-3, hello', new SpentDate('2022-08-09')));
230327
}
231328

232329
/**
233-
* Tempo started rejecting `/core/3/worklogs?issue=id&issue=id` queries when
330+
* Tempo started rejecting `/core/3/worklogs?issueId=id&issueId=id` queries when
234331
* the `id` is the same: that kind of query now leads to a 404 error.
235332
*
236333
* In order to avoid this problem, we de-duplicate any queried issue IDs before
@@ -239,12 +336,21 @@ public function testWillRejectNon200HttpResponses(): void
239336
#[Group('#39')]
240337
public function testWillDeDuplicateJiraIssueIdsBeforeQuerying(): void
241338
{
242-
$response = $this->responseFactory->createResponse();
339+
$response = $this->responseFactory->createResponse()
340+
->withAddedHeader('Content-Type', 'application/json');
243341

244342
$response->getBody()
245343
->write(
246344
<<<'JSON'
247345
{
346+
"self": "https://example.com/page",
347+
"metadata": {
348+
"count": 1,
349+
"limit": 50,
350+
"offset": 0,
351+
"next": "https://example.com/page/next",
352+
"previous": "https://example.com/page/previous"
353+
},
248354
"results": []
249355
}
250356
JSON,
@@ -255,7 +361,7 @@ public function testWillDeDuplicateJiraIssueIdsBeforeQuerying(): void
255361
->with(self::callback(static function (RequestInterface $request): bool {
256362
self::assertSame('GET', $request->getMethod());
257363
self::assertSame(
258-
'https://api.tempo.io/4/worklogs?issue=AB1-2&issue=AB1-3&issue=FALLBACK-123&from=2022-08-09&to=2022-08-09&limit=1000',
364+
'https://api.tempo.io/4/worklogs?issueId=285638&issueId=413713&issueId=265303&from=2022-08-09&to=2022-08-09&limit=1000',
259365
$request->getUri()->__toString(),
260366
);
261367
self::assertSame(
@@ -268,7 +374,10 @@ public function testWillDeDuplicateJiraIssueIdsBeforeQuerying(): void
268374

269375
return true;
270376
}))
271-
->willReturn($response);
377+
->willReturnCallback(WrapResponseCallbackInValidationCallback::wrap(
378+
__DIR__ . '/tempo-core.yaml',
379+
static fn (): ResponseInterface => $response,
380+
));
272381

273382
self::assertEmpty(
274383
($this->getEntries)(new TimeEntry(

0 commit comments

Comments
 (0)