Add time limit to locking facility

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>

5 files changed, 58 insertions, 340 deletions

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