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

Problem With getCurrentJsonPointer() #105

Open
XedinUnknown opened this issue Sep 13, 2023 · 4 comments
Open

Problem With getCurrentJsonPointer() #105

XedinUnknown opened this issue Sep 13, 2023 · 4 comments

Comments

@XedinUnknown
Copy link
Contributor

XedinUnknown commented Sep 13, 2023

Hi!

The Problem

  • My Decoder::decode(StreamInterface $stream): iterable layer is configured with JSON pointers for use with JSON Machine. In the subject case, it is configured with an all-matching empty string pointer, but could be configured with e.g. JSON pointers /items and /total.
  • I have a SelectResult implements Traversable layer, which wraps an iterable Items collection, and yields certain (perhaps decorated) items from an /items path, while also remembering the scalar value of /total path.
  • For the sake of loose coupling, SelectResult only knows that the decoded collection is iterable at the moment; it has no awareness of the Items type. I'd like to keep it that way. For the same reason, SelectResult also should not care what pointers the Decoder was configured with; only that the keys it needs are present in the iterable.
  • I am wrapping Items in a Generator that will yield the value of Items::getCurrentJsonPointer() instead of the original key. Since the actual current (not matched) path is unique for any value in the document, this will not yield different values for the same key.
  • This elegantly solves my problem, because the SelectResult can work on the values based on their key now, without knowing of the Items interface. In this example, the SelectResult can yield values from the keys that match e.g. /items/(\d*), while memoizing the value of the /total key. Also forward-compatible with your announced future plans to provide content as a stream without having already consumed it.
  • Except for some reason, the value of getCurrentJsonPointer() seems to always be empty. I have also used xDebug to inspect the values at runtime, and it seems that the value of Parser#currentPath is never set.

Questions

  1. Could this be a bug?
  2. Is there a better way to achieve this?
  3. If not, would you consider changing the default behaviour of yielded keys to be the full path?

Thank you!

@XedinUnknown
Copy link
Contributor Author

Seems to be connected to $iteratorLevel, which depends on the configured JSON pointers: no pointers - no path.

I don't know the codebase well, and so I wonder: perhaps, the $currentLevel needs simply to be greater than 0, and not depend on $iteratorLevel at all?

@XedinUnknown
Copy link
Contributor Author

Digging further, I understand why you probably wouldn't want the iterable to yield full paths as keys: each structure, including the top-most one, should probably have original keys, such that when converted to an array with e.g. iterator_to_array() it retains its key structure.
Also, it's trivial to wrap this into a generator that yields getCurrentJsonPointer() instead of keys, if this is what's needed.

So, currently the only problem with this is that apparently, when not specifying pointers, there's never any current path/pointer to retrieve. To my mind, the relationship between pointers and the path should not exist at all: the path should be available regardless of absence or presence of any pointers, because each value in a JSON document has a path.

@XedinUnknown
Copy link
Contributor Author

By the way, here's what I mean by wrapping Items in a Generator:

        // For each item, yield its full path instead of just the key
        return call_user_func_array(function (Items $data): Generator {
            foreach ($data as $key => $value) {
                $path = $data->getCurrentJsonPointer();
                // Path may be empty if all-matching pointer '' was specified
                // https://github.com/halaxa/json-machine/issues/105
                $newKey = !empty($path)
                    // Remove root prefix to make working with key easier, especially if top-level
                    ? ltrim($path, '/')
                    : $key;
                yield $newKey => $value;
            }
        }, [$data]);

@halaxa
Copy link
Owner

halaxa commented Nov 18, 2023

Tests of getCurrentJsonPointer() seem to be ok. Can you look at them and check your code for possible flaws?

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

No branches or pull requests

2 participants