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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Events support in bid responses #1597

Merged
merged 20 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions endpoints/events/vtrack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"sort"
"strings"
"time"

"github.com/golang/glog"
"github.com/julienschmidt/httprouter"
accountService "github.com/prebid/prebid-server/account"
Expand All @@ -13,12 +20,6 @@ import (
"github.com/prebid/prebid-server/errortypes"
"github.com/prebid/prebid-server/prebid_cache_client"
"github.com/prebid/prebid-server/stored_requests"
"io"
"io/ioutil"
"net/http"
"sort"
"strings"
"time"
)

const (
Expand Down Expand Up @@ -230,7 +231,7 @@ func (v *vtrackEndpoint) cachePutObjects(ctx context.Context, req *BidCacheReque
}

if _, ok := biddersAllowingVastUpdate[c.Bidder]; ok && nc.Data != nil {
nc.Data = modifyVastXml(v.Cfg.ExternalURL, nc.Data, c.BidID, c.Bidder, accountId, c.Timestamp)
nc.Data = ModifyVastXmlJSON(v.Cfg.ExternalURL, nc.Data, c.BidID, c.Bidder, accountId, c.Timestamp)
}

cacheables = append(cacheables, *nc)
Expand Down Expand Up @@ -269,25 +270,34 @@ func getAccountId(httpRequest *http.Request) string {
return httpRequest.URL.Query().Get(AccountParameter)
}

// modifyVastXml modifies BidCacheRequest element Vast XML data
func modifyVastXml(externalUrl string, data json.RawMessage, bidid string, bidder string, accountId string, timestamp int64) json.RawMessage {
c := string(data)
// modifyVastXmlString rewrites and returns the string vastXML or an empty string if it was left unmodified
func ModifyVastXmlString(externalUrl string, c string, bidid string, bidder string, accountId string, timestamp int64) string {
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
ci := strings.Index(c, ImpressionCloseTag)

// no impression tag - pass it as it is
if ci == -1 {
return data
return ""
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}

vastUrlTracking := GetVastUrlTracking(externalUrl, bidid, bidder, accountId, timestamp)
impressionUrl := "<![CDATA[" + vastUrlTracking + "]]>"
oi := strings.Index(c, ImpressionOpenTag)

if ci-oi == len(ImpressionOpenTag) {
return json.RawMessage(strings.Replace(c, ImpressionOpenTag, ImpressionOpenTag+impressionUrl, 1))
return strings.Replace(c, ImpressionOpenTag, ImpressionOpenTag+impressionUrl, 1)
}

return json.RawMessage(strings.Replace(c, ImpressionCloseTag, ImpressionCloseTag+ImpressionOpenTag+impressionUrl+ImpressionCloseTag, 1))
return strings.Replace(c, ImpressionCloseTag, ImpressionCloseTag+ImpressionOpenTag+impressionUrl+ImpressionCloseTag, 1)
}

// modifyVastXml modifies BidCacheRequest element Vast XML data
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
func ModifyVastXmlJSON(externalUrl string, data json.RawMessage, bidid string, bidder string, accountId string, timestamp int64) json.RawMessage {
c := string(data) // FIXME: should decode json, or escapes like "\u003e" will not be recognized as "<"
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
c = ModifyVastXmlString(externalUrl, c, bidid, bidder, accountId, timestamp)
if len(c) == 0 {
return data
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we fall back to string(data) or just data here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought we can just return the original data unmodified as it's already json.RawMessage.

}
return json.RawMessage(c)
}

func contains(s []string, e string) bool {
Expand Down
1 change: 1 addition & 0 deletions endpoints/openrtb2/auction_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) {
nil,
&config.Configuration{},
newTestMetrics(),
infos,
gdpr.AlwaysAllow{},
currencies.NewRateConverter(&http.Client{}, "", time.Duration(0)),
empty_fetcher.EmptyFetcher{},
Expand Down
10 changes: 6 additions & 4 deletions exchange/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (a *auction) setRoundedPrices(priceGranularity openrtb_ext.PriceGranularity
a.roundedPrices = roundedPrices
}

func (a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client, targData *targetData, bidRequest *openrtb.BidRequest, ttlBuffer int64, defaultTTLs *config.DefaultTTLs, bidCategory map[string]string, debugLog *DebugLog) []error {
func (a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client, targData *targetData, evData *eventsData, bidRequest *openrtb.BidRequest, ttlBuffer int64, defaultTTLs *config.DefaultTTLs, bidCategory map[string]string, debugLog *DebugLog) []error {
var bids, vast, includeBidderKeys, includeWinners bool = targData.includeCacheBids, targData.includeCacheVast, targData.includeBidderKeys, targData.includeWinners
if !((bids || vast) && (includeBidderKeys || includeWinners)) {
return nil
Expand Down Expand Up @@ -176,7 +176,7 @@ func (a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client,
expByImp[imp.ID] = imp.Exp
}
for _, topBidsPerImp := range a.winningBidsByBidder {
for _, topBidPerBidder := range topBidsPerImp {
for bidderName, topBidPerBidder := range topBidsPerImp {
impID := topBidPerBidder.bid.ImpID
isOverallWinner := a.winningBids[impID] == topBidPerBidder
if !includeBidderKeys && !isOverallWinner {
Expand All @@ -195,6 +195,7 @@ func (a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client,
}
if bids {
if jsonBytes, err := json.Marshal(topBidPerBidder.bid); err == nil {
jsonBytes = evData.modifyBidJSON(topBidPerBidder.bid, bidderName, jsonBytes)
if useCustomCacheKey {
// not allowed if bids is true; log error and cache normally
errs = append(errs, errors.New("cannot use custom cache key for non-vast bids"))
Expand All @@ -210,8 +211,9 @@ func (a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client,
}
}
if vast && topBidPerBidder.bidType == openrtb_ext.BidTypeVideo {
vast := makeVAST(topBidPerBidder.bid)
if jsonBytes, err := json.Marshal(vast); err == nil {
vastXML := makeVAST(topBidPerBidder.bid)
vastXML = evData.modifyVAST(topBidPerBidder.bid, bidderName, vastXML)
if jsonBytes, err := json.Marshal(vastXML); err == nil {
if useCustomCacheKey {
toCache = append(toCache, prebid_cache_client.Cacheable{
Type: prebid_cache_client.TypeXML,
Expand Down
153 changes: 74 additions & 79 deletions exchange/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import (
"encoding/xml"
"fmt"
"io/ioutil"
"path/filepath"
"regexp"
"strconv"
"strings"
"testing"

"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/config"
"github.com/prebid/prebid-server/openrtb_ext"
"github.com/prebid/prebid-server/prebid_cache_client"
Expand Down Expand Up @@ -100,57 +101,21 @@ func TestBuildCacheString(t *testing.T) {
// TestCacheJSON executes tests for all the *.json files in cachetest.
// customcachekey.json test here verifies custom cache key not used for non-vast video
func TestCacheJSON(t *testing.T) {
if specFiles, err := ioutil.ReadDir("./cachetest"); err == nil {
for _, specFile := range specFiles {
fileName := "./cachetest/" + specFile.Name()
fileDisplayName := "exchange/cachetest/" + specFile.Name()
specData, err := loadCacheSpec(fileName)
if err != nil {
t.Fatalf("Failed to load contents of file %s: %v", fileDisplayName, err)
for _, dir := range []string{"cachetest", "customcachekeytest", "impcustomcachekeytest", "eventscachetest"} {
if specFiles, err := ioutil.ReadDir(dir); err == nil {
for _, specFile := range specFiles {
fileName := filepath.Join(dir, specFile.Name())
fileDisplayName := "exchange/" + fileName
t.Run(fileDisplayName, func(t *testing.T) {
specData, err := loadCacheSpec(fileName)
if assert.NoError(t, err, "Failed to load contents of file %s: %v", fileDisplayName, err) {
runCacheSpec(t, fileDisplayName, specData)
}
})
}

runCacheSpec(t, fileDisplayName, specData)
}
} else {
t.Fatalf("Failed to read contents of directory exchange/cachetest/: %v", err)
}
}

// TestCacheJSON executes tests for all the *.json files in customcachekeytest.
// customcachekey.json test here verifies custom cache key is used for vast video
func TestCustomCacheKeyJSON(t *testing.T) {
if specFiles, err := ioutil.ReadDir("./customcachekeytest"); err == nil {
for _, specFile := range specFiles {
fileName := "./customcachekeytest/" + specFile.Name()
fileDisplayName := "exchange/customcachekeytest/" + specFile.Name()
specData, err := loadCacheSpec(fileName)
if err != nil {
t.Fatalf("Failed to load contents of file %s: %v", fileDisplayName, err)
}

runCacheSpec(t, fileDisplayName, specData)
}
} else {
t.Fatalf("Failed to read contents of directory exchange/customcachekeytest/: %v", err)
}
}

// TestMultiImpCache executes multi-Imp test cases found in *.json files in
// impcustomcachekeytest.
func TestCustomCacheKeyMultiImp(t *testing.T) {
if specFiles, err := ioutil.ReadDir("./impcustomcachekeytest"); err == nil {
for _, specFile := range specFiles {
fileName := "./impcustomcachekeytest/" + specFile.Name()
fileDisplayName := "exchange/impcustomcachekeytest/" + specFile.Name()
multiImpSpecData, err := loadCacheSpec(fileName)
if err != nil {
t.Fatalf("Failed to load contents of file %s: %v", fileDisplayName, err)
}

runCacheSpec(t, fileDisplayName, multiImpSpecData)
} else {
t.Fatalf("Failed to read contents of directory exchange/%s: %v", dir, err)
}
} else {
t.Fatalf("Failed to read contents of directory exchange/customcachekeytest/: %v", err)
}
}

Expand All @@ -169,9 +134,8 @@ func loadCacheSpec(filename string) (*cacheSpec, error) {
return &spec, nil
}

// runCacheSpec has been modified to handle multi-Imp and multi-bid Json test files,
// it cycles through the bids found in the test cases hardcoded in json files and
// finds the highest bid of every Imp.
// runCacheSpec cycles through the bids found in the json test cases and
// finds the highest bid of every Imp, then tests doCache() with resulting auction object
func runCacheSpec(t *testing.T, fileDisplayName string, specData *cacheSpec) {
var bid *pbsOrtbBid
winningBidsByImp := make(map[string]*pbsOrtbBid)
Expand Down Expand Up @@ -245,7 +209,19 @@ func runCacheSpec(t *testing.T, fileDisplayName string, specData *cacheSpec) {
winningBidsByBidder: winningBidsByBidder,
roundedPrices: roundedPrices,
}
_ = testAuction.doCache(ctx, cache, targData, &specData.BidRequest, 60, &specData.DefaultTTLs, bidCategory, &specData.DebugLog)
evData := &eventsData{
accountID: "ACCOUNT_ID",
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
enabledForAccount: specData.EventsDataEnabledForAccount,
enabledForRequest: specData.EventsDataEnabledForRequest,
externalURL: "http://localhost",
auctionTimestampMs: 1234567890,
bidderInfos: adapters.BidderInfos{
"openx": adapters.BidderInfo{ModifyingVastXmlAllowed: specData.BidderModifyingVastXmlAllowed},
"appnexus": adapters.BidderInfo{ModifyingVastXmlAllowed: specData.BidderModifyingVastXmlAllowed},
"pubmatic": adapters.BidderInfo{ModifyingVastXmlAllowed: specData.BidderModifyingVastXmlAllowed},
},
}
_ = testAuction.doCache(ctx, cache, targData, evData, &specData.BidRequest, 60, &specData.DefaultTTLs, bidCategory, &specData.DebugLog)

if len(specData.ExpectedCacheables) > len(cache.items) {
t.Errorf("%s: [CACHE_ERROR] Less elements were cached than expected \n", fileDisplayName)
Expand All @@ -254,27 +230,43 @@ func runCacheSpec(t *testing.T, fileDisplayName string, specData *cacheSpec) {
} else { // len(specData.ExpectedCacheables) == len(cache.items)
// We cached the exact number of elements we expected, now we compare them side by side in n^2
var matched int = 0
var formattedExpectedData string
for i := 0; i < len(specData.ExpectedCacheables); i++ {
if specData.ExpectedCacheables[i].Type == prebid_cache_client.TypeJSON {
ExpectedData := strings.Replace(string(specData.ExpectedCacheables[i].Data), "\\", "", -1)
ExpectedData = strings.Replace(ExpectedData, " ", "", -1)
formattedExpectedData = ExpectedData[1 : len(ExpectedData)-1]
} else {
formattedExpectedData = string(specData.ExpectedCacheables[i].Data)
for i, expectedCacheable := range specData.ExpectedCacheables {
found := false
var expectedData interface{}
if err := json.Unmarshal(expectedCacheable.Data, &expectedData); err != nil {
t.Fatalf("Failed to decode expectedCacheables[%d].value: %v", i, err)
}
for j := 0; j < len(cache.items); j++ {
if formattedExpectedData == string(cache.items[j].Data) &&
specData.ExpectedCacheables[i].TTLSeconds == cache.items[j].TTLSeconds &&
specData.ExpectedCacheables[i].Type == cache.items[j].Type &&
len(specData.ExpectedCacheables[i].Key) <= len(cache.items[j].Key) &&
specData.ExpectedCacheables[i].Key == cache.items[j].Key[:len(specData.ExpectedCacheables[i].Key)] {
matched++
if s, ok := expectedData.(string); ok && expectedCacheable.Type == prebid_cache_client.TypeJSON {
// decode again if we have pre-encoded json string values
if err := json.Unmarshal([]byte(s), &expectedData); err != nil {
t.Fatalf("Failed to re-decode expectedCacheables[%d].value :%v", i, err)
}
}
for j, cachedItem := range cache.items {
var actualData interface{}
if err := json.Unmarshal(cachedItem.Data, &actualData); err != nil {
t.Fatalf("Failed to decode actual cache[%d].value: %s", j, err)
}
if assert.ObjectsAreEqual(expectedData, actualData) &&
expectedCacheable.TTLSeconds == cachedItem.TTLSeconds &&
expectedCacheable.Type == cachedItem.Type &&
len(expectedCacheable.Key) <= len(cachedItem.Key) &&
expectedCacheable.Key == cachedItem.Key[:len(expectedCacheable.Key)] {
found = true
cache.items = append(cache.items[:j], cache.items[j+1:]...) // remove matched item
break
}
}
if found {
matched++
} else {
t.Errorf("%s: [CACHE_ERROR] Did not see expected cacheable #%d: type=%s, ttl=%d, value=%s", fileDisplayName, i, expectedCacheable.Type, expectedCacheable.TTLSeconds, string(expectedCacheable.Data))
}
}
if matched != len(specData.ExpectedCacheables) {
t.Errorf("%s: [CACHE_ERROR] One or more keys were not cached as we expected \n", fileDisplayName)
for i, item := range cache.items {
t.Errorf("%s: [CACHE_ERROR] Got unexpected cached item #%d: type=%s, ttl=%d, value=%s", fileDisplayName, i, item.Type, item.TTLSeconds, string(item.Data))
}
t.FailNow()
}
}
Expand Down Expand Up @@ -500,15 +492,18 @@ func TestNewAuction(t *testing.T) {
}

type cacheSpec struct {
BidRequest openrtb.BidRequest `json:"bidRequest"`
PbsBids []pbsBid `json:"pbsBids"`
ExpectedCacheables []prebid_cache_client.Cacheable `json:"expectedCacheables"`
DefaultTTLs config.DefaultTTLs `json:"defaultTTLs"`
TargetDataIncludeWinners bool `json:"targetDataIncludeWinners"`
TargetDataIncludeBidderKeys bool `json:"targetDataIncludeBidderKeys"`
TargetDataIncludeCacheBids bool `json:"targetDataIncludeCacheBids"`
TargetDataIncludeCacheVast bool `json:"targetDataIncludeCacheVast"`
DebugLog DebugLog `json:"debugLog,omitempty"`
BidRequest openrtb.BidRequest `json:"bidRequest"`
PbsBids []pbsBid `json:"pbsBids"`
ExpectedCacheables []prebid_cache_client.Cacheable `json:"expectedCacheables"`
DefaultTTLs config.DefaultTTLs `json:"defaultTTLs"`
TargetDataIncludeWinners bool `json:"targetDataIncludeWinners"`
TargetDataIncludeBidderKeys bool `json:"targetDataIncludeBidderKeys"`
TargetDataIncludeCacheBids bool `json:"targetDataIncludeCacheBids"`
TargetDataIncludeCacheVast bool `json:"targetDataIncludeCacheVast"`
EventsDataEnabledForAccount bool `json:"eventsDataEnabledForAccount"`
EventsDataEnabledForRequest bool `json:"eventsDataEnabledForRequest"`
BidderModifyingVastXmlAllowed bool `json:"bidderModifyingVastXMLAllowed`
DebugLog DebugLog `json:"debugLog,omitempty"`
}

type pbsBid struct {
Expand Down
Loading