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

Add API for querying JSON documents #356

Closed
yorickpeterse opened this issue Jan 23, 2023 · 11 comments
Closed

Add API for querying JSON documents #356

yorickpeterse opened this issue Jan 23, 2023 · 11 comments
Assignees
Labels
accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers feature New things to add to Inko, such as a new standard library module std Changes related to the standard library
Milestone

Comments

@yorickpeterse
Copy link
Collaborator

master introduces the std::json module, but querying it is a bit of a pain. For example, given this JSON document:

{ "config": { "value": 42 } }

You have to write something like this to extract 42:

match document {
  case Object(root) -> match root['config'] {
    case Object(config) -> match config['value'] {
      case Int(v) -> Option.Some(v)
      case _ -> Option.None
    case _ -> Option.None
  }
  case _ -> Option.None
}

Instead it would be nice if one could write something like this instead:

document.object('config').int('value') # => Option[Int]
@yorickpeterse yorickpeterse added feature New things to add to Inko, such as a new standard library module and removed type::feature labels Mar 15, 2023
@yorickpeterse yorickpeterse removed this from the 0.2.5 milestone Mar 15, 2023
@yorickpeterse yorickpeterse added the accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers label Mar 18, 2023
@a-lafrance
Copy link

I'm looking for something relatively easy to get started as a contributor so I'd be interested in taking this on.

In terms of exact interface, what I had in mind was to define methods to "downcast" a Json object into its variants:

fn pub object(key: String) -> Option[Map[String, Json]]
# And so on for all the other variants

Then your example would look something like this:

document.object('config').then(fn (obj) { return obj['value'].int() })

Which isn't quite as elegant as what you described, but seems like a decently intuitive solution, and at least a solid starting point for more discussion.

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance Thanks for looking into this! 😃 I think a first start would be to gather some more information on what other languages do, if anything at all. That should help us get a better understanding of what options we might want to consider.

In theory we could have query methods returning types like Object, Integer, etc. This allows chaining, and you can enforce type safety (e.g. calling object on an Integer doesn't make sense). So something like this:

root
  .object('config') # => Object
  .int('value')     # => Integer
  .get              # => resolve the query into its final value, returning Option[Int] in this case

The downside is that each query would have to allocate a new value, which I'd like to avoid if at all possible.

@a-lafrance
Copy link

a-lafrance commented May 22, 2023

I just looked at how serde does it in Rust, which seems like one of the closest analogs to Inko, so that might be a good source of inspiration.

The gist is that their JSON type has a .get method that does indexing into both objects and arrays (it's generic over the index type), which would correspond to .object() here. They also have .as_* methods that do the downcasting to each variant (e.g. .as_int for the example here).

So the example here would become root.get('config').get('value').as_int(). Personally the Rust way seems pretty intuitive and elegant, thoughts?

Edit: I should probably also note that .get() automatically returns None if the index is invalid (in any one of three possible ways). They also have an overload of the indexing operator that instead returns the JSON value Value::Null -- both of which seem like valid options, not sure which is better.

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance I like the Serde approach 😃, though we'd have to use separate methods as we don't support argument overloading. So I think we'd end up with key for accessing object indexes, and index for accessing array indexes. I think these methods should return a ref Json. At some point we could add key_opt and index_opt that return Option[Json], but I prefer waiting with that until the need arises. The methods key_mut and index_mut would return a mut Json.

This means that for something like this:

{
  "name": "Alice",
  "houses": [
    {
      "street": "Foo street",
      "number": 42,
    }
  ]
}

You'd write the query like so:

root.key('houses').index(0).key('number').to_int # => Option[Int]

@a-lafrance
Copy link

So serde actually has just one indexing method that works for both objects and arrays because it's generic over some special serde_json::Index trait which I assume encapsulates both behaviors. I'm not sure exactly how that trait works but I assume it's something we could emulate here if desired. Although to be honest I kind of like having those methods separate since it's more clear how you're interpreting the JSON value.

Anyway, not sure if you're interested into looking into that traits/generics approach or if proceeding with the key/index double method solution would be better

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance Separate methods would be better. If we use a single method with some sort of Index trait as the argument, we'd probably have to allocate an object to figure out what kind of index we're dealing with. Using separate methods this isn't needed. This is also why there's write_string and write_bytes, instead of a generic write method that supports both.

@a-lafrance
Copy link

Gotcha, makes sense. If we’re happy enough with the current interface we’ve outlined I can move forward with implementation.

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance That sounds like a good idea, so feel free to start on the changes 😃

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance Are you still looking into this, or is it OK for somebody else to pick this up if they want to?

@a-lafrance
Copy link

Hey, sorry for the inactivity — I’ve been on a variety of out of town errands/vacations since our initial discussion so I haven’t been able to implement it myself.

I’m planning on working on it when I’m back home in 2 weeks or so, but I don’t want to slow progress just because of my schedule, so if someone else wants to pick it up in the meantime, by all means they’re free to do so :)

@yorickpeterse
Copy link
Collaborator Author

@a-lafrance No problem, and no rush! 😃

@yorickpeterse yorickpeterse added this to the 0.14.0 milestone Dec 14, 2023
@yorickpeterse yorickpeterse self-assigned this Dec 15, 2023
yorickpeterse added a commit that referenced this issue Dec 16, 2023
TODO: write description here.

This fixes #356.

Changelog: added
yorickpeterse added a commit that referenced this issue Dec 16, 2023
TODO: write description here.

This fixes #356.

Changelog: added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers feature New things to add to Inko, such as a new standard library module std Changes related to the standard library
Projects
None yet
Development

No branches or pull requests

2 participants