From 9db21cff33a8eadadb85071a37d2239f4ee6f068 Mon Sep 17 00:00:00 2001 From: Francis Toth Date: Fri, 29 May 2020 10:12:10 -0400 Subject: Extract ComponentCache from CsarUtils This commit aims to decouple the component caching logic from CsarUtils to its own class. Signed-off-by: Francis Toth Change-Id: Ia7e9284639ec8cd87ca5107e12f295e2eb599768 Issue-ID: SDC-2812 --- .../org/openecomp/sdc/be/tosca/ComponentCache.java | 156 +++++++++++++++++++++ .../java/org/openecomp/sdc/be/tosca/CsarUtils.java | 117 +++++++--------- .../openecomp/sdc/be/tosca/ComponentCacheTest.java | 97 +++++++++++++ .../org/openecomp/sdc/be/tosca/CsarUtilsTest.java | 25 +--- 4 files changed, 306 insertions(+), 89 deletions(-) create mode 100644 catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ComponentCache.java create mode 100644 catalog-be/src/test/java/org/openecomp/sdc/be/tosca/ComponentCacheTest.java (limited to 'catalog-be/src') diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ComponentCache.java b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ComponentCache.java new file mode 100644 index 0000000000..7871176dab --- /dev/null +++ b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ComponentCache.java @@ -0,0 +1,156 @@ +/*- + * ============LICENSE_START======================================================= + * SDC + * ================================================================================ + * Copyright (C) 2020 AT&T Intellectual Property. All rights reserved. + * ================================================================================ + * 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. + * ============LICENSE_END========================================================= + */ + +package org.openecomp.sdc.be.tosca; + +import static org.openecomp.sdc.be.utils.CommonBeUtils.compareAsdcComponentVersions; + +import io.vavr.collection.HashMap; +import io.vavr.collection.Map; +import io.vavr.collection.Stream; +import java.util.function.BiConsumer; +import java.util.function.BinaryOperator; +import lombok.EqualsAndHashCode; +import org.apache.commons.lang3.tuple.ImmutableTriple; +import org.openecomp.sdc.be.model.Component; + +/** + * Provides caching abilities for components + */ +public final class ComponentCache { + + // TODO: Make this final whenever possible. The current code using the class + // does not allow this. + private Map entries = HashMap.empty(); + private final BinaryOperator merge; + + private ComponentCache(BinaryOperator merge) { + this.merge = merge; + } + + /** + * Creates an overwritable cache based on a merging strategy + * @param merge The strategy used to merge two values which keys are the same + */ + public static ComponentCache overwritable(BinaryOperator merge) { + return new ComponentCache(merge); + } + + /** + * Creates a cached entry + * @param id The id of the entry + * @param fileName the filename of the entry + * @param component the cached component + */ + public static CacheEntry entry(String id, String fileName, Component component) { + return new CacheEntry(id, fileName, component); + } + + /** + * Decorate the cache with a listener called whenever a value is merged + * @param bc the consumer called when a value is merged + */ + public ComponentCache onMerge(BiConsumer bc) { + return new ComponentCache((oldValue, newValue) -> { + CacheEntry value = merge.apply(oldValue, newValue); + if(value.equals(newValue)) { + bc.accept(oldValue, newValue); + } + return value; + }); + } + + public interface MergeStrategy { + + /** + * A strategy designed to favour the latest component version when merging two cached entries + */ + static BinaryOperator overwriteIfSameVersions() { + return (oldValue, newValue) -> + compareAsdcComponentVersions(newValue.getComponentVersion(), oldValue.getComponentVersion()) ? + newValue : oldValue; + } + } + + /** + * Entry stored by the cache + */ + @EqualsAndHashCode + public static final class CacheEntry { + final String id; + + final String fileName; + + final Component component; + CacheEntry(String id, String fileName, Component component) { + this.id = id; + this.fileName = fileName; + this.component = component; + } + + public String getComponentVersion() { + return component.getVersion(); + } + } + + // TODO: Encapsulate the cache and expose functions to interact with it + // For now we'll keep this as is, to prevent the refactoring to be too big + public Iterable> iterable() { + return all().map(e -> + new ImmutableTriple<>(e.id, e.fileName, e.component) + ); + } + + /** + * Streams all the entries stored in the cache + */ + public Stream all() { + return entries.values().toStream(); + } + + /** + * Tells if an entry has been cached for a specific key + * @param key The key used to index the entry + */ + public boolean notCached(String key) { + return !entries.get(key).isDefined(); + } + + /** + * Store an entry in the cache. Keep in mind that currently this mutates the cache and does not work in a + * referentially transparent way (This should be fixed whenever possible). + * + * @param id The id of the entry + * @param fileName the filename of the entry + * @param component the cached component + */ + public ComponentCache put( + String id, + String fileName, + Component component + ) { + String uuid = component.getInvariantUUID(); + CacheEntry entry = new CacheEntry(id, fileName, component); + // TODO: Make the entries final whenever possible. The current code using the class does not allow this + entries = entries.put(uuid, entry, merge); + + return this; + } +} diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/CsarUtils.java b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/CsarUtils.java index 29efb1f1ee..291d2bb508 100644 --- a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/CsarUtils.java +++ b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/CsarUtils.java @@ -21,10 +21,13 @@ package org.openecomp.sdc.be.tosca; +import static org.openecomp.sdc.be.tosca.ComponentCache.MergeStrategy.overwriteIfSameVersions; + import fj.F; import fj.data.Either; import io.vavr.Tuple2; import io.vavr.control.Try; +import io.vavr.control.Option; import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; import java.io.File; @@ -92,7 +95,6 @@ import org.openecomp.sdc.be.plugins.CsarEntryGenerator; import org.openecomp.sdc.be.resources.data.DAOArtifactData; import org.openecomp.sdc.be.tosca.model.ToscaTemplate; import org.openecomp.sdc.be.tosca.utils.OperationArtifactUtil; -import org.openecomp.sdc.be.utils.CommonBeUtils; import org.openecomp.sdc.be.utils.TypeUtils.ToscaTagNamesEnum; import org.openecomp.sdc.common.api.ArtifactGroupTypeEnum; import org.openecomp.sdc.common.api.ArtifactTypeEnum; @@ -276,9 +278,7 @@ public class CsarUtils { } //UID - Map> innerComponentsCache = new HashMap<>(); - - Either responseFormat = getZipOutputStreamResponseFormatEither(zip, dependencies, innerComponentsCache); + Either responseFormat = getZipOutputStreamResponseFormatEither(zip, dependencies); if (responseFormat != null) return responseFormat; //retrieve SDC.zip from Cassandra @@ -472,9 +472,17 @@ public class CsarUtils { private Either getZipOutputStreamResponseFormatEither( ZipOutputStream zip, - List> dependencies, - Map> innerComponentsCache + List> dependencies ) throws IOException { + + ComponentCache innerComponentsCache = ComponentCache + .overwritable(overwriteIfSameVersions()) + .onMerge((oldValue, newValue) -> { + log.warn("Overwriting component invariantID {} of version {} with a newer version {}", + oldValue.id, oldValue.getComponentVersion(), newValue.getComponentVersion() + ); + }); + if (dependencies != null && !dependencies.isEmpty()) { for (Triple d : dependencies) { String cassandraId = d.getMiddle(); @@ -489,27 +497,21 @@ public class CsarUtils { //fill innerComponentsCache String fileName = d.getLeft(); - // TODO: Extract the cache related functions to their own class - addComponentToCache(innerComponentsCache, cassandraId, fileName, childComponent); + innerComponentsCache.put(cassandraId, fileName, childComponent); addInnerComponentsToCache(innerComponentsCache, childComponent); } //add inner components to CSAR - Either responseFormat = addInnerComponentsToCSAR(zip, - innerComponentsCache); - if (responseFormat != null) { - return responseFormat; - } + return addInnerComponentsToCSAR(zip, innerComponentsCache); } return null; } private Either addInnerComponentsToCSAR( ZipOutputStream zip, - Map> innerComponentsCache + ComponentCache innerComponentsCache ) throws IOException { - for (Entry> entry : innerComponentsCache.entrySet()) { - ImmutableTriple ict = entry.getValue(); + for (ImmutableTriple ict : innerComponentsCache.iterable()) { Component innerComponent = ict.getRight(); String icFileName = ict.getMiddle(); @@ -550,7 +552,6 @@ public class CsarUtils { * @param zipOutputStream stores the input stream content * @param schemaFileZip zip data from Cassandra * @param nodesFromPackage list of all nodes found on the onboarded package - * @param isHeatPackage true if the onboardead package is a Heat package */ private void addSchemaFilesFromCassandra(final ZipOutputStream zipOutputStream, final byte[] schemaFileZip, @@ -618,29 +619,34 @@ public class CsarUtils { updateZipEntry(byteArrayOutputStream, nodesYaml); } - private void addInnerComponentsToCache(Map> componentCache, - Component childComponent) { - - List instances = childComponent.getComponentInstances(); - - if(instances != null) { - instances.forEach(ci -> { - ImmutableTriple componentRecord = componentCache.get(ci.getComponentUid()); - if (componentRecord == null) { - // all resource must be only once! - Either resource = toscaOperationFacade.getToscaElement(ci.getComponentUid()); - Component componentRI = checkAndAddComponent(componentCache, ci, resource); - - //if not atomic - insert inner components as well - if(!ModelConverter.isAtomicComponent(componentRI)) { - addInnerComponentsToCache(componentCache, componentRI); - } + private void addInnerComponentsToCache(ComponentCache componentCache, Component childComponent) { + javaListToVavrList(childComponent.getComponentInstances()) + .filter(ci -> componentCache.notCached(ci.getComponentUid())) + .forEach(ci -> { + // all resource must be only once! + Either resource = + toscaOperationFacade.getToscaElement(ci.getComponentUid()); + Component componentRI = checkAndAddComponent(componentCache, ci, resource); + //if not atomic - insert inner components as well + // TODO: This could potentially create a StackOverflowException if the call stack + // happens to be too large. Tail-recursive optimization should be used here. + if (!ModelConverter.isAtomicComponent(componentRI)) { + addInnerComponentsToCache(componentCache, componentRI); } }); - } } - private Component checkAndAddComponent(Map> componentCache, ComponentInstance ci, Either resource) { + // TODO: Move this function in FJToVavrHelper.java once Change 108540 is merged + private io.vavr.collection.List javaListToVavrList( + List componentInstances + ) { + return Option + .of(componentInstances) + .map(io.vavr.collection.List::ofAll) + .getOrElse(io.vavr.collection.List::empty); + } + + private Component checkAndAddComponent(ComponentCache componentCache, ComponentInstance ci, Either resource) { if (resource.isRight()) { log.debug("Failed to fetch resource with id {} for instance {}", ci.getComponentUid(), ci.getName()); } @@ -650,44 +656,15 @@ public class CsarUtils { ArtifactDefinition childArtifactDefinition = childToscaArtifacts.get(ToscaExportHandler.ASSET_TOSCA_TEMPLATE); if (childArtifactDefinition != null) { //add to cache - addComponentToCache(componentCache, childArtifactDefinition.getEsId(), childArtifactDefinition.getArtifactName(), componentRI); + componentCache.put( + childArtifactDefinition.getEsId(), + childArtifactDefinition.getArtifactName(), + componentRI + ); } return componentRI; } - private void addComponentToCache(Map> componentCache, - String id, String fileName, Component component) { - - String uuid = component.getInvariantUUID(); - String version = component.getVersion(); - - Supplier> sup = - () -> new ImmutableTriple<>(id, fileName, component); - - componentCache.put(uuid, updateWith(componentCache, uuid, - cc -> overwriteIfSameVersions(id, version, cc, sup), - sup - )); - } - - private static V updateWith(Map kvs, K k, Function f, Supplier orElse) { - return Optional.ofNullable(kvs.get(k)).map(f).orElseGet(orElse); - } - - private ImmutableTriple overwriteIfSameVersions( - String id, String version, - ImmutableTriple cc, - Supplier> newValue - ) { - if (CommonBeUtils.compareAsdcComponentVersions(version, cc.getRight().getVersion())) { - log.warn("Overwriting component invariantID {} of version {} with a newer version {}", id, - cc.getRight().getVersion(), - version); - return newValue.get(); - } - return cc; - } - private Either writeComponentInterface( Component component, ZipOutputStream zip, diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/ComponentCacheTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/ComponentCacheTest.java new file mode 100644 index 0000000000..12027a755d --- /dev/null +++ b/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/ComponentCacheTest.java @@ -0,0 +1,97 @@ +/*- + * ============LICENSE_START======================================================= + * SDC + * ================================================================================ + * Copyright (C) 2020 AT&T Intellectual Property. All rights reserved. + * ================================================================================ + * 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. + * ============LICENSE_END========================================================= + */ + +package org.openecomp.sdc.be.tosca; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.openecomp.sdc.be.components.impl.utils.TestDataUtils.aUniqueId; +import static org.openecomp.sdc.be.components.impl.utils.TestDataUtils.alphaNum; +import static org.openecomp.sdc.be.tosca.ComponentCache.entry; + +import io.vavr.collection.List; +import java.util.function.BinaryOperator; +import org.junit.jupiter.api.Test; +import org.openecomp.sdc.be.components.impl.utils.TestDataUtils; +import org.openecomp.sdc.be.model.Component; +import org.openecomp.sdc.be.tosca.ComponentCache.CacheEntry; + +public class ComponentCacheTest { + private static final BinaryOperator RETURN_LATEST = (oldValue, newValue) -> newValue; + private static final BinaryOperator NO_MERGE = (oldValue, newValue) -> { + fail("Should not merge"); + return oldValue; + }; + + @Test + public void emptyCase() { + ComponentCache cache = ComponentCache.overwritable(NO_MERGE); + List actual = cache.all().toList(); + assertTrue(actual.isEmpty()); + } + + @Test + public void emptyCacheShouldNotMerge() { + ComponentCache cache = ComponentCache.overwritable(NO_MERGE); + + String id = alphaNum(10); + String fileName = alphaNum(10); + Component component = aComponent(); + + List actual = cache + .put(id, fileName, component) + .all().toList(); + + List expected = List.of(entry(id, fileName, component)); + assertEquals(expected, actual); + } + + @Test + public void nonEmptyCacheShouldMerge() { + ComponentCache cache = ComponentCache.overwritable(RETURN_LATEST); + + String id = alphaNum(10); + String fileName = alphaNum(10); + String invariantId = aUniqueId(); + + Component oldComponent = aComponent(invariantId); + Component newComponent = aComponent(invariantId); + + CacheEntry actual = cache + .put(id, fileName, oldComponent) + .put(id, fileName, newComponent) + .all().toList().head(); + + assertEquals(newComponent, actual.component); + assertEquals(1, cache.all().size()); + } + + private static Component aComponent() { + return aComponent(aUniqueId()); + } + + private static Component aComponent(String invariantId) { + Component component = TestDataUtils.aComponent(); + component.setInvariantUUID(invariantId); + return component; + } +} + diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/CsarUtilsTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/CsarUtilsTest.java index 8f1e9f9199..07cf727a9e 100644 --- a/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/CsarUtilsTest.java +++ b/catalog-be/src/test/java/org/openecomp/sdc/be/tosca/CsarUtilsTest.java @@ -27,6 +27,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.openecomp.sdc.be.tosca.ComponentCache.MergeStrategy.overwriteIfSameVersions; +import static org.openecomp.sdc.be.tosca.ComponentCache.entry; import fj.data.Either; import java.io.File; @@ -85,6 +87,7 @@ import org.openecomp.sdc.be.model.jsonjanusgraph.operations.ToscaOperationFacade import org.openecomp.sdc.be.model.operations.api.StorageOperationStatus; import org.openecomp.sdc.be.resources.data.DAOArtifactData; import org.openecomp.sdc.be.resources.data.SdcSchemaFilesData; +import org.openecomp.sdc.be.tosca.ComponentCache.CacheEntry; import org.openecomp.sdc.be.tosca.CsarUtils.NonMetaArtifactInfo; import org.openecomp.sdc.be.tosca.model.ToscaTemplate; import org.openecomp.sdc.common.api.ArtifactGroupTypeEnum; @@ -509,7 +512,7 @@ public class CsarUtilsTest extends BeConfDependentTest { @Test public void testAddInnerComponentsToCache() { - Map> componentCache = new HashMap<>(); + ComponentCache componentCache = ComponentCache.overwritable(overwriteIfSameVersions()); Component childComponent = new Resource(); Component componentRI = new Service(); List componentInstances = new ArrayList<>(); @@ -536,7 +539,8 @@ public class CsarUtilsTest extends BeConfDependentTest { Deencapsulation.invoke(testSubject, "addInnerComponentsToCache", componentCache, childComponent); - assertTrue(componentCache.containsValue(ImmutableTriple.of("esId","artifactName",componentRI))); + io.vavr.collection.List expected = io.vavr.collection.List.of(entry("esId","artifactName",componentRI)); + assertEquals(expected, componentCache.all().toList()); } @Test @@ -572,23 +576,6 @@ public class CsarUtilsTest extends BeConfDependentTest { assertTrue(componentCache.isEmpty()); } - @Test - public void testAddComponentToCache() { - Map> componentCache = new HashMap<>(); - String id = "id"; - String fileName = "fileName"; - Component component = new Resource(); - component.setInvariantUUID("key"); - component.setVersion("1.0"); - - Component cachedComponent = new Resource(); - cachedComponent.setVersion("0.3"); - - componentCache.put("key", new ImmutableTriple(id, fileName, cachedComponent)); - - Deencapsulation.invoke(testSubject, "addComponentToCache", componentCache, id, fileName, component); - } - @Test public void testWriteComponentInterface() throws IOException { String fileName = "name.hello"; -- cgit 1.2.3-korg