breaking: Issue-112 Fix Default Nested Protobuf Types

[MikeDalby]

Fixes #112

This change accounts for the fact that the protobufjs Type class extends the Namespace class. When traversing down the Namespaces we should stop on the first message Type instance, not just the first Namespace without an inner/nested Namespace.

[MikeDalby]

Open Question: Is it enough to just check that it's not a Type and has a nested object?

if(!(reflection instanceof Type) && reflection.nested)

[haraldkubota]

I tried the patch as per @MikeDalby and it helped me for a nested ProtoBuf. Went from

  decodedValue: MealItems { itemName: 'pizza' }

to a much better

  decodedValue: Meal {
    item: [],
    drink: [ [DrinkItems], [DrinkItems] ],
    name: 'pizza'
  }

Producer was this Python snippet:

mybeer = meal_pb2.Meal.DrinkItems(drink_name="beer")
mywine = meal_pb2.Meal.DrinkItems(drink_name="wine")

meal = meal_pb2.Meal(name='pizza', drink=[mybeer,mywine])
# Less meal (for testing kafkajs which seems to miss the drinks
#meal = meal_pb2.Meal(name='pizza', drink=[])

producer.produce(topic=topic, key=str(uuid4()), value=meal)

While I'm missing the "beer" and "wine", at least I got drinks now.

[Nevon]

Let's test that this works as intended. There's a test file called something like newapi that has a few different protobuf schemas, but none with nested message types. You could extend that suite with a test checks that this finds the correct message type and can encode/decode with it.

[MikeDalby]

@Nevon I've added some tests based on the existing ones. Feel free to let me know if there's anything else you'd like to see here.

[Nevon]

Excellent! Thanks for the contribution.

One thing that would be really helpful is if you could provide a short summary of how this change affects Protobuf users and what they need to do in order to maintain backwards compatibility (I guess explicitly provide a message type?), so I can add that in the release notes.

[MikeDalby]

@Nevon How does something like this sound:

When creating an instance of SchemaRegistry for Protobuf without the messageName parameter confluent-schema-registry would, under certain circumstances, default to the wrong message type in the schema. Specifically, instead of defaulting to the first message type in the schema it would erroneously default to the first message type that did not define a nested type.

If you were relying on this behavior may need to either:

• Start passing the messageName parameter instead of relying on the default behavior
• Update your Schema's and re-ingest messages accordingly

See Issue #112 for more info

[MikeDalby]

@Nevon Just curious what the release process looks like / if there's a release schedule. Thanks!

[MikeDalby]

@Nevon Is there anything I can do to help get a release published?

[Nevon]

I've just had a lot going on both professionally and personally, so I haven't gotten around to it. Managed to get v3.0.0 out just now. Sorry about the delay.

[MikeDalby]

@Nevon I totally understand and sorry for pestering you. Thank you for all of your hard work!