Merge pull request #4293 from guardian/fix-reading-config-arrays

FIX `getOptionalStringSet` in CommonConfig

common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala

@@ -209,9 +209,9 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientBui
 
   final def getOptionalStringSet(key: String): Option[Set[String]] = Try {
     configuration.getOptional[Seq[String]](key)
-  }.recover {
-    case _:ConfigException.WrongType => configuration.getOptional[String](key).map(_.split(",").toSeq.map(_.trim))
-  }.toOption.flatten.map(_.toSet)
+  }.getOrElse(
+    configuration.getOptional[String](key).map(_.split(",").toSeq.map(_.trim))
+  ).map(_.toSet)
 
   final def getStringSet(key: String): Set[String] = getOptionalStringSet(key).getOrElse(Set.empty)
 

common-lib/src/test/scala/com/gu/mediaservice/lib/config/CommonConfigTest.scala

@@ -0,0 +1,29 @@
+package com.gu.mediaservice.lib.config
+
+import org.mockito.Mockito.when
+import org.scalatest.funsuite.AnyFunSuiteLike
+import org.scalatest.matchers.should.Matchers.convertToAnyShouldWrapper
+import org.scalatestplus.mockito.MockitoSugar
+import play.api.Configuration
+
+class CommonConfigTest extends AnyFunSuiteLike with MockitoSugar {
+
+  private val commonConf = mock[CommonConfig]
+  when(commonConf.configuration).thenReturn(Configuration(
+    "setAsCommaSepString" -> "a, b,c",
+    "setAsActualArray" -> Set("a", "b", "c")
+  ))
+
+  test("testGetOptionalStringSet") {
+      commonConf.getOptionalStringSet("doesnt.exist") shouldBe None
+      commonConf.getOptionalStringSet("setAsCommaSepString") shouldBe Some(Set("a", "b", "c"))
+      commonConf.getOptionalStringSet("setAsActualArray") shouldBe Some(Set("a", "b", "c"))
+  }
+
+  test("testGetStringSet") {
+    commonConf.getStringSet("doesnt.exist") shouldBe Set.empty
+    commonConf.getStringSet("setAsCommaSepString") shouldBe Set("a", "b", "c")
+    commonConf.getStringSet("setAsActualArray") shouldBe Set("a", "b", "c")
+  }
+
+}