From 330097b7d455fbae689c890e5b5fef1a59d885e7 Mon Sep 17 00:00:00 2001
From: Michael Davidsaver <mdavidsaver@gmail.com>
Date: Wed, 31 Jul 2024 17:05:28 -0700
Subject: [PATCH] cache_sync() copy Any/Union

Delta may be later be changed by user code,
so copy to ensure cache is not modified.

cf. 92fb0a4afa5054b6cc3e8eb395facaa337d1c454
---
 src/data.cpp       | 10 ++-------
 src/dataimpl.h     |  1 +
 test/testdata.cpp  | 56 +++++++++++++++++++++++++++++++++++++++++++++-
 test/testxcode.cpp |  1 +
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/src/data.cpp b/src/data.cpp
index 8dcb7ddfe..8754da0cd 100644
--- a/src/data.cpp
+++ b/src/data.cpp
@@ -1231,13 +1231,7 @@ void cache_sync(Value& cache, Value& dlt)
             dst->as<shared_array<const void>>() = src->as<shared_array<const void>>();
             break;
         case StoreType::Compound:
-        {
-            std::shared_ptr<impl::FieldStorage> sstore(Value::Helper::store(dlt),
-                                                      src);
-            auto& dfld(dst->as<Value>());
-            Value::Helper::set_desc(dfld, &desc[i]);
-            Value::Helper::store(dfld) = std::move(sstore);
-        }
+            dst->as<Value>() = src->as<Value>().clone();
             break;
         }
     }
@@ -1264,7 +1258,7 @@ void FieldStorage::init(StoreType code)
         new(&store) std::string();
         return;
     case StoreType::Compound:
-        new(&store) std::shared_ptr<FieldStorage>();
+        new(&store) Value();
         return;
     case StoreType::Array:
         new(&store) shared_array<void>();
diff --git a/src/dataimpl.h b/src/dataimpl.h
index 98c7fd72a..7e455ad3e 100644
--- a/src/dataimpl.h
+++ b/src/dataimpl.h
@@ -47,6 +47,7 @@ struct Value::Helper {
  * copy   marked from delta -> cache
  * copy unmarked from cache -> delta
  */
+PVXS_API
 void cache_sync(Value& cache, Value& delta);
 
 namespace impl {
diff --git a/test/testdata.cpp b/test/testdata.cpp
index 4168fd979..51b295f01 100644
--- a/test/testdata.cpp
+++ b/test/testdata.cpp
@@ -493,11 +493,64 @@ void testClear()
     testFalse(val.isMarked(true, true));
 }
 
+void test_cache_sync()
+{
+    testShow()<<__func__;
+
+    auto cache(TypeDef(TypeCode::Struct, {
+                           members::UInt32("int"),
+                           members::String("string"),
+                           members::UInt32A("arr"),
+                           members::Any("any"),
+                           Member(TypeCode::Union, "choice", {
+                               Member(TypeCode::Float32, "a"),
+                               Member(TypeCode::String, "b"),
+                           }),
+                       }).create());
+    auto delta(cache.cloneEmpty());
+
+    delta["int"] = 42;
+    delta["string"] = "hello";
+    delta["arr"] = shared_array<const uint32_t>({1, 2, 3});
+    delta["any"] = 5;
+    delta["choice->a"] = 6;
+
+    // updates cache <- delta
+    cache_sync(cache, delta);
+
+    testEq(delta["int"].as<uint32_t>(), 42u);
+    testEq(delta["string"].as<std::string>(), "hello");
+    testArrEq(delta["arr"].as<shared_array<const uint32_t>>(), shared_array<const uint32_t>({1, 2, 3}));
+    testEq(delta["any"].as<uint32_t>(), 5u);
+    testEq(delta["choice"].as<uint32_t>(), 6u);
+
+    testEq(cache["int"].as<uint32_t>(), 42u);
+    testEq(cache["string"].as<std::string>(), "hello");
+    testArrEq(cache["arr"].as<shared_array<const uint32_t>>(), shared_array<const uint32_t>({1, 2, 3}));
+    testEq(cache["any"].as<uint32_t>(), 5u);
+    testEq(cache["choice"].as<uint32_t>(), 6u);
+
+    delta = cache.cloneEmpty();
+
+    // updates cache -> delta
+    cache_sync(cache, delta);
+
+    testEq(delta["int"].as<uint32_t>(), 42u);
+    testEq(delta["string"].as<std::string>(), "hello");
+    testArrEq(delta["arr"].as<shared_array<const uint32_t>>(), shared_array<const uint32_t>({1, 2, 3}));
+    testEq(delta["any"].as<uint32_t>(), 5u);
+    testEq(delta["choice"].as<uint32_t>(), 6u);
+
+    // Any/Union should be copied to allow consumer of delta to modify
+    testFalse(delta["any"].as<Value>().equalInst(cache["any"].as<Value>()));
+    testFalse(delta["choice"].as<Value>().equalInst(cache["choice"].as<Value>()));
+}
+
 } // namespace
 
 MAIN(testdata)
 {
-    testPlan(172);
+    testPlan(189);
     testSetup();
     testTraverse();
     testAssign();
@@ -581,6 +634,7 @@ MAIN(testdata)
     testUnionMagicAssign();
     testExtract();
     testClear();
+    test_cache_sync();
     cleanup_for_valgrind();
     return testDone();
 }
diff --git a/test/testxcode.cpp b/test/testxcode.cpp
index b577fda25..ee20aed44 100644
--- a/test/testxcode.cpp
+++ b/test/testxcode.cpp
@@ -168,6 +168,7 @@ void testDeserialize1()
 
 }
 
+const
 TypeDef simpledef(TypeCode::Struct, "simple_t", {
                 Member(TypeCode::UInt64A, "value"),
                 Member(TypeCode::Struct, "timeStamp", "time_t", {