Skip to content

Commit 6700d0c

Browse files
pepamartinecpepamartinec
authored
Merge pull request #430 from keboola/pepa_PST-1919_recordBranchId
PST-1919 Always store branchId when creating job
2 parents 3a53cf7 + 13234a3 commit 6700d0c

File tree

5 files changed

+254
-328
lines changed

5 files changed

+254
-328
lines changed

‎src/JobFactory/JobRuntimeResolver.php‎

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use Keboola\PermissionChecker\BranchType;
1313
use Keboola\StorageApi\ClientException as StorageClientException;
1414
use Keboola\StorageApi\Components;
15-
use Keboola\StorageApi\DevBranches;
15+
use Keboola\StorageApiBranch\ClientWrapper;
1616
use Keboola\StorageApiBranch\Factory\ClientOptions;
1717
use Keboola\StorageApiBranch\Factory\StorageClientPlainFactory;
1818
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
@@ -28,14 +28,15 @@ class JobRuntimeResolver
2828

2929
private const PAY_AS_YOU_GO_FEATURE = 'pay-as-you-go';
3030

31-
private StorageClientPlainFactory $storageClientFactory;
31+
private ClientWrapper $clientWrapper;
32+
private Components $componentsApiClient;
3233
private ?array $configuration;
3334
private array $componentData;
3435
private array $jobData;
3536

36-
public function __construct(StorageClientPlainFactory $storageClientFactory)
37-
{
38-
$this->storageClientFactory = $storageClientFactory;
37+
public function __construct(
38+
private readonly StorageClientPlainFactory $storageClientFactory,
39+
) {
3940
}
4041

4142
public function resolveJobData(array $jobData, array $tokenInfo): array
@@ -44,13 +45,19 @@ public function resolveJobData(array $jobData, array $tokenInfo): array
4445
$this->jobData = $jobData;
4546

4647
try {
47-
$this->componentData = $this->getComponentsApiClient(null)
48-
->getComponent($jobData['componentId']);
48+
$this->clientWrapper = $this->storageClientFactory->createClientWrapper(new ClientOptions(
49+
token: $jobData['#tokenString'],
50+
branchId: ((string) $jobData['branchId']) ?: null,
51+
));
52+
53+
$this->componentsApiClient = new Components($this->clientWrapper->getBranchClient());
54+
$this->componentData = $this->componentsApiClient->getComponent($jobData['componentId']);
55+
4956
$jobData['tag'] = $this->resolveTag($jobData);
5057
$variableValues = $this->resolveVariables();
5158
$jobData['parallelism'] = $this->resolveParallelism($jobData);
5259
$jobData['executor'] = $this->resolveExecutor($jobData)->value;
53-
$jobData['branchType'] = $this->resolveBranchType($jobData)->value;
60+
$jobData = $this->resolveBranchType($jobData);
5461

5562
// set type after resolving parallelism
5663
$jobData['type'] = $this->resolveJobType($jobData)->value;
@@ -230,10 +237,7 @@ private function getConfiguration(): array
230237
$this->jobData['configId'] !== null &&
231238
$this->jobData['configId'] !== ''
232239
) {
233-
$componentsApi = $this->getComponentsApiClient(
234-
!empty($this->jobData['branchId']) ? (string) $this->jobData['branchId'] : null,
235-
);
236-
$this->configuration = $componentsApi->getConfiguration(
240+
$this->configuration = $this->componentsApiClient->getConfiguration(
237241
$this->jobData['componentId'],
238242
$this->jobData['configId'],
239243
);
@@ -257,17 +261,6 @@ private function getConfiguration(): array
257261
return $this->configuration;
258262
}
259263

260-
private function getComponentsApiClient(?string $branchId): Components
261-
{
262-
return new Components(
263-
$this->storageClientFactory->createClientWrapper(new ClientOptions(
264-
null,
265-
$this->jobData['#tokenString'],
266-
$branchId,
267-
))->getBranchClient(),
268-
);
269-
}
270-
271264
private function resolveIsForceRunMode(): bool
272265
{
273266
return isset($this->jobData['mode']) && $this->jobData['mode'] === JobInterface::MODE_FORCE_RUN;
@@ -304,21 +297,13 @@ private function resolveExecutor(array $jobData): Executor
304297
return Executor::from($value);
305298
}
306299

307-
private function getBranchesApiClient(): DevBranches
300+
public function resolveBranchType(array $jobData): array
308301
{
309-
return new DevBranches(
310-
$this->storageClientFactory->createClientWrapper(new ClientOptions(
311-
token: $this->jobData['#tokenString'],
312-
))->getBasicClient(),
313-
);
314-
}
302+
$branchType = $this->clientWrapper->isDefaultBranch() ? BranchType::DEFAULT : BranchType::DEV;
315303

316-
public function resolveBranchType(array $jobData): BranchType
317-
{
318-
if ($jobData['branchId'] === 'default' || $jobData['branchId'] === null) {
319-
return BranchType::DEFAULT;
320-
}
321-
$branch = $this->getBranchesApiClient()->getBranch((int) $jobData['branchId']);
322-
return $branch['isDefault'] ? BranchType::DEFAULT : BranchType::DEV;
304+
$jobData['branchType'] = $branchType->value;
305+
$jobData['branchId'] = $this->clientWrapper->getBranchId();
306+
307+
return $jobData;
323308
}
324309
}

‎tests/ClientFunctionalTest.php‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Keboola\StorageApi\Components;
1919
use Keboola\StorageApi\DevBranches;
2020
use Keboola\StorageApi\Options\Components\Configuration;
21+
use Keboola\StorageApiBranch\ClientWrapper;
22+
use Keboola\StorageApiBranch\Factory\ClientOptions;
2123

2224
class ClientFunctionalTest extends BaseClientFunctionalTest
2325
{
@@ -29,16 +31,19 @@ class ClientFunctionalTest extends BaseClientFunctionalTest
2931
private static string $componentId1Tag;
3032

3133
private static StorageClient $client;
34+
private static string $defaultBranchId;
3235

3336
public static function setUpBeforeClass(): void
3437
{
3538
parent::setUpBeforeClass();
36-
self::$client = new StorageClient(
37-
[
38-
'token' => (string) getenv('TEST_STORAGE_API_TOKEN'),
39-
'url' => (string) getenv('TEST_STORAGE_API_URL'),
40-
],
41-
);
39+
40+
$clientWrapper = new ClientWrapper(new ClientOptions(
41+
url: self::getRequiredEnv('TEST_STORAGE_API_URL'),
42+
token: self::getRequiredEnv('TEST_STORAGE_API_TOKEN'),
43+
));
44+
self::$defaultBranchId = $clientWrapper->getDefaultBranch()->id;
45+
46+
self::$client = $clientWrapper->getBasicClient();
4247
$componentsApi = new Components(self::$client);
4348
$configuration = new Configuration();
4449
$configuration->setConfiguration([]);
@@ -160,7 +165,7 @@ public function testCreateJob(
160165
'startTime' => null,
161166
'endTime' => null,
162167
'durationSeconds' => 0,
163-
'branchId' => null,
168+
'branchId' => self::$defaultBranchId,
164169
'variableValuesId' => null,
165170
'variableValuesData' => [
166171
'values' => [],
@@ -273,7 +278,7 @@ public function testCreateJobsBatch(
273278
'startTime' => null,
274279
'endTime' => null,
275280
'durationSeconds' => 0,
276-
'branchId' => null,
281+
'branchId' => self::$defaultBranchId,
277282
'variableValuesId' => null,
278283
'variableValuesData' => [
279284
'values' => [],
@@ -559,7 +564,7 @@ public function testListJobsByComponent(): void
559564
/** @var Job $listedJob */
560565
$listedJob = $response[0];
561566
self::assertEquals($createdJob->jsonSerialize(), $listedJob->jsonSerialize());
562-
self::assertNull($listedJob->jsonSerialize()['branchId']);
567+
self::assertSame(self::$defaultBranchId, $listedJob->jsonSerialize()['branchId']);
563568

564569
// list more components
565570
$response = $client->listJobs(
@@ -750,7 +755,7 @@ public function testListJobsBranchId(): void
750755
$listedJob = $response[0];
751756

752757
self::assertEquals($createdJob2->jsonSerialize(), $listedJob->jsonSerialize());
753-
self::assertNull($listedJob->jsonSerialize()['branchId']);
758+
self::assertSame(self::$defaultBranchId, $listedJob->jsonSerialize()['branchId']);
754759
$branchesApi->deleteBranch($branchId);
755760
}
756761

@@ -814,7 +819,7 @@ public function testListJobsConfigRowIds(): void
814819
/** @var Job $listedJob */
815820
$listedJob = $response[0];
816821
self::assertEquals($createdJob2->jsonSerialize(), $listedJob->jsonSerialize());
817-
self::assertNull($listedJob->jsonSerialize()['branchId']);
822+
self::assertSame(self::$defaultBranchId, $listedJob->jsonSerialize()['branchId']);
818823

819824
// no jobs
820825
$response = $client->listJobs(

‎tests/ClientListConfigurationsJobsFunctionalTest.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use Keboola\StorageApi\Components;
1010
use Keboola\StorageApi\DevBranches;
1111
use Keboola\StorageApi\Options\Components\Configuration;
12+
use Keboola\StorageApiBranch\ClientWrapper;
13+
use Keboola\StorageApiBranch\Factory\ClientOptions;
1214

1315
class ClientListConfigurationsJobsFunctionalTest extends BaseClientFunctionalTest
1416
{
@@ -334,7 +336,12 @@ public function testJobsWithoutBranchAreListed(): void
334336
self::assertCount(2, $response);
335337
self::assertEquals($createdJob2->jsonSerialize(), $response[0]->jsonSerialize());
336338

339+
$defaultBranchId = (new ClientWrapper(new ClientOptions(
340+
url: self::getRequiredEnv('TEST_STORAGE_API_URL'),
341+
token: self::getRequiredEnv('TEST_STORAGE_API_TOKEN'),
342+
)))->getDefaultBranch()->id;
343+
337344
self::assertNotSame($createdJob1->getId(), $response[1]->getId());
338-
self::assertNull($response[1]->getBranchId());
345+
self::assertSame($defaultBranchId, $response[1]->getBranchId());
339346
}
340347
}

0 commit comments

Comments
 (0)