From 0b80343610a215f26a7d764cc849f8e9ca44fea0 Mon Sep 17 00:00:00 2001 From: emaclee Date: Fri, 6 May 2022 10:36:29 +0100 Subject: [PATCH] Add graceful shutdown for Session Manager Introduce singleton session manager Add unit test for session manager Issue-Id: CPS-898 Signed-off-by: emaclee Change-Id: Iaf91f1aa6c1ebfe0ab907e7f7d80a01e940a0fdd --- .../ncmp/api/impl/config/NcmpConfiguration.java | 5 +- .../org/onap/cps/spi/config/CpsSessionFactory.java | 70 ++++++++++++++++++++ .../spi/impl/CpsDataPersistenceServiceImpl.java | 2 +- .../org/onap/cps/spi/utils/SessionManager.java | 54 +++++++++------ .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 2 +- .../spi/utils/SessionManagerIntegrationSpec.groovy | 18 ++++- .../onap/cps/spi/utils/SessionManagerSpec.groovy | 77 ++++++++++++++++------ 7 files changed, 184 insertions(+), 44 deletions(-) create mode 100644 cps-ri/src/main/java/org/onap/cps/spi/config/CpsSessionFactory.java 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 60b44c246..b04619e33 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 000000000..5241ea009 --- /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 daf4dd757..ded234bb4 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 e2786887a..6f96cffdc 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 sessionMap = new ConcurrentHashMap<>(); + private final ConcurrentHashMap 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 b37f471e7..a96b6aff9 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 9b58c8bc3..a1f6d580f 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 a2df06ef0..db766cd1f 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() + } + } -- 2.16.6