descriptor bytes and fully qualified name helpers

[nerdrew]

• add the descriptor proto bytes for each generated proto file
• add a helper for the fully qualified name of each proto.

[nerdrew]

This has failing specs for the functional code generation specs. Those specs seem to rely on the generated message from a pretty old version of protoc. The failure output is also pretty hard to read. Thoughts on refactoring the tests?

I'd like to remove the bytes comparisons and instead require the generated files and check some of the fields, check encode -> decode roundtrip, check the custom options work as expected.

[nerdrew]

e.g., I'm thinking of tests like this:

  before(:all) do
    require PROTOS_PATH.join('google_unittest.pb')
    require PROTOS_PATH.join('map-test.pb')
    require PROTOS_PATH.join('google_unittest_custom_options.pb')
  end

  it "generates code for google's unittest.proto" do
    attrs = {
      optional_int32: 1,
      repeated_string: %w[boom],
      optional_nested_message: { bb: 10 },
      optional_nested_enum: ::Protobuf_unittest::TestAllTypes::NestedEnum::BAZ,
      optional_foreign_message: { c: 12 },
      repeated_import_message: [{ d: 13 }],
    }
    bytes = ::Protobuf_unittest::TestAllTypes.new(attrs).encode
    message = ::Protobuf_unittest::TestAllTypes.decode(bytes)
    expect(message.to_hash).to eq(attrs)
    expect(message.default_int32).to eq(41)
    expect(::Protobuf_unittest::TestAllTypes::FULLY_QUALIFIED_NAME).to eq('protobuf_unittest.TestAllTypes')
    descriptor_file = ::Protobuf_unittest.descriptor_set.file.first
    expect(descriptor_file.name).to eq('protos/google_unittest.proto')
    expect(descriptor_file.package).to eq('protobuf_unittest')
  end

  it "generates code for google's unittest.proto extensions" do
    attrs = { optional_import_message_extension: { d: 14 } }
    bytes = ::Protobuf_unittest::TestAllExtensions.new(attrs).encode
    message = ::Protobuf_unittest::TestAllExtensions.decode(bytes)
    expect(message.to_hash).to eq(attrs)
    expect(message.default_int64_extension).to eq(42)
  end

[nerdrew]

cc @embark

[dsimmsatsquare]

I am completely biased, but I would love to have something like this! Thanks @nerdrew !

[nerdrew]

The tests currently pass, but rubocop fails. I can update rubocop if this PR otherwise looks good. Thoughts on dropping support for EOL-ed ruby versions and updating rubocop to a modern version? (Both would be separate PRs).

[nerdrew]

Related to: #345

[nerdrew]

Any thoughts on this?

[dsimmsatsquare]

Any thoughts on this?

I for one love these changes to include the descriptors and type names! The functional specs I am less help with, but would help if you need.

[nerdrew]

We've been dog fooding this for years now. Any objection to a rebase + merge? cc @film42

[film42]

@nerdrew I'm down to rebase and merge. Y'all have been great with your previous patches. A few questions:

1. Any breaking changes with this?
2. Is there a reason for printing the proto bytes?

[nerdrew]

• Any breaking changes with this?

I don't think there are any breaking changes. We added constants all over with the fully qualified name for messages, enums, etc.

• Is there a reason for printing the proto bytes?

I.e. why'd we add the proto descriptor bytes after __END__ in these files? We have some dynamic systems that don't know what the proto message it will handle looks like, so you send in a descriptor proto along with the serialized message. The service can then dynamically generate code to deserialize the proto. By packaging the descriptor + serialized bytes, the combo becomes "self describing" in a way.

[nerdrew]

I'll rebase and cleanup soon.

[film42]

Thanks @nerdrew ! Makes sense about the descriptor being printed. Only nit would be how it's added to the files themselves. Could this be something we add after calling a dsl method like:

class SomeMessage < ::ProtobufMessage
  descriptor_bytes "<base64_encoded_message>"
end

where descriptor_bytes sets up the descriptors method calls?

More specifically, the instance variable checking as emitted code doesn't look particularly great for something that's supposed to be a human readable DSL.