Import of OFX files with many securities opens too many matching dialogs

Because ofx import is currently split per target account, and since each security has its own accounts, importing such OFX is a tedious process.
The fix is to only split the transactions if we identify a potential transfer, currently based on amount, date and accounts.
To do that, we insert transactions one by one into a list, making sure we have not already inserted one that has the same date, and the same absolute amount. If we have, we keep this potential transfer for a second phase.
A naive approach would loop through added transactions for each new transaction by that ends up being O(N^2), which matters if we have many transactions. Instead, I'm using a hash to make this O(N log N).
This commit is contained in:
jean 2021-10-13 09:29:51 -07:00
parent 003b0a5deb
commit b95df85121

View File

@ -31,6 +31,7 @@
#include <string.h>
#include <sys/time.h>
#include <math.h>
#include <inttypes.h>
#include <libofx/libofx.h>
#include "import-account-matcher.h"
@ -1179,32 +1180,69 @@ reconcile_when_close_toggled_cb (GtkToggleButton *togglebutton, ofx_info* info)
info->run_reconcile = gtk_toggle_button_get_active (togglebutton);
}
static gchar* make_date_amount_key (time64 date, gnc_numeric amount)
{
// Create a string that combines date and amount, we'll use that for our hash
gchar buf[64];
gnc_numeric _amount = gnc_numeric_reduce(amount);
g_snprintf (buf, sizeof(buf), "%" PRId64 "%" PRId64 "%" PRId64, _amount.num , _amount.denom, date);
return g_strdup (buf);
}
static void
runMatcher(ofx_info* info, char * selected_filename, gboolean go_to_next_file)
runMatcher (ofx_info* info, char * selected_filename, gboolean go_to_next_file)
{
GtkWindow *parent = info->parent;
GList* trans_list_remain = NULL;
Account* first_account = NULL;
/* If we have multiple accounts in the ofx file, we need to
* process transactions one account at a time, in case there are
* transfers between accounts.
* avoid processing transfers between accounts together because this will
* create duplicate entries.
*/
GHashTable* trans_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, NULL);
info->num_trans_processed = 0;
// Add transactions, but verify that there isn't one that was already added with identical
// amounts and date, and a different account. To do that, create a hash table whose key is
// a hash of amount and date, and whose value is the account in which they appear.
for(GList* node = info->trans_list; node; node=node->next)
{
Transaction* trans = node->data;
Split* split = xaccTransGetSplit (trans, 0);
if (first_account == NULL) first_account = xaccSplitGetAccount (split);
if (xaccSplitGetAccount (split) == first_account)
Account* account = xaccSplitGetAccount (split);
gchar *date_amount_key = make_date_amount_key (xaccTransGetDate (trans),
gnc_numeric_abs (xaccSplitGetAmount (split)));
// Test if date_amount_key is already in trans_hash.
Account* _account = g_hash_table_lookup (trans_hash, date_amount_key);
if (_account && _account != account)
{
// There is a transaction with identical amounts and
// dates, but a different account. That's a potential
// transfer so process this transaction in a later call.
gchar *name1 = gnc_account_get_full_name (account);
gchar *name2 = gnc_account_get_full_name (_account);
gchar *amtstr = gnc_numeric_to_string (xaccSplitGetAmount (split));
gchar *datestr = qof_print_date (xaccTransGetDate (trans));
DEBUG ("Potential transfer %s %s %s %s\n", name1, name2, amtstr, datestr);
g_free (name1);
g_free (name2);
g_free (amtstr);
g_free (datestr);
trans_list_remain = g_list_prepend (trans_list_remain, trans);
g_free (date_amount_key);
}
else
{
g_hash_table_insert (trans_hash, date_amount_key, account);
gnc_gen_trans_list_add_trans (info->gnc_ofx_importer_gui, trans);
info->num_trans_processed ++;
}
else trans_list_remain = g_list_prepend (trans_list_remain, trans);
}
g_list_free (info->trans_list);
g_hash_table_destroy (trans_hash);
info->trans_list = g_list_reverse (trans_list_remain);
DEBUG("%d transactions remaining to process in file %s\n", g_list_length (info->trans_list), selected_filename);
// See whether the view has anything in it and warn the user if not.
if (gnc_gen_trans_list_empty (info->gnc_ofx_importer_gui))
@ -1212,10 +1250,8 @@ runMatcher(ofx_info* info, char * selected_filename, gboolean go_to_next_file)
gnc_gen_trans_list_delete (info->gnc_ofx_importer_gui);
if (info->num_trans_processed)
{
gchar* acct_name = gnc_get_account_name_for_register (first_account);
gnc_info_dialog (parent, _("While importing transactions from OFX file '%s' into account '%s', found %d previously imported transactions, no new transactions."),
selected_filename, acct_name, info->num_trans_processed);
g_free (acct_name);
gnc_info_dialog (parent, _("While importing transactions from OFX file '%s' found %d previously imported transactions, no new transactions."),
selected_filename, info->num_trans_processed);
// This is required to ensure we don't mistakenly assume the user canceled.
info->response = GTK_RESPONSE_OK;
gnc_ofx_match_done (NULL,info);