[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) <noreply@anthropic.com>
This commit is contained in:
DevEnv nis2-agile
parent
2fd4b7ff26
commit
5413730b00
@ -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<array{line:int,text:string}>, removed: list<array{line:int,text:string}>}
|
||||
*/
|
||||
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
|
||||
{
|
||||
|
||||
36
docs/sql/030_policy_versions_unique.sql
Normal file
36
docs/sql/030_policy_versions_unique.sql
Normal file
@ -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;
|
||||
Loading…
Reference in New Issue
Block a user