Skip to content

fix(migrate): handle legacy periods outside millisecond range - #52

Closed
albertovincenzi wants to merge 1 commit into
fix/overflow-safe-window-warningfrom
fix/saturating-v1-migration
Closed

fix(migrate): handle legacy periods outside millisecond range#52
albertovincenzi wants to merge 1 commit into
fix/overflow-safe-window-warningfrom
fix/saturating-v1-migration

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Dipendenza

Questa PR è impilata su #43. Dopo la migrazione, il PUT costruisce anche le warning del documento v2; #43 rende sicuro quel secondo passaggio per lo stesso valore estremo.

Problema

Il wire v1 rappresenta periodSeconds con un i64; il wire v2 rappresenta timeMs ancora con un i64. La migrazione faceva direttamente periodSeconds * 1000, quindi un documento v1 JSON valido sopra i64::MAX / 1000:

  • panicava nei build debug;
  • poteva wrapparsi nei build senza overflow checks;
  • aveva un secondo overflow nel testo della warning rolling con 2 * count.

Il risultato era un errore di processo durante un percorso che promette di leggere documenti legacy.

Fix

  • converte i secondi in millisecondi con saturating_mul;
  • emette la warning esplicita period-clamped quando il tipo v2 non può rappresentare il periodo originale;
  • usa aritmetica saturante anche per i valori 2 × count mostrati nella warning;
  • aggiunge un test con periodSeconds = i64::MAX e count estremo, includendo il successivo passaggio warnings().

Test

  • cargo test -p gate-core (97 test + doc-test, tutti verdi);
  • cargo clippy -p gate-core --all-targets -- -D warnings;
  • cargo fmt --all -- --check;
  • git diff --check.

Per Alice

Il trade-off da valutare è esplicito: v2 non può rappresentare in millisecondi l'intero range in secondi di v1. Questa PR conserva la promessa di migrazione totale, limita il valore al massimo rappresentabile e rende la perdita di range visibile al caller. L'alternativa sarebbe rifiutare questi soli documenti legacy; sarebbe più rigida ma romperebbe la compatibilità dichiarata in §12.1.

@alice-viola

Copy link
Copy Markdown
Contributor

Landed on master via #67 (merge commit 944ee9b) as part of the 62-PR integration — this PR's head commit 36f2077 is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/overflow-safe-window-warning, not master. Closing as landed.

@alice-viola alice-viola closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants