Handle DB error codes in distributed locking

The commons library wraps the SQLExceptions within its own SQLException,
so changed the code to simply look for a cause that's SQLTransientException,
eliminating the need to check specific error codes.  Deleted the error
code property now that it is no longer needed.
Also updated the distributed locking properties to include examples.

Issue-ID: POLICY-2113
Signed-off-by: Jim Hahn <jrh3@att.com>
Change-Id: If46e85a81cfc952e561174fea670df81efb8309a

6 files changed, 30 insertions, 74 deletions

diff --git a/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties b/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties
index c609c8f9..9acea3c0 100644
--- a/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties
+++ b/feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties
@@ -19,7 +19,12 @@
 ###
 
 #Database properties
-javax.persistence.jdbc.driver= org.mariadb.jdbc.Driver
+javax.persistence.jdbc.driver=org.mariadb.jdbc.Driver
 javax.persistence.jdbc.url=jdbc:mariadb://${env:SQL_HOST}:3306/pooling
 javax.persistence.jdbc.user=${env:SQL_USER}
 javax.persistence.jdbc.password=${env:SQL_PASSWORD}
+
+# default property values are commented out
+#distributed.locking.expire.check.seconds=900
+#distributed.locking.retry.seconds=60
+#distributed.locking.max.retries=2
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java
index 523c0d93..7ee786b9 100644
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockManager.java
@@ -24,6 +24,7 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.SQLTransientException;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
@@ -77,8 +78,7 @@ import org.slf4j.LoggerFactory;
  * instance.</li>
  * </dl>
  */
-public class DistributedLockManager
-                implements PolicyResourceLockManager, PolicyEngineFeatureApi {
+public class DistributedLockManager implements PolicyResourceLockManager, PolicyEngineFeatureApi {
 
     private static final Logger logger = LoggerFactory.getLogger(DistributedLockManager.class);
 
@@ -396,6 +396,8 @@ public class DistributedLockManager
      * Distributed Lock implementation.
      */
     public static class DistributedLock extends LockImpl {
+        private static final String SQL_FAILED_MSG = "request failed for lock: {}";
+
         private static final long serialVersionUID = 1L;
 
         /**
@@ -702,9 +704,9 @@ public class DistributedLockManager
                     req.run();
 
                 } catch (SQLException e) {
-                    logger.warn("request failed for lock: {}", this, e);
+                    logger.warn(SQL_FAILED_MSG, this, e);
 
-                    if (feature.featProps.isTransient(e.getErrorCode())) {
+                    if (e.getCause() instanceof SQLTransientException) {
                         // retry the request a little later
                         rescheduleRequest(req);
                     } else {
@@ -712,7 +714,7 @@ public class DistributedLockManager
                     }
 
                 } catch (RuntimeException e) {
-                    logger.warn("request failed for lock: {}", this, e);
+                    logger.warn(SQL_FAILED_MSG, this, e);
                     removeFromMap();
                 }
             }
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java
index f470c8e2..fff19447 100644
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockProperties.java
@@ -20,10 +20,7 @@
 
 package org.onap.policy.distributed.locking;
 
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.Properties;
-import java.util.Set;
 import lombok.Getter;
 import lombok.Setter;
 import org.onap.policy.common.utils.properties.BeanConfigurator;
@@ -40,7 +37,6 @@ public class DistributedLockProperties {
     public static final String DB_URL = "javax.persistence.jdbc.url";
     public static final String DB_USER = "javax.persistence.jdbc.user";
     public static final String DB_PASS = "javax.persistence.jdbc.password";
-    public static final String TRANSIENT_ERROR_CODES = PREFIX + "transient.error.codes";
     public static final String EXPIRE_CHECK_SEC = PREFIX + "expire.check.seconds";
     public static final String RETRY_SEC = PREFIX + "retry.seconds";
     public static final String MAX_RETRIES = PREFIX + "max.retries";
@@ -70,16 +66,6 @@ public class DistributedLockProperties {
     private String dbPwd;
 
     /**
-     * Vendor-specific error codes that are "transient", meaning they may go away if the
-     * command is repeated (e.g., connection issue), as opposed to something like a syntax
-     * error or a duplicate key.
-     */
-    @Property(name = TRANSIENT_ERROR_CODES)
-    private String errorCodeStrings;
-
-    private final Set<Integer> transientErrorCodes;
-
-    /**
      * Time, in seconds, to wait between checks for expired locks.
      */
     @Property(name = EXPIRE_CHECK_SEC, defaultValue = "900")
@@ -105,32 +91,5 @@ public class DistributedLockProperties {
      */
     public DistributedLockProperties(Properties props) throws PropertyException {
         new BeanConfigurator().configureFromProperties(this, props);
-
-        Set<Integer> set = new HashSet<>();
-        for (String text : errorCodeStrings.split(",")) {
-            text = text.trim();
-            if (text.isEmpty()) {
-                continue;
-            }
-
-            try {
-                set.add(Integer.valueOf(text));
-
-            } catch (NumberFormatException e) {
-                throw new PropertyException(TRANSIENT_ERROR_CODES, "errorCodeStrings", e);
-            }
-        }
-
-        transientErrorCodes = Collections.unmodifiableSet(set);
-    }
-
-    /**
-     * Determines if an error is transient.
-     *
-     * @param errorCode error code to check
-     * @return {@code true} if the error is transient, {@code false} otherwise
-     */
-    public boolean isTransient(int errorCode) {
-        return transientErrorCodes.contains(errorCode);
     }
 }
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java
index 59b56224..0ba92c51 100644
--- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockManagerTest.java
@@ -48,6 +48,7 @@ import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.SQLTransientException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
@@ -100,8 +101,8 @@ public class DistributedLockManagerTest {
     private static final int HOLD_SEC2 = 120;
     private static final int MAX_THREADS = 5;
     private static final int MAX_LOOPS = 100;
-    private static final int TRANSIENT = 500;
-    private static final int PERMANENT = 600;
+    private static final boolean TRANSIENT = true;
+    private static final boolean PERMANENT = false;
 
     // number of execute() calls before the first lock attempt
     private static final int PRE_LOCK_EXECS = 1;
@@ -1673,15 +1674,15 @@ public class DistributedLockManagerTest {
      * Feature whose data source all throws exceptions.
      */
     private class InvalidDbLockingFeature extends MyLockingFeature {
-        private int errcode;
+        private boolean isTransient;
         private boolean freeLock = false;
 
-        public InvalidDbLockingFeature(int errcode) {
+        public InvalidDbLockingFeature(boolean isTransient) {
             // pass "false" because we have to set the error code BEFORE calling
             // afterStart()
             super(false);
 
-            this.errcode = errcode;
+            this.isTransient = isTransient;
 
             this.beforeCreateLockManager(engine, new Properties());
             this.afterStart(engine);
@@ -1695,13 +1696,22 @@ public class DistributedLockManagerTest {
                     lock.free();
                 }
 
-                throw new SQLException(EXPECTED_EXCEPTION, "", errcode);
+                throw makeEx();
             });
 
-            doThrow(new SQLException(EXPECTED_EXCEPTION, "", errcode)).when(datasrc).close();
+            doThrow(makeEx()).when(datasrc).close();
 
             return datasrc;
         }
+
+        private SQLException makeEx() {
+            if (isTransient) {
+                return new SQLException(new SQLTransientException(EXPECTED_EXCEPTION));
+
+            } else {
+                return new SQLException(EXPECTED_EXCEPTION);
+            }
+        }
     }
 
     /**
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java
index 5f76d657..5abca889 100644
--- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockPropertiesTest.java
@@ -20,13 +20,9 @@
 
 package org.onap.policy.distributed.locking;
 
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
 import java.util.Properties;
-import java.util.TreeSet;
 import org.junit.Before;
 import org.junit.Test;
 import org.onap.policy.common.utils.properties.exception.PropertyException;
@@ -46,7 +42,6 @@ public class DistributedLockPropertiesTest {
         props.setProperty(DistributedLockProperties.DB_URL, "my url");
         props.setProperty(DistributedLockProperties.DB_USER, "my user");
         props.setProperty(DistributedLockProperties.DB_PASS, "my pass");
-        props.setProperty(DistributedLockProperties.TRANSIENT_ERROR_CODES, "10,-20,,,30");
         props.setProperty(DistributedLockProperties.EXPIRE_CHECK_SEC, "100");
         props.setProperty(DistributedLockProperties.RETRY_SEC, "200");
         props.setProperty(DistributedLockProperties.MAX_RETRIES, "300");
@@ -60,22 +55,8 @@ public class DistributedLockPropertiesTest {
         assertEquals("my url", dlp.getDbUrl());
         assertEquals("my user", dlp.getDbUser());
         assertEquals("my pass", dlp.getDbPwd());
-        assertEquals("10,-20,,,30", dlp.getErrorCodeStrings());
-        assertEquals("[-20, 10, 30]", new TreeSet<>(dlp.getTransientErrorCodes()).toString());
         assertEquals(100, dlp.getExpireCheckSec());
         assertEquals(200, dlp.getRetrySec());
         assertEquals(300, dlp.getMaxRetries());
-
-        assertTrue(dlp.isTransient(10));
-        assertTrue(dlp.isTransient(-20));
-        assertTrue(dlp.isTransient(30));
-
-        assertFalse(dlp.isTransient(-10));
-
-        // invalid value
-        props.setProperty(DistributedLockProperties.TRANSIENT_ERROR_CODES, "10,abc,30");
-
-        assertThatThrownBy(() -> new DistributedLockProperties(props)).isInstanceOf(PropertyException.class)
-                        .hasMessageContaining(DistributedLockProperties.TRANSIENT_ERROR_CODES);
     }
 }
diff --git a/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties b/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties
index 061cc608..0fca3c04 100644
--- a/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties
+++ b/feature-distributed-locking/src/test/resources/feature-distributed-locking.properties
@@ -23,7 +23,6 @@ javax.persistence.jdbc.url=jdbc:h2:mem:pooling
 javax.persistence.jdbc.user=user
 javax.persistence.jdbc.password=password
 
-distributed.locking.transient.error.codes=500
 distributed.locking.expire.check.seconds=900
 distributed.locking.retry.seconds=60
 distributed.locking.max.retries=2