[jdbc] Improve error handling (#13719)

* Enable wrapped exceptions being thrown by Yank

Fixes #13718

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

* Fix SAT warning about hashCode implementation

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

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

bundles/org.openhab.persistence.jdbc/pom.xml

@@ -22,7 +22,7 @@
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <hikari.version>2.4.7</hikari.version>
     <dbutils.version>1.6</dbutils.version>
-    <yank.version>3.2.0</yank.version>
+    <yank.version>3.4.0</yank.version>
 
     <!-- JDBC database driver versions -->
     <derby.version>10.14.2.0</derby.version>

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/console/JdbcCommandExtension.java

@@ -19,6 +19,7 @@ import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.knowm.yank.exceptions.YankSQLException;
 import org.openhab.core.io.console.Console;
 import org.openhab.core.io.console.ConsoleCommandCompleter;
 import org.openhab.core.io.console.StringsCompleter;
@@ -70,22 +71,14 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
         if (persistenceService == null) {
             return;
         }
-        if (SUBCMD_TABLES_LIST.equalsIgnoreCase(args[1])) {
+        try {
-            listTables(persistenceService, console, args.length == 3 && PARAMETER_ALL.equalsIgnoreCase(args[2]));
+            if (!execute(persistenceService, args, console)) {
-            return;
-        } else if (SUBCMD_TABLES_CLEAN.equalsIgnoreCase(args[1])) {
-            if (args.length == 3) {
-                cleanupItem(persistenceService, console, args[2], false);
-                return;
-            } else if (args.length == 4 && PARAMETER_FORCE.equalsIgnoreCase(args[3])) {
-                cleanupItem(persistenceService, console, args[2], true);
-                return;
-            } else {
-                cleanupTables(persistenceService, console);
-                return;
-            }
-        }
                 printUsage(console);
+                return;
+            }
+        } catch (YankSQLException e) {
+            console.println(e.toString());
+        }
     }
 
     private @Nullable JdbcPersistenceService getPersistenceService() {
@@ -97,12 +90,32 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
         return null;
     }
 
+    private boolean execute(JdbcPersistenceService persistenceService, String[] args, Console console) {
+        if (SUBCMD_TABLES_LIST.equalsIgnoreCase(args[1])) {
+            listTables(persistenceService, console, args.length == 3 && PARAMETER_ALL.equalsIgnoreCase(args[2]));
+            return true;
+        } else if (SUBCMD_TABLES_CLEAN.equalsIgnoreCase(args[1])) {
+            if (args.length == 3) {
+                cleanupItem(persistenceService, console, args[2], false);
+                return true;
+            } else if (args.length == 4 && PARAMETER_FORCE.equalsIgnoreCase(args[3])) {
+                cleanupItem(persistenceService, console, args[2], true);
+                return true;
+            } else {
+                cleanupTables(persistenceService, console);
+                return true;
+            }
+        }
+        return false;
+    }
+
     private void listTables(JdbcPersistenceService persistenceService, Console console, Boolean all) {
         List<ItemTableCheckEntry> entries = persistenceService.getCheckedEntries();
         if (!all) {
             entries.removeIf(t -> t.getStatus() == ItemTableCheckEntryStatus.VALID);
         }
         entries.sort(Comparator.comparing(ItemTableCheckEntry::getTableName));
+        // FIXME: NoSuchElement when empty table - because of get()
         int itemNameMaxLength = Math
                 .max(entries.stream().map(t -> t.getItemName().length()).max(Integer::compare).get(), 4);
         int tableNameMaxLength = Math

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/dto/ItemsVO.java

@@ -13,6 +13,7 @@
 package org.openhab.persistence.jdbc.dto;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 /**
  * Represents the table naming data.
@@ -96,11 +97,7 @@ public class ItemsVO implements Serializable {
      */
     @Override
     public int hashCode() {
-        final int prime = 31;
+        return Objects.hash(itemName, itemId);
-        int result = 1;
-        result = prime * result + ((itemName == null) ? 0 : itemName.hashCode());
-        result = prime * result + (itemId ^ (itemId >>> 32));
-        return result;
     }
 
     /*

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java

@@ -25,6 +25,7 @@ import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.knowm.yank.Yank;
+import org.knowm.yank.exceptions.YankSQLException;
 import org.openhab.core.i18n.TimeZoneProvider;
 import org.openhab.core.items.Item;
 import org.openhab.core.persistence.FilterCriteria;
@@ -66,7 +67,7 @@ public class JdbcMapper {
     /*****************
      * MAPPER ITEMS *
      *****************/
-    public boolean pingDB() {
+    private boolean pingDB() {
         logger.debug("JDBC::pingDB");
         boolean ret = false;
         long timerStart = System.currentTimeMillis();
@@ -90,15 +91,7 @@ public class JdbcMapper {
         return ret;
     }
 
-    public String getDB() {
+    private boolean ifItemsTableExists() {
-        logger.debug("JDBC::getDB");
-        long timerStart = System.currentTimeMillis();
-        String res = conf.getDBDAO().doGetDB();
-        logTime("getDB", timerStart, System.currentTimeMillis());
-        return res != null ? res : "";
-    }
-
-    public boolean ifItemsTableExists() {
         logger.debug("JDBC::ifItemsTableExists");
         long timerStart = System.currentTimeMillis();
         boolean res = conf.getDBDAO().doIfTableExists(new ItemsVO());
@@ -106,7 +99,7 @@ public class JdbcMapper {
         return res;
     }
 
-    public boolean ifTableExists(String tableName) {
+    protected boolean ifTableExists(String tableName) {
         logger.debug("JDBC::ifTableExists");
         long timerStart = System.currentTimeMillis();
         boolean res = conf.getDBDAO().doIfTableExists(tableName);
@@ -114,7 +107,7 @@ public class JdbcMapper {
         return res;
     }
 
-    public ItemsVO createNewEntryInItemsTable(ItemsVO vo) {
+    private ItemsVO createNewEntryInItemsTable(ItemsVO vo) {
         logger.debug("JDBC::createNewEntryInItemsTable");
         long timerStart = System.currentTimeMillis();
         Long i = conf.getDBDAO().doCreateNewEntryInItemsTable(vo);
@@ -123,7 +116,7 @@ public class JdbcMapper {
         return vo;
     }
 
-    public boolean createItemsTableIfNot(ItemsVO vo) {
+    private boolean createItemsTableIfNot(ItemsVO vo) {
         logger.debug("JDBC::createItemsTableIfNot");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doCreateItemsTableIfNot(vo);
@@ -131,7 +124,7 @@ public class JdbcMapper {
         return true;
     }
 
-    public boolean dropItemsTableIfExists(ItemsVO vo) {
+    private boolean dropItemsTableIfExists(ItemsVO vo) {
         logger.debug("JDBC::dropItemsTableIfExists");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doDropItemsTableIfExists(vo);
@@ -139,14 +132,14 @@ public class JdbcMapper {
         return true;
     }
 
-    public void dropTable(String tableName) {
+    protected void dropTable(String tableName) {
         logger.debug("JDBC::dropTable");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doDropTable(tableName);
         logTime("doDropTable", timerStart, System.currentTimeMillis());
     }
 
-    public ItemsVO deleteItemsEntry(ItemsVO vo) {
+    protected ItemsVO deleteItemsEntry(ItemsVO vo) {
         logger.debug("JDBC::deleteItemsEntry");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doDeleteItemsEntry(vo);
@@ -154,7 +147,7 @@ public class JdbcMapper {
         return vo;
     }
 
-    public List<ItemsVO> getItemIDTableNames() {
+    private List<ItemsVO> getItemIDTableNames() {
         logger.debug("JDBC::getItemIDTableNames");
         long timerStart = System.currentTimeMillis();
         List<ItemsVO> vo = conf.getDBDAO().doGetItemIDTableNames(new ItemsVO());
@@ -162,7 +155,7 @@ public class JdbcMapper {
         return vo;
     }
 
-    public List<ItemsVO> getItemTables() {
+    protected List<ItemsVO> getItemTables() {
         logger.debug("JDBC::getItemTables");
         long timerStart = System.currentTimeMillis();
         ItemsVO vo = new ItemsVO();
@@ -175,14 +168,14 @@ public class JdbcMapper {
     /****************
      * MAPPERS ITEM *
      ****************/
-    public void updateItemTableNames(List<ItemVO> vol) {
+    private void updateItemTableNames(List<ItemVO> vol) {
         logger.debug("JDBC::updateItemTableNames");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doUpdateItemTableNames(vol);
         logTime("updateItemTableNames", timerStart, System.currentTimeMillis());
     }
 
-    public ItemVO createItemTable(ItemVO vo) {
+    private ItemVO createItemTable(ItemVO vo) {
         logger.debug("JDBC::createItemTable");
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doCreateItemTable(vo);
@@ -190,7 +183,7 @@ public class JdbcMapper {
         return vo;
     }
 
-    public Item storeItemValue(Item item, State itemState, @Nullable ZonedDateTime date) {
+    protected Item storeItemValue(Item item, State itemState, @Nullable ZonedDateTime date) {
         logger.debug("JDBC::storeItemValue: item={} state={} date={}", item, itemState, date);
         String tableName = getTable(item);
         long timerStart = System.currentTimeMillis();
@@ -208,7 +201,7 @@ public class JdbcMapper {
         return conf.getDBDAO().doGetRowCount(tableName);
     }
 
-    public List<HistoricItem> getHistItemFilterQuery(FilterCriteria filter, int numberDecimalcount, String table,
+    protected List<HistoricItem> getHistItemFilterQuery(FilterCriteria filter, int numberDecimalcount, String table,
             Item item) {
         logger.debug(
                 "JDBC::getHistItemFilterQuery filter='{}' numberDecimalcount='{}' table='{}' item='{}' itemName='{}'",
@@ -221,13 +214,12 @@ public class JdbcMapper {
         return result;
     }
 
-    public boolean deleteItemValues(FilterCriteria filter, String table) {
+    protected void deleteItemValues(FilterCriteria filter, String table) {
         logger.debug("JDBC::deleteItemValues filter='{}' table='{}' itemName='{}'", true, table, filter.getItemName());
         long timerStart = System.currentTimeMillis();
         conf.getDBDAO().doDeleteItemValues(filter, table, timeZoneProvider.getTimeZone());
         logTime("deleteItemValues", timerStart, System.currentTimeMillis());
         errCnt = 0;
-        return true;
     }
 
     /***********************
@@ -239,6 +231,7 @@ public class JdbcMapper {
             logger.info("JDBC::openConnection: Driver is available::Yank setupDataSource");
             try {
                 Yank.setupDefaultConnectionPool(conf.getHikariConfiguration());
+                Yank.setThrowWrappedExceptions(true);
                 conf.setDbConnected(true);
                 return true;
             } catch (PoolInitializationException e) {
@@ -271,6 +264,7 @@ public class JdbcMapper {
         if (initialized) {
             return true;
         }
+        try {
             // first
             boolean p = pingDB();
             if (p) {
@@ -282,6 +276,10 @@ public class JdbcMapper {
                 logger.debug("JDBC::checkDBAcessability, second try connection: {}", p);
                 return (p && !(conf.getErrReconnectThreshold() > 0 && errCnt <= conf.getErrReconnectThreshold()));
             }
+        } catch (YankSQLException e) {
+            logger.warn("Unable to ping database", e);
+            return false;
+        }
     }
 
     /**************************
@@ -412,7 +410,7 @@ public class JdbcMapper {
         initialized = tmpinit;
     }
 
-    public Set<PersistenceItemInfo> getItems() {
+    protected Set<PersistenceItemInfo> getItems() {
         // TODO: in general it would be possible to query the count, earliest and latest values for each item too but it
         // would be a very costly operation
         return itemNameToTableNameMap.keySet().stream().map(itemName -> new JdbcPersistenceItemInfo(itemName))

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java

@@ -25,6 +25,7 @@ import java.util.stream.Collectors;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.knowm.yank.exceptions.YankSQLException;
 import org.openhab.core.config.core.ConfigurableService;
 import org.openhab.core.i18n.TimeZoneProvider;
 import org.openhab.core.items.GroupItem;
@@ -155,12 +156,16 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
                     state, item, errCnt, conf.getErrReconnectThreshold());
             return;
         }
+        try {
             long timerStart = System.currentTimeMillis();
             storeItemValue(item, state, date);
             if (logger.isDebugEnabled()) {
                 logger.debug("JDBC: Stored item '{}' as '{}' in SQL database at {} in {} ms.", item.getName(), state,
                         new Date(), System.currentTimeMillis() - timerStart);
             }
+        } catch (YankSQLException e) {
+            logger.warn("JDBC::store: Unable to store item", e);
+        }
     }
 
     @Override
@@ -215,16 +220,20 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
             return List.of();
         }
 
+        try {
             long timerStart = System.currentTimeMillis();
             List<HistoricItem> items = getHistItemFilterQuery(filter, conf.getNumberDecimalcount(), table, item);
             if (logger.isDebugEnabled()) {
                 logger.debug("JDBC: Query for item '{}' returned {} rows in {} ms", itemName, items.size(),
                         System.currentTimeMillis() - timerStart);
             }
-
             // Success
             errCnt = 0;
             return items;
+        } catch (YankSQLException e) {
+            logger.warn("JDBC::query: Unable to query item", e);
+            return List.of();
+        }
     }
 
     public void updateConfig(Map<Object, Object> configuration) {
@@ -233,9 +242,14 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
         conf = new JdbcConfiguration(configuration);
         if (conf.valid && checkDBAccessability()) {
             namingStrategy = new NamingStrategy(conf);
+            try {
                 checkDBSchema();
                 // connection has been established ... initialization completed!
                 initialized = true;
+            } catch (YankSQLException e) {
+                logger.error("Failed to check database schema", e);
+                initialized = false;
+            }
         } else {
             initialized = false;
         }
@@ -269,14 +283,18 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
             return false;
         }
 
+        try {
             long timerStart = System.currentTimeMillis();
-        boolean result = deleteItemValues(filter, table);
+            deleteItemValues(filter, table);
             if (logger.isDebugEnabled()) {
                 logger.debug("JDBC: Deleted values for item '{}' in SQL database at {} in {} ms.", itemName, new Date(),
                         System.currentTimeMillis() - timerStart);
             }
-
+            return true;
-        return result;
+        } catch (YankSQLException e) {
+            logger.debug("JDBC::remove: Unable to remove values for item", e);
+            return false;
+        }
     }
 
     /**