ruby-protocol-buffers icon indicating copy to clipboard operation
ruby-protocol-buffers copied to clipboard

Rename Message#fields

Open macat opened this issue 12 years ago • 3 comments
trafficstars

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.

macat avatar May 05 '13 02:05 macat

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.

codekitchen avatar May 06 '13 23:05 codekitchen

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.

bufdev avatar Aug 01 '13 11:08 bufdev

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.

codekitchen avatar Aug 01 '13 20:08 codekitchen