refactor(integration): enhance association logic to delete unused instances and improve transaction handling
This commit is contained in:
@@ -27,17 +27,68 @@ class IntegrationAssociationService
|
|||||||
|
|
||||||
public function associate(Client|WebsiteType $owner, string $code, IntegrationInstance $instance): ClientIntegration|WebsiteTypeIntegration
|
public function associate(Client|WebsiteType $owner, string $code, IntegrationInstance $instance): ClientIntegration|WebsiteTypeIntegration
|
||||||
{
|
{
|
||||||
abort_unless($instance->integration_code === $code, 422, 'The instance belongs to another integration.');
|
return DB::transaction(function () use ($owner, $code, $instance): ClientIntegration|WebsiteTypeIntegration {
|
||||||
|
$association = $owner->integrations()
|
||||||
|
->where('integration_code', $code)
|
||||||
|
->lockForUpdate()
|
||||||
|
->first();
|
||||||
|
$previousInstanceId = $association?->integration_instance_id;
|
||||||
|
|
||||||
return $owner->integrations()->updateOrCreate(
|
$instanceIds = array_values(array_unique(array_filter([
|
||||||
['integration_code' => $code],
|
$previousInstanceId,
|
||||||
['integration_instance_id' => $instance->id],
|
$instance->id,
|
||||||
)->load('integrationInstance');
|
])));
|
||||||
|
sort($instanceIds);
|
||||||
|
|
||||||
|
$instances = IntegrationInstance::query()
|
||||||
|
->whereKey($instanceIds)
|
||||||
|
->orderBy('id')
|
||||||
|
->lockForUpdate()
|
||||||
|
->get()
|
||||||
|
->keyBy('id');
|
||||||
|
$instance = $instances->get($instance->id) ?? IntegrationInstance::query()->findOrFail($instance->id);
|
||||||
|
abort_unless($instance->integration_code === $code, 422, 'The instance belongs to another integration.');
|
||||||
|
|
||||||
|
$association = $owner->integrations()->updateOrCreate(
|
||||||
|
['integration_code' => $code],
|
||||||
|
['integration_instance_id' => $instance->id],
|
||||||
|
);
|
||||||
|
|
||||||
|
if ($previousInstanceId && $previousInstanceId !== $instance->id) {
|
||||||
|
$this->deleteIfUnused($previousInstanceId);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $association->load('integrationInstance');
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
public function detach(Client|WebsiteType $owner, string $code): void
|
public function detach(Client|WebsiteType $owner, string $code): void
|
||||||
{
|
{
|
||||||
$owner->integrations()->where('integration_code', $code)->delete();
|
DB::transaction(function () use ($owner, $code): void {
|
||||||
|
$association = $owner->integrations()
|
||||||
|
->where('integration_code', $code)
|
||||||
|
->lockForUpdate()
|
||||||
|
->first();
|
||||||
|
|
||||||
|
if (! $association) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
$instanceId = $association->integration_instance_id;
|
||||||
|
$association->delete();
|
||||||
|
$this->deleteIfUnused($instanceId);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
private function deleteIfUnused(int $instanceId): void
|
||||||
|
{
|
||||||
|
$instance = IntegrationInstance::query()->lockForUpdate()->find($instanceId);
|
||||||
|
|
||||||
|
if ($instance
|
||||||
|
&& ! $instance->clientIntegrations()->exists()
|
||||||
|
&& ! $instance->websiteTypeIntegrations()->exists()) {
|
||||||
|
$instance->delete();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function ownerName(Client|WebsiteType $owner): string
|
private function ownerName(Client|WebsiteType $owner): string
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
- `IntegrationInstance`: configuración interna concreta con nombre. `integration_data` se cifra con `EncryptedIntegrationData`, se almacena en `longText` y nunca se devuelve en la API.
|
- `IntegrationInstance`: configuración interna concreta con nombre. `integration_data` se cifra con `EncryptedIntegrationData`, se almacena en `longText` y nunca se devuelve en la API.
|
||||||
- `ClientIntegration` y `WebsiteTypeIntegration`: asociaciones a instancias. La clave compuesta verifica el código de la instancia y la unicidad permite una instancia por integración y propietario.
|
- `ClientIntegration` y `WebsiteTypeIntegration`: asociaciones a instancias. La clave compuesta verifica el código de la instancia y la unicidad permite una instancia por integración y propietario.
|
||||||
|
|
||||||
Las instancias no se administran directamente por HTTP. Cada configuración enviada desde un cliente o tipo de sitio crea una instancia interna nueva y reemplaza únicamente la asociación de ese propietario. Desvincularla solo elimina la asociación.
|
Las instancias no se administran directamente por HTTP. Cada configuración enviada desde un cliente o tipo de sitio crea una instancia interna nueva y reemplaza únicamente la asociación de ese propietario. Al reemplazar o desvincular una instancia, esta se elimina si ya no tiene asociaciones con ningún cliente ni tipo de sitio; las instancias compartidas se conservan mientras tengan al menos una asociación.
|
||||||
|
|
||||||
## Resolución
|
## Resolución
|
||||||
|
|
||||||
|
|||||||
@@ -126,6 +126,36 @@ class IntegrationInstanceTest extends TestCase
|
|||||||
self::assertSame('changed', $this->probe()->forClient($b)->setting());
|
self::assertSame('changed', $this->probe()->forClient($b)->setting());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_reconfiguring_deletes_the_previous_instance_when_it_is_no_longer_used(): void
|
||||||
|
{
|
||||||
|
$client = Client::create(['code' => 'acme', 'name' => 'Acme']);
|
||||||
|
$associations = new IntegrationAssociationService;
|
||||||
|
$previous = $this->makeInstance('previous');
|
||||||
|
|
||||||
|
$associations->associate($client, 'test', $previous);
|
||||||
|
$current = $associations->configure($client, $this->integration, ['api_key' => 'current']);
|
||||||
|
|
||||||
|
$this->assertDatabaseMissing('integration_instances', ['id' => $previous->id]);
|
||||||
|
$this->assertDatabaseHas('integration_instances', ['id' => $current->integration_instance_id]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_an_instance_is_deleted_only_after_its_last_association_is_removed(): void
|
||||||
|
{
|
||||||
|
$client = Client::create(['code' => 'acme', 'name' => 'Acme']);
|
||||||
|
$type = WebsiteType::create(['codigo' => 'demo', 'nombre' => 'Demo']);
|
||||||
|
$shared = $this->makeInstance('shared');
|
||||||
|
$associations = new IntegrationAssociationService;
|
||||||
|
|
||||||
|
$associations->associate($client, 'test', $shared);
|
||||||
|
$associations->associate($type, 'test', $shared);
|
||||||
|
|
||||||
|
$associations->detach($client, 'test');
|
||||||
|
$this->assertDatabaseHas('integration_instances', ['id' => $shared->id]);
|
||||||
|
|
||||||
|
$associations->detach($type, 'test');
|
||||||
|
$this->assertDatabaseMissing('integration_instances', ['id' => $shared->id]);
|
||||||
|
}
|
||||||
|
|
||||||
public function test_telepagos_shares_tokens_by_instance_and_refreshes_after_credential_changes(): void
|
public function test_telepagos_shares_tokens_by_instance_and_refreshes_after_credential_changes(): void
|
||||||
{
|
{
|
||||||
Integration::create(['integration_code' => 'telepagos_homo', 'name' => 'Telepagos', 'url' => 'https://payments.test']);
|
Integration::create(['integration_code' => 'telepagos_homo', 'name' => 'Telepagos', 'url' => 'https://payments.test']);
|
||||||
|
|||||||
Reference in New Issue
Block a user