Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for :oneof fields on protobuf messages #929

Merged
merged 1 commit into from May 12, 2022

Conversation

andrewn617
Copy link
Contributor

@andrewn617 andrewn617 commented May 12, 2022

Motivation

Google Protobuf messages support oneof field types:

add_message "MyCart" do
  oneof :contact_info do
    optional :phone_number, :int32, 1
    optional :email, :string, 2
  end
end

MyCart will have a #contact_info method that returns the name of the optional that has been used (or nil).

This PR adds a method definition to RBIs for messages with oneof fields.

Implementation

Descriptor provides an each_oneof method. I iterate over the oneof descriptors and create an RBI method for each.

Tests

馃憤

Copy link

@asoules asoules left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃弳

Copy link
Contributor

@slinkp slinkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 馃帺 this on Shopify/shopify and see the desired method sigs for oneOf fields. Thanks!!

Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure how oneof fields worked, and it was not clear to me why the method would be returning a nilable Symbol. So I looked up the documentation for it and understood that the oneof field holds the name of the field that ends up being populated.

Here is a code snippet that describes the situation for everyone's sake:

cart = MyCart.new

# Fields have their defaults.
raise unless cart.email == ""
raise unless cart.phone_number == 0
raise unless cart.contact_info == nil

cart.email = "foo@example.com"
raise unless cart.email == "foo@example.com"
raise unless cart.phone_number == 0
raise unless cart.contact_info == :email

# Setting phone_number clears email.
cart.phone_number = 5552456789
raise unless cart.email == ""
raise unless cart.contact_info == :phone_number

# Setting phone_number to nil clears the oneof.
cart.phone_number = nil
raise unless cart.contact_info == nil

@paracycle paracycle merged commit c4263d7 into main May 12, 2022
@paracycle paracycle deleted the add-support-for-protobuf-oneof-fields branch May 12, 2022 22:37
@shopify-shipit shopify-shipit bot temporarily deployed to production May 13, 2022 23:08 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-8-stable May 30, 2022 13:45 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants