From 340d8c783ef7aa3be07b76f3e410235a89ffbec0 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Wed, 6 Feb 2013 17:54:54 +0100 Subject: [PATCH 1/4] [FIX] Sepa line in normal abnamro tab/txt import file will not crash on any key. There were some tests for length of items and known keys that were not effective, because we have to support values with embedded slashes. Therefore the code has been streamlined to make this clear. The sepa dictionary is extended will new information, but as yet this information in not processed in other parts of the code. --- account_banking_nl_abnamro/abnamro.py | 55 +++++++++++++++++++-------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/account_banking_nl_abnamro/abnamro.py b/account_banking_nl_abnamro/abnamro.py index 8e951f22f..280ed1c82 100644 --- a/account_banking_nl_abnamro/abnamro.py +++ b/account_banking_nl_abnamro/abnamro.py @@ -155,28 +155,51 @@ class transaction(models.mem_bank_transaction): The string consists of slash separated KEY/VALUE pairs, but the slash is allowed to and known to occur in VALUE as well! """ + other_keys = [ + 'ADDR', 'BIC', 'CPRP', 'CREF', 'CSID', 'ISDT', 'MARF', 'NRTX', + 'NRTXR', 'PREF', 'PURP', 'REFOB', 'RREF', 'RTYP', 'SVCL', + 'SWOD' + ] + # Patch keys with embedded double slashes, but first check, + # for performance reasons, wether this problem might occur at all. + if '//' in field: + patch_double_slash_keys = [ + '/BENM//ID/', '/ORDP//ID/', '/ORDP//RID/', '/ORIG//CSID/', + '/ORIG//MARF/', '/ULTD//NAME/', '/ULTD//ID/', + '/ULTB//NAME/', '/ULTB//ID/' + ] + for pdsk in patch_double_slash_keys: + psdk_key = pdsk.replace('//', '~~') + field = field.replace(pdsk, psdk_key) + other_keys.append(psdk_key.replace('/', '')) items = field[1:].split('/') # skip leading slash sepa_dict = {} - prev_key = False known_keys = ['TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', 'SWOC', 'REMI', ] + # There should be at least two items, a key and a value, + # and the first item should be a valid key. (But it will be, as + # we only get here when field starts with /TRTP !!) + if len(items) < 2: + raise osv.except_osv( + _('Error !'), + _('unable to parse SEPA string: %s - %s') % + (field, _('too few items'))) + sepa_key = items[0] + if not (sepa_key in known_keys or sepa_key in other_keys): + raise osv.except_osv( + _('Error !'), + _('unable to parse SEPA string: %s - %s') % + (field, _('First key %s unknown') % sepa_key)) + sepa_values = None while items: - if len(items) == 1: - raise osv.except_osv( - _('Error !'), - _("unable to parse SEPA string: %s") % field) - key = items.pop(0) - if key not in known_keys: - # either an unknown key or a value containing a slash - if prev_key: - sepa_dict[prev_key] = sepa_dict[prev_key] + '/' + key - else: - raise osv.except_osv( - _('Error !'), - _("unable to parse SEPA string: %s") % field) + item = items.pop(0) + if item in known_keys or item in other_keys: + if not sepa_values is None: + sepa_dict[sepa_key] = '/'.join(sepa_values) + sepa_key = item + sepa_values = [] else: - sepa_dict[key] = items.pop(0).strip() - prev_key = key + sepa_values.append(item.strip()) return sepa_dict def parse_type(field): From 924d0e0fefce027d918cc8bf2085a00e69bc16e6 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Wed, 6 Feb 2013 23:48:01 +0100 Subject: [PATCH 2/4] [FIX] Improved on bug fix in previous revision, to not forget the last value in the sepa dictionary. Go through all items in sepa line in old school JSP nested loop. --- account_banking_nl_abnamro/abnamro.py | 75 +++++++++++++-------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/account_banking_nl_abnamro/abnamro.py b/account_banking_nl_abnamro/abnamro.py index 280ed1c82..df14ef272 100644 --- a/account_banking_nl_abnamro/abnamro.py +++ b/account_banking_nl_abnamro/abnamro.py @@ -155,51 +155,46 @@ class transaction(models.mem_bank_transaction): The string consists of slash separated KEY/VALUE pairs, but the slash is allowed to and known to occur in VALUE as well! """ - other_keys = [ + items = field[1:].split('/') # skip leading slash + assert len(items) > 1, ( + _('unable to parse SEPA string: %s - %s') % + (field, _('too few items'))) + known_keys = [ + 'TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', 'SWOC', 'REMI', 'ADDR', 'BIC', 'CPRP', 'CREF', 'CSID', 'ISDT', 'MARF', 'NRTX', 'NRTXR', 'PREF', 'PURP', 'REFOB', 'RREF', 'RTYP', 'SVCL', 'SWOD' ] - # Patch keys with embedded double slashes, but first check, - # for performance reasons, wether this problem might occur at all. - if '//' in field: - patch_double_slash_keys = [ - '/BENM//ID/', '/ORDP//ID/', '/ORDP//RID/', '/ORIG//CSID/', - '/ORIG//MARF/', '/ULTD//NAME/', '/ULTD//ID/', - '/ULTB//NAME/', '/ULTB//ID/' - ] - for pdsk in patch_double_slash_keys: - psdk_key = pdsk.replace('//', '~~') - field = field.replace(pdsk, psdk_key) - other_keys.append(psdk_key.replace('/', '')) - items = field[1:].split('/') # skip leading slash + # Xtended keys have an extra part, separated from the first part + # with a double slash. For example: 'ORIG//CSID'. + # The double slash will make that the first part in items is + # followed by an empty string item, and then the other part of the + # key. + extended_keys = [ + 'BENM', 'ORDP', 'ORIG', 'ULTD', 'ULTB' + ] + known_keys.extend(extended_keys) + assert items[0] in known_keys, ( + _('unable to parse SEPA string: %s - %s') % + (field, _('First key unknown %s') % items[0])) sepa_dict = {} - known_keys = ['TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', - 'SWOC', 'REMI', ] - # There should be at least two items, a key and a value, - # and the first item should be a valid key. (But it will be, as - # we only get here when field starts with /TRTP !!) - if len(items) < 2: - raise osv.except_osv( - _('Error !'), - _('unable to parse SEPA string: %s - %s') % - (field, _('too few items'))) - sepa_key = items[0] - if not (sepa_key in known_keys or sepa_key in other_keys): - raise osv.except_osv( - _('Error !'), - _('unable to parse SEPA string: %s - %s') % - (field, _('First key %s unknown') % sepa_key)) - sepa_values = None - while items: - item = items.pop(0) - if item in known_keys or item in other_keys: - if not sepa_values is None: - sepa_dict[sepa_key] = '/'.join(sepa_values) - sepa_key = item - sepa_values = [] - else: - sepa_values.append(item.strip()) + item_index = 0 + items_len = len(items) + while item_index < items_len: + sepa_key = items[item_index] + sepa_values = [] + if sepa_key in extended_keys: + item_index += 2 + assert item_index < items_len, ( + _('SEPA key %s missing extention') % sepa_key) + # For the moment no test on validity of extension + sepa_key = '//'.join([sepa_key, items[item_index]]) + item_index += 1 + while (item_index < items_len + and items[item_index] not in known_keys): + sepa_values.append(items[item_index].strip()) + item_index += 1 + sepa_dict[sepa_key] = '/'.join(sepa_values) return sepa_dict def parse_type(field): From cc722f9f1fdfee08a6271a2c088d7cd30bfb0e77 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Fri, 8 Feb 2013 10:33:21 +0100 Subject: [PATCH 3/4] [FIX] Updated parsing of abnamro sepa string to handle even more unlikely cases. --- account_banking_nl_abnamro/abnamro.py | 81 +++++++++++++++------------ 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/account_banking_nl_abnamro/abnamro.py b/account_banking_nl_abnamro/abnamro.py index df14ef272..89e5fba66 100644 --- a/account_banking_nl_abnamro/abnamro.py +++ b/account_banking_nl_abnamro/abnamro.py @@ -155,46 +155,53 @@ class transaction(models.mem_bank_transaction): The string consists of slash separated KEY/VALUE pairs, but the slash is allowed to and known to occur in VALUE as well! """ - items = field[1:].split('/') # skip leading slash - assert len(items) > 1, ( - _('unable to parse SEPA string: %s - %s') % - (field, _('too few items'))) - known_keys = [ - 'TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', 'SWOC', 'REMI', - 'ADDR', 'BIC', 'CPRP', 'CREF', 'CSID', 'ISDT', 'MARF', 'NRTX', - 'NRTXR', 'PREF', 'PURP', 'REFOB', 'RREF', 'RTYP', 'SVCL', - 'SWOD' - ] - # Xtended keys have an extra part, separated from the first part - # with a double slash. For example: 'ORIG//CSID'. - # The double slash will make that the first part in items is - # followed by an empty string item, and then the other part of the - # key. - extended_keys = [ - 'BENM', 'ORDP', 'ORIG', 'ULTD', 'ULTB' - ] - known_keys.extend(extended_keys) - assert items[0] in known_keys, ( - _('unable to parse SEPA string: %s - %s') % - (field, _('First key unknown %s') % items[0])) + def _sepa_message(field, reason): + return _( + 'unable to parse SEPA string: %s - %s' % (field, reason)) + + def _get_next_key(items, start): + '''Find next key, starting from start, returns the key found, + the start position in the array and the end position + 1''' + known_keys = [ + 'TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', 'SWOC', + 'REMI', 'ADDR', 'CPRP', 'CREF', 'CSID', 'ISDT', 'MARF', + 'NRTX', 'NRTXR', 'PREF', 'PURP', 'REFOB', 'RREF', 'RTYP', + 'SVCL', 'SWOD', 'BENM//ID', 'ORDP//ID', 'ORDP//RID', + 'ORIG//CSID', 'ORIG//MARF', 'ULTD//NAME', 'ULTD//ID', + 'ULTB//NAME', 'ULTB//ID' + ] + items_len = len(items) + si = start + # Search until start after end of items + while si < items_len: + ei = si + 1 + while ei < items_len: + key = '/'.join(items[si:ei]) + if key in known_keys: + return (key, si, ei) + ei += 1 + si += 1 + return False + + items = field[1:].split('/') + assert len(items) > 1, _sepa_message(field, _('too few items')) sepa_dict = {} item_index = 0 items_len = len(items) - while item_index < items_len: - sepa_key = items[item_index] - sepa_values = [] - if sepa_key in extended_keys: - item_index += 2 - assert item_index < items_len, ( - _('SEPA key %s missing extention') % sepa_key) - # For the moment no test on validity of extension - sepa_key = '//'.join([sepa_key, items[item_index]]) - item_index += 1 - while (item_index < items_len - and items[item_index] not in known_keys): - sepa_values.append(items[item_index].strip()) - item_index += 1 - sepa_dict[sepa_key] = '/'.join(sepa_values) + key_info = _get_next_key(items, item_index) + assert key_info, _sepa_message( + field, _('no key found for start %d') % item_index) + assert key_info[1] == 0, _sepa_message( + field, _('invalid data found before key %s') % key_info[0]) + while key_info: + sepa_key = key_info[0] + item_index = key_info[2] + # Find where next key - if any - starts + key_info = _get_next_key(items, item_index) + ve = (key_info and key_info[1]) or items_len + sepa_value = ( + (ve > item_index) and '/'.join(items[item_index:ve])) or '' + sepa_dict[sepa_key] = sepa_value return sepa_dict def parse_type(field): From 6bf75273f8b54227f7dcfe67911f1df14c298cb9 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Wed, 17 Apr 2013 16:24:33 +0200 Subject: [PATCH 4/4] [FIX] More meaningfull names for index variable names. --- account_banking_nl_abnamro/abnamro.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/account_banking_nl_abnamro/abnamro.py b/account_banking_nl_abnamro/abnamro.py index 89e5fba66..f191ea335 100644 --- a/account_banking_nl_abnamro/abnamro.py +++ b/account_banking_nl_abnamro/abnamro.py @@ -171,16 +171,16 @@ class transaction(models.mem_bank_transaction): 'ULTB//NAME', 'ULTB//ID' ] items_len = len(items) - si = start + start_index = start # Search until start after end of items - while si < items_len: - ei = si + 1 - while ei < items_len: - key = '/'.join(items[si:ei]) + while start_index < items_len: + end_index = start_index + 1 + while end_index < items_len: + key = '/'.join(items[start_index:end_index]) if key in known_keys: - return (key, si, ei) - ei += 1 - si += 1 + return (key, start_index, end_index) + end_index += 1 + start_index += 1 return False items = field[1:].split('/') @@ -198,9 +198,11 @@ class transaction(models.mem_bank_transaction): item_index = key_info[2] # Find where next key - if any - starts key_info = _get_next_key(items, item_index) - ve = (key_info and key_info[1]) or items_len + value_end_index = (key_info and key_info[1]) or items_len sepa_value = ( - (ve > item_index) and '/'.join(items[item_index:ve])) or '' + ((value_end_index > item_index) + and '/'.join(items[item_index:value_end_index])) + or '') sepa_dict[sepa_key] = sepa_value return sepa_dict