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

Allow traversal of all keywords, not only those implementing JSONPather #106

Open
dan-j opened this issue Aug 4, 2021 · 2 comments
Open

Comments

@dan-j
Copy link

dan-j commented Aug 4, 2021

What feature or capability would you like?

Implement https://github.com/qri-io/jsonpointer WalkJSON() api by adding JSONProps() map[string]interface{} methods to all relevant keywords.

At the moment there is no API to traverse parsed schemas down to keywords with scalar values, because scalar keywords do not implement JSONPather. This makes sense, because a keyword like Description shouldn't have a JSONProp(name string) interface{} method.

Do you have a proposed solution?

Adding a new method JSONProps() map[string]interface{} which returns all keywords would support this use-case, whilst JSONChildren() can maintain it's current functionality and only return keywords which are JSONPathers.

Have you considered any alternative solutions or workarounds?

Implement an exported function WalkJSON() which works similarly to https://github.com/qri-io/jsonpointer, and uses the s.keywords slice internally so that all children are traversed. However, for my specific use-case, I have a problem with the current implementation of WalkJSON() because I also need to know the path being traversed, i.e. when visit is called, I need to know which element in the JSON I'm visiting, i.e. /properties/myProperty/type.

Please link any related issues.

I initially created the following issue to add support for using their WalkJSON() function. However, if you look at my second comment, it's not really appropriate: qri-io/jsonpointer#10

@dan-j
Copy link
Author

dan-j commented Aug 5, 2021

I've just pulled the code locally and started making a few changes to support this. I've changed the JSONChildren() map[string]JSONPather functions to JSONChildren() map[string]interface{} just to see how this affected the tests and the only issue is traversal_test.go.

=== RUN   TestReferenceTraversal
    traversal_test.go:66: expected 14 elements, got: 29
    traversal_test.go:99: case 0: expected 2 elements, got: 3

I'm not sure what these tests are actually proving, so I don't know if this is something which can just be updated in the assertions, or if a bug actually has been introduced. Any ideas?

@dan-j
Copy link
Author

dan-j commented Aug 10, 2021

Just following this up, is there a contributor around who could comment?

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

1 participant