From c69649bf06a49e66b47d4f1a3eab0d3036075b41 Mon Sep 17 00:00:00 2001 From: dbemiller <27972385+dbemiller@users.noreply.github.com> Date: Thu, 28 Jun 2018 14:29:16 -0400 Subject: [PATCH] Adding a decorator which enforces the {bidder}.yaml info files (#594) * Added some code to enforce info.yaml files, and a few tests * Used testify in these tests. * Added more tests, and fixed some bugs. * Made sure the decorator returned BadInputErrors. * Moved BadInputError construction into a factory method to keep things concise. --- adapters/bidder.go | 6 ++ adapters/info.go | 117 ++++++++++++++++++++++++++++++++++ adapters/info_test.go | 143 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 249 insertions(+), 17 deletions(-) diff --git a/adapters/bidder.go b/adapters/bidder.go index 7305ebd3c8a..190314fb681 100644 --- a/adapters/bidder.go +++ b/adapters/bidder.go @@ -32,6 +32,12 @@ type Bidder interface { MakeBids(internalRequest *openrtb.BidRequest, externalRequest *RequestData, response *ResponseData) (*BidderResponse, []error) } +func BadInput(msg string) *BadInputError { + return &BadInputError{ + Message: msg, + } +} + // BadInputError should be used when returning errors which are caused by bad input. // It should _not_ be used if the error is a server-side issue (e.g. failed to send the external request). // diff --git a/adapters/info.go b/adapters/info.go index 8f4027279f8..0e052b6bb79 100644 --- a/adapters/info.go +++ b/adapters/info.go @@ -1,13 +1,130 @@ package adapters import ( + "fmt" "io/ioutil" "github.com/golang/glog" + "github.com/mxmCherry/openrtb" "github.com/prebid/prebid-server/openrtb_ext" yaml "gopkg.in/yaml.v2" ) +// EnforceBidderInfo decorates the input Bidder by making sure that all the requests +// to it are in sync with its static/bidder-info/{bidder}.yaml file. +// +// It adjusts incoming requests in the following ways: +// 1. If App or Site traffic is not supported by the info file, then requests from +// those sources will be rejected before the delegate is called. +// 2. If a given MediaType is not supported for the platform, then it will be set +// to nil before the request is forwarded to the delegate. +// 3. Any Imps which have no MediaTypes left will be removed. +// 4. If there are no valid Imps left, the delegate won't be called at all. +func EnforceBidderInfo(bidder Bidder, info BidderInfo) Bidder { + return &InfoAwareBidder{ + Bidder: bidder, + info: info, + } +} + +type InfoAwareBidder struct { + Bidder + info BidderInfo +} + +func (i *InfoAwareBidder) MakeRequests(request *openrtb.BidRequest) ([]*RequestData, []error) { + var allowedMediaTypes []openrtb_ext.BidType + if request.Site != nil { + if i.info.Capabilities.Site == nil { + return nil, []error{BadInput("this bidder does not support site requests")} + } + allowedMediaTypes = i.info.Capabilities.Site.MediaTypes + } + if request.App != nil { + if i.info.Capabilities.App == nil { + return nil, []error{BadInput("this bidder does not support app requests")} + } + allowedMediaTypes = i.info.Capabilities.App.MediaTypes + } + + // Filtering imps is quite expensive (array filter with large, non-pointer elements)... but should be rare, + // because it only happens if the publisher makes a really bad request. + // + // To avoid allocating new arrays and copying in the normal case, we'll make one pass to + // see if any imps need to be removed, and another to do the removing if necessary. + numToFilter, errs := i.pruneImps(request.Imp, allowedMediaTypes) + if numToFilter != 0 { + filteredImps, newErrs := i.filterImps(request.Imp, numToFilter) + request.Imp = filteredImps + errs = append(errs, newErrs...) + } + reqs, delegateErrs := i.Bidder.MakeRequests(request) + return reqs, append(errs, delegateErrs...) +} + +// pruneImps trims invalid media types from each imp, and returns true if any of the +// Imps have _no_ valid Media Types left. +func (i *InfoAwareBidder) pruneImps(imps []openrtb.Imp, allowedTypes []openrtb_ext.BidType) (int, []error) { + allowBanner, allowVideo, allowAudio, allowNative := parseAllowedTypes(allowedTypes) + numToFilter := 0 + var errs []error + for i := 0; i < len(imps); i++ { + if !allowBanner && imps[i].Banner != nil { + imps[i].Banner = nil + errs = append(errs, BadInput(fmt.Sprintf("request.imp[%d] uses banner, but this bidder doesn't support it", i))) + } + if !allowVideo && imps[i].Video != nil { + imps[i].Video = nil + errs = append(errs, BadInput(fmt.Sprintf("request.imp[%d] uses video, but this bidder doesn't support it", i))) + } + if !allowAudio && imps[i].Audio != nil { + imps[i].Audio = nil + errs = append(errs, BadInput(fmt.Sprintf("request.imp[%d] uses audio, but this bidder doesn't support it", i))) + } + if !allowNative && imps[i].Native != nil { + imps[i].Native = nil + errs = append(errs, BadInput(fmt.Sprintf("request.imp[%d] uses native, but this bidder doesn't support it", i))) + } + if !hasAnyTypes(&imps[i]) { + numToFilter = numToFilter + 1 + } + } + return numToFilter, errs +} + +func parseAllowedTypes(allowedTypes []openrtb_ext.BidType) (allowBanner bool, allowVideo bool, allowAudio bool, allowNative bool) { + for _, allowedType := range allowedTypes { + switch allowedType { + case openrtb_ext.BidTypeBanner: + allowBanner = true + case openrtb_ext.BidTypeVideo: + allowVideo = true + case openrtb_ext.BidTypeAudio: + allowAudio = true + case openrtb_ext.BidTypeNative: + allowNative = true + } + } + return +} + +func hasAnyTypes(imp *openrtb.Imp) bool { + return imp.Banner != nil || imp.Video != nil || imp.Audio != nil || imp.Native != nil +} + +func (i *InfoAwareBidder) filterImps(imps []openrtb.Imp, numToFilter int) ([]openrtb.Imp, []error) { + newImps := make([]openrtb.Imp, 0, len(imps)-numToFilter) + errs := make([]error, 0, numToFilter) + for i := 0; i < len(imps); i++ { + if hasAnyTypes(&imps[i]) { + newImps = append(newImps, imps[i]) + } else { + errs = append(errs, BadInput(fmt.Sprintf("request.imp[%d] has no supported MediaTypes. It will be ignored", i))) + } + } + return newImps, errs +} + type BidderInfos map[string]BidderInfo // ParseBidderInfos reads all the static/bidder-info/{bidder}.yaml files from the filesystem. diff --git a/adapters/info_test.go b/adapters/info_test.go index 5bccc182fdf..805eec6d87e 100644 --- a/adapters/info_test.go +++ b/adapters/info_test.go @@ -1,35 +1,144 @@ package adapters_test import ( + "errors" "testing" + "github.com/mxmCherry/openrtb" "github.com/prebid/prebid-server/adapters" "github.com/prebid/prebid-server/openrtb_ext" + "github.com/stretchr/testify/assert" ) +func TestAppNotSupported(t *testing.T) { + bidder := &mockBidder{} + info := adapters.BidderInfo{ + Capabilities: &adapters.CapabilitiesInfo{ + Site: &adapters.PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner}, + }, + }, + } + constrained := adapters.EnforceBidderInfo(bidder, info) + bids, errs := constrained.MakeRequests(&openrtb.BidRequest{ + App: &openrtb.App{}, + }) + if !assert.Len(t, errs, 1) { + return + } + assert.EqualError(t, errs[0], "this bidder does not support app requests") + assert.IsType(t, &adapters.BadInputError{}, errs[0]) + assert.Len(t, bids, 0) +} + +func TestSiteNotSupported(t *testing.T) { + bidder := &mockBidder{} + info := adapters.BidderInfo{ + Capabilities: &adapters.CapabilitiesInfo{ + App: &adapters.PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner}, + }, + }, + } + constrained := adapters.EnforceBidderInfo(bidder, info) + bids, errs := constrained.MakeRequests(&openrtb.BidRequest{ + Site: &openrtb.Site{}, + }) + if !assert.Len(t, errs, 1) { + return + } + assert.EqualError(t, errs[0], "this bidder does not support site requests") + assert.IsType(t, &adapters.BadInputError{}, errs[0]) + assert.Len(t, bids, 0) +} + +func TestImpFiltering(t *testing.T) { + bidder := &mockBidder{} + info := adapters.BidderInfo{ + Capabilities: &adapters.CapabilitiesInfo{ + Site: &adapters.PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeVideo}, + }, + App: &adapters.PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner}, + }, + }, + } + + constrained := adapters.EnforceBidderInfo(bidder, info) + _, errs := constrained.MakeRequests(&openrtb.BidRequest{ + Imp: []openrtb.Imp{ + { + ID: "imp-1", + Video: &openrtb.Video{}, + }, + { + Native: &openrtb.Native{}, + }, + { + ID: "imp-2", + Video: &openrtb.Video{}, + Native: &openrtb.Native{}, + }, + { + Banner: &openrtb.Banner{}, + }, + }, + Site: &openrtb.Site{}, + }) + if !assert.Len(t, errs, 6) { + return + } + assert.EqualError(t, errs[0], "request.imp[1] uses native, but this bidder doesn't support it") + assert.EqualError(t, errs[1], "request.imp[2] uses native, but this bidder doesn't support it") + assert.EqualError(t, errs[2], "request.imp[3] uses banner, but this bidder doesn't support it") + assert.EqualError(t, errs[3], "request.imp[1] has no supported MediaTypes. It will be ignored") + assert.EqualError(t, errs[4], "request.imp[3] has no supported MediaTypes. It will be ignored") + assert.EqualError(t, errs[5], "mock MakeRequests error") + assert.IsType(t, &adapters.BadInputError{}, errs[0]) + assert.IsType(t, &adapters.BadInputError{}, errs[1]) + assert.IsType(t, &adapters.BadInputError{}, errs[2]) + assert.IsType(t, &adapters.BadInputError{}, errs[3]) + assert.IsType(t, &adapters.BadInputError{}, errs[4]) + + req := bidder.gotRequest + if !assert.Len(t, req.Imp, 2) { + return + } + assert.Equal(t, "imp-1", req.Imp[0].ID) + assert.Equal(t, "imp-2", req.Imp[1].ID) + assert.Nil(t, req.Imp[1].Native) +} + +type mockBidder struct { + gotRequest *openrtb.BidRequest +} + +func (m *mockBidder) MakeRequests(request *openrtb.BidRequest) ([]*adapters.RequestData, []error) { + m.gotRequest = request + return nil, []error{errors.New("mock MakeRequests error")} +} + +func (m *mockBidder) MakeBids(internalRequest *openrtb.BidRequest, externalRequest *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) { + return nil, []error{errors.New("mock MakeBids error")} +} + func TestParsing(t *testing.T) { mockBidderName := openrtb_ext.BidderName("someBidder") infos := adapters.ParseBidderInfos("./adapterstest/bidder-info", []openrtb_ext.BidderName{mockBidderName}) if infos[string(mockBidderName)].Maintainer.Email != "some-email@domain.com" { t.Errorf("Bad maintainer email. Got %s", infos[string(mockBidderName)].Maintainer.Email) } - assertBoolsEqual(t, true, infos.HasAppSupport(mockBidderName)) - assertBoolsEqual(t, true, infos.HasSiteSupport(mockBidderName)) + assert.Equal(t, true, infos.HasAppSupport(mockBidderName)) + assert.Equal(t, true, infos.HasSiteSupport(mockBidderName)) - assertBoolsEqual(t, true, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeBanner)) - assertBoolsEqual(t, false, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeVideo)) - assertBoolsEqual(t, false, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeAudio)) - assertBoolsEqual(t, true, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeNative)) + assert.Equal(t, true, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeBanner)) + assert.Equal(t, false, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeVideo)) + assert.Equal(t, false, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeAudio)) + assert.Equal(t, true, infos.SupportsAppMediaType(mockBidderName, openrtb_ext.BidTypeNative)) - assertBoolsEqual(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeBanner)) - assertBoolsEqual(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeVideo)) - assertBoolsEqual(t, false, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeAudio)) - assertBoolsEqual(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeNative)) -} - -func assertBoolsEqual(t *testing.T, expected bool, actual bool) { - t.Helper() - if actual != expected { - t.Errorf("expected %t, got %t", expected, actual) - } + assert.Equal(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeBanner)) + assert.Equal(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeVideo)) + assert.Equal(t, false, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeAudio)) + assert.Equal(t, true, infos.SupportsWebMediaType(mockBidderName, openrtb_ext.BidTypeNative)) }