Extract ComponentCache from CsarUtils

This commit aims to decouple the component caching logic from CsarUtils to its own class.

Signed-off-by: Francis Toth <francis.toth@yoppworks.com>
Change-Id: Ia7e9284639ec8cd87ca5107e12f295e2eb599768
Issue-ID: SDC-2812

4 files changed, 306 insertions, 89 deletions

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<String, CacheEntry> entries = HashMap.empty();
+    private final BinaryOperator<CacheEntry> merge;
+
+    private ComponentCache(BinaryOperator<CacheEntry> 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<CacheEntry> 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<CacheEntry, CacheEntry> 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<CacheEntry> 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<ImmutableTriple<String, String, Component>> iterable() {
+        return all().map(e ->
+            new ImmutableTriple<>(e.id, e.fileName, e.component)
+        );
+    }
+
+    /**
+     * Streams all the entries stored in the cache
+     */
+    public Stream<CacheEntry> 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 <cassandraId,filename,component>
-        Map<String, ImmutableTriple<String,String, Component>> innerComponentsCache = new HashMap<>();
-
-        Either<ZipOutputStream, ResponseFormat> responseFormat = getZipOutputStreamResponseFormatEither(zip, dependencies, innerComponentsCache);
+        Either<ZipOutputStream, ResponseFormat> responseFormat = getZipOutputStreamResponseFormatEither(zip, dependencies);
         if (responseFormat != null) return responseFormat;
 
         //retrieve SDC.zip from Cassandra
@@ -472,9 +472,17 @@ public class CsarUtils {
 
     private Either<ZipOutputStream, ResponseFormat> getZipOutputStreamResponseFormatEither(
         ZipOutputStream zip,
-        List<Triple<String, String, Component>> dependencies,
-        Map<String, ImmutableTriple<String, String, Component>> innerComponentsCache
+        List<Triple<String, String, Component>> 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<String, String, Component> 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<ZipOutputStream, ResponseFormat> responseFormat = addInnerComponentsToCSAR(zip,
-                innerComponentsCache);
-            if (responseFormat != null) {
-                return responseFormat;
-            }
+            return addInnerComponentsToCSAR(zip, innerComponentsCache);
         }
         return null;
     }
 
     private Either<ZipOutputStream, ResponseFormat> addInnerComponentsToCSAR(
         ZipOutputStream zip,
-        Map<String, ImmutableTriple<String, String, Component>> innerComponentsCache
+        ComponentCache innerComponentsCache
     ) throws IOException {
-        for (Entry<String, ImmutableTriple<String, String, Component>> entry : innerComponentsCache.entrySet()) {
-            ImmutableTriple<String, String, Component> ict = entry.getValue();
+        for (ImmutableTriple<String, String, Component> 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<String, ImmutableTriple<String, String, Component>> componentCache,
-            Component childComponent) {
-
-        List<ComponentInstance> instances = childComponent.getComponentInstances();
-
-        if(instances != null) {
-            instances.forEach(ci -> {
-                ImmutableTriple<String, String, Component> componentRecord = componentCache.get(ci.getComponentUid());
-                if (componentRecord == null) {
-                    // all resource must be only once!
-                    Either<Resource, StorageOperationStatus> 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, StorageOperationStatus> 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<String, ImmutableTriple<String, String, Component>> componentCache, ComponentInstance ci, Either<Resource, StorageOperationStatus> resource) {
+    // TODO: Move this function in FJToVavrHelper.java once Change 108540 is merged
+    private io.vavr.collection.List<ComponentInstance> javaListToVavrList(
+        List<ComponentInstance> 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, StorageOperationStatus> 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<String, ImmutableTriple<String, String, Component>> componentCache,
-            String id, String fileName, Component component) {
-
-        String uuid = component.getInvariantUUID();
-        String version = component.getVersion();
-
-        Supplier<ImmutableTriple<String, String, Component>> sup =
-            () -> new ImmutableTriple<>(id, fileName, component);
-
-        componentCache.put(uuid, updateWith(componentCache, uuid,
-            cc -> overwriteIfSameVersions(id, version, cc, sup),
-            sup
-        ));
-    }
-
-    private static <K, V> V updateWith(Map<K, V> kvs, K k, Function<V, V> f, Supplier<V> orElse) {
-        return Optional.ofNullable(kvs.get(k)).map(f).orElseGet(orElse);
-    }
-
-    private ImmutableTriple<String, String, Component> overwriteIfSameVersions(
-        String id, String version,
-        ImmutableTriple<String, String, Component> cc,
-        Supplier<ImmutableTriple<String, String, Component>> 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<ZipOutputStream, ResponseFormat> 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<CacheEntry> RETURN_LATEST = (oldValue, newValue) -> newValue;
+    private static final BinaryOperator<CacheEntry> NO_MERGE = (oldValue, newValue) ->  {
+        fail("Should not merge");
+        return oldValue;
+    };
+
+    @Test
+    public void emptyCase() {
+        ComponentCache cache = ComponentCache.overwritable(NO_MERGE);
+        List<CacheEntry> 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<CacheEntry> actual = cache
+            .put(id, fileName, component)
+            .all().toList();
+
+        List<CacheEntry> 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<String, ImmutableTriple<String, String, Component>> componentCache = new HashMap<>();
+		ComponentCache componentCache = ComponentCache.overwritable(overwriteIfSameVersions());
 		Component childComponent = new Resource();
 		Component componentRI = new Service();
 		List<ComponentInstance> 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<CacheEntry> expected = io.vavr.collection.List.of(entry("esId","artifactName",componentRI));
+		assertEquals(expected, componentCache.all().toList());
 	}
 
 	@Test
@@ -573,23 +577,6 @@ public class CsarUtilsTest extends BeConfDependentTest {
 	}
 
 	@Test
-	public void testAddComponentToCache() {
-		Map<String, ImmutableTriple<String, String, Component>> 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<String, String, Component>(id, fileName, cachedComponent));
-
-		Deencapsulation.invoke(testSubject, "addComponentToCache", componentCache, id, fileName, component);
-	}
-
-	@Test
 	public void testWriteComponentInterface() throws IOException {
 		String fileName = "name.hello";
 		ToscaRepresentation tosca = new ToscaRepresentation();