Fix simultaneous write access to policy provider 01/107001/2
authorliamfallon <liam.fallon@est.tech>
Sat, 2 May 2020 00:35:07 +0000 (01:35 +0100)
committerliamfallon <liam.fallon@est.tech>
Mon, 4 May 2020 20:20:38 +0000 (21:20 +0100)
This change serializes write access to the policy database via the
AUthorative TOSCA provider by making sure that only one request is
executed at a time. His change should be repolaced by proper session
handling in the next release of the Policy Framework project.

Issue-ID: POLICY-2533
Change-Id: I5fe4c0f2846981a66eb2f4e1da936fe3c9490ae5
Signed-off-by: liamfallon <liam.fallon@est.tech>
models-provider/pom.xml
models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyToscaPersistenceTest.java
models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyTypePersistenceTest.java
models-tosca/src/main/java/org/onap/policy/models/tosca/authorative/provider/AuthorativeToscaProvider.java

index 20ea9ab..f5dd6ae 100644 (file)
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.awaitility</groupId>
+            <artifactId>awaitility</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
index b7145d0..cbc57fe 100644 (file)
@@ -20,6 +20,7 @@
 
 package org.onap.policy.models.provider.impl;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
@@ -29,6 +30,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import lombok.NonNull;
 
@@ -238,8 +241,19 @@ public class PolicyToscaPersistenceTest {
     public void testPolicyPersistence(@NonNull final ToscaServiceTemplate serviceTemplate) throws Exception {
         assertNotNull(serviceTemplate);
 
-        databaseProvider.createPolicies(serviceTemplate);
-        databaseProvider.updatePolicies(serviceTemplate);
+        CountDownLatch threadCountDownLatch = new CountDownLatch(10);
+
+        for (int i = 0; i < 10; i++) {
+            new Thread() {
+                public void run() {
+                    assertThatCode(() -> databaseProvider.createPolicies(serviceTemplate)).doesNotThrowAnyException();
+                    assertThatCode(() -> databaseProvider.updatePolicies(serviceTemplate)).doesNotThrowAnyException();
+                    threadCountDownLatch.countDown();
+                }
+            }.start();
+        }
+
+        threadCountDownLatch.await(10, TimeUnit.SECONDS);
 
         for (Map<String, ToscaPolicy> policyMap : serviceTemplate.getToscaTopologyTemplate().getPolicies()) {
             for (ToscaPolicy policy : policyMap.values()) {
index 2272218..6cda57b 100644 (file)
 package org.onap.policy.models.provider.impl;
 
 import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.awaitility.Awaitility.await;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Base64;
 import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.After;
 import org.junit.Before;
@@ -82,7 +85,7 @@ public class PolicyTypePersistenceTest {
             String policyTypeString = ResourceUtils.getResourceAsString(policyTypeFilePath);
 
             ToscaServiceTemplate foundPolicyTypeSt =
-                    yamlTranslator.fromYaml(policyTypeString, ToscaServiceTemplate.class);
+                yamlTranslator.fromYaml(policyTypeString, ToscaServiceTemplate.class);
 
             serviceTemplate.setDerivedFrom(foundPolicyTypeSt.getDerivedFrom());
             serviceTemplate.setDescription(foundPolicyTypeSt.getDescription());
@@ -107,13 +110,25 @@ public class PolicyTypePersistenceTest {
             }
         }
 
-        assertThatCode(() -> databaseProvider.createPolicyTypes(serviceTemplate)).doesNotThrowAnyException();
+        CountDownLatch threadCountDownLatch = new CountDownLatch(10);
+
+        for (int i = 0; i < 10; i++) {
+            new Thread() {
+                public void run() {
+                    assertThatCode(() -> databaseProvider.createPolicyTypes(serviceTemplate))
+                        .doesNotThrowAnyException();
+                    threadCountDownLatch.countDown();
+                }
+            }.start();
+        }
+
+        threadCountDownLatch.await(9, TimeUnit.SECONDS);
 
         ToscaEntityKey resourceOptimizationPtKey =
-                new ToscaEntityKey("onap.policies.optimization.resource.OptimizationPolicy", "1.0.0");
+            new ToscaEntityKey("onap.policies.optimization.resource.OptimizationPolicy", "1.0.0");
 
         ToscaServiceTemplate resOptPolicyTypeSt = databaseProvider.getPolicyTypes(resourceOptimizationPtKey.getName(),
-                resourceOptimizationPtKey.getVersion());
+            resourceOptimizationPtKey.getVersion());
 
         assertEquals(3, resOptPolicyTypeSt.getPolicyTypesAsMap().size());
         assertTrue(resOptPolicyTypeSt.getPolicyTypesAsMap().containsKey(resourceOptimizationPtKey));
index a4a6e59..c595b55 100644 (file)
@@ -56,6 +56,9 @@ import org.slf4j.LoggerFactory;
 public class AuthorativeToscaProvider {
     private static final Logger LOGGER = LoggerFactory.getLogger(AuthorativeToscaProvider.class);
 
+    // TODO: In next release this locking mechanism should be removed and replaced with proper session handling
+    private static final Object providerLockObject = "providerLockObject";
+
     /**
      * Get policy types.
      *
@@ -178,13 +181,15 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate createPolicyTypes(@NonNull final PfDao dao,
         @NonNull final ToscaServiceTemplate serviceTemplate) throws PfModelException {
 
-        LOGGER.debug("->createPolicyTypes: serviceTemplate={}", serviceTemplate);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->createPolicyTypes: serviceTemplate={}", serviceTemplate);
 
-        ToscaServiceTemplate createdServiceTempalate = new SimpleToscaProvider()
-            .createPolicyTypes(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
+            ToscaServiceTemplate createdServiceTempalate = new SimpleToscaProvider()
+                .createPolicyTypes(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
 
-        LOGGER.debug("<-createPolicyTypes: createdServiceTempalate={}", createdServiceTempalate);
-        return createdServiceTempalate;
+            LOGGER.debug("<-createPolicyTypes: createdServiceTempalate={}", createdServiceTempalate);
+            return createdServiceTempalate;
+        }
     }
 
     /**
@@ -198,13 +203,15 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate updatePolicyTypes(@NonNull final PfDao dao,
         @NonNull final ToscaServiceTemplate serviceTemplate) throws PfModelException {
 
-        LOGGER.debug("->updatePolicyTypes: serviceTempalate={}", serviceTemplate);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->updatePolicyTypes: serviceTempalate={}", serviceTemplate);
 
-        ToscaServiceTemplate updatedServiceTempalate = new SimpleToscaProvider()
-            .updatePolicyTypes(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
+            ToscaServiceTemplate updatedServiceTempalate = new SimpleToscaProvider()
+                .updatePolicyTypes(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
 
-        LOGGER.debug("<-updatePolicyTypes: updatedServiceTempalate={}", updatedServiceTempalate);
-        return updatedServiceTempalate;
+            LOGGER.debug("<-updatePolicyTypes: updatedServiceTempalate={}", updatedServiceTempalate);
+            return updatedServiceTempalate;
+        }
     }
 
     /**
@@ -219,14 +226,16 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate deletePolicyType(@NonNull final PfDao dao, @NonNull final String name,
         @NonNull final String version) throws PfModelException {
 
-        LOGGER.debug("->deletePolicyType: name={}, version={}", name, version);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->deletePolicyType: name={}, version={}", name, version);
 
-        ToscaServiceTemplate deletedServiceTempalate =
-            new SimpleToscaProvider().deletePolicyType(dao, new PfConceptKey(name, version)).toAuthorative();
+            ToscaServiceTemplate deletedServiceTempalate =
+                new SimpleToscaProvider().deletePolicyType(dao, new PfConceptKey(name, version)).toAuthorative();
 
-        LOGGER.debug("<-deletePolicyType: name={}, version={}, deletedServiceTempalate={}", name, version,
-            deletedServiceTempalate);
-        return deletedServiceTempalate;
+            LOGGER.debug("<-deletePolicyType: name={}, version={}, deletedServiceTempalate={}", name, version,
+                deletedServiceTempalate);
+            return deletedServiceTempalate;
+        }
     }
 
     /**
@@ -348,13 +357,15 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate createPolicies(@NonNull final PfDao dao,
         @NonNull final ToscaServiceTemplate serviceTemplate) throws PfModelException {
 
-        LOGGER.debug("->createPolicies: serviceTempalate={}", serviceTemplate);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->createPolicies: serviceTempalate={}", serviceTemplate);
 
-        ToscaServiceTemplate createdServiceTempalate =
-            new SimpleToscaProvider().createPolicies(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
+            ToscaServiceTemplate createdServiceTempalate = new SimpleToscaProvider()
+                .createPolicies(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
 
-        LOGGER.debug("<-createPolicies: createdServiceTempalate={}", createdServiceTempalate);
-        return createdServiceTempalate;
+            LOGGER.debug("<-createPolicies: createdServiceTempalate={}", createdServiceTempalate);
+            return createdServiceTempalate;
+        }
     }
 
     /**
@@ -368,13 +379,15 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate updatePolicies(@NonNull final PfDao dao,
         @NonNull final ToscaServiceTemplate serviceTemplate) throws PfModelException {
 
-        LOGGER.debug("->updatePolicies: serviceTempalate={}", serviceTemplate);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->updatePolicies: serviceTempalate={}", serviceTemplate);
 
-        ToscaServiceTemplate updatedServiceTempalate =
-            new SimpleToscaProvider().updatePolicies(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
+            ToscaServiceTemplate updatedServiceTempalate = new SimpleToscaProvider()
+                .updatePolicies(dao, new JpaToscaServiceTemplate(serviceTemplate)).toAuthorative();
 
-        LOGGER.debug("<-updatePolicies: updatedServiceTempalate={}", updatedServiceTempalate);
-        return updatedServiceTempalate;
+            LOGGER.debug("<-updatePolicies: updatedServiceTempalate={}", updatedServiceTempalate);
+            return updatedServiceTempalate;
+        }
     }
 
     /**
@@ -389,14 +402,16 @@ public class AuthorativeToscaProvider {
     public ToscaServiceTemplate deletePolicy(@NonNull final PfDao dao, @NonNull final String name,
         @NonNull final String version) throws PfModelException {
 
-        LOGGER.debug("->deletePolicy: name={}, version={}", name, version);
+        synchronized (providerLockObject) {
+            LOGGER.debug("->deletePolicy: name={}, version={}", name, version);
 
-        ToscaServiceTemplate deletedServiceTempalate =
-            new SimpleToscaProvider().deletePolicy(dao, new PfConceptKey(name, version)).toAuthorative();
+            ToscaServiceTemplate deletedServiceTempalate =
+                new SimpleToscaProvider().deletePolicy(dao, new PfConceptKey(name, version)).toAuthorative();
 
-        LOGGER.debug("<-deletePolicy: name={}, version={}, deletedServiceTempalate={}", name, version,
-            deletedServiceTempalate);
-        return deletedServiceTempalate;
+            LOGGER.debug("<-deletePolicy: name={}, version={}, deletedServiceTempalate={}", name, version,
+                deletedServiceTempalate);
+            return deletedServiceTempalate;
+        }
     }
 
     /**