json-machine icon indicating copy to clipboard operation
json-machine copied to clipboard

Problem With `getCurrentJsonPointer()`

Open XedinUnknown opened this issue 2 years ago • 4 comments

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 avatar Sep 13 '23 18:09 XedinUnknown

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 avatar Sep 13 '23 19:09 XedinUnknown

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 avatar Sep 14 '23 11:09 XedinUnknown

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]);

XedinUnknown avatar Sep 14 '23 17:09 XedinUnknown

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

halaxa avatar Nov 18 '23 20:11 halaxa