Refactor duplicate code from lock managers

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

3 files changed, 449 insertions, 168 deletions

diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java
new file mode 100644
index 00000000..d4e4f5fc
--- /dev/null
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java
@@ -0,0 +1,211 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2019 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.system.internal;
+
+import java.util.concurrent.ScheduledExecutorService;
+import org.onap.policy.drools.core.lock.LockCallback;
+import org.onap.policy.drools.core.lock.LockImpl;
+import org.onap.policy.drools.core.lock.LockState;
+import org.onap.policy.drools.system.PolicyEngineConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Lock implementation used by locking features.
+ */
+public abstract class FeatureLockImpl extends LockImpl {
+    private static final long serialVersionUID = 1L;
+    private static final Logger logger = LoggerFactory.getLogger(FeatureLockImpl.class);
+
+    public static final String LOCK_LOST_MSG = "lock lost";
+
+    /**
+     * {@code True} if this lock is attached to a feature, {@code false} if it is not.
+     */
+    private transient boolean attached;
+
+    /**
+     * Constructs the object.
+     */
+    public FeatureLockImpl() {
+        this.attached = false;
+    }
+
+    /**
+     * Constructs the object.
+     *
+     * @param state initial state of the lock
+     * @param resourceId identifier of the resource to be locked
+     * @param ownerKey information identifying the owner requesting the lock
+     * @param holdSec amount of time, in seconds, for which the lock should be held, after
+     *        which it will automatically be released
+     * @param callback callback to be invoked once the lock is granted, or subsequently
+     *        lost; must not be {@code null}
+     */
+    public FeatureLockImpl(LockState state, String resourceId, String ownerKey, int holdSec, LockCallback callback) {
+        super(state, resourceId, ownerKey, holdSec, callback);
+        this.attached = true;
+    }
+
+    /**
+     * Grants this lock. The notification is <i>always</i> invoked via a background
+     * thread.
+     *
+     * @param foreground {@code true} if to invoke the callback in the foreground thread,
+     *        {@code false} otherwise
+     */
+    protected synchronized void grant(boolean foreground) {
+        if (isUnavailable()) {
+            return;
+        }
+
+        setState(LockState.ACTIVE);
+        updateGrant();
+
+        logger.info("lock granted: {}", this);
+
+        if (foreground) {
+            notifyAvailable();
+        } else {
+            getThreadPool().execute(this::notifyAvailable);
+        }
+    }
+
+    /**
+     * Permanently denies this lock.
+     *
+     * @param reason the reason the lock was denied
+     * @param foreground {@code true} if to invoke the callback in the foreground thread,
+     *        {@code false} otherwise
+     */
+    public void deny(String reason, boolean foreground) {
+        synchronized (this) {
+            setState(LockState.UNAVAILABLE);
+        }
+
+        logger.info("{}: {}", reason, this);
+
+        if (foreground) {
+            notifyUnavailable();
+        } else {
+            getThreadPool().execute(this::notifyUnavailable);
+        }
+    }
+
+    /**
+     * The subclass should make use of {@link #freeAllowed()} in its implementation of
+     * {@link #free()}.
+     */
+    @Override
+    public abstract boolean free();
+
+    /**
+     * Determines if the lock can be freed.
+     *
+     * @return {@code true} if the lock can be freed, {@code false} if the lock is
+     *         unavailable
+     */
+    protected boolean freeAllowed() {
+        // do a quick check of the state
+        if (isUnavailable()) {
+            return false;
+        }
+
+        logger.info("releasing lock: {}", this);
+
+        if (!attachFeature()) {
+            setState(LockState.UNAVAILABLE);
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * The subclass should make use of {@link #extendAllowed()} in its implementation of
+     * {@link #extend()}.
+     */
+    @Override
+    public abstract void extend(int holdSec, LockCallback callback);
+
+    /**
+     * Determines if the lock can be extended.
+     *
+     * @param holdSec the additional amount of time to hold the lock, in seconds
+     * @param callback callback to be invoked when the extension completes
+     * @return {@code true} if the lock can be extended, {@code false} if the lock is
+     *         unavailable
+     */
+    protected boolean extendAllowed(int holdSec, LockCallback callback) {
+        if (holdSec < 0) {
+            throw new IllegalArgumentException("holdSec is negative");
+        }
+
+        setHoldSec(holdSec);
+        setCallback(callback);
+
+        // do a quick check of the state
+        if (isUnavailable() || !attachFeature()) {
+            deny(LOCK_LOST_MSG, true);
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Attaches to the feature instance, if not already attached.
+     *
+     * @return {@code true} if the lock is now attached to a feature, {@code false}
+     *         otherwise
+     */
+    private synchronized boolean attachFeature() {
+        if (!attached) {
+            attached = addToFeature();
+        }
+
+        return attached;
+    }
+
+    /**
+     * Updates a lock when it is granted. The default method does nothing.
+     */
+    protected void updateGrant() {
+        // do nothing
+    }
+
+    /**
+     * Adds the lock to the relevant feature.
+     *
+     * @return {@code true} if the lock was added, {@code false} if it could not be added
+     *         (e.g., because there is no feature yet)
+     */
+    protected abstract boolean addToFeature();
+
+    /**
+     * Gets the thread pool.
+     *
+     * @return the thread pool
+     */
+    protected ScheduledExecutorService getThreadPool() {
+        return PolicyEngineConstants.getManager().getExecutorService();
+    }
+}
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java
new file mode 100644
index 00000000..7e4505be
--- /dev/null
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java
@@ -0,0 +1,197 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2019 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.system.internal;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import lombok.AccessLevel;
+import lombok.Getter;
+import org.onap.policy.drools.core.lock.AlwaysFailLock;
+import org.onap.policy.drools.core.lock.Lock;
+import org.onap.policy.drools.core.lock.LockCallback;
+import org.onap.policy.drools.core.lock.LockState;
+import org.onap.policy.drools.core.lock.PolicyResourceLockManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Super class for Lock Features.
+ *
+ * <p/>
+ * Note: this implementation does <i>not</i> honor the waitForLocks={@code true}
+ * parameter.
+ *
+ * <p/>
+ * When a lock is deserialized, it will not initially appear in this feature's map; it
+ * will be added to the map once free() or extend() is invoked, provided there isn't
+ * already an entry.
+ */
+public abstract class LockManager<T extends FeatureLockImpl> implements PolicyResourceLockManager {
+
+    private static final Logger logger = LoggerFactory.getLogger(LockManager.class);
+
+    public static final String NOT_LOCKED_MSG = "not locked";
+
+    /**
+     * Maps a resource to the lock that owns it.
+     */
+    @Getter(AccessLevel.PROTECTED)
+    private final Map<String, T> resource2lock = new ConcurrentHashMap<>();
+
+    /**
+     * {@code True} if this feature is running, {@code false} otherwise.
+     */
+    private boolean alive = false;
+
+    /**
+     * {@code True} if this feature is locked, {@code false} otherwise.
+     */
+    private boolean locked = false;
+
+
+    /**
+     * Constructs the object.
+     */
+    public LockManager() {
+        super();
+    }
+
+    @Override
+    public boolean isAlive() {
+        return alive;
+    }
+
+    @Override
+    public synchronized boolean start() {
+        if (alive) {
+            return false;
+        }
+
+        alive = true;
+        return true;
+    }
+
+    /**
+     * Stops the expiration checker. Does <i>not</i> invoke any lock call-backs.
+     */
+    @Override
+    public synchronized boolean stop() {
+        if (!alive) {
+            return false;
+        }
+
+        alive = false;
+        return true;
+    }
+
+    @Override
+    public void shutdown() {
+        stop();
+    }
+
+    @Override
+    public boolean isLocked() {
+        return locked;
+    }
+
+    @Override
+    public synchronized boolean lock() {
+        if (locked) {
+            return false;
+        }
+
+        locked = true;
+        return true;
+    }
+
+    @Override
+    public synchronized boolean unlock() {
+        if (!locked) {
+            return false;
+        }
+
+        locked = false;
+        return true;
+    }
+
+    /**
+     * After performing checks, this invokes
+     * {@link #makeLock(LockState, String, String, int, LockCallback)} to create a lock
+     * object, inserts it into the map, and then invokes {@link #finishLock(MgrLock)}.
+     */
+    @Override
+    public Lock createLock(String resourceId, String ownerKey, int holdSec, LockCallback callback,
+                    boolean waitForLock) {
+
+        if (hasInstanceChanged()) {
+            AlwaysFailLock lock = new AlwaysFailLock(resourceId, ownerKey, holdSec, callback);
+            lock.notifyUnavailable();
+            return lock;
+        }
+
+        T lock = makeLock(LockState.WAITING, resourceId, ownerKey, holdSec, callback);
+
+        T existingLock = resource2lock.putIfAbsent(resourceId, lock);
+
+        if (existingLock == null) {
+            logger.debug("added lock to map {}", lock);
+            finishLock(lock);
+        } else {
+            lock.deny("resource is busy", true);
+        }
+
+        return lock;
+    }
+
+    /**
+     * Determines if this object is no longer the current instance of this feature type.
+     *
+     * @return {@code true} if this object is no longer the current instance,
+     *         {@code false} otherwise
+     */
+    protected abstract boolean hasInstanceChanged();
+
+    /**
+     * Finishes the steps required to establish a lock, changing its state to ACTIVE, when
+     * appropriate.
+     *
+     * @param lock the lock to be locked
+     */
+    protected abstract void finishLock(T lock);
+
+    // these may be overridden by junit tests
+
+    /**
+     * Makes a lock of the appropriate type.
+     *
+     * @param state initial state of the lock
+     * @param resourceId identifier of the resource to be locked
+     * @param ownerKey information identifying the owner requesting the lock
+     * @param holdSec amount of time, in seconds, for which the lock should be held, after
+     *        which it will automatically be released
+     * @param callback callback to be invoked once the lock is granted, or subsequently
+     *        lost; must not be {@code null}
+     * @return a new lock
+     */
+    protected abstract T makeLock(LockState waiting, String resourceId, String ownerKey, int holdSec,
+                    LockCallback callback);
+}
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java
index f5163e9b..839c17dc 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java
@@ -23,8 +23,6 @@ package org.onap.policy.drools.system.internal;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -34,13 +32,10 @@ import lombok.Getter;
 import lombok.Setter;
 import org.onap.policy.common.utils.properties.exception.PropertyException;
 import org.onap.policy.common.utils.time.CurrentTime;
-import org.onap.policy.drools.core.lock.AlwaysFailLock;
-import org.onap.policy.drools.core.lock.Lock;
 import org.onap.policy.drools.core.lock.LockCallback;
-import org.onap.policy.drools.core.lock.LockImpl;
 import org.onap.policy.drools.core.lock.LockState;
-import org.onap.policy.drools.core.lock.PolicyResourceLockManager;
 import org.onap.policy.drools.system.PolicyEngine;
+import org.onap.policy.drools.system.PolicyEngineConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -58,13 +53,10 @@ import org.slf4j.LoggerFactory;
  * will be added to the map once free() or extend() is invoked, provided there isn't
  * already an entry.
  */
-public class SimpleLockManager implements PolicyResourceLockManager {
+public class SimpleLockManager extends LockManager<SimpleLockManager.SimpleLock> {
 
     private static final Logger logger = LoggerFactory.getLogger(SimpleLockManager.class);
 
-    private static final String NOT_LOCKED_MSG = "not locked";
-    private static final String LOCK_LOST_MSG = "lock lost";
-
     /**
      * Provider of current time. May be overridden by junit tests.
      */
@@ -74,12 +66,6 @@ public class SimpleLockManager implements PolicyResourceLockManager {
     @Setter(AccessLevel.PROTECTED)
     private static SimpleLockManager latestInstance = null;
 
-
-    /**
-     * Engine with which this manager is associated.
-     */
-    private final PolicyEngine engine;
-
     /**
      * Feature properties.
      */
@@ -88,13 +74,7 @@ public class SimpleLockManager implements PolicyResourceLockManager {
     /**
      * Maps a resource to the lock that owns it.
      */
-    private final Map<String, SimpleLock> resource2lock = new ConcurrentHashMap<>();
-
-    /**
-     * Thread pool used to check for lock expiration and to notify owners when locks are
-     * lost.
-     */
-    private ScheduledExecutorService exsvc = null;
+    private final Map<String, SimpleLock> resource2lock;
 
     /**
      * Used to cancel the expiration checker on shutdown.
@@ -110,8 +90,8 @@ public class SimpleLockManager implements PolicyResourceLockManager {
      */
     public SimpleLockManager(PolicyEngine engine, Properties properties) {
         try {
-            this.engine = engine;
             this.featProps = new SimpleLockProperties(properties);
+            this.resource2lock = getResource2lock();
 
         } catch (PropertyException e) {
             throw new SimpleLockManagerException(e);
@@ -119,24 +99,17 @@ public class SimpleLockManager implements PolicyResourceLockManager {
     }
 
     @Override
-    public boolean isAlive() {
-        return (checker != null);
-    }
-
-    @Override
-    public boolean start() {
-        if (checker != null) {
+    public synchronized boolean start() {
+        if (isAlive()) {
             return false;
         }
 
-        exsvc = getThreadPool();
-
-        checker = exsvc.scheduleWithFixedDelay(this::checkExpired, featProps.getExpireCheckSec(),
-                        featProps.getExpireCheckSec(), TimeUnit.SECONDS);
+        checker = PolicyEngineConstants.getManager().getExecutorService().scheduleWithFixedDelay(this::checkExpired,
+                        featProps.getExpireCheckSec(), featProps.getExpireCheckSec(), TimeUnit.SECONDS);
 
         setLatestInstance(this);
 
-        return true;
+        return super.start();
     }
 
     /**
@@ -144,9 +117,7 @@ public class SimpleLockManager implements PolicyResourceLockManager {
      */
     @Override
     public synchronized boolean stop() {
-        exsvc = null;
-
-        if (checker == null) {
+        if (!super.stop()) {
             return false;
         }
 
@@ -158,49 +129,6 @@ public class SimpleLockManager implements PolicyResourceLockManager {
         return true;
     }
 
-    @Override
-    public void shutdown() {
-        stop();
-    }
-
-    @Override
-    public boolean isLocked() {
-        return false;
-    }
-
-    @Override
-    public boolean lock() {
-        return true;
-    }
-
-    @Override
-    public boolean unlock() {
-        return true;
-    }
-
-    @Override
-    public Lock createLock(String resourceId, String ownerKey, int holdSec, LockCallback callback,
-                    boolean waitForLock) {
-
-        if (latestInstance != this) {
-            AlwaysFailLock lock = new AlwaysFailLock(resourceId, ownerKey, holdSec, callback);
-            lock.notifyUnavailable();
-            return lock;
-        }
-
-        SimpleLock lock = makeLock(LockState.WAITING, resourceId, ownerKey, holdSec, callback);
-
-        SimpleLock existingLock = resource2lock.putIfAbsent(resourceId, lock);
-
-        if (existingLock == null) {
-            lock.grant();
-        } else {
-            lock.deny("resource is busy");
-        }
-
-        return lock;
-    }
-
     /**
      * Checks for expired locks.
      */
@@ -230,15 +158,25 @@ public class SimpleLockManager implements PolicyResourceLockManager {
 
             SimpleLock lock = lockref.get();
             if (lock != null) {
-                lock.deny("lock expired");
+                lock.deny("lock expired", false);
             }
         }
     }
 
+    @Override
+    protected void finishLock(SimpleLock lock) {
+        lock.grant(true);
+    }
+
+    @Override
+    protected boolean hasInstanceChanged() {
+        return (getLatestInstance() != this);
+    }
+
     /**
      * Simple Lock implementation.
      */
-    public static class SimpleLock extends LockImpl {
+    public static class SimpleLock extends FeatureLockImpl {
         private static final long serialVersionUID = 1L;
 
         /**
@@ -248,17 +186,16 @@ public class SimpleLockManager implements PolicyResourceLockManager {
         private long holdUntilMs;
 
         /**
-         * Feature containing this lock. May be {@code null} until the feature is
-         * identified. Note: this can only be null if the lock has been de-serialized.
+         * Map that should hold this lock.
          */
-        private transient SimpleLockManager feature;
+        private transient Map<String, SimpleLock> resource2lock;
 
         /**
          * Constructs the object.
          */
         public SimpleLock() {
             this.holdUntilMs = 0;
-            this.feature = null;
+            this.resource2lock = null;
         }
 
         /**
@@ -276,7 +213,7 @@ public class SimpleLockManager implements PolicyResourceLockManager {
         public SimpleLock(LockState state, String resourceId, String ownerKey, int holdSec, LockCallback callback,
                         SimpleLockManager feature) {
             super(state, resourceId, ownerKey, holdSec, callback);
-            this.feature = feature;
+            this.resource2lock = feature.resource2lock;
         }
 
         /**
@@ -289,62 +226,15 @@ public class SimpleLockManager implements PolicyResourceLockManager {
             return (holdUntilMs <= currentMs);
         }
 
-        /**
-         * Grants this lock. The notification is <i>always</i> invoked via a background
-         * thread.
-         */
-        protected synchronized void grant() {
-            if (isUnavailable()) {
-                return;
-            }
-
-            setState(LockState.ACTIVE);
-            holdUntilMs = currentTime.getMillis() + TimeUnit.SECONDS.toMillis(getHoldSec());
-
-            logger.info("lock granted: {}", this);
-
-            feature.exsvc.execute(this::notifyAvailable);
-        }
-
-        /**
-         * Permanently denies this lock. The notification is invoked via a background
-         * thread, if a feature instance is attached, otherwise it uses the foreground
-         * thread.
-         *
-         * @param reason the reason the lock was denied
-         */
-        protected void deny(String reason) {
-            synchronized (this) {
-                setState(LockState.UNAVAILABLE);
-            }
-
-            logger.info("{}: {}", reason, this);
-
-            if (feature == null) {
-                notifyUnavailable();
-
-            } else {
-                feature.exsvc.execute(this::notifyUnavailable);
-            }
-        }
-
         @Override
         public boolean free() {
-            // do a quick check of the state
-            if (isUnavailable()) {
-                return false;
-            }
-
-            logger.info("releasing lock: {}", this);
-
-            if (!attachFeature()) {
-                setState(LockState.UNAVAILABLE);
+            if (!freeAllowed()) {
                 return false;
             }
 
             AtomicBoolean result = new AtomicBoolean(false);
 
-            feature.resource2lock.computeIfPresent(getResourceId(), (resourceId, curlock) -> {
+            resource2lock.computeIfPresent(getResourceId(), (resourceId, curlock) -> {
 
                 if (curlock == this) {
                     // this lock was the owner - resource is now available
@@ -362,46 +252,33 @@ public class SimpleLockManager implements PolicyResourceLockManager {
 
         @Override
         public void extend(int holdSec, LockCallback callback) {
-            if (holdSec < 0) {
-                throw new IllegalArgumentException("holdSec is negative");
-            }
-
-            setHoldSec(holdSec);
-            setCallback(callback);
-
-            // do a quick check of the state
-            if (isUnavailable() || !attachFeature()) {
-                deny(LOCK_LOST_MSG);
+            if (!extendAllowed(holdSec, callback)) {
                 return;
             }
 
-            if (feature.resource2lock.get(getResourceId()) == this) {
-                grant();
+            if (resource2lock.get(getResourceId()) == this) {
+                grant(true);
             } else {
-                deny(NOT_LOCKED_MSG);
+                deny(NOT_LOCKED_MSG, true);
             }
         }
 
-        /**
-         * Attaches to the feature instance, if not already attached.
-         *
-         * @return {@code true} if the lock is now attached to a feature, {@code false}
-         *         otherwise
-         */
-        private synchronized boolean attachFeature() {
-            if (feature != null) {
-                // already attached
-                return true;
-            }
+        @Override
+        protected void updateGrant() {
+            holdUntilMs = currentTime.getMillis() + TimeUnit.SECONDS.toMillis(getHoldSec());
+        }
 
-            feature = latestInstance;
+        @Override
+        protected boolean addToFeature() {
+            SimpleLockManager feature = getLatestInstance();
             if (feature == null) {
                 logger.warn("no feature yet for {}", this);
                 return false;
             }
 
             // put this lock into the map
-            feature.resource2lock.putIfAbsent(getResourceId(), this);
+            resource2lock = feature.resource2lock;
+            resource2lock.putIfAbsent(getResourceId(), this);
 
             return true;
         }
@@ -415,10 +292,6 @@ public class SimpleLockManager implements PolicyResourceLockManager {
 
     // these may be overridden by junit tests
 
-    protected ScheduledExecutorService getThreadPool() {
-        return engine.getExecutorService();
-    }
-
     protected SimpleLock makeLock(LockState waiting, String resourceId, String ownerKey, int holdSec,
                     LockCallback callback) {
         return new SimpleLock(waiting, resourceId, ownerKey, holdSec, callback, this);