flipp-oss / protoc-gen-avro

Generate Avro schemas from Protobuf files.
MIT License
0 stars 2 forks source link

Bug fixes / features #13

Open ikstewa opened 4 months ago

ikstewa commented 4 months ago

Hey @dorner !

I have a collection of new features and a few bug fixes we needed to add in order to leverage this for our use case. A lot of these are based of the goal of having the avro schemas match the similar behavior described in the proto json mapping.

Let me know if you wanted to merge any of these in!

Features

Bug Fixes

ikstewa commented 4 months ago

Here's one regarding the enums I am struggling with if you have any input.

Here's a sample WIP: https://github.com/ikstewa/protoc-gen-avro/pull/8

When referencing an enum from another message, the Ref ends up not aligning with the definition of the type:

message BleepEnum {
  enum Bleep {
    UNKOWN = 0;
    BLEEP_ME = 1;
    BLEEP_YOU = 2;
  }
}

message Foobar {
  ...
  BleepEnum.Bleep bleep = 18;
  BleepEnum.Bleep bleep_again = 19;
}
...
    {
      "name": "bleep",
      "type": {
        "type": "enum",
        "name": "Bleep",
        "symbols": [
          "UNKOWN",
          "BLEEP_ME",
          "BLEEP_YOU"
        ],
        "default": "UNKOWN"
      },
      "default": "UNKOWN"
    },
    {
      "name": "bleepAgain",
      "type": "testdata.BleepEnum.Bleep"
    }
...

Note: The type names don't match up as the Bleep type is name namespaced under BleepEnum

dorner commented 4 months ago

Hey @ikstewa - I'm more than happy to accept PRs with both features and bug fixes. Right now this tool is pretty niche, so I'm good to configure it as deeply as needed to get your use cases going.

What assistance do you need with the WIP?

ikstewa commented 4 months ago

What assistance do you need with the WIP?

I'm having a hard time figuring out how to best update the code to handle the naming of the type. The example from the WIP results in invalid avro as the type testdata.BleepEnum.Bleep is not found. When nesting the type initially the in the record it needs to either have "name": "BleepEnum.Bleep" or needs to be referenced as "type": "testdata.Bleep" instead.

I'm not certain how to best do that, as when toJSON is called on the enum object it doesn't have reference to the namespace where it's being written so it's not able to respect relative type namings.

dorner commented 4 months ago

Oof... it's been a while since I've had to dive into this. I wonder if there's a way to store the current nesting somewhere in the typeRepo so you can access it as you go into it?