Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with assignment in case root element is a list. #109

Merged
merged 2 commits into from
Sep 20, 2023

Conversation

rolling-robot
Copy link

This is a fix for #108

@michaelmior
Copy link
Collaborator

@rolling-robot Would you be able to add a test for this new case?

@rolling-robot
Copy link
Author

Here you go.
An error message with older clean algorithm

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ jsonpath_ng/jsonpath.py:295: in update_or_create
    return _clean_list_keys(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
dict_ = [{'foo': 1}, {'bar': 2, 'foo': 42}]

    def _clean_list_keys(dict_):
        """
        Replace {LIST_KEY: ['foo', 'bar']} with ['foo', 'bar'].
    
        >>> _clean_list_keys({LIST_KEY: ['foo', 'bar']})
        ['foo', 'bar']
    
        """
>       for key, value in dict_.items():
E       AttributeError: 'list' object has no attribute 'items'

jsonpath_ng/jsonpath.py:809: AttributeError

@rolling-robot
Copy link
Author

Older version implicitly suggests that root element is a dict, but it may not.

@rolling-robot
Copy link
Author

rolling-robot commented Sep 16, 2023

BTW, upon looking at original code now, a year later, I started to doubt: although everything works, this may be a wrong fix for this bug.
It may be intended by original author of this code that library-internal representation for list is dictionary with special structure {LIST_KEY: [.......]}, so zero-th level in the tree MUST be a dict type, but somehow _create_list_key() is not run for 0-th level in the first place, so bug manifests as an exception in _clean_list_keys . I did not dig into implementation and philosophy of structure representation of jsonpath-ng that deep.

@rolling-robot
Copy link
Author

rolling-robot commented Sep 16, 2023

No, it seems like the bugfix is safe and does not break conventions.
_create_list_key() is used twice in the whole repo, and both times its argument is an empty dictionary. So IMHO LIST_KEY hack looks like a generic way to allow assignment or access to uninitialized or empty element be it either a list, or a dict in more or less uniform way.

@michaelmior michaelmior merged commit ac28e1d into h2non:master Sep 20, 2023
@michaelmior
Copy link
Collaborator

@rolling-robot Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants