Merge "Add graceful shutdown for Session Manager"

7 files changed, 184 insertions, 44 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/config/NcmpConfiguration.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/config/NcmpConfiguration.java
index 3aa17b70cb..a981022256 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/config/NcmpConfiguration.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/config/NcmpConfiguration.java
@@ -1,6 +1,6 @@
 /*
  * ============LICENSE_START=======================================================
- *  Copyright (C) 2021 Nordix Foundation
+ *  Copyright (C) 2021-2022 Nordix Foundation
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@ package org.onap.cps.ncmp.api.impl.config;
 import java.util.Arrays;
 import lombok.Getter;
 import org.springframework.beans.factory.annotation.Value;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.boot.web.client.RestTemplateBuilder;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -53,7 +54,7 @@ public class NcmpConfiguration {
      * @return rest template instance
      */
     @Bean
-    @Scope("singleton")
+    @Scope(ConfigurableBeanFactory.SCOPE_SINGLETON)
     public static RestTemplate restTemplate(final RestTemplateBuilder restTemplateBuilder) {
         final RestTemplate restTemplate = restTemplateBuilder.build();
         setRestTemplateMessageConverters(restTemplate);
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/config/CpsSessionFactory.java b/cps-ri/src/main/java/org/onap/cps/spi/config/CpsSessionFactory.java
new file mode 100644
index 0000000000..5241ea0096
--- /dev/null
+++ b/cps-ri/src/main/java/org/onap/cps/spi/config/CpsSessionFactory.java
@@ -0,0 +1,70 @@
+/*
+ *  ============LICENSE_START=======================================================
+ *  Copyright (C) 2022 Nordix Foundation
+ *  ================================================================================
+ *  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.
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *  ============LICENSE_END=========================================================
+ */
+
+package org.onap.cps.spi.config;
+
+import org.hibernate.HibernateException;
+import org.hibernate.Session;
+import org.hibernate.SessionFactory;
+import org.onap.cps.spi.entities.AnchorEntity;
+import org.onap.cps.spi.entities.DataspaceEntity;
+import org.onap.cps.spi.entities.SchemaSetEntity;
+import org.onap.cps.spi.entities.YangResourceEntity;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.context.annotation.Scope;
+import org.springframework.stereotype.Component;
+
+@Component
+@Scope(ConfigurableBeanFactory.SCOPE_SINGLETON)
+public class CpsSessionFactory {
+
+    private SessionFactory sessionFactory = null;
+
+    /**
+     * Open a session from session factory.
+     *
+     * @return session
+     * @throws HibernateException hibernate exception
+     */
+    public Session openSession() throws HibernateException {
+        return getSessionFactory().openSession();
+    }
+
+    /**
+     * Close session factory.
+     *
+     * @throws HibernateException hibernate exception
+     */
+    public void closeSessionFactory() throws HibernateException {
+        getSessionFactory().close();
+    }
+
+    private SessionFactory getSessionFactory() {
+        if (sessionFactory == null) {
+            sessionFactory = new org.hibernate.cfg.Configuration().configure("hibernate.cfg.xml")
+                    .addAnnotatedClass(AnchorEntity.class)
+                    .addAnnotatedClass(DataspaceEntity.class)
+                    .addAnnotatedClass(SchemaSetEntity.class)
+                    .addAnnotatedClass(YangResourceEntity.class)
+                    .buildSessionFactory();
+        }
+        return sessionFactory;
+    }
+}
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
index daf4dd757b..ded234bb48 100644
--- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
@@ -217,7 +217,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
 
     @Override
     public void closeSession(final String sessionId) {
-        sessionManager.closeSession(sessionId);
+        sessionManager.closeSession(sessionId, SessionManager.WITH_COMMIT);
     }
 
     @Override
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/utils/SessionManager.java b/cps-ri/src/main/java/org/onap/cps/spi/utils/SessionManager.java
index e2786887ac..6f96cffdc1 100644
--- a/cps-ri/src/main/java/org/onap/cps/spi/utils/SessionManager.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/utils/SessionManager.java
@@ -29,44 +29,54 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import javax.annotation.PostConstruct;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.Session;
-import org.hibernate.SessionFactory;
-import org.hibernate.cfg.Configuration;
+import org.onap.cps.spi.config.CpsSessionFactory;
 import org.onap.cps.spi.entities.AnchorEntity;
 import org.onap.cps.spi.entities.DataspaceEntity;
-import org.onap.cps.spi.entities.SchemaSetEntity;
-import org.onap.cps.spi.entities.YangResourceEntity;
 import org.onap.cps.spi.exceptions.SessionManagerException;
 import org.onap.cps.spi.exceptions.SessionTimeoutException;
 import org.onap.cps.spi.repository.AnchorRepository;
 import org.onap.cps.spi.repository.DataspaceRepository;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
 
 @RequiredArgsConstructor
 @Slf4j
 @Component
+@Scope(ConfigurableBeanFactory.SCOPE_SINGLETON)
 public class SessionManager {
 
+    private final CpsSessionFactory cpsSessionFactory;
     private final TimeLimiterProvider timeLimiterProvider;
     private final DataspaceRepository dataspaceRepository;
     private final AnchorRepository anchorRepository;
-    private static SessionFactory sessionFactory;
-    private static ConcurrentHashMap<String, Session> sessionMap = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<String, Session> sessionMap = new ConcurrentHashMap<>();
+    public static final boolean WITH_COMMIT = true;
+    public static final boolean WITH_ROLLBACK = false;
 
-    private synchronized void buildSessionFactory() {
-        if (sessionFactory == null) {
-            sessionFactory = new Configuration().configure("hibernate.cfg.xml")
-                    .addAnnotatedClass(AnchorEntity.class)
-                    .addAnnotatedClass(DataspaceEntity.class)
-                    .addAnnotatedClass(SchemaSetEntity.class)
-                    .addAnnotatedClass(YangResourceEntity.class)
-                    .buildSessionFactory();
+    @PostConstruct
+    private void postConstruct() {
+        final Thread shutdownHook = new Thread(this::closeAllSessionsInShutdown);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);
+    }
+
+    private void closeAllSessionsInShutdown() {
+        for (final String sessionId : sessionMap.keySet()) {
+            try {
+                closeSession(sessionId, WITH_ROLLBACK);
+                log.info("Session with session ID {} rolled back and closed", sessionId);
+            } catch (final Exception e) {
+                log.warn("Session with session ID {} failed to close", sessionId);
+            }
         }
+        cpsSessionFactory.closeSessionFactory();
     }
 
     /**
@@ -75,8 +85,7 @@ public class SessionManager {
      * @return Session ID string
      */
     public String startSession() {
-        buildSessionFactory();
-        final Session session = sessionFactory.openSession();
+        final Session session = cpsSessionFactory.openSession();
         final String sessionId = UUID.randomUUID().toString();
         sessionMap.put(sessionId, session);
         session.beginTransaction();
@@ -85,14 +94,20 @@ public class SessionManager {
 
     /**
      * Close session.
-     * Locks will be released and changes will be committed.
+     * Changes are committed when commit boolean is set to true.
+     * Rollback will execute when commit boolean is set to false.
      *
      * @param sessionId session ID
+     * @param commit indicator whether session will commit or rollback
      */
-    public void closeSession(final String sessionId) {
+    public void closeSession(final String sessionId, final boolean commit) {
         try {
             final Session session = getSession(sessionId);
-            session.getTransaction().commit();
+            if (commit) {
+                session.getTransaction().commit();
+            } else {
+                session.getTransaction().rollback();
+            }
             session.close();
         } catch (final HibernateException e) {
             throw new SessionManagerException("Cannot close session",
@@ -162,4 +177,5 @@ public class SessionManager {
         }
         return session;
     }
+
 }
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
index b37f471e76..a96b6aff9b 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
@@ -127,7 +127,7 @@ class CpsDataPersistenceServiceSpec extends Specification {
         when: 'close session method is called with session ID as parameter'
             objectUnderTest.closeSession(someSessionId)
         then: 'the session manager method to close session is invoked with parameter'
-            1 * mockSessionManager.closeSession(someSessionId)
+            1 * mockSessionManager.closeSession(someSessionId, mockSessionManager.WITH_COMMIT)
     }
 
     def 'Lock anchor.'(){
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerIntegrationSpec.groovy
index 9b58c8bc32..a1f6d580fd 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerIntegrationSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerIntegrationSpec.groovy
@@ -20,6 +20,7 @@
 
 package org.onap.cps.spi.utils
 
+import org.onap.cps.spi.config.CpsSessionFactory
 import org.onap.cps.spi.exceptions.SessionManagerException
 import org.onap.cps.spi.impl.CpsPersistenceSpecBase
 import org.springframework.beans.factory.annotation.Autowired
@@ -32,6 +33,9 @@ class SessionManagerIntegrationSpec extends CpsPersistenceSpecBase{
     @Autowired
     SessionManager objectUnderTest
 
+    @Autowired
+    CpsSessionFactory cpsSessionFactory
+
     def sessionId
     def shortTimeoutForTesting = 200L
 
@@ -40,7 +44,7 @@ class SessionManagerIntegrationSpec extends CpsPersistenceSpecBase{
     }
 
     def cleanup(){
-        objectUnderTest.closeSession(sessionId)
+        objectUnderTest.closeSession(sessionId, objectUnderTest.WITH_COMMIT)
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -62,8 +66,18 @@ class SessionManagerIntegrationSpec extends CpsPersistenceSpecBase{
             def thrown = thrown(SessionManagerException)
             thrown.message.contains('Timeout')
         then: 'when the other session holding the lock is closed, lock can finally be acquired'
-            objectUnderTest.closeSession(otherSessionId)
+            objectUnderTest.closeSession(otherSessionId, objectUnderTest.WITH_COMMIT)
             objectUnderTest.lockAnchor(sessionId,DATASPACE_NAME,ANCHOR_NAME1,shortTimeoutForTesting)
     }
 
+    @Sql([CLEAR_DATA, SET_DATA])
+    def 'Lock anchor twice using the same session.'(){
+        given: 'session that already holds an anchor lock'
+            objectUnderTest.lockAnchor(sessionId, DATASPACE_NAME, ANCHOR_NAME1, shortTimeoutForTesting)
+        when: 'same session tries to acquire same anchor lock'
+            objectUnderTest.lockAnchor(sessionId, DATASPACE_NAME, ANCHOR_NAME1, shortTimeoutForTesting)
+        then: 'no exception is thrown'
+            noExceptionThrown()
+    }
+
 }
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerSpec.groovy
index a2df06ef0e..db766cd1f3 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/SessionManagerSpec.groovy
@@ -23,28 +23,36 @@ package org.onap.cps.spi.utils
 import com.google.common.util.concurrent.TimeLimiter
 import org.hibernate.HibernateException
 import org.hibernate.Transaction
+import org.onap.cps.spi.config.CpsSessionFactory
 import org.onap.cps.spi.entities.AnchorEntity
 import org.onap.cps.spi.exceptions.SessionManagerException
 import org.onap.cps.spi.repository.AnchorRepository
 import org.onap.cps.spi.repository.DataspaceRepository
-import org.testcontainers.shaded.com.google.common.util.concurrent.UncheckedExecutionException
 import spock.lang.Specification
 import org.hibernate.Session
-
 import java.util.concurrent.ExecutionException
 
 class SessionManagerSpec extends Specification {
 
+    def mockCpsSessionFactory = Mock(CpsSessionFactory)
     def spiedTimeLimiterProvider = Spy(TimeLimiterProvider)
     def mockDataspaceRepository = Mock(DataspaceRepository)
     def mockAnchorRepository = Mock(AnchorRepository)
-    def mockSession = Mock(Session)
+    def mockSession1 = Mock(Session)
+    def mockSession2 = Mock(Session)
+    def mockTransaction1 = Mock(Transaction)
+    def mockTransaction2 = Mock(Transaction)
+
+    def objectUnderTest = new SessionManager(mockCpsSessionFactory, spiedTimeLimiterProvider, mockDataspaceRepository, mockAnchorRepository)
 
-    def objectUnderTest = new SessionManager(spiedTimeLimiterProvider, mockDataspaceRepository, mockAnchorRepository)
+    def setup(){
+        mockSession1.getTransaction() >> mockTransaction1
+        mockSession2.getTransaction() >> mockTransaction2
+    }
 
-    def 'Lock anchor entity with #exceptionDuringTest exception.'(){
+    def 'Lock anchor entity with #exceptionDuringTest exception.'() {
         given: 'a dummy session'
-            objectUnderTest.sessionMap.put('dummySession', mockSession)
+            objectUnderTest.sessionMap.put('dummy-session', mockSession1)
         and: 'the anchor name can be resolved'
             def mockAnchorEntity = Mock(AnchorEntity)
             mockAnchorEntity.getId() > 456
@@ -54,33 +62,49 @@ class SessionManagerSpec extends Specification {
             spiedTimeLimiterProvider.getTimeLimiter(_) >> mockTimeLimiter
             mockTimeLimiter.callWithTimeout(*_) >> { throw exceptionDuringTest }
         when: 'session tries to acquire anchor lock'
-            objectUnderTest.lockAnchor('dummySession', 'some-dataspace','some-anchor', 123L)
+            objectUnderTest.lockAnchor('dummy-session', 'some-dataspace', 'some-anchor', 123L)
         then: 'a session manager exception is thrown with the expected detail'
             def thrown = thrown(SessionManagerException)
             thrown.details.contains(expectedExceptionDetail)
         where:
-            exceptionDuringTest               || expectedExceptionDetail
-            new InterruptedException()        || 'interrupted'
-            new ExecutionException()          || 'aborted'
+            exceptionDuringTest        || expectedExceptionDetail
+            new InterruptedException() || 'interrupted'
+            new ExecutionException()   || 'aborted'
+    }
+
+    def 'Close a session' () {
+        given: 'a session in the session map'
+            objectUnderTest.sessionMap.putAll([testSessionId1:mockSession1])
+        when: 'the session manager closes session'
+            objectUnderTest.closeSession('testSessionId1', commit)
+        then: 'commit or rollback is called on the transaction as appropriate'
+            if (commit) {
+                1 * mockTransaction1.commit()
+            } else {
+                1 * mockTransaction1.rollback()
+            }
+        and: 'the correct session is closed'
+            1 * mockSession1.close()
+        where:
+            commit << [SessionManager.WITH_COMMIT, SessionManager.WITH_ROLLBACK]
     }
 
     def 'Close session that does not exist.'() {
         when: 'attempt to close session that does not exist'
-            objectUnderTest.closeSession('unknown session id')
+            objectUnderTest.closeSession('unknown session id', SessionManager.WITH_COMMIT)
         then: 'a session manager exception is thrown with the unknown id in the details'
             def thrown = thrown(SessionManagerException)
             assert thrown.details.contains('unknown session id')
     }
 
     def 'Hibernate exception while closing session.'() {
-        given: 'a test session with a transaction'
-            objectUnderTest.sessionMap.put('testSessionId', mockSession)
-            mockSession.getTransaction() >> Mock(Transaction)
+        given: 'a test session in session map'
+            objectUnderTest.sessionMap.put('testSessionId', mockSession1)
         and: 'an hibernate exception when closing that session'
             def hibernateException = new HibernateException('test')
-            mockSession.close() >> { throw hibernateException }
+            mockSession1.close() >> { throw hibernateException }
         when: 'attempt to close session'
-            objectUnderTest.closeSession('testSessionId')
+            objectUnderTest.closeSession('testSessionId', SessionManager.WITH_COMMIT)
         then: 'a session manager exception is thrown with the session id in the details'
             def thrown = thrown(SessionManagerException)
             assert thrown.details.contains('testSessionId')
@@ -88,12 +112,27 @@ class SessionManagerSpec extends Specification {
             assert thrown.cause == hibernateException
     }
 
-    def 'Attempt to lock anchor entity with session Id that does not exists'(){
-        when: 'attempt to acquire anchor lock with session that does not exists'
-            objectUnderTest.lockAnchor('unknown session id','','',123L)
+    def 'Attempt to lock anchor entity with session Id that does not exist'() {
+        when: 'attempt to acquire anchor lock with session that does not exist'
+            objectUnderTest.lockAnchor('unknown session id', '', '', 123L)
         then: 'a session manager exception is thrown with the unknown id in the details'
             def thrown = thrown(SessionManagerException)
             thrown.details.contains('unknown session id')
     }
 
+    def 'Close all sessions in shutdown.'() {
+        given: 'sessions that holds transactions in the session map'
+            objectUnderTest.sessionMap.putAll([testSessionId1:mockSession1, otherSessionId:mockSession2])
+        when: 'shutdown method to close all sessions is called'
+            objectUnderTest.closeAllSessionsInShutdown()
+        then: 'commit is called on each transaction'
+            1 * mockTransaction1.rollback()
+            1 * mockTransaction2.rollback()
+        and: 'each session is closed'
+            1 * mockSession1.close()
+            1 * mockSession2.close()
+        then: 'session factory is closed'
+            1 * mockCpsSessionFactory.closeSessionFactory()
+    }
+
 }