ponylang / json

A JSON library for Pony
https://ponylang.io/
BSD 2-Clause "Simplified" License
4 stars 1 forks source link

Pony JSON - difficult to modify `iso` JsonDoc #16

Open sgebbie opened 2 years ago

sgebbie commented 2 years ago

Currently json.JsonDoc provides accessors via the data field. However, if one has an iso reference to a JsonDoc that has already been populated, then it is difficult to modify the data after the fact.

Background

For example, the following code will work (where a ref alias is recovered):

be dostuff(jdoc: JsonDoc iso) =>
    let jdoc': JsonDoc ref = recover ref consume jdoc end
    try
        (jdoc'.data as JsonObject).data("seq") = I64(123)
    end

But then jdoc' is no longer iso.

One could replace jdoc.data altogether:

    let jom = recover iso Map[String, JsonType] end
    jom("seq") = I64(123)
    jdoc.data = recover iso JsonObject.from_map(consume jom) end

But then the full map would need to be recreated.

Proposal

Based on feedback from Borja o'Cook we could instead introduce "getters" into JsonDoc and JsonObject (and JsonArray) such that viewpoint adaptation could be applied to help achieve the desired access to the data while satisfying the reference capability constraints.

class JsonDoc
  // ....
  fun get_data(): this->JsonType! =>
    data

// ...

class JsonObject
  // ...
  fun get_data(): this->Map[String, JsonType]! =>
    data

// ---

actor Main
  let _env: Env

  new create(env: Env) =>
    _env = env
    try
      let json_string = "{\"key\":\"value\"}"
      let sendable_doc: JsonDoc iso = recover iso JsonDoc.>parse(json_string)? end
      this.dostuff(consume sendable_doc)
    end

  be dostuff(jdoc: JsonDoc iso) =>
    try
      let new_doc: JsonDoc iso = recover
        let doc_ref = consume ref jdoc
        let obj: JsonObject ref = doc_ref.get_data() as JsonObject
        let data = obj.get_data()
        data("seq") = I64(123)
        doc_ref
      end
      this.morestuff(consume new_doc)
    end

  be morestuff(jdoc: JsonDoc iso) =>
   _env.out.print(jdoc.string(where indent="  ", pretty_print=true))
sgebbie commented 2 years ago

In terms of naming, we could go with: get_data, which is not my favourite, but makes sense. Unfortunately, we can't use data() for backwards compatibility. We could also consider payload() or simply d().

Another option that may work well is apply() so as to leverage the syntactic sugar.

sgebbie commented 2 years ago

I've started putting together the following PR:

which allows one the write the following:

be dostuff(jdoc: JsonDoc iso) =>
  _seq = _seq + 1 // update the sequence
  let jdoc': JsonDoc iso = recover
    let jdoc_ref = consume ref jdoc
    try
      let m: Map[String, JsonType] ref = (jdoc_ref() as JsonObject)()
      m("seq") = _seq.i64()
    end
    jdoc_ref
end
ergl commented 2 years ago

@sgebbie Since this changes the standard library, I think this will be required to go through the RFC process.

sgebbie commented 2 years ago

@sgebbie Since this changes the standard library, I think this will be required to go through the RFC process.

Cool, that makes sense. I am just going to add a unit test to my PR and then I will take a look at the RFC documentation.

sgebbie commented 2 years ago

I've started drafting and RFC for this change:

I'll send through a PR once I've completed the documentation.

ergl commented 2 years ago

@sgebbie Cool. One point to consider is that since pony is pre-1.0, we can be more relaxed about breaking changes. In this case, I'd at least write down the option of making the data field private, or of calling the getter data. You can put these alternatives on the "Alternatives" section of the RFC.

sgebbie commented 2 years ago

I've posted an RFC for this proposed change:

jasoncarr0 commented 2 years ago

I'm not going to suggest that we should hold this up for it, but I'm marking this as another use case that is solved very easily by recover blocks with receiver https://github.com/ponylang/rfcs/pull/182

sgebbie commented 2 years ago

Following the Pony sync 2021-11-23 it seems like the issue in the JSON API has more to do with the difference in how the compiler is handling field access versus how it is handling method access (with viewpoint adaption).

The suggestion was to rather file a new bug report that isolates an example of this difference in behaviour.