Add time limit to locking facility 87/55487/4
authorJim Hahn <jrh3@att.com>
Wed, 27 Jun 2018 14:12:45 +0000 (10:12 -0400)
committerJim Hahn <jrh3@att.com>
Wed, 27 Jun 2018 16:58:08 +0000 (12:58 -0400)
Modified the locking facility to add a time limit and remove the
callback parameter.  This affected both the default facility as
well as the distributed locking feature.  It will also require
a change to the rules for Closed Loop.
Changed testUnlock() to try locking with a different owner.
Default feature API should be OPER_UNHANDLED.
Put a few things back so-as not to break the drools-applications
build.  They can be removed once drools-applications is updated.
Fix newlines in API java.

Change-Id: I3ed7835cac6a582493a9bc8f6d1d4f3e6cb6289e
Issue-ID: POLICY-872
Signed-off-by: Jim Hahn <jrh3@att.com>
15 files changed:
feature-distributed-locking/src/main/feature/config/feature-distributed-locking.properties
feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java
feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/Heartbeat.java [deleted file]
feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java
feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java [deleted file]
policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java
policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java [deleted file]
policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java
policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java

index 665f822..33e2d78 100644 (file)
@@ -23,12 +23,3 @@ javax.persistence.jdbc.driver= org.mariadb.jdbc.Driver
 javax.persistence.jdbc.url=jdbc:mariadb://${{SQL_HOST}}:3306/pooling
 javax.persistence.jdbc.user=${{SQL_USER}}
 javax.persistence.jdbc.password=${{SQL_PASSWORD}}
-
-#This value is added to System.currentTimeMs to
-#set expirationTime when a lock is obtained. 
-#distributed.locking.lock.aging=1000
-
-#The frequency (in milliseconds) that the heartbeat
-#thread refreshes locks owned by the current host
-#distributed.locking.heartbeat.interval=5000
-
index b30fca7..03872e1 100644 (file)
@@ -24,14 +24,9 @@ import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.util.Properties;
 import java.util.UUID;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
 import org.apache.commons.dbcp2.BasicDataSource;
 import org.apache.commons.dbcp2.BasicDataSourceFactory;
 import org.onap.policy.common.utils.properties.exception.PropertyException;
-import org.onap.policy.drools.core.lock.LockRequestFuture;
 import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI;
 import org.onap.policy.drools.features.PolicyEngineFeatureAPI;
 import org.onap.policy.drools.persistence.SystemPersistence;
@@ -56,11 +51,6 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
         */
        private DistributedLockingProperties lockProps;
        
-       /**
-        *ScheduledExecutorService for LockHeartbeat 
-        */
-       private ScheduledExecutorService scheduledExecutorService;
-       
        /**
         * Data source used to connect to the DB containing locks.
         */
@@ -71,43 +61,36 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
         */
        private static final UUID uuid = UUID.randomUUID();
        
-
-       /**
-        * Reference to Heartbeat
-        */
-       private static Heartbeat heartbeat = null;
-       
        @Override
        public int getSequenceNumber() {
         return 1000;
        }
        
        @Override
-       public Future<Boolean> beforeLock(String resourceId, String owner, Callback callback) {
-               
-               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps, dataSource);
+    public OperResult beforeLock(String resourceId, String owner, int holdSec) {
                
-               return new LockRequestFuture(resourceId, owner, tLock.lock());
-                               
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, dataSource);
+
+        return(tLock.lock(holdSec) ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);                               
        }
 
        @Override
        public OperResult beforeUnlock(String resourceId, String owner) {
-               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps, dataSource);
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, dataSource);
                
                return(tLock.unlock() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
        }
        
        @Override
        public OperResult beforeIsLockedBy(String resourceId, String owner) {
-               TargetLock tLock = new TargetLock(resourceId, uuid, owner, lockProps, dataSource);
+               TargetLock tLock = new TargetLock(resourceId, uuid, owner, dataSource);
 
         return(tLock.isActive() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
        }
        
        @Override
        public OperResult beforeIsLocked(String resourceId) {
-               TargetLock tLock = new TargetLock(resourceId, uuid, "dummyOwner", lockProps, dataSource);
+               TargetLock tLock = new TargetLock(resourceId, uuid, "dummyOwner", dataSource);
 
         return(tLock.isLocked() ? OperResult.OPER_ACCEPTED : OperResult.OPER_DENIED);
        }
@@ -130,13 +113,8 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
             throw new DistributedLockingFeatureException(e);
                }
                
-               long heartbeatInterval = this.lockProps.getHeartBeatIntervalProperty();
-               
                cleanLockTable();
-               initHeartbeat();
                
-               this.scheduledExecutorService = Executors.newScheduledThreadPool(1);
-               this.scheduledExecutorService.scheduleAtFixedRate(heartbeat, heartbeatInterval, heartbeatInterval, TimeUnit.MILLISECONDS);
                return false;
        }
        
@@ -164,7 +142,6 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
         */
        @Override
        public boolean beforeShutdown(PolicyEngine engine) {
-               scheduledExecutorService.shutdown();
                cleanLockTable();
                return false;
        }
@@ -186,18 +163,5 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
                        logger.error("error in refreshLockTable()", e);
                }
                
-       }
-
-       /**
-        * Initialize the static heartbeat object
-        */
-       private void initHeartbeat() {
-               heartbeat = new Heartbeat(uuid, lockProps, dataSource);
-               
-       }
-       
-       public static Heartbeat getHeartbeat() {
-               return heartbeat;
-       }
-       
+       }       
 }
index b82f4b0..1f55a4c 100644 (file)
@@ -35,8 +35,6 @@ public class DistributedLockingProperties extends PropertyConfiguration {
     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_PWD = "javax.persistence.jdbc.password";
-    public static final String AGING_PROPERTY = PREFIX + "lock.aging";
-    public static final String HEARTBEAT_INTERVAL_PROPERTY = PREFIX + "heartbeat.interval";
 
     /**
      * Properties from which this was constructed.
@@ -67,18 +65,6 @@ public class DistributedLockingProperties extends PropertyConfiguration {
     @Property(name = DB_PWD)
     private String dbPwd;
 
-    /**
-     * Used to set expiration time for lock.
-     */
-    @Property(name = AGING_PROPERTY, defaultValue = "300000")
-    private long agingProperty;
-
-    /**
-     * Indicates intervals at which we refresh locks.
-     */
-    @Property(name = HEARTBEAT_INTERVAL_PROPERTY, defaultValue = "60000")
-    private long heartBeatIntervalProperty;
-
     public DistributedLockingProperties(Properties props) throws PropertyException {
         super(props);
         source = props;
@@ -110,16 +96,6 @@ public class DistributedLockingProperties extends PropertyConfiguration {
     }
 
 
-    public long getAgingProperty() {
-        return agingProperty;
-    }
-
-
-    public long getHeartBeatIntervalProperty() {
-        return heartBeatIntervalProperty;
-    }
-
-
     public void setDbDriver(String dbDriver) {
         this.dbDriver = dbDriver;
     }
@@ -139,14 +115,4 @@ public class DistributedLockingProperties extends PropertyConfiguration {
         this.dbPwd = dbPwd;
     }
 
-
-    public void setAgingProperty(long agingProperty) {
-        this.agingProperty = agingProperty;
-    }
-
-
-    public void setHeartBeatIntervalProperty(long heartBeatIntervalProperty) {
-        this.heartBeatIntervalProperty = heartBeatIntervalProperty;
-    }
-
 }
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/Heartbeat.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/Heartbeat.java
deleted file mode 100644 (file)
index ccfb4c7..0000000
+++ /dev/null
@@ -1,107 +0,0 @@
-/*
- * ============LICENSE_START=======================================================
- * feature-distributed-locking
- * ================================================================================
- * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
- * ================================================================================
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-
-package org.onap.policy.distributed.locking;
-
-import java.sql.Connection;
-import java.sql.PreparedStatement;
-import java.sql.SQLException;
-import java.util.UUID;
-import java.util.concurrent.CountDownLatch;
-import org.apache.commons.dbcp2.BasicDataSource;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * 
- * This runnable class scans the locks table for all locks owned by this host.
- * It refreshes the expiration time of each lock using the locking.distributed.aging
- * property
- *
- */
-public class Heartbeat implements Runnable{
-
-       private static final Logger logger = LoggerFactory.getLogger(Heartbeat.class);
-
-       /**
-        * Properties object containing properties needed by class
-        */
-       private DistributedLockingProperties lockProps;
-    
-    /**
-     * Data source used to connect to the DB containing locks.
-     */
-    private BasicDataSource dataSource;
-
-       /**
-        * UUID 
-        */
-       private UUID uuid;
-       
-       /**
-        * Countdown latch for testing 
-        */
-       private volatile CountDownLatch latch;
-       
-       /**
-        * 
-        * @param uuid
-        * @param lockProps
-        * @param dataSource 
-        */
-       public Heartbeat(UUID uuid, DistributedLockingProperties lockProps, BasicDataSource dataSource) {
-               this.lockProps = lockProps;
-               this.dataSource = dataSource;
-               this.uuid = uuid;
-               this.latch = new CountDownLatch(1);
-       }
-       /**
-        * 
-        * @param latch
-        * Used for testing purposes
-        */
-       protected void giveLatch(CountDownLatch latch) {
-               this.latch = latch;
-       }
-       
-       @Override
-       public void run() {
-
-               
-               long expirationAge = lockProps.getAgingProperty();
-
-               try (Connection conn = dataSource.getConnection();
-                       PreparedStatement statement = conn
-                                               .prepareStatement("UPDATE pooling.locks SET expirationTime = ? WHERE host = ?");) {
-
-                       statement.setLong(1, System.currentTimeMillis() + expirationAge);
-                       statement.setString(2, this.uuid.toString());
-                       statement.executeUpdate();
-                       
-                       latch.countDown();
-                       
-               } catch (SQLException e) {
-                       logger.error("error in Heartbeat.run()", e);
-               }
-
-       }
-       
-       
-}
index d57de1f..0853f12 100644 (file)
@@ -25,6 +25,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.UUID;
 import org.apache.commons.dbcp2.BasicDataSource;
+import java.util.concurrent.TimeUnit;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -36,11 +37,6 @@ public class TargetLock {
         * The Target resource we want to lock
         */
        private String resourceId;
-       
-       /**
-        * Properties object containing properties needed by class
-        */
-       private DistributedLockingProperties lockProps;
     
     /**
      * Data source used to connect to the DB containing locks.
@@ -61,23 +57,22 @@ public class TargetLock {
         * Constructs a TargetLock object.
         * 
         * @param resourceId ID of the entity we want to lock
-        * @param lockProps Properties object containing properties needed by class
         * @param dataSource used to connect to the DB containing locks
         */
-       public TargetLock (String resourceId, UUID uuid, String owner, DistributedLockingProperties lockProps, BasicDataSource dataSource) {
+       public TargetLock (String resourceId, UUID uuid, String owner, BasicDataSource dataSource) {
                this.resourceId = resourceId;
                this.uuid = uuid;
                this.owner = owner;
-               this.lockProps = lockProps;
                this.dataSource = dataSource;
        }
        
        /**
         * obtain a lock
+     * @param holdSec the amount of time, in seconds, that the lock should be held
         */
-       public boolean lock() {
+       public boolean lock(int holdSec) {
                
-               return grabLock();
+               return grabLock(TimeUnit.SECONDS.toMillis(holdSec));
        }
        
        /**
@@ -91,8 +86,9 @@ public class TargetLock {
         * "Grabs" lock by attempting to insert a new record in the db.
         *  If the insert fails due to duplicate key error resource is already locked
         *  so we call secondGrab. 
+     * @param holdMs the amount of time, in milliseconds, that the lock should be held
         */
-       private boolean grabLock() {
+       private boolean grabLock(long holdMs) {
 
                // try to insert a record into the table(thereby grabbing the lock)
                try (Connection conn = dataSource.getConnection();
@@ -100,16 +96,17 @@ public class TargetLock {
                                PreparedStatement statement = conn.prepareStatement(
                                                "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)")) {
                        
-                       statement.setString(1, this.resourceId);
-                       statement.setString(2, this.uuid.toString());
-                       statement.setString(3, this.owner);
-                       statement.setLong(4, System.currentTimeMillis() + lockProps.getAgingProperty());
+                   int i = 1;
+                       statement.setString(i++, this.resourceId);
+                       statement.setString(i++, this.uuid.toString());
+                       statement.setString(i++, this.owner);
+                       statement.setLong(i++, System.currentTimeMillis() + holdMs);
                        statement.executeUpdate();
                }
 
                catch (SQLException e) {
                        logger.error("error in TargetLock.grabLock()", e);
-                       return secondGrab();
+                       return secondGrab(holdMs);
                }
 
                return true;
@@ -118,22 +115,25 @@ public class TargetLock {
        /**
         * A second attempt at grabbing a lock. It first attempts to update the lock in case it is expired.
         * If that fails, it attempts to insert a new record again
+     * @param holdMs the amount of time, in milliseconds, that the lock should be held
         */
-       private boolean secondGrab() {
+       private boolean secondGrab(long holdMs) {
 
                try (Connection conn = dataSource.getConnection();
 
                                PreparedStatement updateStatement = conn.prepareStatement(
-                                               "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = ? WHERE expirationTime <= ? AND resourceId = ?");
+                                               "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = ? WHERE resourceId = ? AND (owner = ? OR expirationTime <= ?)");
 
                                PreparedStatement insertStatement = conn.prepareStatement(
                                                "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)");) {
 
-                       updateStatement.setString(1, this.uuid.toString());
-                       updateStatement.setString(2, this.owner);
-                       updateStatement.setLong(3, System.currentTimeMillis() + lockProps.getAgingProperty());
-                       updateStatement.setLong(4, System.currentTimeMillis());
-                       updateStatement.setString(5, this.resourceId);
+                   int i = 1;
+                       updateStatement.setString(i++, this.uuid.toString());
+                       updateStatement.setString(i++, this.owner);
+                       updateStatement.setLong(i++, System.currentTimeMillis() + holdMs);
+            updateStatement.setString(i++, this.resourceId);
+            updateStatement.setString(i++, this.owner);
+                       updateStatement.setLong(i++, System.currentTimeMillis());
 
                        // The lock was expired and we grabbed it.
                        // return true
@@ -144,10 +144,11 @@ public class TargetLock {
                        // If our update does not return 1 row, the lock either has not expired
                        // or it was removed. Try one last grab
                        else {
-                               insertStatement.setString(1, this.resourceId);
-                               insertStatement.setString(2, this.uuid.toString());
-                               insertStatement.setString(3, this.owner);
-                               insertStatement.setLong(4, System.currentTimeMillis() + lockProps.getAgingProperty());
+                           i = 1;
+                               insertStatement.setString(i++, this.resourceId);
+                               insertStatement.setString(i++, this.uuid.toString());
+                               insertStatement.setString(i++, this.owner);
+                               insertStatement.setLong(i++, System.currentTimeMillis() + holdMs);
 
                                // If our insert returns 1 we successfully grabbed the lock
                                return (insertStatement.executeUpdate() == 1);
index b01d967..a4c292c 100644 (file)
 
 package org.onap.policy.distributed.locking;
 
+import static org.junit.Assert.assertEquals;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.util.concurrent.ExecutionException;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.onap.policy.distributed.locking.DistributedLockingFeature;
 import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult;
 import org.onap.policy.drools.persistence.SystemPersistence;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import java.sql.Connection;
-import java.sql.DriverManager;
-import java.sql.PreparedStatement;
-import java.sql.SQLException;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
 
 public class TargetLockTest {
        private static final Logger logger = LoggerFactory.getLogger(TargetLockTest.class);
+    private static final int MAX_AGE_SEC = 4 * 60;
        private static final String DB_CONNECTION = "jdbc:h2:mem:pooling;INIT=CREATE SCHEMA IF NOT EXISTS pooling\\;SET SCHEMA pooling";
        private static final String DB_USER = "user";
        private static final String DB_PASSWORD = "password";
@@ -82,7 +77,7 @@ public class TargetLockTest {
        
        @Test
        public void testGrabLockSuccess() throws InterruptedException, ExecutionException {
-               assertTrue(distLockFeat.beforeLock("resource1", "owner1", null).get());
+           assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC));
        
                        //attempt to grab expiredLock
                try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = ? WHERE resourceId = ?");)
@@ -96,68 +91,49 @@ public class TargetLockTest {
                        throw new RuntimeException(e);
                }
                
-               assertTrue(distLockFeat.beforeLock("resource1", "owner1", null).get());
+               assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC));
+
+               // extend the lock
+        assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC));
        }
 
        @Test
-       public void testExpiredLocks() throws InterruptedException, ExecutionException {
-
-               CountDownLatch rowExpireLatch = new CountDownLatch(1);
-               CountDownLatch heartbeatLatch = new CountDownLatch(1);
+       public void testExpiredLocks() throws Exception {
                        
                        //grab lock
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                
-                       //Wait for lock to expire
-               try {
-                       rowExpireLatch.await(150, TimeUnit.MILLISECONDS);
-               } catch (InterruptedException e) {
-                       logger.error("Error in testExpiredLocks", e);
-               }
-               
-               // Grab reference to heartbeat object
-               Heartbeat heartbeat = DistributedLockingFeature.getHeartbeat();
-
-               // Pass heartbeat object countdown latch
-               try {
-                       heartbeat.giveLatch(heartbeatLatch);
-                       heartbeatLatch.await(5, TimeUnit.SECONDS);
-               } catch (InterruptedException e) {
-                       logger.error("Error in testExpiredLocks", e);
-               }
+                       //force lock to expire
+        try (PreparedStatement lockExpire = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = ?");) {
+            lockExpire.setLong(1, System.currentTimeMillis() - MAX_AGE_SEC - 1);
+            lockExpire.executeUpdate();
+        }
        
-                       //Heartbeat should keep it active
-               assertFalse(distLockFeat.beforeLock("resource1", "owner1", null).get());
+               assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC));
        }
        
        @Test
        public void testGrabLockFail() throws InterruptedException, ExecutionException {
-               CountDownLatch latch = new CountDownLatch(1);
                
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                
-               try {
-                       latch.await(10, TimeUnit.MILLISECONDS);
-               } catch (InterruptedException e) {
-                       logger.error("Error in testExpiredLocks", e);
-               }
-               assertFalse(distLockFeat.beforeLock("resource1", "owner1", null).get());
+               assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC));
 
        }
        
        
        @Test
        public void testUnlock() throws InterruptedException, ExecutionException {
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
 
                assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
-               assertTrue(distLockFeat.beforeLock("resource1", "owner1", null).get());
+               assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC));
        }
        
        @Test
        public void testIsActive() {
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner2"));
 
@@ -175,49 +151,17 @@ public class TargetLockTest {
 
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
 
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                        //Unlock record, next isActive attempt should fail
                distLockFeat.beforeUnlock("resource1", "owner1");
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
                
        }
        
-       @Test
-       public void testHeartbeat() {
-               
-               CountDownLatch rowExpireLatch = new CountDownLatch(1);
-               CountDownLatch heartbeatLatch = new CountDownLatch(1);
-               
-                       //grab lock
-               distLockFeat.beforeLock("resource1", "owner1", null);
-               
-                       //Wait for lock to expire
-               try {
-                       rowExpireLatch.await(150, TimeUnit.MILLISECONDS);
-               } catch (InterruptedException e) {
-                       logger.error("Error in testExpiredLocks", e);
-               }
-               
-                       //Grab reference to heartbeat object
-               Heartbeat heartbeat = DistributedLockingFeature.getHeartbeat();
-               
-                       //Pass heartbeat object countdown latch
-               try {
-                       heartbeat.giveLatch(heartbeatLatch);
-                       heartbeatLatch.await(5, TimeUnit.SECONDS);
-               } catch (InterruptedException e) {
-                       logger.error("Error in testExpiredLocks", e);
-               }
-                       //At this point the heartbeat object should hve
-                       //refreshed the lock. assert that resource1 is
-                       //locked
-               assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
-       }
-       
        @Test
        public void unlockBeforeLock() {
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeUnlock("resource1", "owner1"));
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeUnlock("resource1", "owner1"));
        }
@@ -225,7 +169,7 @@ public class TargetLockTest {
        @Test
        public void testIsLocked() {
                assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLocked("resource1"));
-               distLockFeat.beforeLock("resource1", "owner1", null);
+               distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
                assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
        
        }
index ea5e252..d376491 100644 (file)
@@ -102,7 +102,7 @@ public class Lock<T> {
      */
     public boolean add(String requester, T item) {
         if (item == null) {
-            throw LockRequestFuture.makeNullArgException("lock requester item is null");
+            throw SimpleLockManager.makeNullArgException("lock requester item is null");
         }
 
         if (requester.equals(owner)) {
diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java
deleted file mode 100644 (file)
index a2e9e62..0000000
+++ /dev/null
@@ -1,261 +0,0 @@
-/*
- * ============LICENSE_START=======================================================
- * ONAP
- * ================================================================================
- * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
- * ================================================================================
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-
-package org.onap.policy.drools.core.lock;
-
-import java.util.concurrent.CancellationException;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicReference;
-import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * Future associated with a lock request.
- */
-public class LockRequestFuture implements Future<Boolean> {
-
-    // messages used in exceptions
-    public static final String MSG_NULL_RESOURCE_ID = "null resourceId";
-    public static final String MSG_NULL_OWNER = "null owner";
-
-    private static Logger logger = LoggerFactory.getLogger(LockRequestFuture.class);
-
-    /**
-     * The resource on which the lock was requested.
-     */
-    private final String resourceId;
-
-    /**
-     * The owner for which the lock was requested.
-     */
-    private final String owner;
-
-    /**
-     * Possible states for this future.
-     */
-    private enum State {
-        WAITING, CANCELLED, ACQUIRED, DENIED
-    };
-
-    private AtomicReference<State> state;
-
-    /**
-     * Used to wait for the lock request to complete.
-     */
-    private CountDownLatch waiter = new CountDownLatch(1);
-
-    /**
-     * Callback to invoke once the lock is acquired (or denied). This is set to
-     * {@code null} once the callback has been invoked.
-     */
-    private final AtomicReference<Callback> callback;
-
-    /**
-     * Constructs a future that has already been completed.
-     * 
-     * @param resourceId
-     * @param owner owner for which the lock was requested
-     * @param locked {@code true} if the lock has been acquired, {@code false} if the lock
-     *        request has been denied
-     * @throws IllegalArgumentException if any of the arguments are {@code null}
-     */
-    public LockRequestFuture(String resourceId, String owner, boolean locked) {
-        if (resourceId == null) {
-            throw makeNullArgException(MSG_NULL_RESOURCE_ID);
-        }
-
-        if (owner == null) {
-            throw makeNullArgException(MSG_NULL_OWNER);
-        }
-
-        this.resourceId = resourceId;
-        this.owner = owner;
-        this.callback = new AtomicReference<>(null);
-        this.state = new AtomicReference<>(locked ? State.ACQUIRED : State.DENIED);
-
-        // indicate that it's already done
-        this.waiter.countDown();
-    }
-
-    /**
-     * Constructs a future that has not yet been completed.
-     * 
-     * @param resourceId
-     * @param owner owner for which the lock was requested
-     * @param callback item to be wrapped
-     * @throws IllegalArgumentException if the resourceId or owner is {@code null}
-     */
-    public LockRequestFuture(String resourceId, String owner, Callback callback) {
-        if (resourceId == null) {
-            throw makeNullArgException(MSG_NULL_RESOURCE_ID);
-        }
-
-        if (owner == null) {
-            throw makeNullArgException(MSG_NULL_OWNER);
-        }
-
-        this.resourceId = resourceId;
-        this.owner = owner;
-        this.callback = new AtomicReference<>(callback);
-        this.state = new AtomicReference<>(State.WAITING);
-    }
-
-    public String getResourceId() {
-        return resourceId;
-    }
-
-    public String getOwner() {
-        return owner;
-    }
-
-    @Override
-    public boolean cancel(boolean mayInterruptIfRunning) {
-        boolean cancelled = state.compareAndSet(State.WAITING, State.CANCELLED);
-
-        if (cancelled) {
-            logger.info("resource {} owner {} cancelled lock request", resourceId, owner);
-            waiter.countDown();
-        }
-
-        return cancelled;
-    }
-
-    /**
-     * Indicates that the lock has been acquired or denied.
-     * 
-     * @param locked {@code true} if the lock has been acquired, {@code false} if the lock
-     *        request has been denied
-     * 
-     * @return {@code true} if it was not already completed, {@code false} otherwise
-     */
-    protected boolean setLocked(boolean locked) {
-        State newState = (locked ? State.ACQUIRED : State.DENIED);
-        if (state.compareAndSet(State.WAITING, newState)) {
-            waiter.countDown();
-            return true;
-
-        } else {
-            return false;
-        }
-    }
-
-    @Override
-    public boolean isCancelled() {
-        return (state.get() == State.CANCELLED);
-    }
-
-    @Override
-    public boolean isDone() {
-        return (state.get() != State.WAITING);
-    }
-
-    /**
-     * Gets the current status of the lock.
-     * 
-     * @return {@code true} if the lock has been acquired, {@code false} otherwise
-     */
-    public boolean isLocked() {
-        return (state.get() == State.ACQUIRED);
-    }
-
-    /**
-     * @return {@code true} if the lock was acquired, {@code false} if it was denied
-     */
-    @Override
-    public Boolean get() throws InterruptedException {
-        waiter.await();
-
-        switch (state.get()) {
-            case CANCELLED:
-                throw new CancellationException("lock request was cancelled");
-            case ACQUIRED:
-                return true;
-            default:
-                // should only be DENIED at this point
-                return false;
-        }
-    }
-
-    /**
-     * @return {@code true} if the lock was acquired, {@code false} if it was denied
-     */
-    @Override
-    public Boolean get(long timeout, TimeUnit unit)
-                    throws InterruptedException, TimeoutException {
-
-        if (!waiter.await(timeout, unit)) {
-            throw new TimeoutException("lock request did not complete in time");
-        }
-
-        return get();
-    }
-
-    /**
-     * Invokes the callback, indicating whether or not the lock was acquired.
-     * 
-     * @throws IllegalStateException if the request was previously cancelled, has not yet
-     *         completed, or if the callback has already been invoked
-     */
-    protected void invokeCallback() {
-        boolean locked;
-
-        switch (state.get()) {
-            case ACQUIRED:
-                locked = true;
-                break;
-            case DENIED:
-                locked = false;
-                break;
-            case CANCELLED:
-                throw new IllegalStateException("cancelled lock request callback");
-            default:
-                // only other choice is WAITING
-                throw new IllegalStateException("incomplete lock request callback");
-        }
-
-        Callback cb = callback.get();
-        if (cb == null || !callback.compareAndSet(cb, null)) {
-            throw new IllegalStateException("already invoked lock request callback");
-        }
-
-
-        // notify the callback
-        try {
-            cb.set(locked);
-
-        } catch (RuntimeException e) {
-            logger.info("lock request callback for resource {} owner {} threw an exception", resourceId, owner, e);
-        }
-    }
-
-    /**
-     * Makes an exception for when an argument is {@code null}.
-     * 
-     * @param msg exception message
-     * @return a new Exception
-     */
-    public static IllegalArgumentException makeNullArgException(String msg) {
-        return new IllegalArgumentException(msg);
-    }
-}
index d4e7bee..1184b01 100644 (file)
@@ -20,7 +20,6 @@
 
 package org.onap.policy.drools.core.lock;
 
-import java.util.concurrent.Future;
 import org.onap.policy.drools.utils.OrderedService;
 import org.onap.policy.drools.utils.OrderedServiceImpl;
 
@@ -62,6 +61,7 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
     }
 
     /**
+         * 
      * Result of a requested operation.
      */
     public enum OperResult {
@@ -87,40 +87,21 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService {
     }
 
     /**
-     * This method is called before a lock is acquired on a resource. If a callback is
-     * provided, and the implementer is unable to acquire the lock immediately, then the
-     * implementer will invoke the callback once the lock is acquired. If the implementer
-     * handled the request, then it will return a future, which may be in one of three
-     * states:
-     * <dl>
-     * <dt>isDone()=true and get()=true</dt>
-     * <dd>the lock has been acquired; the callback may or may not have been invoked</dd>
-     * <dt>isDone()=true and get()=false</dt>
-     * <dd>the lock request has been denied; the callback may or may not have been
-     * invoked</dd>
-     * <dt>isDone()=false</dt>
-     * <dd>the lock was not immediately available and a callback was provided. The
-     * callback will be invoked once the lock is acquired (or denied). In this case, the
-     * future may be used to cancel the request</dd>
-     * </dl>
+     * This method is called before a lock is acquired on a resource.  It may be
+     * invoked repeatedly to extend the time that a lock is held.
      * 
      * @param resourceId
      * @param owner
-     * @param callback function to invoke, if the requester wishes to wait for the lock to
-     *        come available, {@code null} to provide immediate replies
-     * @return a future for the lock, if the implementer handled the request, {@code null}
-     *         if additional locking logic should be performed
-     * @throws IllegalStateException if the owner already holds the lock or is already in
-     *         the queue to get the lock
+     * @param holdSec the amount of time, in seconds, that the lock should be held
+     * @return the result, where <b>OPER_DENIED</b> indicates that the lock is currently
+     *         held by another owner
      */
-    public default Future<Boolean> beforeLock(String resourceId, String owner, Callback callback) {
-        return null;
+    public default OperResult beforeLock(String resourceId, String owner, int holdSec) {
+        return OperResult.OPER_UNHANDLED;
     }
 
     /**
-     * This method is called after a lock for a resource has been acquired or denied. This
-     * may be invoked immediately, if the status can be determined immediately, or it may
-     * be invoked asynchronously, once the status has been determined.
+     * This method is called after a lock for a resource has been acquired or denied.
      * 
      * @param resourceId
      * @param owner
index 97e7242..afe1cab 100644 (file)
@@ -20,9 +20,6 @@
 
 package org.onap.policy.drools.core.lock;
 
-import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_OWNER;
-import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_RESOURCE_ID;
-import static org.onap.policy.drools.core.lock.LockRequestFuture.makeNullArgException;
 import java.util.List;
 import java.util.concurrent.Future;
 import java.util.function.Function;
@@ -71,9 +68,22 @@ public class PolicyResourceLockManager extends SimpleLockManager {
     protected static void setFactory(Factory factory) {
         PolicyResourceLockManager.factory = factory;
     }
+    
+    /**
+     * This method is here temporarily so-as not to break the drools-applications
+     * build.  It will be removed once drools-applications has been updated.
+     * @param resourceId
+     * @param owner
+     * @param callback
+     * @return nothing; always throws an exception
+     * @throws UnsupportedOperationException
+     */
+    public Future<Boolean> lock(String resourceId, String owner, Callback callback) {
+        throw new UnsupportedOperationException("lock with callback");
+    }
 
     @Override
-    public Future<Boolean> lock(String resourceId, String owner, Callback callback) {
+    public boolean lock(String resourceId, String owner, int holdSec) {
         if (resourceId == null) {
             throw makeNullArgException(MSG_NULL_RESOURCE_ID);
         }
@@ -82,19 +92,16 @@ public class PolicyResourceLockManager extends SimpleLockManager {
             throw makeNullArgException(MSG_NULL_OWNER);
         }
 
-        Future<Boolean> result = doIntercept(null, impl -> impl.beforeLock(resourceId, owner, callback));
-        if (result != null) {
-            return result;
-        }
 
-        // implementer didn't do the work - use superclass
-        result = super.lock(resourceId, owner, callback);
+        return doBoolIntercept(impl -> impl.beforeLock(resourceId, owner, holdSec), () -> {
 
-        boolean locked = ((LockRequestFuture) result).isLocked();
+            // implementer didn't do the work - defer to the superclass
+            boolean locked = super.lock(resourceId, owner, holdSec);
 
-        doIntercept(false, impl -> impl.afterLock(resourceId, owner, locked));
+            doIntercept(false, impl -> impl.afterLock(resourceId, owner, locked));
 
-        return result;
+            return locked;
+        });
     }
 
     @Override
index 9bcf259..79fa0a7 100644 (file)
 
 package org.onap.policy.drools.core.lock;
 
-import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_OWNER;
-import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_RESOURCE_ID;
-import static org.onap.policy.drools.core.lock.LockRequestFuture.makeNullArgException;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.SortedSet;
 import java.util.TreeSet;
-import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import org.onap.policy.common.utils.time.CurrentTime;
-import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -43,10 +38,9 @@ public class SimpleLockManager {
 
     protected static Logger logger = LoggerFactory.getLogger(SimpleLockManager.class);
 
-    /**
-     * Maximum age, in milliseconds, before a lock is considered stale and released.
-     */
-    protected static final long MAX_AGE_MS = TimeUnit.MINUTES.toMillis(15L);
+    // messages used in exceptions
+    public static final String MSG_NULL_RESOURCE_ID = "null resourceId";
+    public static final String MSG_NULL_OWNER = "null owner";
     
     /**
      * Used to access the current time.  May be overridden by junit tests.
@@ -80,31 +74,16 @@ public class SimpleLockManager {
     }
 
     /**
-     * Attempts to lock a resource. This method ignores the callback and always returns a
-     * {@link CompletedLockRequest}.
+     * Attempts to lock a resource, extending the lock if the owner already holds it.
      * 
      * @param resourceId
      * @param owner
-     * @param callback function to invoke, if the requester wishes to wait for the lock to
-     *        be acquired, {@code null} to provide immediate replies
-     * @return a future for the lock request. The future will be in one of three states:
-     *         <dl>
-     *         <dt>isDone()=true and get()=true</dt>
-     *         <dd>the lock has been acquired; the callback may or may not have been
-     *         invoked</dd>
-     *         <dt>isDone()=true and get()=false</dt>
-     *         <dd>the lock request has been denied; the callback may or may not have been
-     *         invoked</dd>
-     *         <dt>isDone()=false</dt>
-     *         <dd>the lock was not immediately available and a callback was provided. The
-     *         callback will be invoked once the lock is acquired (or denied). In this
-     *         case, the future may be used to cancel the request</dd>
-     *         </dl>
+     * @param holdSec the amount of time, in seconds, that the lock should be held
+     * @return {@code true} if locked, {@code false} if the resource is already locked by
+     *         a different owner
      * @throws IllegalArgumentException if the resourceId or owner is {@code null}
-     * @throws IllegalStateException if the owner already holds the lock or is already in
-     *         the queue to get the lock
      */
-    public Future<Boolean> lock(String resourceId, String owner, Callback callback) {
+    public boolean lock(String resourceId, String owner, int holdSec) {
 
         if (resourceId == null) {
             throw makeNullArgException(MSG_NULL_RESOURCE_ID);
@@ -118,22 +97,24 @@ public class SimpleLockManager {
         
         synchronized(locker) {
             cleanUpLocks();
-            
-            if((existingLock = resource2data.get(resourceId)) == null) {
-                Data data = new Data(owner, resourceId, currentTime.getMillis() + MAX_AGE_MS);
+
+            if ((existingLock = resource2data.get(resourceId)) != null && existingLock.getOwner().equals(owner)) {
+                // replace the existing lock with an extended lock
+                locks.remove(existingLock);
+                existingLock = null;
+            }
+
+            if (existingLock == null) {
+                Data data = new Data(owner, resourceId, currentTime.getMillis() + TimeUnit.SECONDS.toMillis(holdSec));
                 resource2data.put(resourceId, data);
                 locks.add(data);
             }
         }
 
         boolean locked = (existingLock == null);
-        if (existingLock != null && owner.equals(existingLock.getOwner())) {
-            throw new IllegalStateException("lock for resource " + resourceId + " already owned by " + owner);
-        }
-
         logger.info("lock {} for resource {} owner {}", locked, resourceId, owner);
 
-        return new LockRequestFuture(resourceId, owner, locked);
+        return locked;
     }
 
     /**
@@ -254,6 +235,16 @@ public class SimpleLockManager {
             }
         }
     }
+
+    /**
+     * Makes an exception for when an argument is {@code null}.
+     * 
+     * @param msg exception message
+     * @return a new Exception
+     */
+    public static IllegalArgumentException makeNullArgException(String msg) {
+        return new IllegalArgumentException(msg);
+    }
     
     /**
      * Data for a single Lock.  Sorts by expiration time, then resource, and
diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java
deleted file mode 100644 (file)
index 7ac9e31..0000000
+++ /dev/null
@@ -1,395 +0,0 @@
-/*
- * ============LICENSE_START=======================================================
- * ONAP
- * ================================================================================
- * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
- * ================================================================================
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-
-package org.onap.policy.drools.core.lock;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.anyBoolean;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.verify;
-import static org.onap.policy.drools.core.lock.TestUtils.expectException;
-import java.util.concurrent.CancellationException;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-import org.junit.Before;
-import org.junit.Test;
-import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback;
-
-public class LockRequestFutureTest {
-
-    private static final int WAIT_SEC = 1;
-    private static final String RESOURCE = "my.resource";
-    private static final String OWNER = "my.owner";
-    private static final String EXPECTED_EXCEPTION = "expected exception";
-
-    private Callback callback;
-    private LockRequestFuture fut;
-
-    @Before
-    public void setUp() {
-        callback = mock(Callback.class);
-        fut = new LockRequestFuture(RESOURCE, OWNER, callback);
-    }
-
-    @Test
-    public void testLockRequestFutureStringStringBoolean_False() throws Exception {
-        fut = new LockRequestFuture(RESOURCE, OWNER, false);
-
-        assertTrue(fut.isDone());
-        assertEquals(RESOURCE, fut.getResourceId());
-        assertEquals(OWNER, fut.getOwner());
-
-        assertFalse(fut.isLocked());
-        assertFalse(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testLockRequestFutureStringStringBoolean_True() throws Exception {
-        fut = new LockRequestFuture(RESOURCE, OWNER, true);
-
-        assertTrue(fut.isDone());
-        assertEquals(RESOURCE, fut.getResourceId());
-        assertEquals(OWNER, fut.getOwner());
-
-        assertTrue(fut.isLocked());
-        assertTrue(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testLockRequestFutureStringStringBoolean_ArgEx() throws Exception {
-
-        // null resource id
-        IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, () -> new LockRequestFuture(null, OWNER, true));
-        assertEquals("null resourceId", ex.getMessage());
-
-
-        // null owner
-        ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, true));
-        assertEquals("null owner", ex.getMessage());
-    }
-
-    @Test
-    public void testLockRequestFutureStringStringCallback() throws Exception {
-        assertFalse(fut.isDone());
-        assertEquals(RESOURCE, fut.getResourceId());
-        assertEquals(OWNER, fut.getOwner());
-
-        fut.setLocked(true);
-        fut.invokeCallback();
-
-        // ensure it invoked the callback
-        verify(callback).set(true);
-    }
-
-    @Test
-    public void testLockRequestFutureStringStringCallback_ArgEx() throws Exception {
-
-        // null resource id
-        IllegalArgumentException ex = expectException(IllegalArgumentException.class,
-                        () -> new LockRequestFuture(null, OWNER, callback));
-        assertEquals("null resourceId", ex.getMessage());
-
-
-        // null owner
-        ex = expectException(IllegalArgumentException.class, () -> new LockRequestFuture(RESOURCE, null, callback));
-        assertEquals("null owner", ex.getMessage());
-
-
-        // null callback is OK
-        new LockRequestFuture(RESOURCE, OWNER, null);
-    }
-
-    @Test
-    public void testGetResourceId() {
-        assertEquals(RESOURCE, fut.getResourceId());
-    }
-
-    @Test
-    public void testGetOwner() {
-        assertEquals(OWNER, fut.getOwner());
-    }
-
-    @Test
-    public void testCancel() throws Exception {
-        // not cancelled yet
-        assertFalse(fut.isDone());
-
-        // cancel it
-        assertTrue(fut.cancel(false));
-        assertTrue(fut.isDone());
-
-        // should not block now
-        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
-
-    }
-
-    @Test
-    public void testCancel_AlreadyCancelled() throws Exception {
-
-        fut.cancel(true);
-
-        assertFalse(fut.cancel(true));
-        assertTrue(fut.isDone());
-
-        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testCancel_AlreadyAcquired() throws Exception {
-
-        fut.setLocked(true);
-
-        assertFalse(fut.cancel(true));
-        assertTrue(fut.isDone());
-        assertTrue(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testCancel_AlreadyDenied() throws Exception {
-
-        fut.setLocked(false);
-
-        assertFalse(fut.cancel(true));
-        assertTrue(fut.isDone());
-        assertFalse(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testSetLocked_True() throws Exception {
-        assertTrue(fut.setLocked(true));
-
-        assertTrue(fut.isDone());
-        assertTrue(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testSetLocked_False() throws Exception {
-        assertTrue(fut.setLocked(false));
-
-        assertTrue(fut.isDone());
-        assertFalse(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testSetLocked_AlreadyCancelled() {
-
-        fut.cancel(true);
-
-        assertFalse(fut.setLocked(true));
-        assertFalse(fut.setLocked(false));
-
-        assertTrue(fut.isDone());
-        expectException(CancellationException.class, () -> fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testSetLocked_AlreadyAcquired() throws Exception {
-        fut.setLocked(true);
-
-        assertTrue(fut.isDone());
-        assertTrue(fut.get(0, TimeUnit.SECONDS));
-
-        assertFalse(fut.cancel(true));
-        assertFalse(fut.setLocked(true));
-        assertFalse(fut.setLocked(false));
-    }
-
-    @Test
-    public void testSetLocked_AlreadyDenied() throws Exception {
-        fut.setLocked(false);
-
-        assertTrue(fut.isDone());
-        assertFalse(fut.get(0, TimeUnit.SECONDS));
-
-        assertFalse(fut.cancel(true));
-        assertFalse(fut.setLocked(true));
-        assertFalse(fut.setLocked(false));
-    }
-
-    @Test
-    public void testIsCancelled() {
-        assertFalse(fut.isCancelled());
-
-        fut.cancel(false);
-        assertTrue(fut.isCancelled());
-    }
-
-    @Test
-    public void testIsCancelled_Acquired() {
-        fut.setLocked(true);
-        assertFalse(fut.isCancelled());
-    }
-
-    @Test
-    public void testIsCancelled_Denied() {
-        fut.setLocked(false);
-        assertFalse(fut.isCancelled());
-    }
-
-    @Test
-    public void testIsDone_Cancelled() {
-        fut.cancel(false);
-        assertTrue(fut.isDone());
-    }
-
-    @Test
-    public void testIsDone_Acquired() {
-        fut.setLocked(true);
-        assertTrue(fut.isDone());
-    }
-
-    @Test
-    public void testIsDone_Denied() {
-        fut.setLocked(false);
-        assertTrue(fut.isDone());
-    }
-
-    @Test
-    public void testIsDone_Waiting() {
-        assertFalse(fut.isDone());
-    }
-
-    @Test
-    public void testIsLocked_Cancelled() {
-        fut.cancel(false);
-        assertFalse(fut.isLocked());
-    }
-
-    @Test
-    public void testIsLocked_Acquired() {
-        fut.setLocked(true);
-        assertTrue(fut.isLocked());
-    }
-
-    @Test
-    public void testIsLocked_Denied() {
-        fut.setLocked(false);
-        assertFalse(fut.isLocked());
-    }
-
-    @Test
-    public void testIsLocked_Waiting() {
-        assertFalse(fut.isLocked());
-    }
-
-    @Test
-    public void testGet_Cancelled() throws Exception {
-        new Thread(() -> fut.cancel(false)).start();
-        expectException(CancellationException.class, () -> fut.get());
-    }
-
-    @Test
-    public void testGet_Acquired() throws Exception {
-        new Thread(() -> fut.setLocked(true)).start();
-        assertTrue(fut.get());
-    }
-
-    @Test
-    public void testGet_Denied() throws Exception {
-        new Thread(() -> fut.setLocked(false)).start();
-        assertFalse(fut.get());
-    }
-
-    @Test
-    public void testGetLongTimeUnit() throws Exception {
-        expectException(TimeoutException.class, () -> fut.get(0, TimeUnit.SECONDS));
-
-        fut.setLocked(true);
-        assertTrue(fut.get(0, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testGetLongTimeUnit_Timeout() throws Exception {
-        expectException(TimeoutException.class, () -> fut.get(0, TimeUnit.SECONDS));
-        expectException(TimeoutException.class, () -> fut.get(2, TimeUnit.MILLISECONDS));
-    }
-
-    @Test
-    public void testGetLongTimeUnit_Cancelled() throws Exception {
-        new Thread(() -> fut.cancel(false)).start();
-        expectException(CancellationException.class, () -> fut.get(WAIT_SEC, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testGetLongTimeUnit_Acquired() throws Exception {
-        new Thread(() -> fut.setLocked(true)).start();
-        assertTrue(fut.get(WAIT_SEC, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testGetLongTimeUnit_Denied() throws Exception {
-        new Thread(() -> fut.setLocked(false)).start();
-        assertFalse(fut.get(WAIT_SEC, TimeUnit.SECONDS));
-    }
-
-    @Test(expected = IllegalStateException.class)
-    public void testInvokeCallback() {
-        fut.setLocked(true);
-        fut.invokeCallback();
-
-        // re-invoke - should throw an exception
-        fut.invokeCallback();
-    }
-
-    @Test(expected = IllegalStateException.class)
-    public void testInvokeCallback_Cancelled() {
-        fut.cancel(false);
-
-        // invoke after cancel - should throw an exception
-        fut.invokeCallback();
-    }
-
-    @Test
-    public void testInvokeCallback_Acquired() {
-        fut.setLocked(true);
-        fut.invokeCallback();
-
-        verify(callback).set(true);
-        verify(callback, never()).set(false);
-    }
-
-    @Test
-    public void testInvokeCallback_Denied() {
-        fut.setLocked(false);
-        fut.invokeCallback();
-
-        verify(callback).set(false);
-        verify(callback, never()).set(true);
-    }
-
-    @Test
-    public void testInvokeCallback_Ex() {
-        doThrow(new RuntimeException(EXPECTED_EXCEPTION)).when(callback).set(anyBoolean());
-
-        fut.setLocked(false);
-        fut.invokeCallback();
-    }
-
-    @Test
-    public void testMakeNullArgException() {
-        IllegalArgumentException ex = LockRequestFuture.makeNullArgException(EXPECTED_EXCEPTION);
-        assertEquals(EXPECTED_EXCEPTION, ex.getMessage());
-    }
-}
index 57adc84..0404877 100644 (file)
@@ -22,7 +22,6 @@ package org.onap.policy.drools.core.lock;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
 import org.junit.Before;
 import org.junit.Test;
 import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult;
@@ -46,7 +45,7 @@ public class PolicyResourceLockFeatureAPITest {
 
     @Test
     public void testBeforeLock() {
-        assertNull(api.beforeLock(RESOURCE_ID, OWNER, null));
+        assertEquals(OperResult.OPER_UNHANDLED, api.beforeLock(RESOURCE_ID, OWNER, 0));
     }
 
     @Test
index 26732e4..92e6026 100644 (file)
@@ -24,8 +24,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -36,17 +36,17 @@ import static org.onap.policy.drools.core.lock.TestUtils.expectException;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.concurrent.Future;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback;
 import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.OperResult;
 import org.onap.policy.drools.core.lock.PolicyResourceLockManager.Factory;
 
 public class PolicyResourceLockManagerTest {
 
+    private static final int MAX_AGE_SEC = 4 * 60;
+
     private static final String NULL_RESOURCE_ID = "null resourceId";
     private static final String NULL_OWNER = "null owner";
 
@@ -63,13 +63,10 @@ public class PolicyResourceLockManagerTest {
      */
     private static Factory saveFactory;
 
-    private Callback callback1;
     private PolicyResourceLockFeatureAPI impl1;
     private PolicyResourceLockFeatureAPI impl2;
     private List<PolicyResourceLockFeatureAPI> implList;
 
-    private Future<Boolean> fut;
-
     private PolicyResourceLockManager mgr;
 
     @BeforeClass
@@ -84,7 +81,6 @@ public class PolicyResourceLockManagerTest {
 
     @Before
     public void setUp() {
-        callback1 = mock(Callback.class);
         impl1 = mock(PolicyResourceLockFeatureAPI.class);
         impl2 = mock(PolicyResourceLockFeatureAPI.class);
 
@@ -111,7 +107,7 @@ public class PolicyResourceLockManagerTest {
      * @param impl
      */
     private void initImplementer(PolicyResourceLockFeatureAPI impl) {
-        when(impl.beforeLock(anyString(), anyString(), any(Callback.class))).thenReturn(null);
+        when(impl.beforeLock(anyString(), anyString(), anyInt())).thenReturn(OperResult.OPER_UNHANDLED);
         when(impl.beforeUnlock(anyString(), anyString())).thenReturn(OperResult.OPER_UNHANDLED);
         when(impl.beforeIsLocked(anyString())).thenReturn(OperResult.OPER_UNHANDLED);
         when(impl.beforeIsLockedBy(anyString(), anyString())).thenReturn(OperResult.OPER_UNHANDLED);
@@ -119,13 +115,10 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testLock() throws Exception {
-        fut = mgr.lock(RESOURCE_A, OWNER1, callback1);
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
-
-        verify(impl1).beforeLock(RESOURCE_A, OWNER1, callback1);
-        verify(impl2).beforeLock(RESOURCE_A, OWNER1, callback1);
+        verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        verify(impl2).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
         verify(impl1).afterLock(RESOURCE_A, OWNER1, true);
         verify(impl2).afterLock(RESOURCE_A, OWNER1, true);
 
@@ -135,43 +128,48 @@ public class PolicyResourceLockManagerTest {
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2));
 
         // null callback - not locked yet
-        fut = mgr.lock(RESOURCE_C, OWNER3, null);
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
+        assertTrue(mgr.lock(RESOURCE_C, OWNER3, MAX_AGE_SEC));
 
         // null callback - already locked
-        fut = mgr.lock(RESOURCE_A, OWNER3, null);
-        assertTrue(fut.isDone());
-        assertFalse(fut.get());
+        assertFalse(mgr.lock(RESOURCE_A, OWNER3, MAX_AGE_SEC));
     }
 
     @Test
     public void testLock_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, callback1));
+                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, MAX_AGE_SEC));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, callback1));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, MAX_AGE_SEC));
         assertEquals(NULL_OWNER, ex.getMessage());
 
         // this should not throw an exception
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
     }
 
     @Test
-    public void testLock_BeforeIntercepted() {
-        fut = mock(LockRequestFuture.class);
+    public void testLock_Acquired_BeforeIntercepted() {
+        // have impl1 intercept
+        when(impl1.beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_ACCEPTED);
 
-        // NOT async
-        when(fut.isDone()).thenReturn(true);
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
+        verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        verify(impl2, never()).beforeLock(anyString(), anyString(), anyInt());
+
+        verify(impl1, never()).afterLock(anyString(), anyString(), anyBoolean());
+        verify(impl2, never()).afterLock(anyString(), anyString(), anyBoolean());
+    }
+
+    @Test
+    public void testLock_Denied_BeforeIntercepted() {
         // have impl1 intercept
-        when(impl1.beforeLock(RESOURCE_A, OWNER1, callback1)).thenReturn(fut);
+        when(impl1.beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC)).thenReturn(OperResult.OPER_DENIED);
 
-        assertEquals(fut, mgr.lock(RESOURCE_A, OWNER1, callback1));
+        assertFalse(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
-        verify(impl1).beforeLock(RESOURCE_A, OWNER1, callback1);
-        verify(impl2, never()).beforeLock(anyString(), anyString(), any(Callback.class));
+        verify(impl1).beforeLock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        verify(impl2, never()).beforeLock(anyString(), anyString(), anyInt());
 
         verify(impl1, never()).afterLock(anyString(), anyString(), anyBoolean());
         verify(impl2, never()).afterLock(anyString(), anyString(), anyBoolean());
@@ -183,10 +181,7 @@ public class PolicyResourceLockManagerTest {
         // impl1 intercepts during afterLock()
         when(impl1.afterLock(RESOURCE_A, OWNER1, true)).thenReturn(true);
 
-        fut = mgr.lock(RESOURCE_A, OWNER1, callback1);
-
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
         // impl1 sees it, but impl2 does not
         verify(impl1).afterLock(RESOURCE_A, OWNER1, true);
@@ -195,10 +190,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testLock_Acquired() throws Exception {
-        fut = mgr.lock(RESOURCE_A, OWNER1, callback1);
-
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
         verify(impl1).afterLock(RESOURCE_A, OWNER1, true);
         verify(impl2).afterLock(RESOURCE_A, OWNER1, true);
@@ -207,16 +199,13 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testLock_Denied_AfterIntercepted() throws Exception {
 
-        mgr.lock(RESOURCE_A, OWNER1, callback1);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // impl1 intercepts during afterLock()
         when(impl1.afterLock(RESOURCE_A, OWNER2, false)).thenReturn(true);
 
         // owner2 tries to lock
-        fut = mgr.lock(RESOURCE_A, OWNER2, null);
-
-        assertTrue(fut.isDone());
-        assertFalse(fut.get());
+        assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC));
 
         // impl1 sees it, but impl2 does not
         verify(impl1).afterLock(RESOURCE_A, OWNER2, false);
@@ -226,10 +215,10 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testLock_Denied() {
 
-        mgr.lock(RESOURCE_A, OWNER1, callback1);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // owner2 tries to lock
-        fut = mgr.lock(RESOURCE_A, OWNER2, null);
+        mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC);
 
         verify(impl1).afterLock(RESOURCE_A, OWNER2, false);
         verify(impl2).afterLock(RESOURCE_A, OWNER2, false);
@@ -237,8 +226,8 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testUnlock() throws Exception {
-        mgr.lock(RESOURCE_A, OWNER1, null);
-        mgr.lock(RESOURCE_B, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.unlock(RESOURCE_A, OWNER1));
 
@@ -261,7 +250,7 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testUnlock_BeforeInterceptedTrue() {
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // have impl1 intercept
         when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_ACCEPTED);
@@ -278,7 +267,7 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testUnlock_BeforeInterceptedFalse() {
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // have impl1 intercept
         when(impl1.beforeUnlock(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED);
@@ -294,7 +283,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testUnlock_Unlocked() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.unlock(RESOURCE_A, OWNER1));
 
@@ -310,7 +299,7 @@ public class PolicyResourceLockManagerTest {
         // have impl1 intercept
         when(impl1.afterUnlock(RESOURCE_A, OWNER1, true)).thenReturn(true);
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.unlock(RESOURCE_A, OWNER1));
 
@@ -348,7 +337,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testIsLocked_True() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.isLocked(RESOURCE_A));
 
@@ -386,7 +375,7 @@ public class PolicyResourceLockManagerTest {
     public void testIsLocked_BeforeIntercepted_False() {
 
         // lock it so we can verify that impl1 overrides the superclass isLocker()
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // have impl1 intercept
         when(impl1.beforeIsLocked(RESOURCE_A)).thenReturn(OperResult.OPER_DENIED);
@@ -399,7 +388,7 @@ public class PolicyResourceLockManagerTest {
 
     @Test
     public void testIsLockedBy_True() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1));
 
@@ -410,7 +399,7 @@ public class PolicyResourceLockManagerTest {
     @Test
     public void testIsLockedBy_False() {
         // different owner
-        mgr.lock(RESOURCE_A, OWNER2, null);
+        mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC);
 
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1));
 
@@ -444,7 +433,7 @@ public class PolicyResourceLockManagerTest {
     public void testIsLockedBy_BeforeIntercepted_False() {
 
         // lock it so we can verify that impl1 overrides the superclass isLocker()
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // have impl1 intercept
         when(impl1.beforeIsLockedBy(RESOURCE_A, OWNER1)).thenReturn(OperResult.OPER_DENIED);
@@ -470,7 +459,7 @@ public class PolicyResourceLockManagerTest {
         // clear the implementer list
         implList.clear();
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertFalse(mgr.isLocked(RESOURCE_B));
index 9aeba53..14964e0 100644 (file)
@@ -26,10 +26,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.onap.policy.drools.core.lock.TestUtils.expectException;
 import java.util.LinkedList;
-import java.util.concurrent.CancellationException;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.AfterClass;
@@ -43,6 +40,12 @@ import org.powermock.reflect.Whitebox;
 
 public class SimpleLockManagerTest {
 
+    // Note: this must be a multiple of four
+    private static final int MAX_AGE_SEC = 4 * 60;
+    private static final int MAX_AGE_MS = MAX_AGE_SEC * 1000;
+    
+    private static final String EXPECTED_EXCEPTION = "expected exception";
+    
     private static final String NULL_RESOURCE_ID = "null resourceId";
     private static final String NULL_OWNER = "null owner";
 
@@ -63,7 +66,6 @@ public class SimpleLockManagerTest {
     private static CurrentTime savedTime;
 
     private TestTime testTime;
-    private Future<Boolean> fut;
     private SimpleLockManager mgr;
     
     @BeforeClass
@@ -91,10 +93,7 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testLock() throws Exception {
-        fut = mgr.lock(RESOURCE_A, OWNER1, null);
-
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1));
@@ -102,49 +101,59 @@ public class SimpleLockManagerTest {
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2));
 
         // null callback - not locked yet
-        fut = mgr.lock(RESOURCE_C, OWNER3, null);
-        assertTrue(fut.isDone());
-        assertTrue(fut.get());
+        assertTrue(mgr.lock(RESOURCE_C, OWNER3, MAX_AGE_SEC));
 
         // null callback - already locked
-        fut = mgr.lock(RESOURCE_A, OWNER3, null);
-        assertTrue(fut.isDone());
-        assertFalse(fut.get());
+        assertFalse(mgr.lock(RESOURCE_A, OWNER3, MAX_AGE_SEC));
     }
 
     @Test
-    public void testLock_AlreadyLocked() throws Exception {
-        mgr.lock(RESOURCE_A, OWNER1, null);
-
-        fut = mgr.lock(RESOURCE_A, OWNER2, null);
-        assertTrue(fut.isDone());
-        assertFalse(fut.get());
+    public void testLock_ExtendLock() throws Exception {
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        
+        // sleep half of the cycle
+        testTime.sleep(MAX_AGE_MS/2);
+        assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1));
+        
+        // extend the lock
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        
+        // verify still locked after sleeping the other half of the cycle
+        testTime.sleep(MAX_AGE_MS/2+1);
+        assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1));
+        
+        // and should release after another half cycle
+        testTime.sleep(MAX_AGE_MS/2);
+        assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1));
     }
 
-    @Test(expected = IllegalStateException.class)
-    public void testLock_SameOwner() throws Exception {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+    @Test
+    public void testLock_AlreadyLocked() throws Exception {
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        
+        // same owner
+        assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC));
 
-        // should throw an exception
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        // different owner
+        assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC));
     }
 
     @Test
     public void testLock_ArgEx() {
         IllegalArgumentException ex =
-                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, null));
+                        expectException(IllegalArgumentException.class, () -> mgr.lock(null, OWNER1, MAX_AGE_SEC));
         assertEquals(NULL_RESOURCE_ID, ex.getMessage());
 
-        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, null));
+        ex = expectException(IllegalArgumentException.class, () -> mgr.lock(RESOURCE_A, null, MAX_AGE_SEC));
         assertEquals(NULL_OWNER, ex.getMessage());
 
         // this should not throw an exception
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
     }
 
     @Test
     public void testUnlock() throws Exception {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // unlock it
         assertTrue(mgr.unlock(RESOURCE_A, OWNER1));
@@ -166,7 +175,7 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testUnlock_DiffOwner() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
         assertFalse(mgr.unlock(RESOURCE_A, OWNER2));
     }
 
@@ -174,8 +183,8 @@ public class SimpleLockManagerTest {
     public void testIsLocked() {
         assertFalse(mgr.isLocked(RESOURCE_A));
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
-        mgr.lock(RESOURCE_B, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
+        mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC);
 
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertTrue(mgr.isLocked(RESOURCE_B));
@@ -204,7 +213,7 @@ public class SimpleLockManagerTest {
     public void testIsLockedBy() {
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER1));
 
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         assertFalse(mgr.isLockedBy(RESOURCE_B, OWNER1));
 
@@ -230,7 +239,7 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testIsLockedBy_NotLocked() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // different resource, thus no lock
         assertFalse(mgr.isLockedBy(RESOURCE_B, OWNER1));
@@ -238,7 +247,7 @@ public class SimpleLockManagerTest {
 
     @Test
     public void testIsLockedBy_LockedButNotOwner() {
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
 
         // different owner
         assertFalse(mgr.isLockedBy(RESOURCE_A, OWNER2));
@@ -247,43 +256,49 @@ public class SimpleLockManagerTest {
     @Test
     public void testCleanUpLocks() throws Exception {
         // note: this assumes that MAX_AGE_MS is divisible by 4
-        mgr.lock(RESOURCE_A, OWNER1, null);
+        mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC);
         assertTrue(mgr.isLocked(RESOURCE_A));
         
         testTime.sleep(10);
-        mgr.lock(RESOURCE_B, OWNER1, null);
+        mgr.lock(RESOURCE_B, OWNER1, MAX_AGE_SEC);
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertTrue(mgr.isLocked(RESOURCE_B));
         
-        testTime.sleep(SimpleLockManager.MAX_AGE_MS/4);
-        mgr.lock(RESOURCE_C, OWNER1, null);
+        testTime.sleep(MAX_AGE_MS/4);
+        mgr.lock(RESOURCE_C, OWNER1, MAX_AGE_SEC);
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertTrue(mgr.isLocked(RESOURCE_B));
         assertTrue(mgr.isLocked(RESOURCE_C));
         
-        testTime.sleep(SimpleLockManager.MAX_AGE_MS/4);
-        mgr.lock(RESOURCE_D, OWNER1, null);
+        testTime.sleep(MAX_AGE_MS/4);
+        mgr.lock(RESOURCE_D, OWNER1, MAX_AGE_SEC);
         assertTrue(mgr.isLocked(RESOURCE_A));
         assertTrue(mgr.isLocked(RESOURCE_B));
         assertTrue(mgr.isLocked(RESOURCE_C));
         assertTrue(mgr.isLocked(RESOURCE_D));
         
         // sleep remainder of max age - first two should expire
-        testTime.sleep(SimpleLockManager.MAX_AGE_MS/2);
+        testTime.sleep(MAX_AGE_MS/2);
         assertFalse(mgr.isLocked(RESOURCE_A));
         assertFalse(mgr.isLocked(RESOURCE_B));
         assertTrue(mgr.isLocked(RESOURCE_C));
         assertTrue(mgr.isLocked(RESOURCE_D));
         
         // another quarter - next one should expire
-        testTime.sleep(SimpleLockManager.MAX_AGE_MS/4);
+        testTime.sleep(MAX_AGE_MS/4);
         assertFalse(mgr.isLocked(RESOURCE_C));
         assertTrue(mgr.isLocked(RESOURCE_D));
         
         // another quarter - last one should expire
-        testTime.sleep(SimpleLockManager.MAX_AGE_MS/4);
+        testTime.sleep(MAX_AGE_MS/4);
         assertFalse(mgr.isLocked(RESOURCE_D));
     }
+
+    @Test
+    public void testMakeNullArgException() {
+        IllegalArgumentException ex = SimpleLockManager.makeNullArgException(EXPECTED_EXCEPTION);
+        assertEquals(EXPECTED_EXCEPTION, ex.getMessage());
+    }
     
     @Test
     public void testDataGetXxx() {
@@ -390,16 +405,13 @@ public class SimpleLockManagerTest {
 
                     try {
                         // some locks will be acquired, some denied
-                        mgr.lock(res, owner, null).get();
+                        mgr.lock(res, owner, MAX_AGE_SEC);
 
                         // do some "work"
                         stopper.await(1L, TimeUnit.MILLISECONDS);
 
                         mgr.unlock(res, owner);
 
-                    } catch (CancellationException | ExecutionException e) {
-                        nfail.incrementAndGet();
-
                     } catch (InterruptedException expected) {
                         Thread.currentThread().interrupt();
                         break;