diff options
15 files changed, 230 insertions, 1148 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 665f8227..33e2d789 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 @@ -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 - diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java index b30fca76..03872e1e 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java @@ -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; @@ -57,11 +52,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. */ private BasicDataSource dataSource; @@ -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; - } - + } } diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java index b82f4b00..1f55a4cb 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingProperties.java @@ -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 index ccfb4c7d..00000000 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/Heartbeat.java +++ /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); - } - - } - - -} diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java index d57de1f7..0853f125 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java @@ -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); diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java index b01d9676..a4c292c1 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java @@ -20,29 +20,24 @@ 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,7 +151,7 @@ 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")); @@ -183,41 +159,9 @@ public class TargetLockTest { } @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")); } diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java index ea5e2521..d3764910 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java @@ -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 index a2e9e62e..00000000 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java +++ /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); - } -} diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java index d4e7bee9..1184b017 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java @@ -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 diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java index 97e7242d..afe1cabe 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java @@ -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 diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java index 9bcf2598..79fa0a7d 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java @@ -20,18 +20,13 @@ 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 index 7ac9e31c..00000000 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/LockRequestFutureTest.java +++ /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()); - } -} diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java index 57adc847..0404877b 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPITest.java @@ -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 diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java index 26732e42..92e60268 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/PolicyResourceLockManagerTest.java @@ -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)); diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java index 9aeba53c..14964e0e 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java @@ -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; |