From 6a1bbf295501650a6a7b8308da4d88835f122fa2 Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Mon, 5 Sep 2022 12:08:09 +0100 Subject: [PATCH] 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 Change-Id: Ic8ff4bdfef646e98ef61a6732c6d5ecb4b762e29 --- .../ncmp/api/inventory/sync/ModuleSyncService.java | 20 +++++++------------- .../api/inventory/sync/ModuleSyncServiceSpec.groovy | 13 ++++++------- .../spi/impl/CpsModulePersistenceServiceImpl.java | 8 ++++---- .../repository/YangResourceNativeRepositoryImpl.java | 4 ++++ ...CpsModulePersistenceServiceIntegrationSpec.groovy | 19 +++++++++++++++++++ .../main/java/org/onap/cps/api/CpsModuleService.java | 4 ++-- .../org/onap/cps/api/impl/CpsModuleServiceImpl.java | 4 ++-- .../onap/cps/spi/CpsModulePersistenceService.java | 8 ++++---- 8 files changed, 48 insertions(+), 32 deletions(-) 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 7f61c476d..7efce1ad5 100644 --- 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 moduleReferencesFromCmHandle = + final Collection allModuleReferencesFromCmHandle = dmiModelOperations.getModuleReferences(yangModelCmHandle); final Collection identifiedNewModuleReferencesFromCmHandle = cpsModuleService - .identifyNewModuleReferences(moduleReferencesFromCmHandle); - - final Collection existingModuleReferencesFromCmHandle = - moduleReferencesFromCmHandle.stream().filter(moduleReferenceFromCmHandle -> - !identifiedNewModuleReferencesFromCmHandle.contains(moduleReferenceFromCmHandle) - ).collect(Collectors.toList()); + .identifyNewModuleReferences(allModuleReferencesFromCmHandle); final Map 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 newModuleNameToContentMap, - final Collection existingModuleReferencesFromCmHandle) { + final Collection 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 78da7eb74..3c4c6f554 100644 --- 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 e9e945a49..400e9b3e8 100755 --- 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 newModuleNameToContentMap, - final Collection moduleReferences) { + final Collection allModuleReferences) { storeSchemaSet(dataspaceName, schemaSetName, newModuleNameToContentMap); final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final SchemaSetEntity schemaSetEntity = schemaSetRepository.getByDataspaceAndName(dataspaceEntity, schemaSetName); - final List listOfYangResourceIds = - yangResourceRepository.getResourceIdsByModuleReferences(moduleReferences); - yangResourceRepository.insertSchemaSetIdYangResourceId(schemaSetEntity.getId(), listOfYangResourceIds); + final List 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 485f839bf..850b274c9 100644 --- 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 getResourceIdsByModuleReferences(final Collection 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 eac28873e..f9ebc52f1 100644 --- 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 5e8eb9f6c..6b17e820c 100644 --- 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 newModuleNameToContentMap, - Collection moduleReferences); + Collection 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 ff725a617..20b4a23a9 100644 --- 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 newModuleNameToContentMap, - final Collection moduleReferences) { + final Collection 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 db2cb60f3..aaf6b38af 100755 --- 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 newModuleNameToContentMap, Collection moduleReferences); + Map newModuleNameToContentMap, Collection allModuleReferences); /** * Deletes Schema Set. -- 2.16.6