Skip to content

Commit b797f38

Browse files
committed
Tidy OAuth2Provider: drop dead getter and de-duplicate emptiness check
- Remove unused getSetting(); getDestinationAppId/SecretFields cover all callers. - getDestinationAppId() now returns ?string (null when unset), so the destination drops its duplicate isEmptyOAuth2Setting() helper and isConfigured() reads as `enabled || appId !== null`. - Document the PROVIDERS target/key routing so adding a provider is self-explanatory.
1 parent 09334e6 commit b797f38

2 files changed

Lines changed: 21 additions & 16 deletions

File tree

src/Migration/Destinations/Appwrite.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3559,7 +3559,7 @@ protected function createOAuth2Provider(OAuth2Provider $resource): bool
35593559
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
35603560

35613561
$appId = $resource->getDestinationAppId();
3562-
if (!$this->isEmptyOAuth2Setting($appId)) {
3562+
if ($appId !== null) {
35633563
$oAuthProviders[$key . 'Appid'] = $appId;
35643564
}
35653565

@@ -3604,11 +3604,6 @@ private function mergeJsonSecret(string $existing, array $fields): string
36043604
return \json_encode($decoded) ?: '';
36053605
}
36063606

3607-
private function isEmptyOAuth2Setting(mixed $value): bool
3608-
{
3609-
return $value === null || $value === '' || $value === [];
3610-
}
3611-
36123607
/**
36133608
* Direct DB write — SDK policy setters reject `total: 0` but `0` is the
36143609
* storage value for "disabled". Shares the `auths` map with

src/Migration/Resources/Auth/OAuth2/OAuth2Provider.php

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ final class OAuth2Provider extends Resource
1414
private const TARGET_SECRET = 'secret';
1515

1616
/**
17-
* Allow-list of readable provider fields that are safe to migrate.
17+
* Allow-list of readable provider fields that are safe to migrate, keyed by
18+
* provider. Each field declares where it lands on the destination:
19+
* - target `appId` -> the provider's `{key}Appid` attribute (one per provider)
20+
* - target `secret` -> merged into the `{key}Secret` JSON blob, renamed via `key`
21+
*
22+
* Anything not listed here (clientSecret, Apple's p8File, …) is never copied,
23+
* so a secret field the server may add upstream cannot leak into a migration.
1824
*
1925
* @var array<string, array<string, array{target: string, key?: string}>>
2026
*/
@@ -150,17 +156,21 @@ public function getSettings(): array
150156
return $this->settings;
151157
}
152158

153-
public function getSetting(string $field): mixed
154-
{
155-
return $this->settings[$field] ?? null;
156-
}
157-
158-
public function getDestinationAppId(): mixed
159+
/**
160+
* Value for the destination's `{key}Appid` attribute (clientId, or serviceId
161+
* for Apple). Null when unset, so callers can skip it without a separate
162+
* emptiness check.
163+
*/
164+
public function getDestinationAppId(): ?string
159165
{
160166
foreach ($this->getDescriptor() as $field => $metadata) {
161-
if ($metadata['target'] === self::TARGET_APP_ID && \array_key_exists($field, $this->settings)) {
162-
return $this->settings[$field];
167+
if ($metadata['target'] !== self::TARGET_APP_ID) {
168+
continue;
163169
}
170+
171+
$value = $this->settings[$field] ?? null;
172+
173+
return self::isEmpty($value) ? null : (string) $value;
164174
}
165175

166176
return null;
@@ -190,7 +200,7 @@ public function getDestinationSecretFields(): array
190200

191201
public function isConfigured(): bool
192202
{
193-
return $this->enabled || !self::isEmpty($this->getDestinationAppId());
203+
return $this->enabled || $this->getDestinationAppId() !== null;
194204
}
195205

196206
/**

0 commit comments

Comments
 (0)