Skip to content

Conversation

macat
Copy link

@macat macat commented May 5, 2013

I'm implementing a client library for Mozilla Heka, and their message.proto contains a fields field to embed more Field message type. Although I don't agree with their decision, the ruby client should support that format.

When the Message message type compiled by ruby-protocol-buffers tries to initiate itself with the fields field in it, runs into an infinite loop. I know, the whole thing seems ridiculous.

My first proposal is to add a message_ prefix to fields. If you have a better naming convention, I'll change this.

@codekitchen
Copy link
Owner

Hmm this doesn't really seem like a general solution to me. What happens when somebody tries to use a .proto message that contains field named message_fields? That said, I don't really have a better suggestion, short of moving every single Message method off the class into some other non-conflicting namespace. Maybe that is the only solution. I'll have to think on it.

@bufdev
Copy link

bufdev commented Aug 1, 2013

This will be a breaking change for everyone else, no? Something should be done though, this will come up again. And protocol buffers allows use of keywords for field names, ie you can do:

optional string message = 1;

So there's no obvious solution. I think it would be cleaner to move non-field methods to a non-conflicting namespace.

@codekitchen
Copy link
Owner

I'm leaning towards putting everything in a non-conflicting namespace, but retaining the current methods on Message as aliases to that namespace, for backwards compatibility. However, we'd change precedence.

For example in a message:

class Foo < ProtocolBuffers::Message
  repeated :string, :fields, 1
end

Calling foo.fields would return the value of the fields attribute, rather than hitting a loop or otherwise breaking. But on other messages that don't have a fields attribute, it'd still return the metadata as it does today.

This is still not strictly backwards compatible, as some generic code could see the change in precedence when reflecting on messages that override methods with fields of the same name. However, that same generic code isn't able to properly handle such messages today, so it seems like an acceptable compromise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants