Performance Improvement: Fix Insert Yang Resource IDs (Schemset)

- Add robustness to handle insertion/get of empty collection
- Fix business logic to pass down ALL module references during module Sync
- Update Java doc etc to clarify ALL module references are needed (not just new)

Issue-ID: CPS-1246
Issue-ID: CPS-1126
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: Ic8ff4bdfef646e98ef61a6732c6d5ecb4b762e29

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncService.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncService.java
index 7f61c47..7efce1a 100644 (file)
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncService.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncService.java
@@ -23,9 +23,8 @@ package org.onap.cps.ncmp.api.inventory.sync;
 import static org.onap.cps.ncmp.api.impl.constants.DmiRegistryConstants.NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME;
 
 import java.util.Collection;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.Map;
-import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.api.CpsAdminService;
@@ -54,33 +53,28 @@ public class ModuleSyncService {
      */
     public void syncAndCreateSchemaSetAndAnchor(final YangModelCmHandle yangModelCmHandle) {
 
-        final Collection<ModuleReference> moduleReferencesFromCmHandle =
+        final Collection<ModuleReference> allModuleReferencesFromCmHandle =
                 dmiModelOperations.getModuleReferences(yangModelCmHandle);
 
         final Collection<ModuleReference> identifiedNewModuleReferencesFromCmHandle = cpsModuleService
-                .identifyNewModuleReferences(moduleReferencesFromCmHandle);
-
-        final Collection<ModuleReference> existingModuleReferencesFromCmHandle =
-                moduleReferencesFromCmHandle.stream().filter(moduleReferenceFromCmHandle ->
-                        !identifiedNewModuleReferencesFromCmHandle.contains(moduleReferenceFromCmHandle)
-                ).collect(Collectors.toList());
+                .identifyNewModuleReferences(allModuleReferencesFromCmHandle);
 
         final Map<String, String> newModuleNameToContentMap;
         if (identifiedNewModuleReferencesFromCmHandle.isEmpty()) {
-            newModuleNameToContentMap = new HashMap<>();
+            newModuleNameToContentMap = Collections.emptyMap();
         } else {
             newModuleNameToContentMap = dmiModelOperations.getNewYangResourcesFromDmi(yangModelCmHandle,
                     identifiedNewModuleReferencesFromCmHandle);
         }
-        createSchemaSetAndAnchor(yangModelCmHandle, newModuleNameToContentMap, existingModuleReferencesFromCmHandle);
+        createSchemaSetAndAnchor(yangModelCmHandle, newModuleNameToContentMap, allModuleReferencesFromCmHandle);
     }
 
     private void createSchemaSetAndAnchor(final YangModelCmHandle yangModelCmHandle,
                                           final Map<String, String> newModuleNameToContentMap,
-                                          final Collection<ModuleReference> existingModuleReferencesFromCmHandle) {
+                                          final Collection<ModuleReference> allModuleReferencesFromCmHandle) {
         final String schemaSetAndAnchorName = yangModelCmHandle.getId();
         cpsModuleService.createSchemaSetFromModules(NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME, schemaSetAndAnchorName,
-                        newModuleNameToContentMap, existingModuleReferencesFromCmHandle);
+                        newModuleNameToContentMap, allModuleReferencesFromCmHandle);
         cpsAdminService.createAnchor(NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME, schemaSetAndAnchorName,
             schemaSetAndAnchorName);
     }

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncServiceSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncServiceSpec.groovy
index 78da7eb..3c4c6f5 100644 (file)
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncServiceSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncServiceSpec.groovy
@@ -47,8 +47,7 @@ class ModuleSyncServiceSpec extends Specification {
             ncmpServiceCmHandle.cmHandleId = 'cmHandleId-1'
             def yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(dmiServiceName, '', '', ncmpServiceCmHandle)
         and: 'DMI operations returns some module references'
-            def moduleReferences =  [ new ModuleReference(moduleName:'module1',revision:'1'),
-                                                            new ModuleReference(moduleName:'module2',revision:'2') ]
+            def moduleReferences =  [ new ModuleReference('module1','1'), new ModuleReference('module2','2') ]
             mockDmiModelOperations.getModuleReferences(yangModelCmHandle) >> moduleReferences
         and: 'CPS-Core returns list of existing module resources'
             mockCpsModuleService.getYangResourceModuleReferences(expectedDataspaceName) >> toModuleReference(existingModuleResourcesInCps)
@@ -58,14 +57,14 @@ class ModuleSyncServiceSpec extends Specification {
             mockCpsModuleService.identifyNewModuleReferences(moduleReferences) >> toModuleReference(identifiedNewModuleReferences)
             objectUnderTest.syncAndCreateSchemaSetAndAnchor(yangModelCmHandle)
         then: 'create schema set from module is invoked with correct parameters'
-            1 * mockCpsModuleService.createSchemaSetFromModules('NFP-Operational', 'cmHandleId-1', newModuleNameContentToMap, existingModuleReferencesInCps)
+            1 * mockCpsModuleService.createSchemaSetFromModules('NFP-Operational', 'cmHandleId-1', newModuleNameContentToMap, moduleReferences)
         and: 'anchor is created with the correct parameters'
             1 * mockCpsAdminService.createAnchor('NFP-Operational', 'cmHandleId-1', 'cmHandleId-1')
         where: 'the following parameters are used'
-            scenario             | existingModuleResourcesInCps           | identifiedNewModuleReferences | newModuleNameContentToMap       | existingModuleReferencesInCps
-            'one new module'     | [['module2' : '2'], ['module3' : '3']] | [['module1' : '1']]           | [module1: 'some yang source']   | [new ModuleReference(moduleName:'module2',revision:'2')]
-            'no add. properties' | [['module2' : '2'], ['module3' : '3']] | [['module1' : '1']]           | [module1: 'some yang source']   | [new ModuleReference(moduleName:'module2',revision:'2')]
-            'no new module'      | [['module1' : '1'], ['module2' : '2']] | []                            | [:]                             | [new ModuleReference(moduleName:'module1',revision:'1'), new ModuleReference(moduleName:'module2',revision:'2')]
+            scenario             | existingModuleResourcesInCps           | identifiedNewModuleReferences | newModuleNameContentToMap
+            'one new module'     | [['module2' : '2'], ['module3' : '3']] | [['module1' : '1']]           | [module1: 'some yang source']
+            'no add. properties' | [['module2' : '2'], ['module3' : '3']] | [['module1' : '1']]           | [module1: 'some yang source']
+            'no new module'      | [['module1' : '1'], ['module2' : '2']] | []                            | [:]
     }
 
     def 'Delete Schema Set for CmHandle' () {

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsModulePersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsModulePersistenceServiceImpl.java
index e9e945a..400e9b3 100755 (executable)
--- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsModulePersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsModulePersistenceServiceImpl.java
@@ -162,14 +162,14 @@ public class CpsModulePersistenceServiceImpl implements CpsModulePersistenceServ
         @Backoff(random = true, delay = 200, maxDelay = 2000, multiplier = 2))
     public void storeSchemaSetFromModules(final String dataspaceName, final String schemaSetName,
                                           final Map<String, String> newModuleNameToContentMap,
-                                          final Collection<ModuleReference> moduleReferences) {
+                                          final Collection<ModuleReference> allModuleReferences) {
         storeSchemaSet(dataspaceName, schemaSetName, newModuleNameToContentMap);
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         final SchemaSetEntity schemaSetEntity =
                 schemaSetRepository.getByDataspaceAndName(dataspaceEntity, schemaSetName);
-        final List<Long> listOfYangResourceIds =
-            yangResourceRepository.getResourceIdsByModuleReferences(moduleReferences);
-        yangResourceRepository.insertSchemaSetIdYangResourceId(schemaSetEntity.getId(), listOfYangResourceIds);
+        final List<Long> allYangResourceIds =
+            yangResourceRepository.getResourceIdsByModuleReferences(allModuleReferences);
+        yangResourceRepository.insertSchemaSetIdYangResourceId(schemaSetEntity.getId(), allYangResourceIds);
     }
 
     @Override

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/YangResourceNativeRepositoryImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/YangResourceNativeRepositoryImpl.java
index 485f839..850b274 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/YangResourceNativeRepositoryImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/YangResourceNativeRepositoryImpl.java
@@ -21,6 +21,7 @@
 package org.onap.cps.spi.repository;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.StringJoiner;
 import javax.persistence.EntityManager;
@@ -42,6 +43,9 @@ public class YangResourceNativeRepositoryImpl implements YangResourceNativeRepos
     @Override
     @Transactional
     public List<Long> getResourceIdsByModuleReferences(final Collection<ModuleReference> moduleReferences) {
+        if (moduleReferences.isEmpty()) {
+            return Collections.emptyList();
+        }
         final Query query = entityManager.createNativeQuery(getCombinedSelectSqlQuery(moduleReferences))
             .unwrap(org.hibernate.query.NativeQuery.class)
             .addScalar("id", StandardBasicTypes.LONG);

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsModulePersistenceServiceIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsModulePersistenceServiceIntegrationSpec.groovy
index eac2887..f9ebc52 100644 (file)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsModulePersistenceServiceIntegrationSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsModulePersistenceServiceIntegrationSpec.groovy
@@ -31,8 +31,10 @@ import org.onap.cps.spi.model.ModuleReference
 import org.onap.cps.spi.repository.AnchorRepository
 import org.onap.cps.spi.repository.SchemaSetRepository
 import org.onap.cps.spi.repository.SchemaSetYangResourceRepositoryImpl
+import org.onap.cps.spi.repository.YangResourceRepository
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.test.context.jdbc.Sql
+import spock.lang.Ignore
 
 class CpsModulePersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
 
@@ -48,6 +50,9 @@ class CpsModulePersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase
     @Autowired
     CpsAdminPersistenceService cpsAdminPersistenceService
 
+    @Autowired
+    YangResourceRepository yangResourceRepository
+
     final static String SET_DATA = '/data/schemaset.sql'
 
     def static EXISTING_SCHEMA_SET_NAME = SCHEMA_SET_NAME1
@@ -76,6 +81,20 @@ class CpsModulePersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase
         dataspaceEntity = dataspaceRepository.getByName(DATASPACE_NAME)
     }
 
+    @Sql([CLEAR_DATA, SET_DATA])
+    def 'Getting yang resource ids from module references'() {
+        when: 'getting yang resources for #scenario'
+            def result = yangResourceRepository.getResourceIdsByModuleReferences(moduleReferences)
+        then: 'the result contains the expected number entries'
+            assert result.size() == expectedResultSize
+        where: 'the following module references are provided'
+            scenario                                 | moduleReferences                                                                                                 || expectedResultSize
+            '2 valid module references'              | [ new ModuleReference('MODULE-NAME-002','REVISION-002'), new ModuleReference('MODULE-NAME-003','REVISION-002') ] || 2
+            '1 invalid module reference'             | [ new ModuleReference('NOT EXIST','IRRELEVANT') ]                                                                || 0
+            '1 valid and 1 invalid module reference' | [ new ModuleReference('MODULE-NAME-002','REVISION-002'), new ModuleReference('NOT EXIST','IRRELEVANT') ]         || 1
+            'no module references'                   | []                                                                                                               || 0
+    }
+
     @Sql([CLEAR_DATA, SET_DATA])
     def 'Store schema set error scenario: #scenario.'() {
         when: 'attempt to store schema set #schemaSetName in dataspace #dataspaceName'

diff --git a/cps-service/src/main/java/org/onap/cps/api/CpsModuleService.java b/cps-service/src/main/java/org/onap/cps/api/CpsModuleService.java
index 5e8eb9f..6b17e82 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/api/CpsModuleService.java
+++ b/cps-service/src/main/java/org/onap/cps/api/CpsModuleService.java
@@ -50,11 +50,11 @@ public interface CpsModuleService {
      * @param dataspaceName             Dataspace name
      * @param schemaSetName             schema set name
      * @param newModuleNameToContentMap YANG resources map where key is a module name and value is content
-     * @param moduleReferences          List of YANG resources module references of the modules
+     * @param allModuleReferences       All YANG resource module references
      */
     void createSchemaSetFromModules(String dataspaceName, String schemaSetName,
                                     Map<String, String> newModuleNameToContentMap,
-                                    Collection<ModuleReference> moduleReferences);
+                                    Collection<ModuleReference> allModuleReferences);
 
     /**
      * Read schema set in the given dataspace.

diff --git a/cps-service/src/main/java/org/onap/cps/api/impl/CpsModuleServiceImpl.java b/cps-service/src/main/java/org/onap/cps/api/impl/CpsModuleServiceImpl.java
index ff725a6..20b4a23 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/api/impl/CpsModuleServiceImpl.java
+++ b/cps-service/src/main/java/org/onap/cps/api/impl/CpsModuleServiceImpl.java
@@ -60,10 +60,10 @@ public class CpsModuleServiceImpl implements CpsModuleService {
     @Override
     public void createSchemaSetFromModules(final String dataspaceName, final String schemaSetName,
         final Map<String, String> newModuleNameToContentMap,
-        final Collection<ModuleReference> moduleReferences) {
+        final Collection<ModuleReference> allModuleReferences) {
         CpsValidator.validateNameCharacters(dataspaceName, schemaSetName);
         cpsModulePersistenceService.storeSchemaSetFromModules(dataspaceName, schemaSetName,
-            newModuleNameToContentMap, moduleReferences);
+            newModuleNameToContentMap, allModuleReferences);
 
     }
 

diff --git a/cps-service/src/main/java/org/onap/cps/spi/CpsModulePersistenceService.java b/cps-service/src/main/java/org/onap/cps/spi/CpsModulePersistenceService.java
index db2cb60..aaf6b38 100755 (executable)
--- a/cps-service/src/main/java/org/onap/cps/spi/CpsModulePersistenceService.java
+++ b/cps-service/src/main/java/org/onap/cps/spi/CpsModulePersistenceService.java
@@ -43,13 +43,13 @@ public interface CpsModulePersistenceService {
     /**
      * Stores a schema set from new modules and existing modules.
      *
-     * @param dataspaceName                          Dataspace name
-     * @param schemaSetName                          Schema set name
+     * @param dataspaceName             Dataspace name
+     * @param schemaSetName             Schema set name
      * @param newModuleNameToContentMap YANG resources map where key is a module name and value is content
-     * @param moduleReferences                       List of YANG resources module references
+     * @param allModuleReferences       All YANG resources module references
      */
     void storeSchemaSetFromModules(String dataspaceName, String schemaSetName,
-        Map<String, String> newModuleNameToContentMap, Collection<ModuleReference> moduleReferences);
+        Map<String, String> newModuleNameToContentMap, Collection<ModuleReference> allModuleReferences);
 
     /**
      * Deletes Schema Set.