nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.58k stars 1.47k forks source link

should json.to() respect parent attributes? #5856

Closed enthus1ast closed 6 years ago

enthus1ast commented 7 years ago
import json

type
  MsgBase = ref object of RootObj
    name*: string

  MsgChallenge = ref object of MsgBase
    challenge*: string

var resp = %* {"name": "challenge", "challenge": "asdf"}
echo repr resp.to(MsgChallenge)
ref 0032A088 --> [challenge = 0032A0A8"asdf",
name = nil]

Please note: the name is nil. I dont know what's the desired behavior but should it maybe also fill the parents attributes?

dom96 commented 7 years ago

Hrm, maybe, or at least it should be explicitly disallowed.

darkmusic commented 6 years ago

@loloiccl - Can you please create a PR for your fix for 5856? I have been using your fix successfully for a while, but it would be great if it could be considered for merging into the Nim devel branch. Thanks.

loloicci commented 6 years ago

@darkmusic @dom96 I have once created a PL but it has been reviewed that it is not suitable (https://github.com/nim-lang/Nim/pull/5879). Should I create another PL again?

darkmusic commented 6 years ago

@loloiccl - I see. I did not realize you already submitted a PR. I do not think it is worth creating another one unless we can convince @dom96 to change his mind on the desired behaviour. I would expect json.to() to produce an accurate representation of the object it is unmarshalling to, and I do not understand why inheritance is allowed by Nim types but ignored by json.to(). But there must be good reasons for this.

dom96 commented 6 years ago

Can you guys explain the use case for this? From what I am seeing of these examples you may as well just have one type with the two fields.

darkmusic commented 6 years ago

To understand my use case, there are two main applications for a website I am building. The first is the server app, which is a Jester server that acts as a REST API layer to interact with a database. The second is a web client app implemented in Karax and compiled using Nim's JavaScript target. For the server, I wanted to use Nim to define the DB model and perform all interactions with the database, so I would not have to write any SQL statements, and so there would be an abstraction that would enable me to switch to a different database provider with minimal effort. So I built an ORM which uses metadata attached via custom pragmas to types defined in Nim to code-generate SQL (Postgresql, currently) schema creation/drop statements, as well as Nim "methods" which allow for CRUD operations, as well as paging, filtering, sorting, and child collection retrieval. All model types inherit from a common type - "BaseDataObj", which has an "id" field defined. Since each model type corresponds to a database table, each model needs this field, which represents the table's primary key field.

type
  BaseDataObj* = ref object {.inheritable.}
    id* {.id.}: int

There are many model types defined, and they look something like this:

type
  Sentence* = ref object of BaseDataObj
    sentence* {.text, notnull, index_like.}: string
    definition* {.text, notnull, index_like.}: string
    kana* {.text, notnull.}: string
    notes* {.text.}: string
    image_id* {.foreignkey: image, index.}: int
    image* {.meta.}: Image

So for an example flow, the web client posts an ajax JSON request to the server, which contains something to the effect of "fetch me sentences with a certain criteria". The server then fetches the data from the database using the model types and their code-generated Nim procs. The server then performs any additional processing to the retrieved data, such as sorting. It then builds the JSON response, which it sends back to the client. The web client has access to all of the Nim model types, so can unmarshal the response directly to these types using json.to(). Everything works great, except json.to() doesn't understand what the "id" field is because it belongs to the base type. Since Nim supports type inheritence, it doesn't make any sense to have to manually define the "id" field on each model type, so I am currently using @loloiccl's fix to allow json.to() to populate the properties of base types. But I hope that this explains my use case enough and that you will consider adding this as a feature to json.to().