[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>

diff --git a/bundles/org.openhab.persistence.jdbc/pom.xml b/bundles/org.openhab.persistence.jdbc/pom.xml
index 7ad8183c05c0dc8784b14c19a572de5bb9ad3d0b..e26f67e6107390206f9b848ca179c5285f836f0c 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/pom.xml
+++ b/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>

diff --git a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/console/JdbcCommandExtension.java b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/console/JdbcCommandExtension.java
index 0bcab36a2ac49fbdb4e3648524ca510196974552..6923430aa7e95719710e5caaee71173236102383 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/console/JdbcCommandExtension.java
+++ b/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])) {
-            listTables(persistenceService, console, args.length == 3 && PARAMETER_ALL.equalsIgnoreCase(args[2]));
-            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);
+        try {
+            if (!execute(persistenceService, args, console)) {
+                printUsage(console);
                 return;
             }
+        } catch (YankSQLException e) {
+            console.println(e.toString());
         }
-        printUsage(console);
     }
 
     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

diff --git a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/dto/ItemsVO.java b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/dto/ItemsVO.java
index 0e203f59817701f3f09d4612915e7eb39bcdef3a..9f8f7da3cbf7cab8dcce6e60d85c041ca25e2d41 100644 (file)
--- a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/dto/ItemsVO.java
+++ b/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;
-        int result = 1;
-        result = prime * result + ((itemName == null) ? 0 : itemName.hashCode());
-        result = prime * result + (itemId ^ (itemId >>> 32));
-        return result;
+        return Objects.hash(itemName, itemId);
     }
 
     /*

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 c956e706ecda8b0708d9f81d242b8d4c79969714..cae0a5346dc4dd89b2d9de2d613bfe2cbf4acaed 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
@@ -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() {
-        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() {
+    private 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,16 +264,21 @@ public class JdbcMapper {
         if (initialized) {
             return true;
         }
-        // first
-        boolean p = pingDB();
-        if (p) {
-            logger.debug("JDBC::checkDBAcessability, first try connection: {}", p);
-            return (p && !(conf.getErrReconnectThreshold() > 0 && errCnt <= conf.getErrReconnectThreshold()));
-        } else {
-            // second
-            p = pingDB();
-            logger.debug("JDBC::checkDBAcessability, second try connection: {}", p);
-            return (p && !(conf.getErrReconnectThreshold() > 0 && errCnt <= conf.getErrReconnectThreshold()));
+        try {
+            // first
+            boolean p = pingDB();
+            if (p) {
+                logger.debug("JDBC::checkDBAcessability, first try connection: {}", p);
+                return (p && !(conf.getErrReconnectThreshold() > 0 && errCnt <= conf.getErrReconnectThreshold()));
+            } else {
+                // second
+                p = pingDB();
+                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))

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 d267ad097045dc11b6f4385ff19bd7e908dd7f87..e0fbf11552b607518bb3c733cf99d5ec4c00656e 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
@@ -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,11 +156,15 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
                     state, item, errCnt, conf.getErrReconnectThreshold());
             return;
         }
-        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);
+        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);
         }
     }
 
@@ -215,16 +220,20 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
             return List.of();
         }
 
-        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);
+        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();
         }
-
-        // Success
-        errCnt = 0;
-        return items;
     }
 
     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);
-            checkDBSchema();
-            // connection has been established ... initialization completed!
-            initialized = true;
+            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;
         }
 
-        long timerStart = System.currentTimeMillis();
-        boolean result = 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);
+        try {
+            long timerStart = System.currentTimeMillis();
+            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;
+        } catch (YankSQLException e) {
+            logger.debug("JDBC::remove: Unable to remove values for item", e);
+            return false;
         }
-
-        return result;
     }
 
     /**