protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Rust Type and Required Messages

Open yordis opened this issue 8 months ago • 11 comments

As requested by @esrauchg, moving the conversation here:
https://github.com/tokio-rs/prost/issues/1031#issuecomment-2932003751.

Although this originated in a prost discussion, the underlying issue concerns how the official protobuf toolchain treats field presence. The intent here is to clarify the expected stance and behavior of the generator.

What language does this apply to?

Primarily Rust, but also interested in Go and TypeScript as they relate to protobuf code generation.

Describe the problem you are trying to solve.

The generated types are structurally imprecise: all fields are wrapped in Option<T>, even those marked as required (explicitly or implicitly) in the proto definition.

This leads to:

  • Verbose boilerplate (Some(...) wrappers)
  • Increased cognitive overhead
  • Risk of runtime errors when unwrapping values that should be guaranteed
fn calculate_minimum_bid_increment(cmd: &StartAuctionRun) -> Result<MoneyAmount, Error> {
    match &cmd.minimum_bid_increment_policy {
        Some(policy) => match &policy.policy {
            Some(minimum_bid_increment_policy::Policy::Fixed(fixed_policy)) => {
              //...
            }
            // ...
            // NOTE: this should never happen, it is required
            None => Err(Error::MinimumBidIncrementPolicyRequired),
        },
        // NOTE: this should never happen, it is required
        None => Err(Error::MinimumBidIncrementPolicyRequired),
    }
}

Despite the domain clearly requiring this field, the type system does not enforce it. Structurally speaking.

We're using protobuf as the canonical schema for all:

  • Commands
  • Events
  • Aggregate Snapshot

That applies across multiple language runtimes (via WASM modules) and is critical for:

  • Schema evolution and change detection
  • Consistent interop across services
  • Serialization correctness
  • Avoiding runtime reflection or manual encoders/decoders

We treat protobuf-generated types as the source of truth and only validate commands post-deserialization (or via protovalidate extension).

Here is the existing callbacks (thus far):

pub type InitialState<State> = fn() -> State;
pub type IsTerminal<State> = fn(state: State) -> bool;
pub type Decide<State, Command, Event, Error> =
    fn(state: &State, command: Command) -> Result<Decision<State, Command, Event, Error>, Error>;
pub type Evolve<State, Event> = fn(state: &State, event: Event) -> State;

pub type GetStreamId<Command> = fn(Command) -> String;
pub type IsOrigin<Event> = fn(Event) -> bool;
pub type GetEventID<Event> = fn(Event) ->String;

Describe the solution you'd like

The code generator should respect the required field modifier in .proto definitions, emitting non-optional Rust fields where appropriate.

That would:

  • Better align with schema intent
  • Eliminate unnecessary Option<T> wrappers
  • Improve safety and ergonomics

Describe alternatives you've considered

Creating application-level copies of protobuf types, but such types have very little value in our context. The distintion between application and serialization matters better little to us, especially when protobuffer files can not break change.

Additional context

  • https://github.com/tokio-rs/prost/pull/1286
  • https://github.com/tokio-rs/prost/issues/1031

yordis avatar Jun 02 '25 19:06 yordis

For anyone else coming across this, much of the conversation happened on the mailing list: https://groups.google.com/g/protobuf/c/4W-lFPqGnWM

Continuing from the last reply there:

Personally, I do not mind the builder pattern or dealing with getters/setters; if that is the answer, I am OK with it, my primary concern is when things are combined and I can construct broken messages (bypassing builder, and setters) and the builder pattern.

Builder pattern can be nice for some things, but from a design level we can't really take first-class concepts that demand builder pattern, actually most implementations of Protobuf don't use builder pattern. Even if we decided that was deeply regretted choice (which it is not currently considered so), it would be far beyond the scope of breaking change we could ever consider doing to move e.g. C++ and Py Protobuf implementations onto builder patterns.

Without sidetracking too much, the biggest pain in the Go version is that everything is a pointer;

I believe this is discussing one of the main pain point relating to the classic Go Proto API that justified creating the newer opaque API instead (https://go.dev/blog/protobuf-opaque). You can see here how you can enable the opaque API.

Purely speaking about Event Sourcing so my events are immutable, never breaking change messages.

I think this topic relates to using Protobuf as an FFI layer, where there's definitively no schema skew because both sides of the communication are shipped atomically. This is a design that works (and Google uses Protobuf this way internally), but its really just not a usecase that Protobuf considers a primary design goal, rather "schemas that will change over time within a distributed system" is the primary usecase. There are other technologies that do prioritize the "schema never changes" or "there can never be schema skew because both sides are shipped together" usecases more like Flatbuffers, CapnProto, and any FFI Bindings library which will handle this case much better.

What do you recommend me to do here? I need to find a way to test the v4 asap, I feel it is prudent to move to it

Flagging that Rust-v4 is only in beta, it is currently published to crates.io here https://crates.io/crates/protobuf/4.31.1-release with an example for how to set it up here: https://crates.io/crates/protobuf_example and some other supporting information has been published here https://protobuf.dev/reference/rust/

We are interested in any early feedback on that package, though there are a number of design choices that may not be seen as ideal from developers who are coming from Prost (which follows a very KISS model, and can achieve nice Rusty ergonomics, with a number of other tradeoffs that involve downsides more relevant to large corporate users compared to smaller users, including that "adding a new field to a preexisting message must not be a breaking change" is extremely important for Google's business needs, but may realistically be less important for how other people use protobuf).

esrauchg avatar Jun 04 '25 13:06 esrauchg

I believe this is discussing one of the main pain point relating to the classic Go Proto API...

Just wanted to share that I had provided through a private email some Deep Lore™ about the Go proto API. Your links are great for explaining the current API shift, but I saw the implied context in mentioning the “scalars are always pointers” pain points and the of the required keyword, as suggesting they could benefit from background on how we addressed many of the proto2 pointer pain points with the shift to the outgoing proto3 “scalars are scalars” design, which itself lead to a whole new constellation of pain points, that we are now addressing with the links you helpfully provided.

So, just FYI, anyone on the thread, I already shared some catchup context on Golang so that hopefully they’re prepared to understand the reasoning and arguments in the links explaining and justifying the shift to an Opaque API .

puellanivis avatar Jun 04 '25 14:06 puellanivis

I believe this is discussing one of the main pain point relating to the classic Go Proto API that justified creating the newer opaque API instead (https://go.dev/blog/protobuf-opaque). You can see here how you can enable the opaque API.

I didn't know about it 😱 TIL, thank you so much for sharing it.

I think this topic relates to using Protobuf as an FFI layer, where there's definitively no schema skew because both sides of the communication are shipped atomically.

That is not our case, by the way. We expect Event Processors (and sometimes Command Handlers) to skew over time, as long as nothing breaks. This is why we are motivated to make it 100% protobuf-based, since "schemas that will change over time within a distributed system" resonate with us as the primary use case.


I will ensure everything works in the beta version and adjust the code to be based on the opaque API using the Go version. I apologize for any inconvenience and appreciate your patience and support.

yordis avatar Jun 04 '25 18:06 yordis

Here are my thoughts so far, for what they're worth, in the Go version, I am unsure if I should not share it here since this was originally for Rust, please let me know, my intent is to document the journey, and hopefully the next person sees something worth in it.

Building Messages

Previously, a significant advantage was the absence of variables, which meant naming wasn't a major concern. Most of our code involved returning an inline constructed data structure.

func startAuction(state *aggregatev1.Aggregate, command *cmdv1.StartAuctionRun) (trogondecider.Decision, error) {
	return trogondecider.Emit(&eventv1.Event{
		Event: &eventv1.Event_AuctionStarted{
			AuctionStarted: &eventv1.AuctionStarted{
				RunId:                     command.RunId,
				StartedAt:                 command.StartedAt,
				InitialPrice:              command.InitialPrice,
				// more fields here
			},
		},
	}), nil
}

With the new version, we have to create many variables. The setter isn't returning the same data type, so we can not "chain" calls:

func startAuction(state *aggregatev1.Aggregate, command *cmdv1.StartAuctionRun) (trogondecider.Decision, error) {
	event := &eventv1.Event{}

	started := &eventv1.AuctionStarted{}
	started.SetRunId(command.GetRunId())
	started.SetStartedAt(command.GetStartedAt())
	started.SetInitialPrice(command.GetInitialPrice())
        // more fields here

	event.SetAuctionStarted(started)

	return trogondecider.Emit(event), nil
}

Reading https://protobuf.dev/reference/go/opaque-faq/ I see I could mimic the previous behavior to some extent using the _builder.

Do you think it would be possible to either return the data type in the Set* or add a With* (eg, WithRunI) functions as well?

Something like:

&eventv1.Event{}.WithAuctionStarted(
		&eventv1.AuctionStarted{}.
			WithRunId(command.GetRunId()).
			WithStartedAt(command.GetStartedAt()).
			WithInitialPrice(command.GetInitialPrice())
	)

So that developers aren't tripping up with naming and creating variables and the order of the call of or initialization and so on. I am leaning towards disabling builders so we do not have 2 ways to deal with the problems 🤔

yordis avatar Jun 04 '25 20:06 yordis

btw the readme from https://crates.io/crates/protobuf_example says

The easiest way to get ahold of protoc is to download a prebuilt binary from the matching release here. Just make sure protoc is on your $PATH when you run cargo.

But I couldn't find the release for it, I do see the tag: https://github.com/protocolbuffers/protobuf/tree/v4.31.1

yordis avatar Jun 04 '25 20:06 yordis

Our numbering scheme is that the major version is per-language (to decouple semver bumps to every language), and then the minor/point versions are the same numbering across all, and the corresponding best protoc version is the same number as that.

So 4.31.1 for Rust is intended to be used with the 31.1 release on the linked page, and theres an "assets" section that has the released protoc binaries there.

Right now we expect an exact match of protoc and Rust runtime version (which is also how it is for C++), but in the future we will either relax that or vendor/automate protoc into cargo to take the sharp edge of having an exact match off.

esrauchg avatar Jun 04 '25 20:06 esrauchg

I would love to be able to remove not_set(std::marker::PhantomData<&'msg ()>) = 0 from the enums and/or oneof

Like the Go version, I am unsure what the decision around setters not returning the type is, but, personally, I would love to be able to chain the calls

from:

let mut auction_end_scheduled = AuctionEndScheduled::new();
auction_end_scheduled.set_run_id(cmd.run_id());
auction_end_scheduled.set_scheduled_at(cmd.started_at());
auction_end_scheduled.set_ends_at(ends_at_timestamp);
auction_end_scheduled.set_extension_id(0);
auction_end_scheduled.set_annotations(cmd.annotations());

to:

let auction_end_scheduled = AuctionEndScheduled::new()
  .set_run_id(cmd.run_id());
  .set_scheduled_at(cmd.started_at());
  .set_ends_at(ends_at_timestamp);
  .set_extension_id(0);
  .set_annotations(cmd.annotations());

yordis avatar Jun 06 '25 00:06 yordis

Here is another example in Go, but it could be the same in Rust since follows the same pattern. I feel letting the setter to return the same type would avoid so many anchor variables or an extra builder that I am failing to see the value off; please correct me here, I could be doing something silly.

I used to do the following:

var EventTypeProvider = trogontypeprovider.MustNewTypeProvider(
	getEventMessageType,
	EventMarshaller,
	&eventv1.Event{Event: &eventv1.Event_AuctionStarted{}},
	&eventv1.Event{Event: &eventv1.Event_BidPlaced{}},
	&eventv1.Event{Event: &eventv1.Event_AuctionEnded{}},
   // more and more events here
)

Now I have to use that _builder

var EventTypeProvider = trogontypeprovider.MustNewTypeProvider(
	getEventMessageType,
	EventMarshaller,
	eventv1.Event_builder{AuctionStarted: &eventv1.AuctionStarted{}}.Build(),
	eventv1.Event_builder{BidPlaced: &eventv1.BidPlaced{}}.Build(),
	eventv1.Event_builder{AuctionEnded: &eventv1.AuctionEnded{}}.Build(),
)

or

func addMoneyAmounts(a, b *typev1.MoneyAmount) *typev1.MoneyAmount {
	if a == nil || b == nil {
		return nil
	}

	return &typev1.MoneyAmount{
		Amount: a.Amount + b.Amount,
	}
}

to

func addMoneyAmounts(a, b *typev1.MoneyAmount) *typev1.MoneyAmount {
	if a == nil || b == nil {
		return nil
	}

	amount := a.GetAmount() + b.GetAmount()
        
        // or using _builder but I rather avoid paying that cost
	c := &typev1.MoneyAmount{}
	c.SetAmount(amount)
        
	return c
}

A lot of my switch statements become if statements where I lost a bit the structure of the programmer. I found Which*() but I can not directly have the new type already, a simply mistyping could lead to check the Which* correctly but still call the wrong function

func Evolve(state *aggregatev1.Aggregate, event *eventv1.Event) *aggregatev1.Aggregate {
	switch e := event.Event.(type) {
	case *eventv1.Event_AuctionStarted:
		state.Started = true
	case *eventv1.Event_BidPlaced:
		state.CurrentPrice = e.BidPlaced.BidAmount
	case *eventv1.Event_AuctionEnded:
		state.Ended = true
	}
	return state
}

Even using the _builder pattern it requires anchor veriables to pass pointers (obviously without importing some helper function such as proto.String())

auctionStarted := eventv1.AuctionStarted_builder{
                 // this does not work GetRunId doesnt return a pointer
		RunId:                     &command.GetRunId(),
	}.Build()

Any advice in terms of what I could be doing wrong here?

yordis avatar Jun 09 '25 04:06 yordis

I am unsure what the decision around setters not returning the type is

Golang does not like chained callers like you purposed. When a setter returns the same type, a natural question is: “is the returned value the mutated receiver, or is it a deep/shallow-copy of the original?” Golang idiom settles this matter by choosing that one should never† return a mutated receiver. The caller already has the original receiver, so we don’t need to return it.

Do you think it would be possible to either return the data type in the Set* or add a With* (eg, WithRunI) functions as well?

This is the wrong issue board for asking about Go protobuf stuff, because we have a different board, but I’m already also here, so I can help you out.

No, we do not want to return the mutated data type with the Set or add a With function. Doing so would imply that the returned value either will be a copy, or could be a copy. (http.Request.WithContext returns a shallow-copy of the request, with the context value set; slice = append(slice, value) may involve a reallocation of the backing array, and thus returns the maybe-new slice; map assignment cannot spontaneously allocate because they are mutating operations: m[key] = value; any map assignment wrapper that could allow for spontaneous allocation should return the maybe-new map, so m = m.Set(key, value), but here it is consider a poor choice, because while slice appends can potentially reallocate many times, a map only needs the single initial allocation. So, this assignment would waste time and effort. Better to just make a mutator with no return value at all.)

I am leaning towards disabling builders so we do not have 2 ways to deal with the problems

There definitely are caveats and gotchas with the builders, particularly if oneofs are being used. But as noted, the Golang protobuf team will not be very receptive of the idea of allowing any sort of mutator receiver method chains as a matter of principle.

†: there are plenty of widely used open-source libraries that do return the mutated receiver… 🤷‍♀ unless a language makes something impossible, some people are going to do Thing We Don’t Recommend™.

puellanivis avatar Jun 09 '25 09:06 puellanivis

Again, slightly out of scope for this board, but:

Context:

var EventTypeProvider = trogontypeprovider.MustNewTypeProvider(
	getEventMessageType,
	EventMarshaller,
	eventv1.Event_builder{AuctionStarted: &eventv1.AuctionStarted{}}.Build(),
	eventv1.Event_builder{BidPlaced: &eventv1.BidPlaced{}}.Build(),
	eventv1.Event_builder{AuctionEnded: &eventv1.AuctionEnded{}}.Build(),
	…
)

I’m unsure what the purpose of this even is. I do not understand how or why such a “type provider” would be necessary, or have to have the types passed in at runtime… It might be better to ask about this stuff in our side-channel discussion.

This really doesn’t have any distinct differences from the return &typev1.MoneyAmount{ Amount: a.GetAmount() + b.GetAmount() }

func addMoneyAmounts(a, b *typev1.MoneyAmount) *typev1.MoneyAmount {
	c := &typev1.MoneyAmount{}
	c.SetAmount(a.GetAmount() + b.GetAmount()) // no need to test for nil, the accessors check and return zero if not set.
        
	return c
}
func Evolve(state *aggregatev1.Aggregate, event *eventv1.Event) *aggregatev1.Aggregate {
	switch {
	case event.HasAuctionStarted():
		state.SetStarted(true)
	case event.HasBidPlaced():
		state.SetCurrentPrice(event.GetBidPlaced().BidAmount())
	case event.HasAuctionEnded():
		state.SetEnded(true)
	}
	return state
}

puellanivis avatar Jun 09 '25 10:06 puellanivis

What I dont like about the event.Has* and then inside event.Get* is that I can see where the condition to be true, and then we call the wrong getter.

I am doing,

if e := event.GetBidPlaced(), e != nil {
  // ...
}

But now the structure looks weird hehe.

Anyway, silly things we can move on from it, no a big deal. I still prefer the chaining 😭 So many anchor veriables or .Build() penalty if I want to bypass it 😭

yordis avatar Jun 09 '25 10:06 yordis

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Sep 08 '25 10:09 github-actions[bot]

@esrauchg any final thoughts here?

yordis avatar Sep 08 '25 10:09 yordis

This thread meandered a bit, but I'm going to close with three resolutions:

  1. Open topic of Rust Proto which is about setters returning -> &mut Self so that you can chain setter calls. I opened https://github.com/protocolbuffers/protobuf/issues/23412 for that.

  2. For the topic that is the issue title (non-optional submessage getters), I think this is resolved as "yes, our implementations don't do that, and Prost is free to make that choice but its not aligned with our philosophy". Optional is intended to mean something subtly different from nullable, and in general official protobuf implementations expose the optional field getters as non-nullable by default. There are edge cases to this including C# which does treat optional submessages as nullable because of a combination of historical reasons, local idioms, and lack of builder pattern, but its an odd-implementation-out and not the philosophy of optional fields.

  3. For the feedback about the oneof-getter being able to opt into not having a not_set case, that one is a wontfix. Protobuf's primary usecase is systems which need to be able to handle schema skew over time, where the data writer and data reader do not have the same schema, and much of our API design philosophy stems from trying to make that stay sane. In this particular case, dropping the not_set of a oneof amounts to the same thing as the old proto2 "required" which is a strongly regretted and discouraged behavior that we do not anyone use going forward. Proto2 didn't let you mark oneofs as required, and we're not realistically going to extend that behavior to oneofs from here.

If there's another piece of feedback for us to consider that doesn't seem covered by those three things that you'd like us to consider, lets restart a new issue focused on that topic and I'm happy to look. If its about Go Proto, that one language is maintained in a separate repo here: https://github.com/golang/protobuf

Thanks for the feedback and discussion here, and I especially appreciate keeping the conversation civil throughout.

esrauchg avatar Sep 08 '25 12:09 esrauchg