From 5413730b006bd2b667a07acd3d388dfb76562d91 Mon Sep 17 00:00:00 2001 From: DevEnv nis2-agile Date: Sat, 30 May 2026 11:39:38 +0200 Subject: [PATCH] [FIX] Policy: UNIQUE(policy_id,version) + diff LCS posizionale (findings review) - Migrazione 030: UNIQUE uq_policy_version su policy_versions (de-dup prima, idempotente). approve() ora usa INSERT ... ON DUPLICATE KEY UPDATE -> riapprovare la stessa versione aggiorna lo snapshot invece di duplicarlo. Verificato E2E: 2x approve v1.0 -> 1 sola riga. - diff(): sostituito il confronto set-based (falsi negativi su righe duplicate/riordino) con un vero diff LCS line-by-line con posizioni. Verificato E2E: bump v1->v2 -> added 2, removed 1 corretti. Co-Authored-By: Claude Opus 4.8 (1M context) --- application/controllers/PolicyController.php | 95 ++++++++++++++++---- docs/sql/030_policy_versions_unique.sql | 36 ++++++++ 2 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 docs/sql/030_policy_versions_unique.sql diff --git a/application/controllers/PolicyController.php b/application/controllers/PolicyController.php index 5290b2c..55fdffb 100644 --- a/application/controllers/PolicyController.php +++ b/application/controllers/PolicyController.php @@ -123,16 +123,24 @@ class PolicyController extends BaseController $policy = Database::fetchOne('SELECT * FROM policies WHERE id = ?', [$id]); - // Snapshot versione per storico/diff/attestation (P3) + // Snapshot versione per storico/diff/attestation (P3). + // Idempotente sulla coppia (policy_id, version): la riapprovazione della + // stessa versione aggiorna lo snapshot invece di duplicarlo (UNIQUE uq_policy_version). try { - Database::insert('policy_versions', [ - 'policy_id' => $id, - 'organization_id' => $this->getCurrentOrgId(), - 'version' => $policy['version'] ?? '1.0', - 'content' => $policy['content'] ?? null, - 'change_note' => $this->getParam('change_note') ?: 'Approvazione', - 'created_by' => $this->getCurrentUserId(), - ]); + Database::query( + 'INSERT INTO policy_versions (policy_id, organization_id, version, content, change_note, created_by) + VALUES (?, ?, ?, ?, ?, ?) + ON DUPLICATE KEY UPDATE content = VALUES(content), change_note = VALUES(change_note), + created_by = VALUES(created_by), created_at = NOW()', + [ + $id, + $this->getCurrentOrgId(), + $policy['version'] ?? '1.0', + $policy['content'] ?? null, + $this->getParam('change_note') ?: 'Approvazione', + $this->getCurrentUserId(), + ] + ); } catch (Throwable $e) { error_log('[POLICY_VER] ' . $e->getMessage()); } @@ -308,20 +316,69 @@ class PolicyController extends BaseController } $linesA = preg_split('/\r\n|\r|\n/', (string) $a['content']); $linesB = preg_split('/\r\n|\r|\n/', (string) $b['content']); - $setA = array_count_values(array_map('trim', $linesA)); - $setB = array_count_values(array_map('trim', $linesB)); - $added = []; $removed = []; - foreach ($linesB as $l) { $t = trim($l); if ($t !== '' && empty($setA[$t])) $added[] = $l; } - foreach ($linesA as $l) { $t = trim($l); if ($t !== '' && empty($setB[$t])) $removed[] = $l; } + $d = self::lcsLineDiff($linesA, $linesB); $this->jsonSuccess([ - 'from' => ['id' => $fromId, 'version' => $a['version']], - 'to' => ['id' => $toId, 'version' => $b['version']], - 'added' => $added, - 'removed' => $removed, - 'summary' => ['added' => count($added), 'removed' => count($removed)], + 'from' => ['id' => $fromId, 'version' => $a['version']], + 'to' => ['id' => $toId, 'version' => $b['version']], + 'added' => $d['added'], + 'removed' => $d['removed'], + 'summary' => ['added' => count($d['added']), 'removed' => count($d['removed'])], ]); } + /** + * Diff line-by-line basato su LCS (Longest Common Subsequence). + * Rileva correttamente righe duplicate, spostamenti e modifiche posizionali, + * a differenza di un confronto set-based. Restituisce le righe aggiunte (in B + * ma non allineate in A) e rimosse (in A ma non allineate in B), con posizione. + * + * @return array{added: list, removed: list} + */ + private static function lcsLineDiff(array $a, array $b): array + { + $n = count($a); + $m = count($b); + // Tabella LCS (lunghezze). Cap di sicurezza per evitare blow-up su documenti enormi. + if ($n > 4000 || $m > 4000) { + // fallback prudente: confronto posizionale semplice riga per riga + $added = []; $removed = []; + $max = max($n, $m); + for ($i = 0; $i < $max; $i++) { + $la = $a[$i] ?? null; $lb = $b[$i] ?? null; + if ($la !== $lb) { + if ($lb !== null && trim($lb) !== '') $added[] = ['line' => $i + 1, 'text' => $lb]; + if ($la !== null && trim($la) !== '') $removed[] = ['line' => $i + 1, 'text' => $la]; + } + } + return ['added' => $added, 'removed' => $removed]; + } + + $dp = array_fill(0, $n + 1, array_fill(0, $m + 1, 0)); + for ($i = $n - 1; $i >= 0; $i--) { + for ($j = $m - 1; $j >= 0; $j--) { + $dp[$i][$j] = ($a[$i] === $b[$j]) + ? $dp[$i + 1][$j + 1] + 1 + : max($dp[$i + 1][$j], $dp[$i][$j + 1]); + } + } + $added = []; $removed = []; + $i = 0; $j = 0; + while ($i < $n && $j < $m) { + if ($a[$i] === $b[$j]) { + $i++; $j++; + } elseif ($dp[$i + 1][$j] >= $dp[$i][$j + 1]) { + if (trim($a[$i]) !== '') $removed[] = ['line' => $i + 1, 'text' => $a[$i]]; + $i++; + } else { + if (trim($b[$j]) !== '') $added[] = ['line' => $j + 1, 'text' => $b[$j]]; + $j++; + } + } + for (; $i < $n; $i++) { if (trim($a[$i]) !== '') $removed[] = ['line' => $i + 1, 'text' => $a[$i]]; } + for (; $j < $m; $j++) { if (trim($b[$j]) !== '') $added[] = ['line' => $j + 1, 'text' => $b[$j]]; } + return ['added' => $added, 'removed' => $removed]; + } + /** GET /api/policies/pending-attestations — policy approvate non ancora attestate dall'utente. */ public function pendingAttestations(): void { diff --git a/docs/sql/030_policy_versions_unique.sql b/docs/sql/030_policy_versions_unique.sql new file mode 100644 index 0000000..5e8ee65 --- /dev/null +++ b/docs/sql/030_policy_versions_unique.sql @@ -0,0 +1,36 @@ +-- ============================================================================ +-- Migration 030 - UNIQUE(policy_id, version) su policy_versions (P3 hardening) +-- ---------------------------------------------------------------------------- +-- Evita snapshot duplicati della stessa versione di policy (riapprovazioni +-- ripetute con la stessa version creavano righe duplicate, confondendo il diff). +-- Idempotente: aggiunge l'indice solo se assente; de-duplica prima se necessario. +-- +-- mysql -h localhost nis2_agile_db -e "source docs/sql/030_policy_versions_unique.sql" +-- ============================================================================ + +-- 1) Rimuovi eventuali duplicati pre-esistenti (mantieni il piu recente per id) +DELETE pv1 FROM policy_versions pv1 +JOIN policy_versions pv2 + ON pv1.policy_id = pv2.policy_id + AND pv1.version = pv2.version + AND pv1.id < pv2.id; + +-- 2) Aggiungi UNIQUE solo se non esiste gia +DELIMITER // +DROP PROCEDURE IF EXISTS _mig030 // +CREATE PROCEDURE _mig030() +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'policy_versions' + AND INDEX_NAME = 'uq_policy_version' + ) THEN + ALTER TABLE policy_versions ADD UNIQUE KEY uq_policy_version (policy_id, version); + END IF; +END // +DELIMITER ; +CALL _mig030(); +DROP PROCEDURE IF EXISTS _mig030; + +-- ROLLBACK: +-- ALTER TABLE policy_versions DROP INDEX uq_policy_version;