diff --git a/openrtb2/banner.go b/openrtb2/banner.go index d3a5254..ea15c8a 100644 --- a/openrtb2/banner.go +++ b/openrtb2/banner.go @@ -123,8 +123,8 @@ type Banner struct { // integer // Description: // Indicates if the banner is in the top frame as opposed to an - // iframe, where 0 = no, 1 = yes. - TopFrame int8 `json:"topframe,omitempty"` + // iframe, where 0 = no, 1 = yes, omission indicates Unknown. + TopFrame *int8 `json:"topframe,omitempty"` // Attribute: // expdir diff --git a/openrtb2/regs.go b/openrtb2/regs.go index a19c6d7..5ba4052 100644 --- a/openrtb2/regs.go +++ b/openrtb2/regs.go @@ -14,9 +14,10 @@ type Regs struct { // integer // Description: // Flag indicating if this request is subject to the COPPA - // regulations established by the USA FTC, where 0 = no, 1 = yes. - // Refer to Section 7.5 for more information. - COPPA int8 `json:"coppa,omitempty"` + // regulations established by the USA FTC, where 0 = no, 1 = yes, + // omission indicates Unknown. Refer to Section 7.5 for more + // information. + COPPA *int8 `json:"coppa,omitempty"` // Attribute: // gdpr diff --git a/openrtb2/video.go b/openrtb2/video.go index 79be589..bd677b2 100644 --- a/openrtb2/video.go +++ b/openrtb2/video.go @@ -276,8 +276,9 @@ type Video struct { // Type: // integer // Description: - // Minimum bit rate in Kbps (kilobits per second). - MinBitRate int64 `json:"minbitrate,omitempty"` + // Minimum bit rate in Kbps (kilobits per second). A value of 0 is + // a meaningful floor, distinct from omitting the attribute. + MinBitRate *int64 `json:"minbitrate,omitempty"` // Attribute: // maxbitrate diff --git a/openrtb2/zero_value_fields_test.go b/openrtb2/zero_value_fields_test.go new file mode 100644 index 0000000..917feb4 --- /dev/null +++ b/openrtb2/zero_value_fields_test.go @@ -0,0 +1,46 @@ +package openrtb2_test + +import ( + "encoding/json" + + . "github.com/prebid/openrtb/v20/openrtb2" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +// These fields (Regs.COPPA, Banner.TopFrame, Video.MinBitRate) were previously typed as +// plain int8/int64 with `omitempty`. Because encoding/json's omitempty treats a value type's +// zero value as empty, explicitly setting one of these fields to a meaningful 0 (COPPA: not +// subject to COPPA, TopFrame: not in the top frame, MinBitRate: a real 0 Kbps floor) was +// indistinguishable, once marshaled, from never having set the field at all - the sender's +// explicit signal was silently dropped. See https://github.com/prebid/openrtb/issues/13. +// +// Changing them to pointer types (matching the existing pattern used by Regs.GDPR, +// Banner.Vcm, and Device.DNT for the same "0/1, omission means Unknown" semantics) lets a +// caller distinguish "not set" (nil) from "explicitly set to 0" (Int8Ptr(0)/Int64Ptr(0)). +var _ = Describe("Zero-valued optional fields survive marshaling", func() { + It("keeps Regs.COPPA when explicitly set to 0", func() { + b, err := json.Marshal(Regs{COPPA: Int8Ptr(0)}) + Expect(err).NotTo(HaveOccurred()) + Expect(b).To(MatchJSON(`{"coppa":0}`)) + }) + + It("omits Regs.COPPA when left unset", func() { + b, err := json.Marshal(Regs{}) + Expect(err).NotTo(HaveOccurred()) + Expect(b).To(MatchJSON(`{}`)) + }) + + It("keeps Banner.TopFrame when explicitly set to 0", func() { + b, err := json.Marshal(Banner{TopFrame: Int8Ptr(0)}) + Expect(err).NotTo(HaveOccurred()) + Expect(b).To(MatchJSON(`{"topframe":0}`)) + }) + + It("keeps Video.MinBitRate when explicitly set to 0", func() { + b, err := json.Marshal(Video{MinBitRate: Int64Ptr(0)}) + Expect(err).NotTo(HaveOccurred()) + Expect(b).To(MatchJSON(`{"mimes":null,"minbitrate":0}`)) + }) +})