Wrong default value for deal auction type.
fredriklagerblad opened this issue · 3 comments
When invoking the getAt() method on a OpenRtb.BidRequest.Imp.Pmp.Deal object, that doesn't have that 'at' value set - the method defaults to OpenRtb.AuctionType.FIRST_PRICE.
According to the OpenRTB specs, if should default to the BidRequest's auction type: "Optional override of the overall auction type of the bid request".
(Which in turn defaults correctly to OpenRtb.AuctionType.SECOND_PRICE)
Notice that the specification states both "optional override", like you quote above, and it doesn't mention any default value in the Type column like it usually does for all fields that have a default. This means this default is implicit, i.e. if we don't provide a value for Deal.at then you should assume its auction type to be whatever you get from BidRequest.at. And the latter field defaults to second price, but this is just a default, another type can be set.
Protobuf doesn't have syntax to specify defaults dynamically with runtime rules referring to other fields, only statically with constant values. If I set a static default value to AuctionType.SECOND_PRICE, and the exchange sets a different auction type in the request but doesn't set any value in the deal, then you will get the wrong value from Deal.at.
You only need to mind the fact that protobuf fields can have absent value, so the right way to code is: first check if the field is set (has method), if it's not set then you should use the request's field instead, i.e. it's caller responsibility to do this default logic because unfortunately we cannot put that in the message as a static default.
Ok, I thought you added some "runtime" code (rules) after generating the code from protobuf, resulting in the final code code.
I understand that defaulting to AuctionType.SECOND_PRICE for Deal.at wouldn't then completely solve the issue, but it wouldn't either be "worse" than it is today (defaulting to FIRST_PRICE). :-)
I had already implemented my work-around as you described with hasAt() and fall-backing in code, but it would have been so much better to get that support directly in the framework.
What you suggest would be ideal indeed, unfortunately protobuf doesn't allow you to customize any of the generated code... well I could take the generated sources and modify them, but that would be hard to maintain :)