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

JSON library - Unmarshalling json, which contains "_key" key #10718

Open
MikhailMS opened this issue Feb 20, 2019 · 24 comments
Open

JSON library - Unmarshalling json, which contains "_key" key #10718

MikhailMS opened this issue Feb 20, 2019 · 24 comments

Comments

@MikhailMS
Copy link

Recently started to learn the Nim language and at the moment facing an interesting use case.

I am building a small app to monitor my Elasticsearch cluster and would love to unmarshall the resposnes I get back from ES. However, the response JSON may contain something like

{
	"_nodes": {
		"total": 1,
		"successful": 1,
		"failed": 0
	},
        ...Long response continues here
}

which so far seems like impossible to unmarshall, because I cannot do something like

type 
  Response = object
    _nodes: whatever


let response = to(parseJson(..), Response)

as it raises error

Error: invalid token: _ (\95)

I believe it would be hard to allow the ussage of _ at the beginning of the variable names (seems like limitation of the language at the moment), so maybe there could be a way to ignore specific keys from JSON strings?

I am pretty sure I can work around of it and stop using unmarshlling feature (to macros) of json library, but it sounds a bit sad :/

Nim Compiler Version 0.19.4
Was it working in the previous Nim releases? - N\A
A link to a related issue or discussion. - haven't found any

PS: if it is desired/designed behaviour of the library, than I'm terribly sorry for disturbing you :)

@haltcase
Copy link
Contributor

haltcase commented Feb 20, 2019

You should be able to escape the identifier by wrapping it in backticks:

type 
  Response = object
    `_nodes`: whatever

This goes for anywhere you bind/use a name by the way:

let `for` = "not a keyword right now"
echo `for`
# -> not a keyword right now

@krux02
Copy link
Contributor

krux02 commented Feb 20, 2019

This seems to be a solved question now.

@krux02 krux02 closed this as completed Feb 20, 2019
@haltcase
Copy link
Contributor

Hmm actually I just tested and while the backticks work for things like keywords it doesn't allow a leading underscore. Whether it's intended to work or not is outside my wheelhouse 😆

@MikhailMS
Copy link
Author

@citycide thank you for checking that - I could have only tested it tomorrow. Sad it doesn't work for leading underscore :/ Maybe someone can confirm that this is intended behaviour

@krux02
Copy link
Contributor

krux02 commented Feb 20, 2019

sorry I am reopening then.

@dom96
Copy link
Contributor

dom96 commented Feb 20, 2019

Two options:

  • Allow field name customisation using a pragma

  • Allow backtick quoting of non-keywords in Nim

@krux02
Copy link
Contributor

krux02 commented Feb 20, 2019

Third option: skip leading traling _ in field names when trying to map them to object members.

@MikhailMS
Copy link
Author

So, from the sound of the proposals, I believe it won't be fixed for some time? Therefore in the meantime, the only workaround I see is not to use to macro and build types from JSON manually

@dom96
Copy link
Contributor

dom96 commented Feb 20, 2019

Yes, but you can use a hybrid approach. For example to(parseJson(...)["_nodes"], Nodes)

@MikhailMS
Copy link
Author

Will try it out tomorrow then. Thank you guys for the advices :) Hope I could help to resolve this issue

@Vindaar
Copy link
Contributor

Vindaar commented Feb 20, 2019

  • Allow backtick quoting of non-keywords in Nim

This was already rejected here: #9014.

And just to clarify, the fact that this is not allowed is indeed the intended behavior. See the grammar for Nim identifiers here: https://nim-lang.github.io/Nim/manual.html#lexical-analysis-identifiers-amp-keywords.

Of the three proposals above field name customization via a pragma to me seems like the most versatile solution, while just skipping leading _ seems somewhat arbitrary in general (although a nice solution for this specific case).

@krux02
Copy link
Contributor

krux02 commented Feb 21, 2019

Of the three proposals above field name customization via a pragma to me seems like the most versatile solution

Only if you own the implementation of the type. You can't add pragmas to types you don't own.

Another solution is to allow custom overloads in the json serializer.

@MikhailMS
Copy link
Author

Hello there

Just went through the propositions once again and my thoughts are

  1. just skipping leading **_** seems somewhat arbitrary in general (although a nice solution for this specific case) may not always work. Even in my use case, ElasticSearch response contains both _nodes and nodes key, so ignoring leading underscores will confuse those 2 keys when we do map them.
  2. Custom overloads maybe a really nice feature to have, as it allows more flexibility.
  3. @krux02 what do you mean by own the implementation of the type? Haven't yet discovered this feature (pragmas) of the language, but looks like I should do so :)

@krux02
Copy link
Contributor

krux02 commented Feb 21, 2019

@MikhailMS

you can tag fields of types with custom pragmas, for example:

template myProperties(someValue: int) {.pragma.}

type
  MyType = object
    field1: int
    field2 {.myProperties(someValue = 123).}: string

Then from macros you can use these pragmas and their value to do some arbitrary logic on it, for example specify the name how it should be serialized when serialized ot json. But the downside is that this way you won't be able to change anything that you don't own. You cannot add a property to type where the implementation is out of your scope.

@LemonBoy
Copy link
Contributor

You'd probably be interested in this package.

@MikhailMS
Copy link
Author

@LemonBoy looks interesting and will nicely fit in. Btw, why is it archived? Would it be supported?

@Araq
Copy link
Member

Araq commented Feb 21, 2019

We will add support for custom names via field {.jsonName: "_field".}: type for the to macro.

@MikhailMS
Copy link
Author

Would be a great addition, @Araq :) Glad that this issue has triggered some good propositions/ideas and would make Nim language better

@Phillips126
Copy link

@Araq Interested to hear if there has been a fix for this or an alternative approach? I am using ArangoDB as a database and the data contains a _key, _id, and _rev. Thanks, love the language!

@krux02
Copy link
Contributor

krux02 commented Oct 9, 2019

Well you can also preprocess your input:

let newInput = inputStr.multiReplace(
  ("\"_nodes\":", "\"nodes\":"),
  ("\"_some\":", "\"some\":"),
  ("\"_more\":", "\"more\":"),
  ("\"_names\":", "\"names\":")
)
let myResponse = to(parseJson(newInput), Response)

@peheje
Copy link

peheje commented Apr 13, 2020

krux02: Don't you risk messing with the data however improbable (or probable) that might be?

Pragmas {.jsonName: "fieldName".} solution OK. Polluting definition a bit? More pagmas to remember, need to know they are being considered by to.
Deserialization options decoubled from deserialization step.
Flexible but you need to repeat yourself.
Can there be more than 1 .jsonName per field (in case of multiple data sources)?
If "I just want to remove all _ from incoming data before deserializing" should I need to decorate all fields?

Propose to take an optional proc(property: string): string for preprocessing. Seems open for extension and closes need for further modification.

proc test(x: string, call: proc(property: string): string): string =
    return call(x)
let hue = test("_hue", x => x.strip(chars={'_'}))
echo hue

proc fromMap(x: string): string =
    let map = { "_id": "id", "$id": "id", "_rev": "revision"}.toTable
    if x in map:
        return map[x]
    else:
        return x
let rev = test("_rev", fromMap)
echo rev

Map could be obtained of off the type definition or global or per api you are calling, up to you during runtime.

@krux02
Copy link
Contributor

krux02 commented Apr 13, 2020

@peheje yes preprocessing the data like I suggested is always a risk. You might screw it up. In this particular case however it could work, as the replacement string contains the quotation marks and colon of the replaced text.

Regarding {.jsonName: "fieldName".}. I have the same concerns about it as you do. But to implement it #11526 needs to be merged first. @Araq you asked for an explanation of what that PR does. Here is a use case for it. The explanation is still the same though, it allows to use custom pragma annotations from the macro system (which is currently not possible).

Propose to take an optional proc(property: string): string for preprocessing. Seems open for extension and closes need for further modification.

I like the idea, I just don't like any implementations that I can currently think of right now.

@metagn
Copy link
Collaborator

metagn commented Sep 26, 2020

The following is a (although ugly) workaround for a field name starting with _ that wasn't mentioned:

type 
  Response = object
    `"_nodes"`: whatever

@mfreeman451
Copy link

this is riddiculous.. running into the same issue, this shouldn't be this hard. about to give up on nim.

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