From 3912487305794bbc926cbaaf0c2d1ef8e33dece6 Mon Sep 17 00:00:00 2001 From: Jacob Laursen Date: Mon, 28 Nov 2022 23:38:55 +0100 Subject: [PATCH] [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 --- .../org.openhab.persistence.jdbc/README.md | 6 +++++ .../persistence/jdbc/internal/JdbcMapper.java | 22 ++++++++++++++++++- .../jdbc/internal/JdbcPersistenceService.java | 16 ++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/bundles/org.openhab.persistence.jdbc/README.md b/bundles/org.openhab.persistence.jdbc/README.md index fbb4ac3d106..611814358f8 100644 --- 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 ` (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 1b3c1652262..7a510527fa3 100644 --- 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 af7da62f953..28fc2c5a973 100644 --- 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 getSchemaIssues(String tableName, String itemName) throws JdbcSQLException { List 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;