Replace GncOptionValue<const QofInstance*> with GncOptionQofInstanceValue.

That stores its values as a pair of <QofIdType, gncGUID> so that it
won't have dangling ptrs if the instance gets deleted.

libgnucash/app-utils/gnc-option-impl.cpp

@@ -38,6 +38,168 @@ static const QofLogModule log_module{"gnc.options"};
 const std::string GncOptionMultichoiceValue::c_empty_string{""};
 const std::string GncOptionMultichoiceValue::c_list_string{"multiple values"};
 
+using GncItem = std::pair<QofIdTypeConst, GncGUID>;
+
+static GncItem
+make_gnc_item(const QofInstance* inst)
+{
+    if (!inst)
+        return std::make_pair<QofIdTypeConst, GncGUID>("", guid_new_return());
+    auto type{qof_collection_get_type(qof_instance_get_collection(inst))};
+    auto guid{qof_instance_get_guid(inst)};
+    return std::make_pair(std::move(type), std::move(*const_cast<GncGUID*>(guid)));
+}
+
+static const QofInstance*
+qof_instance_from_gnc_item(const GncItem& item)
+{
+    auto [type, guid] = item;
+    auto book{gnc_get_current_book()};
+    auto coll{qof_book_get_collection(book, type)};
+    return static_cast<QofInstance*>(qof_collection_lookup_entity(coll, &guid));
+}
+
+static bool
+operator!=(const GncItem& left, const GncItem& right)
+{
+    auto [ltype, lguid]{left};
+    auto [rtype, rguid]{right};
+    return strcmp(rtype, ltype) && !guid_equal(&rguid, &lguid);
+}
+
+GncOptionQofInstanceValue::GncOptionQofInstanceValue(
+    const char* section, const char* name,
+    const char* key, const char* doc_string,
+    const QofInstance* value, GncOptionUIType ui_type) :
+    OptionClassifier{section, name, key, doc_string},
+    m_ui_type(ui_type), m_value{},
+    m_default_value{} {
+    m_value = make_gnc_item(value);
+    m_default_value = make_gnc_item(value);
+}
+
+GncOptionQofInstanceValue::GncOptionQofInstanceValue(const GncOptionQofInstanceValue& from) :
+    OptionClassifier{from.m_section, from.m_name, from.m_sort_tag,
+                     from.m_doc_string},
+    m_ui_type(from.get_ui_type()), m_value{from.get_item()},
+    m_default_value{from.get_default_item()}
+{
+}
+void
+GncOptionQofInstanceValue::set_value(const QofInstance* new_value)
+{
+    m_value = make_gnc_item(new_value);
+}
+
+void
+GncOptionQofInstanceValue::set_default_value(const QofInstance *new_value)
+{
+    m_value = m_default_value = make_gnc_item(new_value);
+
+}
+
+const QofInstance*
+GncOptionQofInstanceValue::get_value() const
+{
+    return qof_instance_from_gnc_item(m_value);
+}
+
+const QofInstance*
+GncOptionQofInstanceValue::get_default_value() const
+{
+    return qof_instance_from_gnc_item(m_default_value);
+}
+
+void
+GncOptionQofInstanceValue::reset_default_value()
+{
+    m_value = m_default_value;
+}
+
+bool
+GncOptionQofInstanceValue::is_changed() const noexcept
+{
+    return m_value != m_default_value;
+}
+
+bool
+GncOptionQofInstanceValue::deserialize(const std::string& str) noexcept
+{
+    QofInstance* inst{};
+    // Commodities are often serialized as Namespace::Mnemonic or just Mnemonic
+    if (m_ui_type == GncOptionUIType::CURRENCY ||
+        m_ui_type == GncOptionUIType::COMMODITY)
+    {
+        auto book{gnc_get_current_book()};
+        auto table = gnc_commodity_table_get_table(book);
+        auto sep{str.find(":")};
+        if (sep != std::string::npos)
+        {
+            auto name_space{str.substr(0, sep)};
+            auto mnemonic{str.substr(sep + 1, -1)};
+            inst = QOF_INSTANCE(gnc_commodity_table_lookup(table,
+                                                              name_space.c_str(),
+                                                              mnemonic.c_str()));
+        }
+        if (!inst && m_ui_type == GncOptionUIType::CURRENCY)
+            inst = QOF_INSTANCE(gnc_commodity_table_lookup(table,
+                                                             "CURRENCY",
+                                                             str.c_str()));
+        if (inst)
+        {
+            m_value = make_gnc_item(inst);
+            return true;
+        }
+    }
+
+    if (!inst)
+    {
+        try {
+            auto guid{static_cast<GncGUID>(gnc::GUID::from_string(str))};
+            inst = qof_instance_from_guid(&guid, m_ui_type);
+            if (inst)
+            {
+                auto coll{qof_instance_get_collection(inst)};
+                m_value = std::make_pair(qof_collection_get_type(coll), guid);
+                return true;
+            }
+        }
+        catch (const gnc::guid_syntax_exception& err)
+        {
+            PWARN("Failed to convert %s to a GUID", str.c_str());
+        }
+    }
+    return false;
+}
+
+std::string
+GncOptionQofInstanceValue::serialize() const noexcept
+{
+    auto inst{get_value()};
+    std::string retval;
+    if (GNC_IS_COMMODITY(inst))
+    {
+        auto commodity{GNC_COMMODITY(inst)};
+        if (!gnc_commodity_is_currency(commodity))
+        {
+            auto name_space{gnc_commodity_get_namespace(GNC_COMMODITY(inst))};
+            if (name_space && *name_space != '\0')
+            {
+                retval = name_space;
+                retval += ":";
+            }
+        }
+        retval += gnc_commodity_get_mnemonic(GNC_COMMODITY(inst));
+        return retval;
+    }
+    else
+    {
+        gnc::GUID guid{m_value.second};
+        retval = guid.to_string();
+    }
+    return retval;
+}
+
 bool
 GncOptionAccountListValue::validate(const GncOptionAccountList& values) const
 {
@@ -536,9 +698,7 @@ GncOptionValidatedValue<ValueType>::serialize() const noexcept
 template <typename ValueType> bool
 GncOptionValidatedValue<ValueType>::deserialize(const std::string& str) noexcept
 {
-    if constexpr(std::is_same_v<ValueType, const QofInstance*>)
+    if constexpr(is_same_decayed_v<ValueType, std::string>)
-        set_value(qof_instance_from_string(str, get_ui_type()));
-    else if constexpr(is_same_decayed_v<ValueType, std::string>)
         set_value(str);
     else if constexpr(is_same_decayed_v<ValueType, bool>)
         set_value(str == "True");
@@ -728,7 +888,6 @@ template GncOptionValue<double>::GncOptionValue(const GncOptionValue<double>&);
 template GncOptionValue<char*>::GncOptionValue(const GncOptionValue<char*>&);
 template GncOptionValue<const char*>::GncOptionValue(const GncOptionValue<const char*>&);
 template GncOptionValue<std::string>::GncOptionValue(const GncOptionValue<std::string>&);
-template GncOptionValue<const QofInstance*>::GncOptionValue(const GncOptionValue<const QofInstance*>&);
 template GncOptionValue<const QofQuery*>::GncOptionValue(const GncOptionValue<const QofQuery*>&);
 template GncOptionValue<const GncOwner*>::GncOptionValue(const GncOptionValue<const GncOwner*>&);
 template GncOptionValue<RelativeDatePeriod>::GncOptionValue(const GncOptionValue<RelativeDatePeriod>&);
@@ -743,7 +902,6 @@ template void GncOptionValue<double>::set_value(double);
 template void GncOptionValue<char*>::set_value(char*);
 template void GncOptionValue<const char*>::set_value(const char*);
 template void GncOptionValue<std::string>::set_value(std::string);
-template void GncOptionValue<const QofInstance*>::set_value(const QofInstance*);
 template void GncOptionValue<const QofQuery*>::set_value(const QofQuery*);
 template void GncOptionValue<const GncOwner*>::set_value(const GncOwner*);
 template void GncOptionValue<RelativeDatePeriod>::set_value(RelativeDatePeriod);
@@ -757,7 +915,6 @@ template void GncOptionValue<double>::set_default_value(double);
 template void GncOptionValue<char*>::set_default_value(char*);
 template void GncOptionValue<const char*>::set_default_value(const char*);
 template void GncOptionValue<std::string>::set_default_value(std::string);
-template void GncOptionValue<const QofInstance*>::set_default_value(const QofInstance*);
 template void GncOptionValue<const QofQuery*>::set_default_value(const QofQuery*);
 template void GncOptionValue<const GncOwner*>::set_default_value(const GncOwner*);
 template void GncOptionValue<RelativeDatePeriod>::set_default_value(RelativeDatePeriod);
@@ -771,7 +928,6 @@ template void GncOptionValue<double>::reset_default_value();
 template void GncOptionValue<char*>::reset_default_value();
 template void GncOptionValue<const char*>::reset_default_value();
 template void GncOptionValue<std::string>::reset_default_value();
-template void GncOptionValue<const QofInstance*>::reset_default_value();
 template void GncOptionValue<const QofQuery*>::reset_default_value();
 template void GncOptionValue<const GncOwner*>::reset_default_value();
 template void GncOptionValue<RelativeDatePeriod>::reset_default_value();
@@ -785,7 +941,6 @@ template std::string GncOptionValue<double>::serialize() const noexcept;
 template std::string GncOptionValue<char*>::serialize() const noexcept;
 template std::string GncOptionValue<const char*>::serialize() const noexcept;
 template std::string GncOptionValue<std::string>::serialize() const noexcept;
-template std::string GncOptionValue<const QofInstance*>::serialize() const noexcept;
 template std::string GncOptionValue<const QofQuery*>::serialize() const noexcept;
 template std::string GncOptionValue<const GncOwner*>::serialize() const noexcept;
 template std::string GncOptionValue<SCM>::serialize() const noexcept;
@@ -808,7 +963,6 @@ template bool GncOptionValue<double>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<char*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<const char*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<std::string>::deserialize(const std::string&) noexcept;
-template bool GncOptionValue<const QofInstance*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<const QofQuery*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<const GncOwner*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<SCM>::deserialize(const std::string&) noexcept;

libgnucash/app-utils/gnc-option-impl.hpp

@@ -141,6 +141,37 @@ private:
     ValueType m_default_value;
 };
 
+using GncItem = std::pair<QofIdTypeConst, GncGUID>;
+
+class GncOptionQofInstanceValue: public OptionClassifier {
+public:
+    GncOptionQofInstanceValue(
+        const char* section, const char* name,
+        const char* key, const char* doc_string,
+        const QofInstance* value,
+        GncOptionUIType ui_type = GncOptionUIType::INTERNAL);
+    GncOptionQofInstanceValue(const GncOptionQofInstanceValue& from);
+    GncOptionQofInstanceValue(GncOptionQofInstanceValue&&) = default;
+    GncOptionQofInstanceValue& operator=(GncOptionQofInstanceValue&&) = default;
+    ~GncOptionQofInstanceValue() = default;
+    const QofInstance* get_value() const;
+    const QofInstance* get_default_value() const;
+    GncItem get_item() const { return m_value; }
+    GncItem get_default_item() const { return m_default_value; }
+    void set_value(const QofInstance* new_value);
+    void set_default_value(const QofInstance* new_value);
+    void reset_default_value();
+    bool is_changed() const noexcept;
+    GncOptionUIType get_ui_type() const noexcept { return m_ui_type; }
+    void make_internal() { m_ui_type = GncOptionUIType::INTERNAL; }
+    std::string serialize() const noexcept;
+    bool deserialize(const std::string& str) noexcept;
+private:
+    GncOptionUIType m_ui_type;
+    GncItem m_value;
+    GncItem m_default_value;
+};
+
 /** class GncOptionValidatedValue
  *  Validated values have an additional member function, provided as a
  *  constructor argument, that checks value parameters for some property before
@@ -221,7 +252,7 @@ template <typename T>
 struct is_QofInstanceValue
 {
     static constexpr bool value =
-         (std::is_same_v<std::decay_t<T>, GncOptionValue<const QofInstance*>> ||
+         (std::is_same_v<std::decay_t<T>, GncOptionQofInstanceValue> ||
           std::is_same_v<std::decay_t<T>,
           GncOptionValidatedValue<const QofInstance*>>);
 };
@@ -276,7 +307,7 @@ operator<< (std::ostream& oss, const OptType& opt)
     if (auto type = opt.get_ui_type(); type == GncOptionUIType::COMMODITY ||
         type == GncOptionUIType::CURRENCY)
     {
-        if (auto type = opt.get_ui_type(); type == GncOptionUIType::COMMODITY)
+        if (type == GncOptionUIType::COMMODITY)
         {
             oss << gnc_commodity_get_namespace(GNC_COMMODITY(value)) << " ";
         }

libgnucash/app-utils/gnc-option.cpp

@@ -35,7 +35,7 @@ extern "C"
 
 template <typename ValueType,
           typename std::enable_if_t<!is_OptionClassifier_v<ValueType>,
-                               int>>
+                                    int>>
 GncOption::GncOption(const char* section, const char* name,
                      const char* key, const char* doc_string,
                      ValueType value, GncOptionUIType ui_type) :
@@ -343,7 +343,7 @@ GncOption::permissible_value_index(const char* value) const
                                             GncOptionDateValue>)
                 return option.permissible_value_index(value);
             else
-                return size_t_max;;
+                return size_t_max;
         }, *m_option);
 }
 
@@ -462,8 +462,6 @@ template GncOption::GncOption(const char*, const char*, const char*,
 //                              const char*, double, GncOptionUIType);
 template GncOption::GncOption(const char*, const char*, const char*,
                               const char*, std::string, GncOptionUIType);
-template GncOption::GncOption(const char*, const char*, const char*,
-                              const char*, const QofInstance*, GncOptionUIType);
 template GncOption::GncOption(const char*, const char*, const char*,
                               const char*, SCM, GncOptionUIType);
 template GncOption::GncOption(const char*, const char*, const char*,
@@ -553,9 +551,4 @@ template GncOption* gnc_make_option<bool>(const char*, const char*, const char*,
 template GncOption* gnc_make_option<int64_t>(const char*, const char*,
                                              const char*, const char*, int64_t,
                                              GncOptionUIType);
-template GncOption* gnc_make_option<const QofInstance*>(const char*,
+
-                                                        const char*,
-                                                        const char*,
-                                                        const char*,
-                                                        const QofInstance*,
-                                                        GncOptionUIType);

libgnucash/app-utils/gnc-option.hpp

@@ -55,6 +55,7 @@ using QofQuery = _QofQuery;
 struct QofInstance_s;
 using QofInstance = QofInstance_s;
 template <typename ValueType> class GncOptionValue;
+class GncOptionQofInstanceValue;
 class GncOptionAccountListValue;
 class GncOptionAccountSelValue;
 class GncOptionMultichoiceValue;
@@ -97,7 +98,7 @@ is_RangeValue_v = is_RangeValue<T>::value;
 using GncOptionVariant = std::variant<GncOptionValue<std::string>,
                                       GncOptionValue<bool>,
                                       GncOptionValue<int64_t>,
-                                      GncOptionValue<const QofInstance*>,
+                                      GncOptionQofInstanceValue,
                                       GncOptionValue<const QofQuery*>,
                                       GncOptionValue<const GncOwner*>,
                                       GncOptionValue<SCM>,

libgnucash/app-utils/gnc-optiondb.cpp

@@ -630,8 +630,9 @@ gnc_register_budget_option(GncOptionDB* db, const char* section,
                            const char* name, const char* key,
                            const char* doc_string, GncBudget *value)
 {
-    GncOption option{section, name, key, doc_string, (const QofInstance*)value,
+    GncOption option{GncOptionQofInstanceValue{section, name, key, doc_string,
-            GncOptionUIType::BUDGET};
+                                               (const QofInstance*)value,
+                                               GncOptionUIType::BUDGET}};
     db->register_option(section, std::move(option));
 }
 
@@ -650,8 +651,9 @@ gnc_register_commodity_option(GncOptionDB* db, const char* section,
                               const char* name, const char* key,
                               const char* doc_string, gnc_commodity *value)
 {
-    GncOption option{section, name, key, doc_string, (const QofInstance*)value,
+    GncOption option{GncOptionQofInstanceValue{section, name, key, doc_string,
-            GncOptionUIType::COMMODITY};
+                                               (const QofInstance*)value,
+                                               GncOptionUIType::COMMODITY}};
     db->register_option(section, std::move(option));
 }
 
@@ -862,8 +864,9 @@ gnc_register_invoice_option(GncOptionDB* db, const char* section,
                             const char* name, const char* key,
                             const char* doc_string, GncInvoice* value)
 {
-    GncOption option{section, name, key, doc_string, (const QofInstance*)value,
+    GncOption option{GncOptionQofInstanceValue{section, name, key, doc_string,
-            GncOptionUIType::INVOICE};
+                                               (const QofInstance*)value,
+                                               GncOptionUIType::INVOICE}};
     db->register_option(section, std::move(option));
 }
 
@@ -872,8 +875,9 @@ gnc_register_taxtable_option(GncOptionDB* db, const char* section,
                              const char* name, const char* key,
                              const char* doc_string, GncTaxTable* value)
 {
-    GncOption option{section, name, key, doc_string, (const QofInstance*)value,
+    GncOption option{GncOptionQofInstanceValue{section, name, key, doc_string,
-            GncOptionUIType::TAX_TABLE};
+                                               (const QofInstance*)value,
+                                               GncOptionUIType::TAX_TABLE}};
     db->register_option(section, std::move(option));
 }
 

libgnucash/app-utils/gnc-optiondb.i

@@ -140,9 +140,10 @@ SCM scm_init_sw_gnc_optiondb_module(void);
                 $descriptor(_gncJob*), $descriptor(_gncVendor*)
                 };
         void* ptr{};
+        SCM instance{$input};
         auto pos = std::find_if(types.begin(), types.end(),
-                                [&$input, &ptr](auto type){
+                                [&instance, &ptr](auto type){
-                                    SWIG_ConvertPtr($input, &ptr, type, 0);
+                                    SWIG_ConvertPtr(instance, &ptr, type, 0);
                                     return ptr != nullptr; });
         if (pos == types.end())
             $1 = nullptr;
@@ -990,7 +991,6 @@ inline SCM return_scm_value(ValueType value)
 %template(gnc_make_string_option) gnc_make_option<std::string>;
 %template(gnc_make_bool_option) gnc_make_option<bool>;
 %template(gnc_make_int64_option) gnc_make_option<int64_t>;
-%template(gnc_make_qofinstance_option) gnc_make_option<const QofInstance*>;
 %template(gnc_make_query_option) gnc_make_option<const QofQuery*>;
 %template(gnc_make_owner_option) gnc_make_option<const GncOwner*>;
 
@@ -1383,6 +1383,26 @@ inline SCM return_scm_value(ValueType value)
         qof_book_commit_edit(book);
     }
 
+    static GncOption*
+    gnc_make_qofinstance_option(const char* section,
+                                const char* name, const char* key,
+                                const char* doc_string,
+                                const QofInstance* value,
+                                GncOptionUIType ui_type)
+    {
+        try {
+            return new GncOption(GncOptionQofInstanceValue{section, name, key,
+                                                           doc_string,
+                                                           value, ui_type});
+        }
+        catch (const std::exception& err)
+        {
+            std::cerr << "Make QofInstance option threw unexpected exception"
+            << err.what() << ", option not created." << std::endl;
+            return nullptr;
+        }
+    }
+
     static GncOption*
     gnc_make_account_list_option(const char* section,
                                  const char* name, const char* key,
@@ -1569,7 +1589,7 @@ inline SCM return_scm_value(ValueType value)
                               const char* key, const char* doc_string,
                               gnc_commodity *value)
     {
-        return new GncOption{GncOptionValue<const QofInstance*>{
+        return new GncOption{GncOptionQofInstanceValue{
                 section, name, key, doc_string, (const QofInstance*)value,
                     GncOptionUIType::COMMODITY}};
     }

libgnucash/app-utils/test/gtest-gnc-option.cpp

@@ -154,8 +154,9 @@ TEST_F(GncOptionTest, test_budget_ctor)
 {
     auto budget = gnc_budget_new(m_book);
     EXPECT_NO_THROW({
-            GncOption option("foo", "bar", "baz", "Phony Option",
+            GncOption option(GncOptionQofInstanceValue{"foo", "bar", "baz",
-                             (const QofInstance*)budget);
+                                                       "Phony Option",
+                                                       (const QofInstance*)budget});
         });
     gnc_budget_destroy(budget);
 }
@@ -163,7 +164,7 @@ TEST_F(GncOptionTest, test_budget_ctor)
 TEST_F(GncOptionTest, test_budget_out)
 {
     auto budget = gnc_budget_new(m_book);
-    GncOption option{"foo", "bar", "baz", "Phony Option", (const QofInstance*)budget};
+    GncOption option{GncOptionQofInstanceValue{"foo", "bar", "baz", "Phony Option", (const QofInstance*)budget}};
 
     auto budget_guid{gnc::GUID{*qof_instance_get_guid(QOF_INSTANCE(budget))}.to_string()};
     std::ostringstream oss;
@@ -177,7 +178,7 @@ TEST_F(GncOptionTest, test_budget_in)
     auto budget = gnc_budget_new(m_book);
     auto budget_guid{gnc::GUID{*qof_instance_get_guid(QOF_INSTANCE(budget))}.to_string()};
     std::istringstream iss{budget_guid};
-    GncOption option{GncOptionValue<const QofInstance*>{"foo", "bar", "baz", "Phony Option", nullptr, GncOptionUIType::BUDGET}};
+    GncOption option{GncOptionQofInstanceValue{"foo", "bar", "baz", "Phony Option", nullptr, GncOptionUIType::BUDGET}};
     iss >> option;
     EXPECT_EQ(QOF_INSTANCE(budget), option.get_value<const QofInstance*>());
     gnc_budget_destroy(budget);
@@ -188,9 +189,10 @@ TEST_F(GncOptionTest, test_commodity_ctor)
     auto hpe = gnc_commodity_new(m_book, "Hewlett Packard Enterprise, Inc.",
                                     "NYSE", "HPE", NULL, 1);
     EXPECT_NO_THROW({
-            GncOption option("foo", "bar", "baz", "Phony Option",
+            GncOption option(GncOptionQofInstanceValue{"foo", "bar", "baz",
-                             (const QofInstance*)hpe);
+                                                       "Phony Option",
-        });
+                                                       (const QofInstance*)hpe});
+    });
     gnc_commodity_destroy(hpe);
 }
 
@@ -319,8 +321,8 @@ TEST_F(GncOptionCommodityTest, test_currency_out)
 
 TEST_F(GncOptionCommodityTest, test_commodity_out)
 {
-    GncOption option{"foo", "bar", "baz", "Phony Option", (const QofInstance*)m_hpe,
+    GncOption option{GncOptionQofInstanceValue{"foo", "bar", "baz", "Phony Option", (const QofInstance*)m_hpe,
-                     GncOptionUIType::COMMODITY};
+                     GncOptionUIType::COMMODITY}};
     std::string hpe_str{make_commodity_str(m_hpe)};
     std::ostringstream oss;
     oss << option;
@@ -348,8 +350,8 @@ TEST_F(GncOptionCommodityTest, test_currency_in)
 
 TEST_F(GncOptionCommodityTest, test_commodity_in)
 {
-    GncOption option{"foo", "bar", "baz", "Phony Option", (const QofInstance*)m_aapl,
+    GncOption option{GncOptionQofInstanceValue{"foo", "bar", "baz", "Phony Option", (const QofInstance*)m_aapl,
-                     GncOptionUIType::COMMODITY};
+                     GncOptionUIType::COMMODITY}};
 
     std::string hpe_str{make_commodity_str(m_hpe)};
     std::istringstream iss{hpe_str};