fix: retry settings catalog bulk apply

This commit is contained in:
Ahmed Darrazi 2025-12-21 15:35:50 +01:00
parent 6910e40b91
commit 23c15400fd
5 changed files with 182 additions and 3 deletions

View File

@ -135,6 +135,19 @@ public function settingsWriteBodyShape(string $policyType): string
return is_string($shape) && $shape !== '' ? $shape : 'collection'; return is_string($shape) && $shape !== '' ? $shape : 'collection';
} }
public function settingsWriteFallbackBodyShape(string $policyType): ?string
{
$contract = $this->get($policyType);
$write = $contract['settings_write'] ?? null;
$shape = is_array($write) ? ($write['fallback_body_shape'] ?? null) : null;
if (! is_string($shape) || $shape === '') {
return null;
}
return $shape;
}
/** /**
* Sanitize a settings_apply payload for settingsCatalogPolicy. * Sanitize a settings_apply payload for settingsCatalogPolicy.
* Preserves `@odata.type` inside `settingInstance` and nested children while * Preserves `@odata.type` inside `settingInstance` and nested children while

View File

@ -408,6 +408,7 @@ private function applySettingsCatalogPolicySettings(
$method = $this->contracts->settingsWriteMethod('settingsCatalogPolicy'); $method = $this->contracts->settingsWriteMethod('settingsCatalogPolicy');
$path = $this->contracts->settingsWritePath('settingsCatalogPolicy', $policyId); $path = $this->contracts->settingsWritePath('settingsCatalogPolicy', $policyId);
$bodyShape = strtolower($this->contracts->settingsWriteBodyShape('settingsCatalogPolicy')); $bodyShape = strtolower($this->contracts->settingsWriteBodyShape('settingsCatalogPolicy'));
$fallbackShape = $this->contracts->settingsWriteFallbackBodyShape('settingsCatalogPolicy');
$buildIssues = function (string $reason) use ($settings): array { $buildIssues = function (string $reason) use ($settings): array {
$issues = []; $issues = [];
@ -455,15 +456,20 @@ private function applySettingsCatalogPolicySettings(
]; ];
} }
$payload = match ($bodyShape) { $buildPayload = function (string $shape) use ($sanitized): array {
return match ($shape) {
'wrapped' => ['settings' => $sanitized], 'wrapped' => ['settings' => $sanitized],
default => $sanitized, default => $sanitized,
}; };
};
$payload = $buildPayload($bodyShape);
$this->graphLogger->logRequest('apply_settings_bulk', $context + [ $this->graphLogger->logRequest('apply_settings_bulk', $context + [
'endpoint' => $path, 'endpoint' => $path,
'method' => $method, 'method' => $method,
'settings_count' => count($sanitized), 'settings_count' => count($sanitized),
'body_shape' => $bodyShape,
]); ]);
$response = $this->graphClient->request($method, $path, ['json' => $payload] + Arr::except($graphOptions, ['platform'])); $response = $this->graphClient->request($method, $path, ['json' => $payload] + Arr::except($graphOptions, ['platform']));
@ -472,8 +478,33 @@ private function applySettingsCatalogPolicySettings(
'endpoint' => $path, 'endpoint' => $path,
'method' => $method, 'method' => $method,
'settings_count' => count($sanitized), 'settings_count' => count($sanitized),
'body_shape' => $bodyShape,
]); ]);
if ($response->failed() && is_string($fallbackShape) && strtolower($fallbackShape) !== $bodyShape) {
$fallbackShape = strtolower($fallbackShape);
if ($this->shouldRetrySettingsBulkApply($response->meta['error_message'] ?? null)) {
$fallbackPayload = $buildPayload($fallbackShape);
$this->graphLogger->logRequest('apply_settings_bulk_retry', $context + [
'endpoint' => $path,
'method' => $method,
'settings_count' => count($sanitized),
'body_shape' => $fallbackShape,
]);
$response = $this->graphClient->request($method, $path, ['json' => $fallbackPayload] + Arr::except($graphOptions, ['platform']));
$this->graphLogger->logResponse('apply_settings_bulk_retry', $response, $context + [
'endpoint' => $path,
'method' => $method,
'settings_count' => count($sanitized),
'body_shape' => $fallbackShape,
]);
}
}
if ($response->successful()) { if ($response->successful()) {
return [ return [
[ [
@ -508,6 +539,19 @@ private function applySettingsCatalogPolicySettings(
]; ];
} }
private function shouldRetrySettingsBulkApply(?string $errorMessage): bool
{
if (! is_string($errorMessage) || $errorMessage === '') {
return false;
}
$message = strtolower($errorMessage);
return str_contains($message, 'empty payload')
|| str_contains($message, 'json content expected')
|| str_contains($message, 'request body');
}
private function assertActiveContext(Tenant $tenant, BackupSet $backupSet): void private function assertActiveContext(Tenant $tenant, BackupSet $backupSet): void
{ {
if (! $tenant->isActive()) { if (! $tenant->isActive()) {

View File

@ -67,6 +67,7 @@
'method' => 'POST', 'method' => 'POST',
'bulk' => true, 'bulk' => true,
'body_shape' => 'collection', 'body_shape' => 'collection',
'fallback_body_shape' => 'wrapped',
], ],
'update_strategy' => 'settings_catalog_policy_with_settings', 'update_strategy' => 'settings_catalog_policy_with_settings',
], ],

View File

@ -305,3 +305,107 @@ public function request(string $method, string $path, array $options = []): Grap
expect($client->requestCalls[0]['payload'][0]['settingInstance']['@odata.type']) expect($client->requestCalls[0]['payload'][0]['settingInstance']['@odata.type'])
->toBe('#microsoft.graph.deviceManagementConfigurationChoiceSettingInstance'); ->toBe('#microsoft.graph.deviceManagementConfigurationChoiceSettingInstance');
}); });
test('restore retries bulk apply with wrapped payload when graph expects json object', function () {
$policyResponse = new GraphResponse(
success: true,
data: [],
status: 200,
errors: [],
warnings: [],
meta: ['request_id' => 'req-policy', 'client_request_id' => 'client-policy'],
);
$firstResponse = new GraphResponse(
success: false,
data: ['error' => ['code' => 'BadRequest', 'message' => 'Empty Payload. JSON content expected.']],
status: 400,
errors: [['code' => 'BadRequest', 'message' => 'Empty Payload. JSON content expected.']],
warnings: [],
meta: [
'error_code' => 'BadRequest',
'error_message' => 'Empty Payload. JSON content expected.',
'request_id' => 'req-1',
'client_request_id' => 'client-1',
],
);
$secondResponse = new GraphResponse(
success: true,
data: [],
status: 200,
errors: [],
warnings: [],
meta: ['request_id' => 'req-2', 'client_request_id' => 'client-2'],
);
$client = new SettingsCatalogRestoreGraphClient($policyResponse, [$firstResponse, $secondResponse]);
app()->instance(GraphClientInterface::class, $client);
$tenant = Tenant::create([
'tenant_id' => 'tenant-4',
'name' => 'Tenant Four',
'metadata' => [],
]);
$policy = Policy::create([
'tenant_id' => $tenant->id,
'external_id' => 'scp-4',
'policy_type' => 'settingsCatalogPolicy',
'display_name' => 'Settings Catalog Delta',
'platform' => 'windows',
]);
$backupSet = BackupSet::create([
'tenant_id' => $tenant->id,
'name' => 'Backup',
'status' => 'completed',
'item_count' => 1,
]);
$payload = [
'displayName' => 'Settings Catalog Delta',
'Settings' => [
[
'id' => 'setting-1',
'settingInstance' => [
'@odata.type' => '#microsoft.graph.deviceManagementConfigurationSimpleSettingInstance',
'settingDefinitionId' => 'test_setting',
'simpleSettingValue' => [
'@odata.type' => '#microsoft.graph.deviceManagementConfigurationIntegerSettingValue',
'value' => 5,
],
],
],
],
];
$backupItem = BackupItem::create([
'tenant_id' => $tenant->id,
'backup_set_id' => $backupSet->id,
'policy_id' => $policy->id,
'policy_identifier' => $policy->external_id,
'policy_type' => $policy->policy_type,
'platform' => $policy->platform,
'payload' => $payload,
]);
$user = User::factory()->create();
$this->actingAs($user);
$service = app(RestoreService::class);
$run = $service->execute(
tenant: $tenant,
backupSet: $backupSet,
selectedItemIds: [$backupItem->id],
dryRun: false,
actorEmail: $user->email,
actorName: $user->name,
)->refresh();
expect($run->status)->toBe('completed');
expect($client->requestCalls)->toHaveCount(2);
expect($client->requestCalls[0]['payload'])->toHaveKey(0);
expect($client->requestCalls[1]['payload'])->toHaveKey('settings');
});

View File

@ -44,6 +44,7 @@
$registry = app(GraphContractRegistry::class); $registry = app(GraphContractRegistry::class);
expect($registry->settingsWriteBodyShape('settingsCatalogPolicy'))->toBe('collection'); expect($registry->settingsWriteBodyShape('settingsCatalogPolicy'))->toBe('collection');
expect($registry->settingsWriteFallbackBodyShape('settingsCatalogPolicy'))->toBeNull();
}); });
it('returns null when settings write contract is missing', function () { it('returns null when settings write contract is missing', function () {
@ -54,3 +55,19 @@
expect($registry->settingsWriteMethod('settingsCatalogPolicy'))->toBeNull(); expect($registry->settingsWriteMethod('settingsCatalogPolicy'))->toBeNull();
expect($registry->settingsWritePath('settingsCatalogPolicy', 'policy-1', 'setting-9'))->toBeNull(); expect($registry->settingsWritePath('settingsCatalogPolicy', 'policy-1', 'setting-9'))->toBeNull();
}); });
it('returns fallback body shape when configured', function () {
config()->set('graph_contracts.types.settingsCatalogPolicy', [
'settings_write' => [
'path_template' => 'deviceManagement/configurationPolicies/{id}/settings',
'method' => 'POST',
'body_shape' => 'collection',
'fallback_body_shape' => 'wrapped',
],
]);
$registry = app(GraphContractRegistry::class);
expect($registry->settingsWriteBodyShape('settingsCatalogPolicy'))->toBe('collection');
expect($registry->settingsWriteFallbackBodyShape('settingsCatalogPolicy'))->toBe('wrapped');
});