From 2e3d4b652906aff5e03805e08b7b7f410c220508 Mon Sep 17 00:00:00 2001
From: Anton Alkin <anton.alkin@cern.ch>
Date: Mon, 11 Dec 2023 14:00:54 +0100
Subject: [PATCH] DPL Analysis: fix for string labeled array + update tests

---
 Framework/Core/include/Framework/Variant.h    | 12 +++-
 .../include/Framework/VariantJSONHelpers.h    |  9 ++-
 Framework/Core/test/test_Variants.cxx         | 59 ++++++++++++++++++-
 3 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h
index 0c055cfb84b12..d9ef196f56e0d 100644
--- a/Framework/Core/include/Framework/Variant.h
+++ b/Framework/Core/include/Framework/Variant.h
@@ -73,6 +73,12 @@ constexpr auto isArray2D()
           V == VariantType::Array2DDouble);
 }
 
+template <VariantType V>
+constexpr auto isLabeledArrayString()
+{
+  return V == VariantType::LabeledArrayString;
+}
+
 template <VariantType V>
 constexpr auto isLabeledArray()
 {
@@ -290,9 +296,9 @@ class Variant
   using storage_t = std::aligned_union<8, int, int8_t, int16_t, int64_t,
                                        uint8_t, uint16_t, uint32_t, uint64_t,
                                        const char*, float, double, bool,
-                                       int*, float*, double*, bool*,
-                                       Array2D<int>, Array2D<float>, Array2D<double>,
-                                       LabeledArray<int>, LabeledArray<float>, LabeledArray<double>>::type;
+                                       int*, float*, double*, bool*, std::string*,
+                                       Array2D<int>, Array2D<float>, Array2D<double>, Array2D<std::string>,
+                                       LabeledArray<int>, LabeledArray<float>, LabeledArray<double>, LabeledArray<std::string>>::type;
 
  public:
   Variant(VariantType type = VariantType::Unknown) : mType{type}, mSize{1} {}
diff --git a/Framework/Core/include/Framework/VariantJSONHelpers.h b/Framework/Core/include/Framework/VariantJSONHelpers.h
index ebf0b308581a1..8fca73efbbb76 100644
--- a/Framework/Core/include/Framework/VariantJSONHelpers.h
+++ b/Framework/Core/include/Framework/VariantJSONHelpers.h
@@ -154,7 +154,7 @@ struct VariantReader : public rapidjson::BaseReaderHandler<rapidjson::UTF8<>, Va
       states.push(State::IN_ERROR);
       return true;
     } else {
-      if (states.top() == State::IN_ARRAY) {
+      if (states.top() == State::IN_ARRAY || states.top() == State::IN_ROW) {
         debug << "added to array" << std::endl;
         if constexpr (isLabeledArray<V>()) {
           if (currentKey == labels_rows_str) {
@@ -163,6 +163,13 @@ struct VariantReader : public rapidjson::BaseReaderHandler<rapidjson::UTF8<>, Va
           } else if (currentKey == labels_cols_str) {
             labels_cols.push_back(str);
             return true;
+          } else if (currentKey == "values") {
+            if constexpr (std::is_same_v<std::string, variant_array_element_type_t<V>>) {
+              accumulatedData.push_back(str);
+            } else {
+              states.push(State::IN_ERROR);
+            }
+            return true;
           } else {
             states.push(State::IN_ERROR);
             return true;
diff --git a/Framework/Core/test/test_Variants.cxx b/Framework/Core/test/test_Variants.cxx
index d774fe160885b..ac0c96fe26565 100644
--- a/Framework/Core/test/test_Variants.cxx
+++ b/Framework/Core/test/test_Variants.cxx
@@ -131,6 +131,16 @@ TEST_CASE("VariantTest")
     REQUIRE(strings[i] == (vscc.get<std::string*>())[i]);
   }
 
+  Variant vsca(vvstr);            // Copy constructor
+  Variant vsma(std::move(vvstr)); // Move constructor
+  Variant vscca = vsma;           // Copy assignment
+  for (auto i = 0u; i < vsma.size(); ++i) {
+    REQUIRE(vstrings[i] == (vsma.get<std::string*>())[i]);
+  }
+  for (auto i = 0u; i < vscca.size(); ++i) {
+    REQUIRE(vstrings[i] == (vscca.get<std::string*>())[i]);
+  }
+
   float m[3][4] = {{0.1, 0.2, 0.3, 0.4}, {0.5, 0.6, 0.7, 0.8}, {0.9, 1.0, 1.1, 1.2}};
   Array2D mm(&m[0][0], 3, 4);
   Variant vmm(mm);
@@ -213,6 +223,11 @@ TEST_CASE("Array2DTest")
       REQUIRE(vv[j] == mm(i, j));
     }
   }
+  std::vector<std::string> s = {"one", "two", "three", "four"};
+  Array2D ms(s, 4, 1);
+  for (auto i = 0U; i < 4; ++i) {
+    REQUIRE(ms(i,0) == s[i]);
+  }
 }
 
 TEST_CASE("LabeledArrayTest")
@@ -236,7 +251,20 @@ TEST_CASE("LabeledArrayTest")
   }
 }
 
-TEST_CASE("VariantConversionsTest")
+TEST_CASE("VariantTreeConversionsTest")
+{
+  std::vector<std::string> vstrings{"0.1", "0.2", "0.3"};
+  Variant vvstr(std::move(vstrings));
+
+  auto tree = vectorToBranch(vvstr.get<std::string*>(), vvstr.size());
+  auto v = Variant(vectorFromBranch<std::string>(tree));
+
+  for (auto i = 0U; i < vvstr.size(); ++i) {
+    REQUIRE(vvstr.get<std::string*>()[i] == v.get<std::string*>()[i]);
+  }
+}
+
+TEST_CASE("VariantJSONConversionsTest")
 {
   int iarr[] = {1, 2, 3, 4, 5};
   Variant viarr(iarr, 5);
@@ -281,4 +309,33 @@ TEST_CASE("VariantConversionsTest")
       REQUIRE(vlaf.get<LabeledArray<float>>().get(i, j) == vlafc.get<LabeledArray<float>>().get(i, j));
     }
   }
+
+  std::string mS[3][4] = {{"a", "b", "c", "d"}, {"e", "f", "g", "h"}, {"i", "l", "m", "n"}};
+  LabeledArray<std::string> las{&mS[0][0], 3, 4, {"r1", "r2", "r3"}, {"c1", "c2", "c3", "c4"}};
+  Variant vms(las);
+  std::stringstream ossl;
+  VariantJSONHelpers::write(ossl, vms);
+
+  std::stringstream issl;
+  issl.str(ossl.str());
+  auto vmsa = VariantJSONHelpers::read<VariantType::LabeledArrayString>(issl);
+
+  for (auto i = 0U; i < vmsa.get<LabeledArray<std::string>>().rows(); ++i) {
+    for (auto j = 0U; j < vmsa.get<LabeledArray<std::string>>().cols(); ++j) {
+      REQUIRE(vmsa.get<LabeledArray<std::string>>().get(i, j) == vms.get<LabeledArray<std::string>>().get(i, j));
+    }
+  }
+
+  std::vector<std::string> vstrings{"s1", "s2", "s3"};
+  Variant vvstr(vstrings);
+  std::stringstream osal;
+  VariantJSONHelpers::write(osal, vvstr);
+
+  std::stringstream isal;
+  isal.str(osal.str());
+  auto vvstra = VariantJSONHelpers::read<VariantType::ArrayString>(isal);
+
+  for (auto i = 0U; i < vvstra.size(); ++i) {
+    REQUIRE(vstrings[i] == vvstra.get<std::string*>()[i]);
+  }
 }