Porta avanti le impostazioni vecchie, o le correzioni di oggi non arrivano

Trovato provando davvero l'applicazione, non dai test. Il settings.json esistente
conteneva MaxRequestsPerSecond 40 e DefaultBidBeforeDeadlineMs 1000, e il valore
su disco vince sempre sui predefiniti: installando la versione nuova, le due
correzioni che pesano di piu' - tetto delle richieste tolto e anticipo a mezzo
secondo - sarebbero state annullate dal file dell'utente, che avrebbe concluso
che non funzionano.

Che il file vinca e' giusto: sono scelte di chi usa il programma. Ma quei due
valori non erano scelte, erano conseguenze di misure sbagliate. Erano stati
tarati quando il tetto stesso accodava le chiamate e gonfiava il ping a 444 ms al
p99,9; senza tetto il ping misurato sta fra 47 e 78 ms.

Migrazione una tantum, con un numero di schema nel file perche' giri una volta
sola. Tocca solo chi ha ancora esattamente il vecchio valore: chi lo aveva gia'
cambiato di suo se lo tiene, perche' quella e' una scelta e non spetta a un
aggiornamento ribaltarla. Il salvataggio avviene dentro Load, che di norma non
tocca il disco, ma una migrazione capita una volta nella vita del file e
lasciarla non salvata la farebbe ripetere a ogni avvio.

Verificato dal vivo sull'eseguibile pubblicato: 40 diventa 0, 1000 diventa 500,
lo schema passa da assente a 1, e le impostazioni nuove (fascia oraria, ritiro
dai duelli) compaiono coi valori giusti.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Mimante/Tests/SettingsMigrationTests.cs

@@ -0,0 +1,90 @@
+using AutoBidder.Utilities;
+using Xunit;
+
+namespace AutoBidder.Tests;
+
+/// <summary>
+/// Migrazione delle impostazioni salvate da una versione precedente.
+///
+/// <para>La proprieta' da difendere: un predefinito nuovo <b>non arriva</b> a chi ha gia' un
+/// file salvato, perche' il valore su disco vince. Va bene per le scelte dell'utente, non
+/// per due valori che erano conseguenze di misure sbagliate — il tetto di 40 richieste al
+/// secondo e l'anticipo di 1000 ms erano stati tarati quando il tetto stesso accodava le
+/// chiamate e gonfiava il ping. Senza migrazione l'aggiornamento sarebbe arrivato con le
+/// correzioni gia' annullate dal file dell'utente.</para>
+///
+/// <para>L'altra proprieta', altrettanto importante: chi quei valori li aveva gia' cambiati
+/// di suo se li tiene. Quella e' una scelta, e non spetta a un aggiornamento ribaltarla.</para>
+/// </summary>
+public class SettingsMigrationTests
+{
+    private static AppSettings Vecchie() => new()
+    {
+        SettingsSchemaVersion = 0,
+        MaxRequestsPerSecond = 40,
+        DefaultBidBeforeDeadlineMs = 1000
+    };
+
+    [Fact]
+    public void I_valori_vecchi_vengono_portati_avanti()
+    {
+        var s = Vecchie();
+
+        Assert.True(SettingsManager.Migra(s));
+
+        Assert.Equal(0, s.MaxRequestsPerSecond);          // nessun tetto
+        Assert.Equal(500, s.DefaultBidBeforeDeadlineMs);
+        Assert.Equal(SettingsManager.SchemaCorrente, s.SettingsSchemaVersion);
+    }
+
+    [Fact]
+    public void Una_scelta_dell_utente_non_viene_ribaltata()
+    {
+        var s = new AppSettings
+        {
+            SettingsSchemaVersion = 0,
+            MaxRequestsPerSecond = 15,          // scelto apposta, non il vecchio predefinito
+            DefaultBidBeforeDeadlineMs = 2000   // idem
+        };
+
+        SettingsManager.Migra(s);
+
+        Assert.Equal(15, s.MaxRequestsPerSecond);
+        Assert.Equal(2000, s.DefaultBidBeforeDeadlineMs);
+    }
+
+    [Fact]
+    public void Gira_una_volta_sola()
+    {
+        var s = Vecchie();
+
+        Assert.True(SettingsManager.Migra(s));
+        Assert.False(SettingsManager.Migra(s));
+
+        // E se poi l'utente rimette 40 di suo, non glielo si toglie al giro dopo.
+        s.MaxRequestsPerSecond = 40;
+        Assert.False(SettingsManager.Migra(s));
+        Assert.Equal(40, s.MaxRequestsPerSecond);
+    }
+
+    [Fact]
+    public void Impostazioni_nuove_di_zecca_sono_gia_allo_schema_corrente()
+    {
+        // Un file che non esiste ancora non e' un file vecchio: nasce gia' aggiornato,
+        // e non deve far scattare nessuna migrazione.
+        var s = new AppSettings { SettingsSchemaVersion = SettingsManager.SchemaCorrente };
+
+        Assert.False(SettingsManager.Migra(s));
+    }
+
+    [Fact]
+    public void I_predefiniti_di_fabbrica_sono_quelli_misurati_oggi()
+    {
+        var s = new AppSettings();
+
+        Assert.Equal(0, s.MaxRequestsPerSecond);
+        Assert.Equal(500, s.DefaultBidBeforeDeadlineMs);
+        Assert.True(s.QuietHoursEnabled);
+        Assert.True(s.AutoBidDuelWithdrawEnabled);
+    }
+}

Mimante/Utilities/SettingsManager.cs

@@ -565,6 +565,17 @@ namespace AutoBidder.Utilities
         // Vedi BiddingHours per i numeri: alle 0 e alle 9 la stessa asta costa quasi il
         // doppio che fra le 10 e le 13.
 
+        /// <summary>
+        /// Versione dello schema delle impostazioni, per le migrazioni una tantum.
+        ///
+        /// <para>Un file scritto da una versione precedente non ha questo campo e vale
+        /// quindi 0. Serve perché un predefinito nuovo <b>non arriva</b> a chi ha già un
+        /// file salvato: il valore su disco vince sempre, ed è giusto che sia così, tranne
+        /// quando il vecchio valore veniva da una misura che si è poi rivelata sbagliata.
+        /// Vedi <see cref="Migra"/>.</para>
+        /// </summary>
+        public int SettingsSchemaVersion { get; set; }
+
         /// <summary>
         /// Sospendere le puntate nella fascia indicata. Acceso di serie.
         /// L'asta resta comunque Attiva e riprende da sola: vedi <see cref="BiddingHours"/>.
@@ -833,6 +844,36 @@ namespace AutoBidder.Utilities
 
         private const int CACHE_TTL_MS = 2000;
 
+        /// <summary>Schema corrente. Alzarlo fa girare <see cref="Migra"/> una volta sola.</summary>
+        public const int SchemaCorrente = 1;
+
+        /// <summary>
+        /// Porta avanti un file scritto da una versione precedente. Restituisce true se
+        /// qualcosa è cambiato, così chi chiama sa che va risalvato.
+        ///
+        /// <para><b>Perché serve.</b> Il valore su disco vince sui predefiniti, ed è
+        /// giusto: sono scelte dell'utente. Ma due di quei valori non erano scelte, erano
+        /// conseguenze di misure sbagliate — il tetto di 40 richieste al secondo e
+        /// l'anticipo di 1000 ms erano stati tarati quando il tetto stesso accodava le
+        /// chiamate e gonfiava il ping a 444 ms. Senza migrazione, chi aggiorna si ritrova
+        /// le correzioni di oggi già annullate dal proprio file, e conclude che non
+        /// funzionano.</para>
+        ///
+        /// <para>Si tocca solo chi ha ancora esattamente il vecchio valore. Chi lo aveva
+        /// già cambiato di suo se lo tiene: quella è una scelta, e non spetta a un
+        /// aggiornamento ribaltarla.</para>
+        /// </summary>
+        public static bool Migra(AppSettings s)
+        {
+            if (s == null || s.SettingsSchemaVersion >= SchemaCorrente) return false;
+
+            if (Math.Abs(s.MaxRequestsPerSecond - 40) < 0.001) s.MaxRequestsPerSecond = 0;
+            if (s.DefaultBidBeforeDeadlineMs == 1000) s.DefaultBidBeforeDeadlineMs = 500;
+
+            s.SettingsSchemaVersion = SchemaCorrente;
+            return true;
+        }
+
         public static AppSettings Load()
         {
             // Percorso comune: nessun lucchetto, nessuna allocazione.
@@ -848,11 +889,20 @@ namespace AutoBidder.Utilities
                     return cached;
 
                 AppSettings loaded;
+                var daRisalvare = false;
+
                 try
                 {
-                    loaded = File.Exists(_file)
+                    var esiste = File.Exists(_file);
+
+                    loaded = esiste
                         ? JsonSerializer.Deserialize<AppSettings>(File.ReadAllText(_file)) ?? new AppSettings()
                         : new AppSettings();
+
+                    // Un file nuovo nasce già allo schema corrente: la migrazione riguarda
+                    // solo chi arriva da una versione precedente.
+                    if (!esiste) loaded.SettingsSchemaVersion = SchemaCorrente;
+                    else daRisalvare = Migra(loaded);
                 }
                 catch
                 {
@@ -860,6 +910,15 @@ namespace AutoBidder.Utilities
                     loaded = _cached ?? new AppSettings();
                 }
 
+                // Scritto qui e non dentro Save: Load è sul percorso caldo e non deve
+                // toccare il disco, ma una migrazione capita una volta sola nella vita
+                // del file, e lasciarla non salvata la farebbe ripetere a ogni avvio.
+                if (daRisalvare)
+                {
+                    try { File.WriteAllText(_file, JsonSerializer.Serialize(loaded, new JsonSerializerOptions { WriteIndented = true })); }
+                    catch { /* si riproverà al prossimo avvio */ }
+                }
+
                 _cached = loaded;
                 Interlocked.Exchange(ref _cacheExpiryTicks, DateTime.UtcNow.AddMilliseconds(CACHE_TTL_MS).Ticks);
                 return loaded;