Skip to content

Commit be03fa0

Browse files
bshafferbshaffer
committed
address all review comments
1 parent 099ecea commit be03fa0

16 files changed

Lines changed: 267 additions & 74 deletions

‎Core/src/ApiHelperTrait.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Google\ApiCore\Options\CallOptions;
2222
use Google\Protobuf\Internal\Message;
2323
use Google\Protobuf\NullValue;
24+
use LogicException;
2425

2526
/**
2627
* @internal
@@ -273,6 +274,8 @@ private function splitOptionalArgs(array $input, array $extraAllowedKeys = []):
273274
* $optionTypes can be an array of string keys, a protobuf Message classname, or a
274275
* the CallOptions classname. Parameters are split and returned in the order
275276
* that the options types are provided.
277+
*
278+
* @throws LogicException
276279
*/
277280
private function validateOptions(array $options, array|Message|string ...$optionTypes): array
278281
{

‎Core/src/LongRunning/LongRunningClientConnection.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,16 @@ class LongRunningClientConnection implements LongRunningConnectionInterface
3434
use RequestProcessorTrait;
3535

3636
public function __construct(
37-
private $gapicClient,
37+
private object $gapicClient,
3838
private Serializer $serializer
3939
) {
4040
}
4141

4242
/**
4343
* @param array $args
44+
* @return array
4445
*/
45-
public function get(array $args)
46+
public function get(array $args): array
4647
{
4748
$operationResponse = $this->gapicClient->resumeOperation($args['name']);
4849

@@ -51,8 +52,9 @@ public function get(array $args)
5152

5253
/**
5354
* @param array $args
55+
* @return array
5456
*/
55-
public function cancel(array $args)
57+
public function cancel(array $args): array
5658
{
5759
$operationResponse = $this->gapicClient->resumeOperation(
5860
$args['name'],
@@ -65,8 +67,9 @@ public function cancel(array $args)
6567

6668
/**
6769
* @param array $args
70+
* @return array
6871
*/
69-
public function delete(array $args)
72+
public function delete(array $args): array
7073
{
7174
$operationResponse = $this->gapicClient->resumeOperation(
7275
$args['name'],
@@ -79,16 +82,17 @@ public function delete(array $args)
7982

8083
/**
8184
* @param array $args
85+
* @return array
8286
*/
83-
public function operations(array $args)
87+
public function operations(array $args): array
8488
{
8589
$request = ListOperationsRequest::build($args['name'], $args['filter'] ?? null);
8690
$response = $this->gapicClient->getOperationsClient()->listOperations($request);
8791

8892
return $this->handleResponse($response);
8993
}
9094

91-
private function operationResponseToArray(OperationResponse $operationResponse)
95+
private function operationResponseToArray(OperationResponse $operationResponse): array
9296
{
9397
$response = $this->handleResponse($operationResponse->getLastProtoResponse());
9498
$metaType = $response['metadata']['typeUrl'];

‎Core/src/OptionsValidator.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ class OptionsValidator
2727
{
2828
use ArrayTrait;
2929

30-
public function __construct(private ?Serializer $serializer = null)
31-
{
30+
public function __construct(
31+
private ?Serializer $serializer = null
32+
) {
3233
}
3334

3435
/**
@@ -61,6 +62,8 @@ public function validateOptions(array $options, array|Message|string ...$optionT
6162
$optionType->mergeFromJsonString(json_encode($messageOptions, JSON_FORCE_OBJECT));
6263
}
6364
$splitOptions[] = $optionType;
65+
} elseif (is_string($optionType)) {
66+
$splitOptions[] = $this->pluck($optionType, $options, false);
6467
} else {
6568
throw new LogicException(sprintf('Invalid option type: %s', $optionType));
6669
}

‎Core/src/TimeTrait.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,14 @@ private function formatTimeAsString(\DateTimeInterface $dateTime, $ns)
7777
$dateTime = clone $dateTime;
7878
}
7979
$dateTime = $dateTime->setTimeZone(new \DateTimeZone('UTC'));
80-
if (is_null($ns)) {
80+
if ($ns === null) {
8181
return $dateTime->format(Timestamp::FORMAT);
82-
} else {
83-
return sprintf(
84-
$dateTime->format(Timestamp::FORMAT_INTERPOLATE),
85-
$this->convertNanoSecondsToFraction($ns)
86-
);
8782
}
83+
84+
return sprintf(
85+
$dateTime->format(Timestamp::FORMAT_INTERPOLATE),
86+
$this->convertNanoSecondsToFraction($ns)
87+
);
8888
}
8989

9090
/**

‎Core/tests/Unit/ApiHelperTraitTest.php‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,20 @@ public function validateOptionsProvider()
366366
(new MockRequest())->setPageToken('foo1'),
367367
]
368368
],
369+
[
370+
[
371+
'baz' => 'bat',
372+
'pageToken' => 'foo1',
373+
],
374+
[
375+
'baz',
376+
new MockRequest(),
377+
],
378+
[
379+
'bat',
380+
(new MockRequest())->setPageToken('foo1'),
381+
]
382+
],
369383
];
370384
}
371385

@@ -381,24 +395,4 @@ public function testValidateOptionsThrowsException()
381395

382396
$this->implementation->validateOptions($options, ['foo']);
383397
}
384-
385-
public function testValidateOptionsWithClassnameThrowsException()
386-
{
387-
$this->expectException(LogicException::class);
388-
$this->expectExceptionMessage('Invalid option type: ' . Blob::class);
389-
390-
$options = [
391-
'foo' => 'bar',
392-
'bar' => 'baz',
393-
];
394-
395-
[$blob, $validated] = $this->implementation->validateOptions(
396-
$options,
397-
Blob::class,
398-
['foo', 'bar']
399-
);
400-
401-
$this->assertEquals([], $blob);
402-
$this->assertEquals($options, $validated);
403-
}
404398
}

‎Core/tests/Unit/Batch/BatchDaemonTraitTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
*/
2828
class BatchDaemonTraitTest extends TestCase
2929
{
30+
private $impl;
31+
3032
public function setUp(): void
3133
{
3234
$this->impl = TestHelpers::impl(BatchDaemonTrait::class);

‎Spanner/src/Backup.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ public function create(
133133
$options['backup']['versionTime'] = $this->formatTimeAsArray($versionTime);
134134
}
135135

136+
/**
137+
* @var CreateBackupRequest $createBackup
138+
* @var array $callOptions
139+
*/
136140
[$createBackup, $callOptions] = $this->validateOptions(
137141
$options,
138142
new CreateBackupRequest(),
@@ -180,6 +184,11 @@ public function createCopy(
180184
'sourceBackup' => $this->fullyQualifiedBackupName($this->name),
181185
'expireTime' => $this->formatTimeAsArray($expireTime)
182186
];
187+
188+
/**
189+
* @var CopyBackupRequest $copyBackup
190+
* @var array $callOptions
191+
*/
183192
[$copyBackup, $callOptions] = $this->validateOptions(
184193
$options,
185194
new CopyBackupRequest(),
@@ -208,6 +217,11 @@ public function delete(array $options = []): void
208217
$options += [
209218
'name' => $this->name
210219
];
220+
221+
/**
222+
* @var DeleteBackupRequest $deleteBackup
223+
* @var array $callOptions
224+
*/
211225
[$deleteBackup, $callOptions] = $this->validateOptions(
212226
$options,
213227
new DeleteBackupRequest(),
@@ -296,6 +310,10 @@ public function reload(array $options = []): array
296310
'name' => $this->name
297311
];
298312

313+
/**
314+
* @var GetBackupRequest $getBackup
315+
* @var array $callOptions
316+
*/
299317
[$getBackup, $callOptions] = $this->validateOptions(
300318
$options,
301319
new GetBackupRequest(),
@@ -364,6 +382,10 @@ public function updateExpireTime(DateTimeInterface $newTimestamp, array $options
364382
]
365383
];
366384

385+
/**
386+
* @var UpdateBackupRequest $updateBackup
387+
* @var array $callOptions
388+
*/
367389
[$updateBackup, $callOptions] = $this->validateOptions(
368390
$options,
369391
new UpdateBackupRequest(),
@@ -427,6 +449,10 @@ public function resumeOperation(string $operationName, array $options = []): Lon
427449
*/
428450
public function longRunningOperations(array $options = []): ItemIterator
429451
{
452+
/**
453+
* @var ListOperationsRequest $listOperations
454+
* @var array $callOptions
455+
*/
430456
[$listOperations, $callOptions] = $this->validateOptions(
431457
$options,
432458
new ListOperationsRequest(),

0 commit comments

Comments
 (0)