Add path-vector get/set functions to KvpFrameImpl.

Needed for Scheme access. The C functions used GList.

src/backend/xml/io-gncxml-v1.cpp

@@ -806,7 +806,7 @@ kvp_frame_slot_end_handler(gpointer data_for_children,
     if (key_node_count != 1) return(FALSE);
 
     value_cr->should_cleanup = TRUE;
-    f->replace_nc(key, value);
+    f->set(key, value);
     if (delete_value)
         delete value;
     return(TRUE);

src/backend/xml/sixtp-dom-parsers.cpp

@@ -446,7 +446,7 @@ dom_tree_to_kvp_frame_given(xmlNodePtr node, KvpFrame *frame)
                 if (val)
                 {
                     //We're deleting the old KvpValue returned by replace_nc().
-                    delete frame->replace_nc(key, val);
+                    delete frame->set(key, val);
                 }
                 else
                 {

src/libqof/qof/kvp-value.cpp

@@ -325,7 +325,7 @@ delete_visitor::operator()(GncGUID * & value)
 template <> void
 delete_visitor::operator()(KvpFrame * & value)
 {
-    kvp_frame_delete(value);
+    delete value;
 }
 
 KvpValueImpl::~KvpValueImpl() noexcept

src/libqof/qof/kvp_frame.cpp

@@ -51,11 +51,12 @@ KvpFrameImpl::KvpFrameImpl(const KvpFrameImpl & rhs) noexcept
     );
 }
 
-KvpValueImpl * KvpFrameImpl::replace_nc(const char * key, KvpValueImpl * value) noexcept
+KvpValue*
+KvpFrameImpl::set(const char* key, KvpValue* value) noexcept
 {
     if (!key) return nullptr;
     auto spot = m_valuemap.find(key);
-    KvpValueImpl * ret {nullptr};
+    KvpValue* ret {nullptr};
     if (spot != m_valuemap.end())
     {
         qof_string_cache_remove(spot->first);
@@ -72,6 +73,53 @@ KvpValueImpl * KvpFrameImpl::replace_nc(const char * key, KvpValueImpl * value)
     return ret;
 }
 
+static inline KvpFrameImpl*
+walk_path_or_nullptr(const KvpFrameImpl* frame, Path& path)
+{
+    KvpFrameImpl* cur_frame = const_cast<KvpFrameImpl*>(frame);
+    for(auto key:path)
+    {
+        auto slot = cur_frame->get_slot(key);
+        if (slot == nullptr || slot->get_type() != KVP_TYPE_FRAME)
+            return nullptr;
+        cur_frame = slot->get<KvpFrame*>();
+    }
+    return cur_frame;
+}
+
+
+KvpValue*
+KvpFrameImpl::set(Path path, KvpValue* value) noexcept
+{
+    const char* last_key = path.back();
+    path.pop_back();
+    auto cur_frame = walk_path_or_nullptr(this, path);
+    if (cur_frame == nullptr)
+        return nullptr;
+    return cur_frame->set(last_key, value);
+}
+
+KvpValue*
+KvpFrameImpl::set_path(Path path, KvpValue* value) noexcept
+{
+    auto cur_frame = this;
+    const char* last_key = path.back();
+    path.pop_back();
+    for(auto key:path)
+    {
+        auto slot = cur_frame->get_slot(key);
+        if (slot == nullptr || slot->get_type() != KVP_TYPE_FRAME)
+        {
+            auto new_frame = new KvpFrame;
+            delete cur_frame->set(key, new KvpValue{new_frame});
+            cur_frame = new_frame;
+            continue;
+        }
+        cur_frame = slot->get<KvpFrame*>();
+    }
+    return cur_frame->set(last_key, value);
+}
+
 std::string
 KvpFrameImpl::to_string() const noexcept
 {
@@ -131,6 +179,18 @@ KvpFrameImpl::get_slot(const char * key) const noexcept
     return spot->second;
 }
 
+KvpValueImpl *
+KvpFrameImpl::get_slot(Path path) const noexcept
+{
+    const char* last_key = path.back();
+    path.pop_back();
+    auto cur_frame = walk_path_or_nullptr(this, path);
+    if (cur_frame == nullptr)
+        return nullptr;
+    return cur_frame->get_slot(last_key);
+
+}
+
 int compare(const KvpFrameImpl * one, const KvpFrameImpl * two) noexcept
 {
     if (one && !two) return 1;
@@ -205,9 +265,7 @@ kvp_frame_get_keys(const KvpFrame * frame)
 gboolean
 kvp_frame_is_empty(const KvpFrame * frame)
 {
-    if (!frame) return TRUE;
-    auto realframe = static_cast<KvpFrameImpl const *>(frame);
-    return realframe->get_keys().size() == 0;
+    return frame->empty();
 }
 
 KvpFrame *
@@ -222,26 +280,20 @@ kvp_frame_copy(const KvpFrame * frame)
  * removing the key from the KVP tree.
  */
 static KvpValue *
-kvp_frame_replace_slot_nc (KvpFrame * frame, const char * slot,
-                           KvpValue * new_value)
+kvp_frame_replace_slot_nc (KvpFrame* frame, const char* slot, KvpValue* value)
 {
-    if (!frame) return NULL;
-
-    auto realframe = static_cast<KvpFrameImpl *>(frame);
-    auto realnewvalue = static_cast<KvpValueImpl *>(new_value);
-    return realframe->replace_nc(slot, realnewvalue);
+    if (!frame) return nullptr;
+    return frame->set(slot, value);
 }
 
 /* Passing in a null value into this routine has the effect
  * of deleting the old value stored at this slot.
  */
 static inline void
-kvp_frame_set_slot_destructively(KvpFrame * frame, const char * slot,
-                                 KvpValue * new_value)
+kvp_frame_set_slot_destructively(KvpFrame* frame, const char* slot,
+                                 KvpValue* value)
 {
-    KvpValue * old_value;
-    old_value = kvp_frame_replace_slot_nc (frame, slot, new_value);
-    kvp_value_delete (old_value);
+    delete frame->set(slot, value);
 }
 
 /* ============================================================ */
@@ -584,14 +636,12 @@ void
 kvp_frame_set_slot(KvpFrame * frame, const char * slot,
                    KvpValue * value)
 {
-    KvpValue *new_value = NULL;
-
+    KvpValue* new_value{nullptr};
     if (!frame) return;
-
     g_return_if_fail (slot && *slot != '\0');
 
     if (value) new_value = kvp_value_copy(value);
-    kvp_frame_set_slot_destructively(frame, slot, new_value);
+    delete frame->set(slot, new_value);
 }
 
 void
@@ -599,18 +649,16 @@ kvp_frame_set_slot_nc(KvpFrame * frame, const char * slot,
                       KvpValue * value)
 {
     if (!frame) return;
-
     g_return_if_fail (slot && *slot != '\0');
 
-    kvp_frame_set_slot_destructively(frame, slot, value);
+    frame->set(slot, value);
 }
 
 KvpValue *
 kvp_frame_get_slot(const KvpFrame * frame, const char * slot)
 {
-    if (!frame) return NULL;
-    auto realframe = static_cast<const KvpFrameImpl *>(frame);
-    return realframe->get_slot(slot);
+    if (!frame) return nullptr;
+    return frame->get_slot(slot);
 }
 
 void

src/libqof/qof/kvp_frame.h

@@ -315,10 +315,10 @@ KvpValue  * kvp_frame_get_value(const KvpFrame *frame, const gchar *path);
  *  exist.
 */
 /*@ dependent @*/
-KvpFrame  * kvp_frame_get_frame(const KvpFrame *frame, const gchar *path);
+KvpFrame* kvp_frame_get_frame(const KvpFrame *frame, const gchar *path);
 
-KvpFrame    * kvp_frame_get_frame_slash (KvpFrame *frame,
-        const gchar *path);
+KvpFrame* kvp_frame_get_frame_slash (KvpFrame *frame,
+                                     const gchar *path);
 
 /** @} */
 /** @name KvpFrame KvpValue low-level storing routines.

src/libqof/qof/kvp_frame.hpp

@@ -41,9 +41,11 @@ class cstring_comparer
     }
 };
 
+using Path = std::vector<const char*>;
+
 struct KvpFrameImpl
 {
-    typedef std::map<const char *, KvpValueImpl *, cstring_comparer> map_type;
+    typedef std::map<const char *, KvpValue*, cstring_comparer> map_type;
 
     public:
     KvpFrameImpl() noexcept {};
@@ -54,22 +56,67 @@ struct KvpFrameImpl
     KvpFrameImpl(const KvpFrameImpl &) noexcept;
 
     /**
-     * Replaces the KvpValueImpl at the specified spot, and returns
-     * the old KvpValueImpl which used to occupy the spot.
-     *
-     * If no KvpValueImpl was at the spot, nullptr is returned.
+     * Set the value with the key in the immediate frame, replacing and
+     * returning the old value if it exists or nullptr if it doesn't.
+     * @param key: The key to insert/replace.
+     * @param newvalue: The value to set at key.
+     * @return The old value if there was one or nullptr.
+     */
+    KvpValue* set(const char * key, KvpValue* newvalue) noexcept;
+    /**
+     * Set the value with the key in a subframe following the keys in path,
+     * replacing and returning the old value if it exists or nullptr if it
+     * doesn't.
+     * @throw invalid_argument if the path doesn't exist.
+     * @param path: The path of subframes leading to the frame in which to
+     * insert/replace.
+     * @param newvalue: The value to set at key.
+     * @return The old value if there was one or nullptr.
+     */
+    KvpValue* set(Path path, KvpValue* newvalue) noexcept;
+    /**
+     * Set the value with the key in a subframe following the keys in path,
+     * replacing and returning the old value if it exists or nullptr if it
+     * doesn't. Creates any missing intermediate frames.
+     * @param path: The path of subframes leading to the frame in which to
+     * insert/replace.
+     * @param newvalue: The value to set at key.
+     * @return The old value if there was one or nullptr.
+     */
+    KvpValue* set_path(Path path, KvpValue* newvalue) noexcept;
+    /**
+     * Make a string representation of the frame. Mostly useful for debugging.
+     * @return A std::string representing the frame and all its children.
      */
-    KvpValueImpl * replace_nc(const char * key, KvpValueImpl * newvalue) noexcept;
-
     std::string to_string() const noexcept;
-
+    /**
+     * Report the keys in the immediate frame. Be sensible about using this, it
+     * isn't a very efficient way to iterate.
+     * @return std::vector of keys as std::strings.
+     */
     std::vector<std::string> get_keys() const noexcept;
 
-    KvpValueImpl * get_slot(const char * key) const noexcept;
+    /** Get the value for the key or nullptr if it doesn't exist.
+     * @param key: The key.
+     * @return The value at the key or nullptr.
+     */
+    KvpValue* get_slot(const char * key) const noexcept;
+    /** Get the value for the tail of the path or nullptr if it doesn't exist.
+     * @param path: Path of keys leading to the desired value.
+     * @return The value at the key or nullptr.
+     */
+    KvpValue* get_slot(Path keys) const noexcept;
+    /** Convenience wrapper for std::for_each, which should be preferred.
+     */
+    void for_each_slot(void (*proc)(const char *key, KvpValue *value,
+                                    void * data),
+                       void *data) const noexcept;
 
-    void for_each_slot(void (*proc)(const char *key, KvpValue *value, void * data), void *data) const noexcept;
-
-    friend int compare(const KvpFrameImpl &, const KvpFrameImpl &) noexcept;
+    /** Test for emptiness
+     * @return true if the frame contains nothing.
+     */
+    bool empty() const noexcept { return m_valuemap.empty(); }
+    friend int compare(const KvpFrameImpl&, const KvpFrameImpl&) noexcept;
 
     private:
     map_type m_valuemap;

src/libqof/qof/test/test-kvp-frame.cpp

@@ -27,6 +27,24 @@
 #include <gtest/gtest.h>
 #include <algorithm>
 
+class KvpFrameTest : public ::testing::Test
+{
+public:
+    KvpFrameTest() :
+        t_int_val{new KvpValue {INT64_C(15)}},
+        t_str_val{new KvpValue{"a value"}}     {
+        auto f1 = new KvpFrame;
+        t_root.set("top", new KvpValue{f1});
+        f1->set("first", t_int_val);
+        f1->set("second", new KvpValue{new KvpFrame});
+        f1->set("third", t_str_val);
+    }
+protected:
+    KvpFrameImpl t_root;
+    KvpValue *t_int_val;
+    KvpValue *t_str_val;
+};
+
 template <typename A, typename B> void
 assert_contains (std::vector<A> vec, B const & b)
 {
@@ -34,57 +52,98 @@ assert_contains (std::vector<A> vec, B const & b)
     EXPECT_NE (val, vec.end ());
 }
 
-TEST (KvpFrameTest, Replace)
+TEST_F (KvpFrameTest, SetLocal)
 {
     auto f1 = new KvpFrameImpl;
     auto v1 = new KvpValueImpl {15.0};
     auto v2 = new KvpValueImpl { (int64_t)52};
     std::string k1 {"first key"};
 
-    EXPECT_EQ (nullptr, f1->replace_nc (k1.c_str (), v1));
-    EXPECT_EQ (v1, f1->replace_nc (k1.c_str (), v2));
+    EXPECT_EQ (nullptr, f1->set (k1.c_str (), v1));
+    EXPECT_EQ (v1, f1->set (k1.c_str (), v2));
 
     delete f1; //this should also delete v2.
     delete v1;
 }
 
-TEST (KvpFrameTest, GetKeys)
+TEST_F (KvpFrameTest, SetPath)
 {
-    auto k1 = "first key";
-    auto k2 = "second key";
-    auto k3 = "first key/third key";
-    auto f1 = new KvpFrameImpl;
-    auto v1 = new KvpValueImpl {15.2};
-    auto v2 = new KvpValueImpl { (int64_t)12};
-    auto v3 = new KvpValueImpl {strdup ("Never again")};
+    Path path1 {"top", "second", "twenty-first"};
+    Path path2 {"top", "third", "thirty-first"};
+    auto v1 = new KvpValueImpl {15.0};
+    auto v2 = new KvpValueImpl { (int64_t)52};
 
-    f1->replace_nc (k1, v1);
-    f1->replace_nc (k2, v2);
-    f1->replace_nc (k3, v3);
+    EXPECT_EQ (nullptr, t_root.set(path1, v1));
+    EXPECT_EQ (v1, t_root.set(path1, v2));
+    EXPECT_EQ (nullptr, t_root.set(path2, v1));
+    EXPECT_EQ (v2, t_root.get_slot(path1));
+    EXPECT_EQ (nullptr, t_root.get_slot(path2));
+    delete v1;
+}
 
-    auto keys = f1->get_keys ();
-    EXPECT_EQ (keys.size (), 3);
+TEST_F (KvpFrameTest, SetPathWithCreate)
+{
+    Path path1 {"top", "second", "twenty-first"};
+    Path path2 {"top", "third", "thirty-first"};
+    auto v1 = new KvpValueImpl {15.0};
+    auto v2 = new KvpValueImpl { (int64_t)52};
+
+    EXPECT_EQ (nullptr, t_root.set_path(path1, v1));
+    EXPECT_EQ (v1, t_root.set_path(path1, v2));
+    EXPECT_EQ (nullptr, t_root.set_path(path2, v1));
+    EXPECT_EQ (v2, t_root.get_slot(path1));
+    EXPECT_EQ (v1, t_root.get_slot(path2));
+}
+
+TEST_F (KvpFrameTest, GetKeys)
+{
+    auto k1 = "top";
+    auto k2 = "first";
+    auto k3 = "second";
+
+    auto keys = t_root.get_keys ();
+    EXPECT_EQ (keys.size (), 1);
 
     assert_contains (keys, k1);
+    auto frameval = t_root.get_slot(k1);
+    ASSERT_EQ(frameval->get_type(), KVP_TYPE_FRAME);
+    keys = frameval->get<KvpFrame*>()->get_keys();
     assert_contains (keys, k2);
     assert_contains (keys, k3);
-
-    delete f1;//This should delete our KvpValueImpls, and our string above, too.
 }
 
-TEST (KvpFrameTest, GetSlot)
+TEST_F (KvpFrameTest, GetLocalSlot)
 {
-    auto f1 = new KvpFrameImpl;
-    auto v1 = new KvpValueImpl {2.2};
-    auto v2 = new KvpValueImpl { (int64_t)4};
-    auto k1 = "first key";
-    auto k2 = "first key/second key";
+    auto k1 = "first";
+    auto k2 = "third";
+    auto k3 = "doesn't exist";
 
-    f1->replace_nc (k1, v1);
-    EXPECT_EQ (v1, f1->get_slot(k1));
-    f1->replace_nc (k2, v2);
-    EXPECT_EQ (v2, f1->get_slot(k2));
-    EXPECT_EQ (v1, f1->get_slot(k1));
-
-    delete f1;//which will delete both kvpvalues, too.
+    auto frameval = t_root.get_slot("top");
+    ASSERT_EQ(frameval->get_type(), KVP_TYPE_FRAME);
+    auto f1 = frameval->get<KvpFrame*>();
+    EXPECT_EQ (t_int_val, f1->get_slot(k1));
+    EXPECT_EQ (t_str_val, f1->get_slot(k2));
+    EXPECT_EQ (nullptr, f1->get_slot(k3));
+}
+
+TEST_F (KvpFrameTest, GetSlotPath)
+{
+    Path path1 {"top", "second", "twenty-first"};
+    Path path2 {"top", "third", "thirty-first"};
+    auto v1 = new KvpValueImpl {15.0};
+    auto v2 = new KvpValueImpl { (int64_t)52};
+
+    EXPECT_EQ (nullptr, t_root.set(path1, v1));
+    EXPECT_EQ (nullptr, t_root.set(path2, v2));
+    EXPECT_EQ (v1, t_root.get_slot(path1));
+    EXPECT_EQ (nullptr, t_root.get_slot(path2));
+    delete v2;
+}
+
+TEST_F (KvpFrameTest, Empty)
+{
+    KvpFrameImpl f1, f2;
+    f2.set("value", new KvpValue {2.2});
+    EXPECT_TRUE(f1.empty());
+    EXPECT_FALSE(f2.empty());
 }