Fixed conversion problem

The conversion assumed there were only three levels to bayes import
map kvp: IMAP token, user-supplied token, GUID/account name. In
actuality, since user-supplied tokens could have the delimiter in them,
there could be several. This fix takes that into account like so:
IMAP token, potentially several user-supplied tokens, GUID/account name.

The import map is undergoing two conversions at the same time: account names
to guids and an hierarchical representation to a flat representation in KVP.
This commit is contained in:
lmat 2017-10-27 11:09:42 -04:00
parent b3667c76fc
commit eb6dad86e3
6 changed files with 57 additions and 194 deletions

View File

@ -1026,7 +1026,6 @@ RESTART:
// Convert imap mappings from account full name to guid strings
qof_event_suspend();
gnc_account_imap_convert_bayes (gnc_get_current_book());
gnc_account_imap_convert_flat (gnc_get_current_book());
qof_event_resume();
return TRUE;

View File

@ -43,6 +43,8 @@
#include "gnc-features.h"
#include "guid.hpp"
#include <numeric>
static QofLogModule log_module = GNC_MOD_ACCOUNT;
/* The Canonical Account Separator. Pre-Initialized. */
@ -5269,8 +5271,8 @@ get_first_pass_probabilities(GncImportMatchMap * imap, GList * tokens)
* in the input tokens list. */
for (auto current_token = tokens; current_token; current_token = current_token->next)
{
auto translated_token = std::string{static_cast<char const *>(current_token->data)};
std::replace(translated_token.begin(), translated_token.end(), '/', '-');
auto translated_token = std::string {static_cast <char const *> (current_token->data)};
std::replace (translated_token.begin (), translated_token.end (), '/', '-');
token_accounts_info tokenInfo{};
auto path = std::string{IMAP_FRAME_BAYES "-"} + translated_token;
qof_instance_foreach_slot_prefix (QOF_INSTANCE (imap->acc), path, &build_token_info, tokenInfo);
@ -5331,7 +5333,7 @@ gnc_account_imap_find_account_bayes (GncImportMatchMap *imap, GList *tokens)
}
static void
change_imap_entry (GncImportMatchMap *imap, gchar *kvp_path, int64_t token_count)
change_imap_entry (GncImportMatchMap *imap, gchar const * kvp_path, int64_t token_count)
{
GValue value = G_VALUE_INIT;
@ -5377,7 +5379,7 @@ gnc_account_imap_add_account_bayes (GncImportMatchMap *imap,
{
GList *current_token;
gint64 token_count;
char *account_fullname, *kvp_path;
char *account_fullname;
char *guid_string;
ENTER(" ");
@ -5405,28 +5407,20 @@ gnc_account_imap_add_account_bayes (GncImportMatchMap *imap,
skip this case here. */
if (!current_token->data || (*((char*)current_token->data) == '\0'))
continue;
/* start off with one token for this account */
token_count = 1;
PINFO("adding token '%s'", (char*)current_token->data);
std::string translated_token {static_cast<char*>(current_token->data)};
std::replace(translated_token.begin(), translated_token.end(), '/', '-');
kvp_path = g_strdup_printf (IMAP_FRAME_BAYES "-%s-%s",
translated_token.c_str(),
guid_string);
std::string translated_token {static_cast <char*> (current_token->data)};
std::replace (translated_token.begin (), translated_token.end (), '/', '-');
auto path = std::string {IMAP_FRAME_BAYES} + '-' + translated_token + '-' + guid_string;
/* change the imap entry for the account */
change_imap_entry (imap, kvp_path, token_count);
g_free (kvp_path);
change_imap_entry (imap, path.c_str (), token_count);
}
/* free up the account fullname and guid string */
qof_instance_set_dirty (QOF_INSTANCE (imap->acc));
xaccAccountCommitEdit (imap->acc);
g_free (account_fullname);
g_free (guid_string);
LEAVE(" ");
}
@ -5655,68 +5649,48 @@ look_for_old_separator_descendants (Account *root, gchar const *full_name, const
return new_name;
}
static Account *
look_for_old_mapping (GncImapInfo *imapInfo)
static std::string
get_guid_from_account_name (Account * root, std::string const & name)
{
Account *root, *map_account = NULL;
const gchar *sep = gnc_get_account_separator_string ();
gchar *full_name;
PINFO("Category Head is '%s', Full Category is '%s'", imapInfo->category_head, imapInfo->full_category);
// do we have a map_account all ready, implying a guid string
if (imapInfo->map_account != NULL)
return NULL;
root = gnc_account_get_root (imapInfo->source_account);
full_name = g_strdup (imapInfo->full_category + strlen (imapInfo->category_head) + 1);
// may be top level or match with existing separator
map_account = gnc_account_lookup_by_full_name (root, full_name);
// do we have a valid account, if not, look for old separator
if (map_account == NULL)
auto map_account = gnc_account_lookup_by_full_name (root, name.c_str ());
if (!map_account)
{
gchar * temp_name = look_for_old_separator_descendants (root, full_name, sep);
g_free(full_name);
full_name = temp_name;
map_account = gnc_account_lookup_by_full_name (root, full_name); // lets try again
auto temp_account_name = look_for_old_separator_descendants (root, name.c_str (),
gnc_get_account_separator_string ());
map_account = gnc_account_lookup_by_full_name (root, temp_account_name);
g_free (temp_account_name);
}
auto temp_guid = gnc::GUID {*xaccAccountGetGUID (map_account)};
return temp_guid.to_string ();
}
PINFO("Full account name is '%s'", full_name);
g_free (full_name);
return map_account;
static std::pair <std::string, KvpValue*>
convert_entry (std::pair <std::vector <std::string>, KvpValue*> entry, Account* root)
{
auto const & account_name = entry.first.back();
entry.first.pop_back();
auto guid_str = get_guid_from_account_name (root, account_name);
entry.first.emplace_back ("/");
entry.first.emplace_back (guid_str);
std::string new_key {std::accumulate (entry.first.begin(), entry.first.end(), std::string {})};
new_key = IMAP_FRAME_BAYES + new_key;
std::replace (new_key.begin(), new_key.end(), '/', '-');
return {new_key, entry.second};
}
static std::vector<std::pair<std::string, KvpValue*>>
get_new_guid_imap (Account * acc)
{
auto frame = qof_instance_get_slots (QOF_INSTANCE (acc));
auto slot = frame->get_slot(IMAP_FRAME_BAYES);
auto slot = frame->get_slot (IMAP_FRAME_BAYES);
if (!slot)
return {};
std::string const imap_frame_str {IMAP_FRAME_BAYES};
std::vector<std::pair<std::string, KvpValue*>> ret;
auto imap_frame = slot->get<KvpFrame*> ();
auto flat_kvp = imap_frame->flatten_kvp ();
auto root = gnc_account_get_root (acc);
auto imap_frame = slot->get<KvpFrame*>();
imap_frame->for_each_slot_temp ([&ret, root, &imap_frame_str] (char const * token, KvpValue* val) {
auto token_frame = val->get<KvpFrame*>();
token_frame->for_each_slot_temp ([&ret, root, &imap_frame_str, token] (char const * account_name, KvpValue* val) {
auto map_account = gnc_account_lookup_by_full_name (root, account_name);
if (!map_account)
{
auto temp_account_name = look_for_old_separator_descendants (root, account_name, gnc_get_account_separator_string());
map_account = gnc_account_lookup_by_full_name (root, temp_account_name);
g_free (temp_account_name);
}
auto temp_guid = gnc::GUID{*xaccAccountGetGUID (map_account)};
auto guid_str = temp_guid.to_string();
ret.push_back({imap_frame_str + "-" + token + "-" + guid_str, val});
});
});
std::vector <std::pair <std::string, KvpValue*>> ret;
for (auto const & flat_entry : flat_kvp)
ret.emplace_back (convert_entry (flat_entry, root));
return ret;
}
@ -5737,7 +5711,6 @@ convert_imap_account_bayes_to_guid (Account *acc)
}
char const * run_once_key_to_guid {"changed-bayesian-to-guid"};
char const * run_once_key_to_flat {"changed-bayesian-to-flat"};
static void
imap_convert_bayes_to_guid (QofBook * book)
@ -5752,55 +5725,6 @@ imap_convert_bayes_to_guid (QofBook * book)
g_list_free (accts);
}
static std::vector<std::pair<std::string, KvpValue*>>
get_new_flat_imap (Account * acc)
{
auto frame = qof_instance_get_slots (QOF_INSTANCE (acc));
auto slot = frame->get_slot(IMAP_FRAME_BAYES);
if (!slot)
return {};
std::string const imap_frame_str {IMAP_FRAME_BAYES};
std::vector<std::pair<std::string, KvpValue*>> ret;
auto root = gnc_account_get_root (acc);
auto imap_frame = slot->get<KvpFrame*>();
imap_frame->for_each_slot_temp ([&ret, &imap_frame_str] (char const * token, KvpValue* val) {
auto token_frame = val->get<KvpFrame*>();
token_frame->for_each_slot_temp ([&ret, &imap_frame_str, token] (char const * account_guid, KvpValue* val) {
ret.push_back({imap_frame_str + "-" + token + "-" + account_guid, val});
});
});
return ret;
}
static void
convert_imap_account_bayes_to_flat (Account * acc)
{
auto flat_imap = get_new_flat_imap (acc);
if (!flat_imap.size())
return;
auto frame = qof_instance_get_slots (QOF_INSTANCE (acc));
xaccAccountBeginEdit(acc);
frame->set(IMAP_FRAME_BAYES, nullptr);
std::for_each(flat_imap.begin(), flat_imap.end(), [&frame] (std::pair<std::string, KvpValue*> const & entry) {
frame->set(entry.first.c_str(), entry.second);
});
qof_instance_set_dirty (QOF_INSTANCE (acc));
xaccAccountCommitEdit(acc);
}
static void
imap_convert_bayes_to_flat (QofBook * book)
{
auto root = gnc_book_get_root_account (book);
auto accts = gnc_account_get_descendants_sorted (root);
for (auto ptr = accts; ptr; ptr = g_list_next (ptr))
{
Account *acc = static_cast <Account*> (ptr->data);
convert_imap_account_bayes_to_flat (acc);
}
g_list_free (accts);
}
static bool
run_once_key_set (char const * key, QofBook * book)
{
@ -5818,22 +5742,13 @@ set_run_once_key (char const * key, QofBook * book)
qof_instance_set_kvp(QOF_INSTANCE (book), key, &value);
}
void
gnc_account_imap_convert_flat (QofBook *book)
{
if (run_once_key_set(run_once_key_to_flat, book))
return;
imap_convert_bayes_to_flat (book);
set_run_once_key(run_once_key_to_flat, book);
}
void
gnc_account_imap_convert_bayes (QofBook *book)
{
if (run_once_key_set(run_once_key_to_guid, book))
if (run_once_key_set (run_once_key_to_guid, book))
return;
imap_convert_bayes_to_guid(book);
set_run_once_key(run_once_key_to_guid, book);
imap_convert_bayes_to_guid (book);
set_run_once_key (run_once_key_to_guid, book);
}
/* ================================================================ */
@ -5843,7 +5758,6 @@ static void
gnc_account_book_end(QofBook* book)
{
Account *root_account = gnc_book_get_root_account(book);
xaccAccountBeginEdit(root_account);
xaccAccountDestroy(root_account);
}

View File

@ -1454,10 +1454,6 @@ void gnc_account_delete_map_entry (Account *acc, char *full_category, gboolean e
*/
void gnc_account_imap_convert_bayes (QofBook *book);
/** Change the bayes imap entries from a nested representation to a flat representation.
*/
void gnc_account_imap_convert_flat (QofBook *);
/** @} */

View File

@ -38,6 +38,7 @@ extern "C"
#include <sstream>
#include <algorithm>
#include <vector>
#include <numeric>
/* This static indicates the debugging module that this .o belongs to. */
static QofLogModule log_module = "qof.kvp";
@ -473,7 +474,7 @@ gnc_value_list_get_type (void)
}
void
KvpFrame::flatten_kvp_impl(std::vector<std::string> path, std::vector<std::pair<std::string, KvpValue*>> & entries) const
KvpFrame::flatten_kvp_impl(std::vector <std::string> path, std::vector <std::pair <std::vector <std::string>, KvpValue*>> & entries) const
{
for (auto const & entry : m_valuemap)
{
@ -486,16 +487,17 @@ KvpFrame::flatten_kvp_impl(std::vector<std::string> path, std::vector<std::pair<
}
else
{
std::string flat_path {std::accumulate(path.begin(), path.end(), std::string{})};
entries.emplace_back(flat_path + "/" + entry.first, entry.second);
std::vector <std::string> new_path {path};
new_path.emplace_back (entry.first);
entries.emplace_back (new_path, entry.second);
}
}
}
std::vector<std::pair<std::string, KvpValue*>>
std::vector <std::pair <std::vector <std::string>, KvpValue*>>
KvpFrame::flatten_kvp(void) const
{
std::vector<std::pair<std::string, KvpValue*>> ret;
std::vector <std::pair <std::vector <std::string>, KvpValue*>> ret;
flatten_kvp_impl({}, ret);
return ret;
}

View File

@ -213,6 +213,10 @@ struct KvpFrameImpl
*/
KvpValue* get_slot(Path keys) const noexcept;
/**
* proc is called with each of the immediate contents of this frame, passing it the key,
* value, and specified data.
*/
void for_each_slot(void (*proc)(const char *key, KvpValue *, void *data), void* data) const noexcept;
/** The function should be of the form:
@ -239,7 +243,7 @@ struct KvpFrameImpl
* Returns all keys and values of this frame recursively, flattening
* the frame-containing values.
*/
std::vector<std::pair<std::string, KvpValue*>>
std::vector <std::pair <std::vector <std::string>, KvpValue*>>
flatten_kvp(void) const;
/** Test for emptiness
@ -251,7 +255,7 @@ struct KvpFrameImpl
private:
map_type m_valuemap;
void flatten_kvp_impl(std::vector<std::string>, std::vector<std::pair<std::string, KvpValue*>> &) const;
void flatten_kvp_impl(std::vector <std::string>, std::vector <std::pair <std::vector <std::string>, KvpValue*>> &) const;
};
template<typename func_type>

View File

@ -318,7 +318,6 @@ TEST_F(ImapBayesTest, AddAccountBayes)
EXPECT_EQ(2, value->get<int64_t>());
}
TEST_F(ImapBayesTest, ConvertAccountBayes)
{
// prevent the embedded beginedit/committedit from doing anything
@ -326,18 +325,15 @@ TEST_F(ImapBayesTest, ConvertAccountBayes)
qof_instance_mark_clean(QOF_INSTANCE(t_bank_account));
gnc_account_imap_add_account_bayes(t_imap, t_list1, t_expense_account1); //Food
gnc_account_imap_add_account_bayes(t_imap, t_list2, t_expense_account2); //Drink
auto root = qof_instance_get_slots(QOF_INSTANCE(t_bank_account));
auto book = qof_instance_get_slots(QOF_INSTANCE(t_imap->book));
auto acct1_guid = guid_to_string (xaccAccountGetGUID(t_expense_account1)); //Food
auto acct2_guid = guid_to_string (xaccAccountGetGUID(t_expense_account2)); //Drink
auto acct3_guid = guid_to_string (xaccAccountGetGUID(t_asset_account2)); //Asset-Bank
auto acct4_guid = guid_to_string (xaccAccountGetGUID(t_sav_account)); //Sav Bank
auto val1 = new KvpValue(static_cast<int64_t>(10));
auto val2 = new KvpValue(static_cast<int64_t>(5));
auto val3 = new KvpValue(static_cast<int64_t>(2));
// Test for existing entries, all will be 1
auto value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + foo + "-" + acct1_guid).c_str());
EXPECT_EQ(1, value->get<int64_t>());
@ -347,84 +343,36 @@ TEST_F(ImapBayesTest, ConvertAccountBayes)
EXPECT_EQ(1, value->get<int64_t>());
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + waldo + "-" + acct2_guid).c_str());
EXPECT_EQ(1, value->get<int64_t>());
// Set up some old entries
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + pepper + "/" + "Asset-Bank").c_str(), val1);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/token/with/slashes/" + "Asset-Bank").c_str(), val1);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + salt + "/" + "Asset-Bank#Bank").c_str(), new KvpValue{*val1});
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + salt + "/" + "Asset>Bank#Bank").c_str(), val2);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + pork + "/" + "Expense#Food").c_str(), new KvpValue{*val2});
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + sausage + "/" + "Expense#Drink").c_str(), val3);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + foo + "/" + "Expense#Food").c_str(), new KvpValue{*val2});
EXPECT_EQ(1, qof_instance_get_editlevel(QOF_INSTANCE(t_bank_account)));
EXPECT_TRUE(qof_instance_get_dirty_flag(QOF_INSTANCE(t_bank_account)));
qof_instance_mark_clean(QOF_INSTANCE(t_bank_account));
// Start Convert
gnc_account_imap_convert_bayes (t_imap->book);
// convert from 'Asset-Bank' to 'Asset-Bank' guid
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + pepper + "-" + acct3_guid).c_str());
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-token-with-slashes-" + acct3_guid).c_str());
EXPECT_EQ(10, value->get<int64_t>());
// convert from 'Asset-Bank#Bank' to 'Sav Bank' guid
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + salt + "-" + acct4_guid).c_str());
EXPECT_EQ(10, value->get<int64_t>());
// convert from 'Expense#Food' to 'Food' guid
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + pork + "-" + acct1_guid).c_str());
EXPECT_EQ(5, value->get<int64_t>());
// convert from 'Expense#Drink' to 'Drink' guid
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + sausage + "-" + acct2_guid).c_str());
EXPECT_EQ(2, value->get<int64_t>());
// convert from 'Expense#Food' to 'Food' guid but add to original value
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + foo + "-" + acct1_guid).c_str());
EXPECT_EQ(5, value->get<int64_t>());
// Check for run once flag
auto vals = book->get_slot("changed-bayesian-to-guid");
EXPECT_STREQ("true", vals->get<const char*>());
EXPECT_EQ(1, qof_instance_get_editlevel(QOF_INSTANCE(t_bank_account)));
EXPECT_TRUE(qof_instance_get_dirty_flag(QOF_INSTANCE(t_bank_account)));
}
TEST_F (ImapBayesTest, convert_map_flat)
{
// prevent the embedded beginedit/committedit from doing anything
qof_instance_increase_editlevel(QOF_INSTANCE(t_bank_account));
qof_instance_mark_clean(QOF_INSTANCE(t_bank_account));
//gnc_account_imap_add_account_bayes(t_imap, t_list1, t_expense_account1); //Food
//gnc_account_imap_add_account_bayes(t_imap, t_list2, t_expense_account2); //Drink
auto root = qof_instance_get_slots(QOF_INSTANCE(t_bank_account));
auto acct1_guid = guid_to_string (xaccAccountGetGUID(t_expense_account1)); //Food
auto acct2_guid = guid_to_string (xaccAccountGetGUID(t_expense_account2)); //Drink
auto acct3_guid = guid_to_string (xaccAccountGetGUID(t_asset_account2)); //Asset-Bank
auto acct4_guid = guid_to_string (xaccAccountGetGUID(t_sav_account)); //Sav Bank
auto val1 = new KvpValue(static_cast<int64_t>(10));
auto val2 = new KvpValue(static_cast<int64_t>(5));
auto val3 = new KvpValue(static_cast<int64_t>(2));
// Set up some old entries
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + pepper + "/" + acct1_guid).c_str(), val1);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + salt + "/" + acct2_guid).c_str(), new KvpValue{*val1});
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + foo + "/" + acct3_guid).c_str(), val2);
root->set_path((std::string{IMAP_FRAME_BAYES} + "/" + pork + "/" + acct4_guid).c_str(), val3);
EXPECT_EQ(1, qof_instance_get_editlevel(QOF_INSTANCE(t_bank_account)));
qof_instance_mark_clean(QOF_INSTANCE(t_bank_account));
gnc_account_imap_convert_flat (t_imap->book);
auto value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + pepper + "-" + acct1_guid).c_str());
EXPECT_EQ(10, value->get<int64_t>());
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + salt + "-" + acct2_guid).c_str());
EXPECT_EQ(10, value->get<int64_t>());
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + foo + "-" + acct3_guid).c_str());
EXPECT_EQ(5, value->get<int64_t>());
value = root->get_slot((std::string{IMAP_FRAME_BAYES} + "-" + pork + "-" + acct4_guid).c_str());
EXPECT_EQ(2, value->get<int64_t>());
auto book = qof_instance_get_slots(QOF_INSTANCE(t_imap->book));
auto vals = book->get_slot("changed-bayesian-to-flat");
EXPECT_STREQ("true", vals->get<const char*>());
EXPECT_EQ(1, qof_instance_get_editlevel(QOF_INSTANCE(t_bank_account)));
EXPECT_TRUE(qof_instance_get_dirty_flag(QOF_INSTANCE(t_bank_account)));
}