[jdbc] Add safety valve for suspicious migrations (openhab#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>
psmedley · Feb 23, 2023 · b2ef2a1 · b2ef2a1
1 parent f7da567
commit b2ef2a1
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 1 deletion.
diff --git 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/...nhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java b/...nhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java
@@ -49,8 +49,9 @@
  */
 @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 @@ private void formatTableNames() throws JdbcSQLException {
                 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/...ence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java b/...ence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java
@@ -322,6 +322,12 @@ public Map<String, String> getItemNameToTableNameMap() {
      */
     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 Collection<String> getSchemaIssues(String tableName, String itemName) thr
      * @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 @@ private ItemTableCheckEntry getCheckedEntry(String itemName, String tableName, b
      * @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;