ScalaPB icon indicating copy to clipboard operation
ScalaPB copied to clipboard

FieldOptions.fromJavaProto does not include unknownFields

Open silverbeak opened this issue 4 months ago • 0 comments

Hello.

Quick note: First-time poster here, and I've read the contribution guidelines, but I may still miss a few details. If so, apologies in advance, any and all guidance is welcome.

Context: I've created a custom DynamicMessage type, similar to the one in google's library. In order to instantiate this message type, I'm reading a serialized in the following fashion:

//** Serializing side **//
val descriptor = HelloMessage.scalaDescriptor.file
val serialized = com.google.protobuf.descriptor.FileDescriptorProto.toByteArray(descriptor.asProto)


//** De-serializing side **/
// The javaFileDescriptorProto contains the proper options/extensions
val javaFileDescriptorProto = com.google.protobuf.DescriptorProtos.FileDescriptorProto.parseFrom(serialized)

// But the scalaFileDescriptorProto does not contain the options/extensions...
val javaFileDescriptor = FileDescriptorProto.fromJavaProto(javaFileDescriptorProto)

// ...and subsequently, the resulting fileDescriptor is also missing the options/extensions
val fileDescriptor = FileDescriptor.buildFrom(javaFileDescriptor, Nil)

Problem The code above will compile and run fine, but when I inspect the resulting object I'm missing data in the following location.

fileDescriptor -> messages[msgIndex] -> fields[fieldIndex] -> asProto -> options

In practice, this means that my decorator extensions will not be properly de-serialized.

Solution? I noticed that unknownFields are not handled here. So I had a bit of a tinker and wrote a rough implementation of what it is I would like to achieve. The implementation can be seen here and here.

There are no tests (not sure how/where to do that yet) and the current implementation is only for scalajvm (don't know if it can/should be implemented for other environments).

Wrap-up So, I hope someone can help me by answering the following:

  1. Is this an actual bug? I.e, should it be fixed in the first place, or is this intentional?
  2. If "yes", then is this the right place to make such an implementation?
  3. If "yes", then should I create tests for this, and where?

I think that's all for now. Thanks!

silverbeak avatar Jun 11 '25 14:06 silverbeak