protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Cannot assign an array to a repeated field in Ruby

Open fables-tales opened this issue 5 years ago • 1 comments

What language does this apply to? Ruby

Describe the problem you are trying to solve. Consider trying to do this:

#!/usr/bin/ruby
require 'google/protobuf'

Google::Protobuf::DescriptorPool.generated_pool.build do
  add_file("google/ads/googleads/v2/services/campaign_budget_service.proto", :syntax => :proto3) do
    add_message "google.ads.googleads.v2.services.MutateCampaignBudgetsRequest" do
      optional :customer_id, :string, 1
      repeated :operations, :message, 2, "google.ads.googleads.v2.services.CampaignBudgetOperation"
    end

    add_message "google.ads.googleads.v2.services.CampaignBudgetOperation" do
      oneof :operation do
        optional :remove, :string, 1
      end
    end
  end
end

module Google
  module Ads
    module GoogleAds
      module V2
        module Services
          MutateCampaignBudgetsRequest = Google::Protobuf::DescriptorPool.generated_pool.lookup("google.ads.googleads.v2.services.MutateCampaignBudgetsRequest").msgclass
          CampaignBudgetOperation = Google::Protobuf::DescriptorPool.generated_pool.lookup("google.ads.googleads.v2.services.CampaignBudgetOperation").msgclass
        end
      end
    end
  end
end

include Google::Ads::GoogleAds::V2::Services

def main
  r = MutateCampaignBudgetsRequest.new
  r.operations = [CampaignBudgetOperation.new]
end

if __FILE__ == $0
  main
end

currently protobuf gives this error:

Traceback (most recent call last):
	2: from hi.rb:40:in `<main>'
	1: from hi.rb:36:in `main'
hi.rb:36:in `method_missing': Expected repeated field array (Google::Protobuf::TypeError)

Describe the solution you'd like Assigning an array to a repeated field should work if all the values are of the right type

Describe alternatives you've considered N/A Additional context my ldap pnlpn@ for on-corp discussions.

fables-tales avatar Jan 31 '20 15:01 fables-tales

I have previously had some reservations about this, mainly because of the following case:

def main
  r = MutateCampaignBudgetsRequest.new
  arr = [CampaignBudgetOperation.new]
  r.operations = arr  # This must copy, as RepeatedField cannot alias Array
  arr << CampaignBudgetOperation.new  # Doesn't mutate r.operations!
  assert_eq 1, r.operations.size  # Some people might expect this to be 2!
end

However after talking about it more, others have convinced me that the convenience of allowing this outweighs the possible confusion that could arise from the case above.

So we should implement this at some point. It's not currently prioritized, but we would accept a PR.

haberman avatar Sep 01 '22 18:09 haberman