Address review: cache parsed LTS versions, unify version parsing, add offline-path and dir/bean guard tests

application/src/main/java/org/thingsboard/server/service/install/DefaultDatabaseSchemaSettingsService.java

@@ -19,6 +19,7 @@ import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.stereotype.Service;
+import org.thingsboard.server.service.install.lts.LtsVersion;
 import org.thingsboard.server.service.install.update.DefaultDataUpdateService;
 
 import java.util.Map;
@@ -132,12 +133,8 @@ public class DefaultDatabaseSchemaSettingsService implements DatabaseSchemaSetti
     }
 
     private long toDbVersion(String version) {
-        String[] versionParts = version.split("\\.");
-        long major = Long.parseLong(versionParts[0]);
-        long minor = versionParts.length > 1 ? Long.parseLong(versionParts[1]) : 0;
-        long maintenance = versionParts.length > 2 ? Long.parseLong(versionParts[2]) : 0;
-        long patch = versionParts.length > 3 ? Long.parseLong(versionParts[3]) : 0;
-        return major * 1_000_000_000L + minor * 1_000_000L + maintenance * 1000L + patch;
+        LtsVersion v = LtsVersion.parse(version);
+        return v.major() * 1_000_000_000L + v.minor() * 1_000_000L + v.maintenance() * 1000L + v.patch();
     }
 
     private void onSchemaSettingsError(String message) {
@@ -146,14 +143,7 @@ public class DefaultDatabaseSchemaSettingsService implements DatabaseSchemaSetti
     }
 
     private String normalizeVersion(String version) {
-        String[] parts = version.split("\\.");
-
-        int major = Integer.parseInt(parts[0]);
-        int minor = parts.length > 1 ? Integer.parseInt(parts[1]) : 0;
-        int maintenance = parts.length > 2 ? Integer.parseInt(parts[2]) : 0;
-        int patch = parts.length > 3 ? Integer.parseInt(parts[3]) : 0;
-
-        return major + "." + minor + "." + maintenance + "." + patch;
+        return LtsVersion.parse(version).toString();
     }
 
 }

application/src/main/java/org/thingsboard/server/service/install/lts/LtsMigrationService.java

@@ -29,6 +29,7 @@ import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
@@ -41,11 +42,14 @@ public class LtsMigrationService {
 
     private static final String SCHEMA_UPDATE_SQL = "schema_update.sql";
 
+    /** A migration paired with its parsed version, so the version is parsed exactly once per bean. */
+    private record VersionedMigration(LtsVersion version, LtsMigration migration) {}
+
     private final JdbcTemplate jdbcTemplate;
     private final InstallScripts installScripts;
     private final DatabaseSchemaSettingsService schemaSettingsService;
     private final TransactionTemplate transactionTemplate;
-    private final List<LtsMigration> migrations;
+    private final List<VersionedMigration> migrations;
 
     public LtsMigrationService(JdbcTemplate jdbcTemplate,
                                InstallScripts installScripts,
@@ -58,25 +62,28 @@ public class LtsMigrationService {
         this.transactionTemplate = new TransactionTemplate(transactionManager);
         this.migrations = validateAndSort(migrations);
         log.info("Discovered {} LTS migration(s): {}", this.migrations.size(),
-                this.migrations.stream().map(LtsMigration::getVersion).toList());
+                this.migrations.stream().map(vm -> vm.migration().getVersion()).toList());
     }
 
-    private static List<LtsMigration> validateAndSort(List<LtsMigration> migrations) {
+    private static List<VersionedMigration> validateAndSort(List<LtsMigration> migrations) {
         Set<String> seen = new HashSet<>();
+        List<VersionedMigration> versioned = new ArrayList<>();
         for (LtsMigration m : migrations) {
-            LtsVersion.parse(m.getVersion()); // fail loud on unparseable version
+            LtsVersion version = LtsVersion.parse(m.getVersion()); // fail loud on unparseable version
             if (!seen.add(m.getVersion())) {
                 throw new IllegalStateException("Duplicate LTS migration version: " + m.getVersion());
             }
+            versioned.add(new VersionedMigration(version, m));
         }
-        return migrations.stream()
-                .sorted(Comparator.comparing(m -> LtsVersion.parse(m.getVersion())))
+        return versioned.stream()
+                .sorted(Comparator.comparing(VersionedMigration::version))
                 .toList();
     }
 
     /** No-downtime path: per migration in (from, to] run SQL, then apply(), then record the version. */
     public void applyMigrations(String fromVersion, String toVersion) {
-        for (LtsMigration migration : select(fromVersion, toVersion)) {
+        for (VersionedMigration vm : select(fromVersion, toVersion)) {
+            LtsMigration migration = vm.migration();
             String version = migration.getVersion();
             transactionTemplate.executeWithoutResult(status -> {
                 runSchemaUpdate(version);
@@ -89,8 +96,8 @@ public class LtsMigrationService {
 
     /** Offline major-upgrade schema phase: per migration in (from, to] run SQL only. No version record. */
     public void runSchemaMigrations(String fromVersion, String toVersion) {
-        for (LtsMigration migration : select(fromVersion, toVersion)) {
-            String version = migration.getVersion();
+        for (VersionedMigration vm : select(fromVersion, toVersion)) {
+            String version = vm.migration().getVersion();
             transactionTemplate.executeWithoutResult(status -> runSchemaUpdate(version));
             log.info("Applied LTS schema migration {}", version);
         }
@@ -98,31 +105,32 @@ public class LtsMigrationService {
 
     /** Offline major-upgrade data phase: per migration in (from, to] run apply() only. No SQL, no version record. */
     public void runDataMigrations(String fromVersion, String toVersion) {
-        for (LtsMigration migration : select(fromVersion, toVersion)) {
-            migration.apply();
-            log.info("Applied LTS data migration {}", migration.getVersion());
+        for (VersionedMigration vm : select(fromVersion, toVersion)) {
+            vm.migration().apply();
+            log.info("Applied LTS data migration {}", vm.migration().getVersion());
         }
     }
 
-    private List<LtsMigration> select(String fromVersion, String toVersion) {
+    private List<VersionedMigration> select(String fromVersion, String toVersion) {
         LtsVersion from = LtsVersion.parse(fromVersion);
         LtsVersion to = LtsVersion.parse(toVersion);
         return migrations.stream()
-                .filter(migration -> {
-                    LtsVersion version = LtsVersion.parse(migration.getVersion());
-                    // Run only migrations whose family matches the target version. Older-family
-                    // migrations (e.g. 4.2.x) ride onto newer-family branches (e.g. 4.3.x) via the
-                    // release-merge cascade, but each branch's own family of migrations is
-                    // self-contained (newer-family copies reproduce the older schema/data changes),
-                    // so a cross-family upgrade is fully handled by the target-family migrations.
-                    // Excluding the dormant older-family beans avoids double-processing.
-                    return version.sameFamily(to)
-                            && version.compareTo(from) > 0
-                            && version.compareTo(to) <= 0;
-                })
+                .filter(vm -> isInRangeForTargetFamily(vm.version(), from, to))
                 .toList();
     }
 
+    // Run only migrations whose family matches the target version. Older-family
+    // migrations (e.g. 4.2.x) ride onto newer-family branches (e.g. 4.3.x) via the
+    // release-merge cascade, but each branch's own family of migrations is
+    // self-contained (newer-family copies reproduce the older schema/data changes),
+    // so a cross-family upgrade is fully handled by the target-family migrations.
+    // Excluding the dormant older-family beans avoids double-processing.
+    static boolean isInRangeForTargetFamily(LtsVersion version, LtsVersion from, LtsVersion to) {
+        return version.sameFamily(to)
+                && version.compareTo(from) > 0
+                && version.compareTo(to) <= 0;
+    }
+
     private void runSchemaUpdate(String version) {
         Path sqlFile = Paths.get(installScripts.getDataDir(), "upgrade", "lts", version, SCHEMA_UPDATE_SQL);
         if (!Files.exists(sqlFile)) {

application/src/test/java/org/thingsboard/server/service/install/lts/LtsMigrationIntegrationTest.java

@@ -31,9 +31,18 @@ import org.thingsboard.server.controller.AbstractControllerTest;
 import org.thingsboard.server.dao.service.DaoSqlTest;
 import org.thingsboard.server.dao.widget.WidgetTypeService;
 import org.thingsboard.server.dao.widget.WidgetsBundleService;
+import org.thingsboard.server.service.install.InstallScripts;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -47,6 +56,10 @@ public class LtsMigrationIntegrationTest extends AbstractControllerTest {
     private static final long V_4_2_2_3 = 4_002_002_003L;
     private static final String OBSOLETE_ALIAS = "air_quality";
 
+    // Versions whose family is older than the current package family ship SQL-less beans intentionally
+    // (their schema/data changes are reproduced by the current-family migrations), so a missing dir is OK.
+    private static final Set<String> SQL_LESS_ALLOWED = Set.of();
+
     @Autowired
     private LtsMigrationService ltsMigrationService;
     @Autowired
@@ -55,6 +68,10 @@ public class LtsMigrationIntegrationTest extends AbstractControllerTest {
     private WidgetTypeService widgetTypeService;
     @Autowired
     private JdbcTemplate jdbcTemplate;
+    @Autowired
+    private InstallScripts installScripts;
+    @Autowired
+    private List<LtsMigration> migrations;
 
     private Long originalSchemaVersion;
     private WidgetsBundleId bundleId;
@@ -136,6 +153,64 @@ public class LtsMigrationIntegrationTest extends AbstractControllerTest {
         assertTrue(widgetTypeService.findWidgetTypeDetailsById(TenantId.SYS_TENANT_ID, widgetTypeId).isDeprecated());
     }
 
+    @Test
+    public void offlinePathRunsSchemaThenDataButRecordsNoVersion() {
+        // Drive the offline major-upgrade path over the real supported range against the real DB.
+        ltsMigrationService.runSchemaMigrations("4.2.2.2", "4.2.2.3");
+        // (a) the schema effects landed: the table the 4.2.2.3 SQL creates now exists.
+        assertTrue(tableExists("iot_hub_installed_item"));
+        // (c) the offline schema phase records NO schema version (unlike applyMigrations).
+        assertEquals(Long.valueOf(V_4_2_2_2),
+                jdbcTemplate.queryForObject("SELECT schema_version FROM tb_schema_settings", Long.class));
+
+        ltsMigrationService.runDataMigrations("4.2.2.2", "4.2.2.3");
+        // (b) the data apply() ran: the obsolete bundle was deleted and its type marked deprecated.
+        assertNull(widgetsBundleService.findWidgetsBundleByTenantIdAndAlias(TenantId.SYS_TENANT_ID, OBSOLETE_ALIAS));
+        WidgetTypeDetails type = widgetTypeService.findWidgetTypeDetailsById(TenantId.SYS_TENANT_ID, widgetTypeId);
+        assertNotNull(type);
+        assertTrue(type.isDeprecated());
+        // (c) the offline data phase also records NO schema version.
+        assertEquals(Long.valueOf(V_4_2_2_2),
+                jdbcTemplate.queryForObject("SELECT schema_version FROM tb_schema_settings", Long.class));
+    }
+
+    @Test
+    public void migrationDirectoriesAndBeansStayInSyncBothWays() {
+        Path ltsDir = Paths.get(installScripts.getDataDir(), "upgrade", "lts");
+        Set<String> dirVersions = listDirVersions(ltsDir);
+        Set<String> beanVersions = migrations.stream().map(LtsMigration::getVersion).collect(Collectors.toSet());
+
+        // Every on-disk migration directory must have a registered bean with the same version.
+        // Otherwise select() (which iterates beans, not dirs) silently skips the SQL dir.
+        Set<String> dirsWithoutBean = dirVersions.stream()
+                .filter(v -> !beanVersions.contains(v))
+                .collect(Collectors.toSet());
+        assertTrue("Migration directories without a registered LtsMigration bean: " + dirsWithoutBean,
+                dirsWithoutBean.isEmpty());
+
+        // Every registered bean must have a matching directory, unless it is explicitly allowed to be SQL-less.
+        // Otherwise a typo'd dir name silently runs no SQL for that bean.
+        Set<String> beansWithoutDir = beanVersions.stream()
+                .filter(v -> !dirVersions.contains(v))
+                .filter(v -> !SQL_LESS_ALLOWED.contains(v))
+                .collect(Collectors.toSet());
+        assertTrue("Registered LtsMigration beans without a matching directory (and not SQL-less allowed): " + beansWithoutDir,
+                beansWithoutDir.isEmpty());
+    }
+
+    private Set<String> listDirVersions(Path ltsDir) {
+        if (!Files.isDirectory(ltsDir)) {
+            return Set.of();
+        }
+        try (Stream<Path> entries = Files.list(ltsDir)) {
+            return entries.filter(Files::isDirectory)
+                    .map(p -> p.getFileName().toString())
+                    .collect(Collectors.toSet());
+        } catch (IOException e) {
+            throw new UncheckedIOException("Failed to list LTS migration directories: " + ltsDir, e);
+        }
+    }
+
     private boolean tableExists(String table) {
         Boolean exists = jdbcTemplate.queryForObject(
                 "SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = ?)", Boolean.class, table);

application/src/test/java/org/thingsboard/server/service/install/lts/LtsMigrationServiceTest.java

@@ -31,7 +31,9 @@ import java.util.ArrayList;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.never;
@@ -128,6 +130,58 @@ class LtsMigrationServiceTest {
         assertEquals(List.of("4.3.1.2", "4.3.1.3"), applied);
     }
 
+    @Test
+    void applyMigrationsSkipsMigrationsOutsideTargetFamilyOnCrossFamilyUpgrade() {
+        List<String> applied = new ArrayList<>();
+        // The dormant 4.2.2.3 bean must not apply or record on a cross-family 4.2 -> 4.3 upgrade.
+        LtsMigrationService service = service(List.of(
+                migration("4.2.2.3", applied),
+                migration("4.3.1.2", applied),
+                migration("4.3.1.3", applied)));
+
+        service.applyMigrations("4.2.2.2", "4.3.1.3");
+
+        assertEquals(List.of("4.3.1.2", "4.3.1.3"), applied);
+        verify(schemaSettingsService, never()).updateSchemaVersion("4.2.2.3");
+        verify(schemaSettingsService).updateSchemaVersion("4.3.1.2");
+        verify(schemaSettingsService).updateSchemaVersion("4.3.1.3");
+    }
+
+    @Test
+    void runSchemaMigrationsSkipsMigrationsOutsideTargetFamilyOnCrossFamilyUpgrade() throws Exception {
+        List<String> applied = new ArrayList<>();
+        writeSql("4.2.2.3", "SELECT 1;");
+        writeSql("4.3.1.2", "SELECT 2;");
+        writeSql("4.3.1.3", "SELECT 3;");
+        LtsMigrationService service = service(List.of(
+                migration("4.2.2.3", applied),
+                migration("4.3.1.2", applied),
+                migration("4.3.1.3", applied)));
+
+        service.runSchemaMigrations("4.2.2.2", "4.3.1.3");
+
+        // The dormant 4.2.2.3 SQL must not run; the 4.3-family SQL must.
+        verify(jdbcTemplate, never()).execute("SELECT 1;");
+        verify(jdbcTemplate).execute("SELECT 2;");
+        verify(jdbcTemplate).execute("SELECT 3;");
+    }
+
+    @Test
+    void isInRangeForTargetFamilyPredicate() {
+        LtsVersion from = LtsVersion.parse("4.3.1.1");
+        LtsVersion to = LtsVersion.parse("4.3.1.3");
+        // same-family in range
+        assertTrue(LtsMigrationService.isInRangeForTargetFamily(LtsVersion.parse("4.3.1.2"), from, to));
+        // older-family bean within the numeric range but wrong family
+        assertFalse(LtsMigrationService.isInRangeForTargetFamily(LtsVersion.parse("4.2.2.3"), from, to));
+        // the target itself (upper boundary, inclusive)
+        assertTrue(LtsMigrationService.isInRangeForTargetFamily(to, from, to));
+        // at from (lower boundary, exclusive)
+        assertFalse(LtsMigrationService.isInRangeForTargetFamily(from, from, to));
+        // below from
+        assertFalse(LtsMigrationService.isInRangeForTargetFamily(LtsVersion.parse("4.3.1.0"), from, to));
+    }
+
     @Test
     void reRunAtCurrentVersionIsNoOp() throws Exception {
         List<String> applied = new ArrayList<>();