Canonical JSON mapping

[connesc]

According to this page, Proto3 defines a canonical JSON mapping. It would be awesome if pq had an option to be compliant.

Currently, the single difference I'm aware of is the name of fields, that should be converted to lowerCamelCase. Handling this would be a great step further, but maybe there are other subtle issues to address in order to be 100% spec-compliant.

[sevagh]

Thanks for pointing this out. Do I basically go to every key in the JSON output and just do some string manipulation to convert them to lowerCamelCase? e.g. python pseudocode: 'snake_case'.split('_')[1] + ''.join([x.capitalize() for x in 'snake_case'.split('_')[1:]])

[connesc]

Yes, sounds good to me ! 👍

[sevagh]

Here's my attempt: https://github.com/sevagh/pq/pull/75/files#diff-d3074ba1689c29fa340ca2ef02837dfaL37

There's a lot of rules but I made some good progress in making a section where we can define rules easily.

[sevagh]

I ran this in prod and it works well. Not sure if I want it to be default or toggleable with a flag. --compliant?

[connesc]

Wow, that was fast, thanks for your efforts! 👍

I have tested v1.0.1. It seems to work very well, but only at the root level, not for sub-objects.

Regarding the default behavior, my guess is that a toggleable flag is perfect to remain compatible while in v1.x (--canonical sounds better than --compliant IMHO).
However, it may also make sense to define the canonical mapping as default for the next major release, while still keeping a --legacy flag for current users.

[sevagh]

I like the idea to switch behaviors and add a legacy flag in the next major release.

For now I need to figure out a way to descend into the Serde::Value object to find every possible key to modify.

[sevagh]

I have tested v1.0.1. It seems to work very well, but only at the root level, not for sub-objects.

Actually maybe I'm misunderstanding the document but it seems to me that sub-objects are considered as map{"k": v} and thus don't need to be modified?

Message field names are mapped to lowerCamelCase and become JSON object keys.

Do message field names only apply to the root keys or even sub-objects? Are sub-objects messages or maps?

map<K,V> | object | {"k": v, …} | All keys are converted to strings.

[connesc]

From what I understand, map<K,V> is for free-style key-value maps, where the keys do not actually have to be converted.
The issue I encounter is when nesting messages like this:

syntax = "proto3";

message Parent {
        Child my_child = 1;
}

message Child {
        string foo_bar = 1;
}

If I decode 0x0A050A0362617A with --canonical, I get this:

{
  "myChild": {
    "foo_bar": "baz"
  }
}

Instead of this:

{
  "myChild": {
    "fooBar": "baz"
  }
}

[sevagh]

Thanks for the test case. I'll try some things.

[sevagh]

I think you're right about maps, this is what a real map looks like:

"myChild": [
    {
      "key": "foo_bar",
      "value": "baz"
    }
  ]

I created this proto to test it:

syntax = "proto3";

message ParentMap {
        map<string, string> my_child = 1;
}

So I think if I recursively check all map keys (where key != "key"), I can safely apply the transformation. The underlying Rust object that I traverse looks like this:

Nested proto case:

Map(
    {
        String(
            "my_child"
        ): Option(
            Some(
                Map(
                    {
                        String(
                            "foo_bar"
                        ): Option(
                            Some(
                                String(
                                    "baz"
                                )
                            )
                        )
                    }
                )
            )
        )
    }
)

Map case:

Map(
    {
        String(
            "my_child"
        ): Seq(
            [
                Map(
                    {
                        String(
                            "key"
                        ): Option(
                            Some(
                                String(
                                    "foo_bar"
                                )
                            )
                        ),
                        String(
                            "value"
                        ): Option(
                            Some(
                                String(
                                    "baz"
                                )
                            )
                        )
                    }
                )
            ]
        )
    }
)

[sevagh]

Tentative fix:

sevagh:python $ ./proto_encoder.py single nested | ../../target/debug/pq --fdsetfile ..
/fdsets/parent_child_nested.fdset --msgtype Parent --canonical
{
  "myChild": {
    "fooBar": "baz"
  }
}

[sevagh]

#76

[sevagh]

Wanna test this hotfix?
pq-bin.tar.gz

It uses rust nightly to recursively peak inside the JSON and modify all keys. 1.0.2

[connesc]

Well done, it works! 🎉
I'm not familiar with Rust, what are the implications of using a nightly? Is it stable enough to release this hotfix?

To be spec-compliant, I think that map<K,V> should be handled differently. With your ParentMap message, I would expect 0x0A0E0A07666F6F5F626172120362617A to be decoded like this (keys are only converted to strings, no case transformation):

{
  "myChild": {
    "foo_bar": "baz"
  }
}

PS: if I understand correctly, what your are currently doing is a post-processing of the deserialized message. So, it's probably difficult to reliably distinguish a map<K,V> from a message. Maybe you can find inspirations in the official Java implementation.