Rollback workflow DB change on errors field

[m4dcoder]

Rollback workflow DB change on errors field because there are cases where action execution result is included in the detail of the error log and the action execution result can contain dictionary with key that has dot notation such as IP address. Also identified and fix mongoescape bug on handling list of dicts in dynamic field and also multiple iteration on unescape.

[blag]

Rolls back part of #4854.

[blag]

It seems like we could have more escaping and unescaping tests in st2common/tests/unit/test_mongoescape.py, but if you feel that your tests adequately cover all of the usecases, then that's fine.

The other thing to consider is that the code in test_mongoescape.py would be easier to debug than all of this.

[m4dcoder]

I can add my test case to st2common/tests/unit/test_mongoescape.py. But i'll keep my specific test for the workflow engine. The basic tests in test_mongoescape.py didn't catch the use case for the workflow engine and definitely didn't catch the bug in the unescape where it's translating twice but not passing the translated value from one _translate_chars to the second _translate_chars. Also, the second _translate_chars is actually redundant and adds performance overhead so I actually consolidate the two calls into one.

[blag]

The second call isn't redundant, it just very much looks like it is. The _translate_chars function mutates its field parameter, which is why calling it twice like that didn't break the tests.

Still very weird and overdue for a refactoring like this.

I believe this code is also an excellent candidate to be implemented with single dispatch:

from functools import singledispatch

# ...


@singledispatch
def _translate_chars(field, translation):
    return field


@_translate_chars.register(list)
def _translate_chars_in_list(field, translation):
    return [_translate_chars(value, translation) for value in field]


def _translate_chars_in_key(key, translation):
    for k, v in [(k, v) for k, v in six.iteritems(translation) if k in key]:
        key = key.replace(k, v)
    return key


@_translate_chars.register(dict)
def _translate_chars_in_dict:
    return {
        _translate_chars_in_key(k, translation): _translate_chars(v, translation)
        for k, v in six.iteritems(field)
    }


def escape_chars(field):
    return _translate_chars(fast_deepcopy(field), ESCAPE_TRANSLATION)


def unescape_chars(field):
    return _translate_chars(fast_deepcopy(field), UNESCAPE_TRANSLATION)

[blag]

@m4dcoder For your consideration: #4894.

[nmaludy]

Thinking about trying to improve performance. Also, probably good to benchmark and see what method wins.

[blag]

As of commit dbf78d4, this PR should be good to go. It is the fastest of the four options (for loop vs. list comprehension and explicit switch vs. single dispatch decorator).

[nmaludy]

LGTM!