#139: Cannot add new additionalInfo in metadata (#140)

* fixes that cannot add new additionalInfo in metadata * GitHUb Action Spark increased to 3.2.2 Co-authored-by: Daniel Kavan <[email protected]>
AbsaOSS · Oct 25, 2022 · e9f72fd · e9f72fd
1 parent 85f5213
commit e9f72fd
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 7 deletions.
diff --git a/.github/workflows/build-sbt.yml b/.github/workflows/build-sbt.yml
@@ -28,10 +28,10 @@ jobs:
       fail-fast: false
       matrix:
         scala: [ 2.11.12, 2.12.15 ]
-        spark: [ 2.4.8, 3.2.1 ]
+        spark: [ 2.4.8, 3.2.2 ]
         exclude:
           - scala: 2.11.12
-            spark: 3.2.1
+            spark: 3.2.2
           - scala: 2.12.15
             spark: 2.4.8
     name: SBT Spark ${{matrix.spark}} on Scala ${{matrix.scala}}

diff --git a/atum/src/main/scala/za/co/absa/atum/core/Accumulator.scala b/atum/src/main/scala/za/co/absa/atum/core/Accumulator.scala
@@ -89,7 +89,7 @@ class Accumulator() {
 
   /** Adds a key-value pair as an additional information stored in the metadata. */
   def setAdditionalInfo(kv: (String, String), replaceIfExists: Boolean): Unit = {
-    controlMeasure.setAdditionalInfo(kv, replaceIfExists)
+    controlMeasure = controlMeasure.setAdditionalInfo(kv, replaceIfExists)
   }
 
   /** The method returns Control Info object as a Json string. */

diff --git a/atum/src/test/scala/za/co/absa/atum/core/AccumulatorSpec.scala b/atum/src/test/scala/za/co/absa/atum/core/AccumulatorSpec.scala
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2018 ABSA Group Limited
+ *
+ * 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.
+ */
+
+package za.co.absa.atum.core
+
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers.convertToAnyShouldWrapper
+import za.co.absa.atum.model.{ControlMeasure, ControlMeasureMetadata}
+import za.co.absa.atum.persistence.ControlMeasuresLoader
+
+class AccumulatorSpec extends AnyFlatSpec {
+
+  private val emptyControlMeasureMetadata = ControlMeasureMetadata(
+    sourceApplication = "",
+    country = "",
+    historyType = "",
+    dataFilename = "",
+    sourceType = "",
+    version = 0,
+    informationDate = "",
+    additionalInfo = Map.empty
+  )
+  private val emptyControlMeasure: ControlMeasure = ControlMeasure(
+    emptyControlMeasureMetadata,
+    None,
+    List.empty
+  )
+  private def initAccumulator(controlMeasure: ControlMeasure = emptyControlMeasure): Accumulator = {
+    val result = new Accumulator
+    val loader: ControlMeasuresLoader = new ControlMeasuresLoader{
+      override def load(): ControlMeasure = controlMeasure
+      override def getInfo: String = ""
+    }
+    result.loadControlMeasurements(loader)
+    result
+  }
+
+  private def initAccumulator(controlMeasureMetadata: ControlMeasureMetadata): Accumulator = {
+    initAccumulator(emptyControlMeasure.copy(metadata = controlMeasureMetadata))
+  }
+
+  "setAdditionalInfo" should "add additional key" in {
+    val expected = emptyControlMeasureMetadata.copy(additionalInfo = Map("Luke"->"Skywalker", "Han"->"Solo"))
+
+    val accumulator = initAccumulator()
+    accumulator.setAdditionalInfo(("Luke","Skywalker"), replaceIfExists = false)
+    accumulator.setAdditionalInfo(("Han","Solo"), replaceIfExists = true)
+    val actual = accumulator.getControlMeasure.metadata
+    actual shouldBe expected
+  }
+
+  it should "overwrite a key with overwrite on" in {
+    val initControlMeasureMetadata = emptyControlMeasureMetadata.copy(additionalInfo = Map("Leia"->"Organa", "Han"->"Solo"))
+    val expected = emptyControlMeasureMetadata.copy(additionalInfo = Map("Leia"->"Organa Solo", "Han"->"Solo"))
+    val accumulator = initAccumulator(initControlMeasureMetadata)
+    accumulator.setAdditionalInfo(("Leia","Organa Solo"), replaceIfExists = true)
+    val actual = accumulator.getControlMeasure.metadata
+    actual shouldBe expected
+  }
+
+  it should "keep the old value if overwrite is off" in {
+    val initControlMeasureMetadata = emptyControlMeasureMetadata.copy(additionalInfo = Map("Luke"->"Skywalker", "Han"->"Solo"))
+    val accumulator = initAccumulator(initControlMeasureMetadata)
+    accumulator.setAdditionalInfo(("Luke","Vader"), replaceIfExists = false)
+    val actual = accumulator.getControlMeasure.metadata
+    actual shouldBe initControlMeasureMetadata
+  }
+
+
+}
diff --git a/model/src/main/scala/za/co/absa/atum/model/ControlMeasure.scala b/model/src/main/scala/za/co/absa/atum/model/ControlMeasure.scala
@@ -52,10 +52,10 @@ case class ControlMeasure
    */
   def setAdditionalInfo(kv: (String, String), replaceIfExists: Boolean): ControlMeasure = {
     kv match {
-      case (key, _) if replaceIfExists || !this.metadata.additionalInfo.contains(kv._1) =>
-          val newInfo = this.metadata.additionalInfo + kv
-          val newMetadata = this.metadata.copy(additionalInfo = newInfo)
-          this.copy(metadata = newMetadata)
+      case (key, _) if replaceIfExists || !this.metadata.additionalInfo.contains(key) =>
+        val newInfo = metadata.additionalInfo + kv
+        val newMetadata = metadata.copy(additionalInfo = newInfo)
+        copy(metadata = newMetadata)
       case _ =>
         this
     }