[jdbc] Add safety valve for suspicious migrations (#13797)

* Abort migration from real names when most tables have table name prefix
* Add missing checks for database connection from console commands
* Add additional documentation for check/fix schema

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

diff --git a/bundles/org.openhab.persistence.jdbc/README.md b/bundles/org.openhab.persistence.jdbc/README.md
index fbb4ac3d10695a70d60651334ee6f8276a71cc97..611814358f8bddfb0d3df1f4938fab0c31a08a9a 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/README.md
+++ b/bundles/org.openhab.persistence.jdbc/README.md
@@ -214,6 +214,12 @@ Use the command `jdbc schema check` to perform an integrity check of the schema.
 
 Identified issues can be fixed automatically using the command `jdbc schema fix` (all items having issues) or `jdbc schema fix <itemName>` (single item).
 
+Issues than can be identified and possibly fixed:
+
+- Wrong column name case (`time` and `name`).
+- Wrong column type. Before fixing this, make sure that time-zone is correctly configured.
+- Unexpected column (identify only).
+
 ### For Developers
 
 * Clearly separated source files for the database-specific part of openHAB logic.

diff --git a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java
index 1b3c165226284b9d4ed9a436016cf897ba71ab2b..7a510527fa3949412208a4de49e11cbffa567858 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java
+++ b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java
@@ -49,8 +49,9 @@ import com.zaxxer.hikari.pool.HikariPool.PoolInitializationException;
  */
 @NonNullByDefault
 public class JdbcMapper {
-    private final Logger logger = LoggerFactory.getLogger(JdbcMapper.class);
+    private static final int MIGRATION_PERCENTAGE_THRESHOLD = 50;
 
+    private final Logger logger = LoggerFactory.getLogger(JdbcMapper.class);
     private final TimeZoneProvider timeZoneProvider;
 
     // Error counter - used to reconnect to database on error
@@ -407,6 +408,25 @@ public class JdbcMapper {
                 initialized = tmpinit;
                 return;
             }
+            // Safety valve to prevent accidental migrations.
+            int numberOfTables = itemTables.size();
+            if (numberOfTables > 0) {
+                String prefix = conf.getTableNamePrefix();
+                long numberOfItemsWithPrefix = itemTables.stream()
+                        .filter(i -> i.startsWith(prefix) || i.toLowerCase().startsWith("item")).count();
+                long percentageWithPrefix = (numberOfItemsWithPrefix * 100) / itemTables.size();
+                if (!prefix.isBlank() && percentageWithPrefix >= MIGRATION_PERCENTAGE_THRESHOLD) {
+                    logger.error(
+                            "JDBC::formatTableNames: {}% of all tables start with table name prefix '{}' or 'item', but items manage table '{}' was not found or is empty. Check configuration parameter 'itemsManageTable'",
+                            percentageWithPrefix, conf.getTableNamePrefix(), conf.getItemsManageTable());
+                    if (ifTableExists("items")) {
+                        logger.error(
+                                "JDBC::formatTableNames: Table 'items' was found, consider updating configuration parameter 'itemsManageTable' accordingly");
+                    }
+                    initialized = tmpinit;
+                    return;
+                }
+            }
             oldNewTableNames = new ArrayList<>();
             for (String itemName : itemTables) {
                 ItemsVO isvo = new ItemsVO();

diff --git a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java
index af7da62f953ccd6de5c58f6c8b954489b55d8c7f..28fc2c5a9730a127fe23878b6f59dfce2fd7df80 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java
+++ b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java
@@ -322,6 +322,12 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
      */
     public Collection<String> getSchemaIssues(String tableName, String itemName) throws JdbcSQLException {
         List<String> issues = new ArrayList<>();
+
+        if (!checkDBAccessability()) {
+            logger.warn("JDBC::getSchemaIssues: database not connected");
+            return issues;
+        }
+
         Item item;
         try {
             item = itemRegistry.getItem(itemName);
@@ -375,6 +381,11 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
      * @throws JdbcSQLException on SQL errors
      */
     public boolean fixSchemaIssues(String tableName, String itemName) throws JdbcSQLException {
+        if (!checkDBAccessability()) {
+            logger.warn("JDBC::fixSchemaIssues: database not connected");
+            return false;
+        }
+
         Item item;
         try {
             item = itemRegistry.getItem(itemName);
@@ -469,6 +480,11 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
      * @throws JdbcSQLException
      */
     public boolean cleanupItem(String itemName, boolean force) throws JdbcSQLException {
+        if (!checkDBAccessability()) {
+            logger.warn("JDBC::cleanupItem: database not connected");
+            return false;
+        }
+
         String tableName = itemNameToTableNameMap.get(itemName);
         if (tableName == null) {
             return false;