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

Find bid duration in linear time #2805

Merged
merged 8 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
50 changes: 32 additions & 18 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -1006,24 +1006,11 @@ func applyCategoryMapping(ctx context.Context, targeting openrtb_ext.ExtRequestT

priceBucket = GetPriceBucket(*bid.Bid, *targData)

newDur := duration
if len(targeting.DurationRangeSec) > 0 {
durationRange := make([]int, len(targeting.DurationRangeSec))
copy(durationRange, targeting.DurationRangeSec)
sort.Ints(durationRange)

//if the bid is above the range of the listed durations (and outside the buffer), reject the bid
if duration > durationRange[len(durationRange)-1] {
bidsToRemove = append(bidsToRemove, bidInd)
rejections = updateRejections(rejections, bidID, "Bid duration exceeds maximum allowed")
continue
}
for _, dur := range durationRange {
if duration <= dur {
newDur = dur
break
}
}
newDur, err := findDurationRange(duration, targeting.DurationRangeSec)
if err != nil {
bidsToRemove = append(bidsToRemove, bidInd)
rejections = updateRejections(rejections, bidID, err.Error())
continue
}

var categoryDuration string
Expand Down Expand Up @@ -1113,6 +1100,33 @@ func applyCategoryMapping(ctx context.Context, targeting openrtb_ext.ExtRequestT
return res, seatBids, rejections, nil
}

// findDurationRange returns the element in the array 'durationRanges' that is both greater than 'dur' and closest
// in value to 'dur' unless a value equal to 'dur' is found. Returns an error if all elements in 'durationRanges'
// are less than 'dur'.
func findDurationRange(dur int, durationRanges []int) (int, error) {
newDur := dur
madeSelection := false
var err error

for i := range durationRanges {
if dur > durationRanges[i] {
continue
}
if dur == durationRanges[i] {
return durationRanges[i], nil
}
// dur < durationRanges[i]
if durationRanges[i] < newDur || !madeSelection {
newDur = durationRanges[i]
madeSelection = true
}
}
if !madeSelection && len(durationRanges) > 0 {
err = errors.New("bid duration exceeds maximum allowed")
}
return newDur, err
}

func removeBidById(seatBid *entities.PbsOrtbSeatBid, bidID string) {
//Find index of bid to remove
dupeBidIndex := -1
Expand Down
84 changes: 78 additions & 6 deletions exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"math"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -3188,7 +3189,7 @@ func TestCategoryMappingBidderNameNoCategories(t *testing.T) {
includeWinners: true,
}

requestExt.Prebid.Targeting.DurationRangeSec = []int{15, 30}
requestExt.Prebid.Targeting.DurationRangeSec = []int{30, 10, 25, 5, 20, 50}

adapterBids := make(map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid)

Expand All @@ -3197,8 +3198,8 @@ func TestCategoryMappingBidderNameNoCategories(t *testing.T) {
bid1 := openrtb2.Bid{ID: "bid_id1", ImpID: "imp_id1", Price: 10.0000, Cat: cats1, W: 1, H: 1}
bid2 := openrtb2.Bid{ID: "bid_id2", ImpID: "imp_id2", Price: 12.0000, Cat: cats2, W: 1, H: 1}

bid1_1 := entities.PbsOrtbBid{&bid1, nil, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, nil, nil, 0, false, "", 10.0000, "USD", ""}
bid1_2 := entities.PbsOrtbBid{&bid2, nil, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 30}, nil, nil, 0, false, "", 12.0000, "USD", ""}
bid1_1 := entities.PbsOrtbBid{&bid1, nil, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 17}, nil, nil, 0, false, "", 10.0000, "USD", ""}
bid1_2 := entities.PbsOrtbBid{&bid2, nil, "video", nil, &openrtb_ext.ExtBidPrebidVideo{Duration: 8}, nil, nil, 0, false, "", 12.0000, "USD", ""}

innerBids1 := []*entities.PbsOrtbBid{
&bid1_1,
Expand All @@ -3220,8 +3221,8 @@ func TestCategoryMappingBidderNameNoCategories(t *testing.T) {

assert.NoError(t, err, "Category mapping error should be empty")
assert.Empty(t, rejections, "There should be 0 bid rejection messages")
assert.Equal(t, "10.00_30s_bidder1", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "12.00_30s_bidder2", bidCategory["bid_id2"], "Category mapping doesn't match")
assert.Equal(t, "10.00_20s_bidder1", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "12.00_10s_bidder2", bidCategory["bid_id2"], "Category mapping doesn't match")
assert.Len(t, adapterBids[bidderName1].Bids, 1, "Bidders number doesn't match")
assert.Len(t, adapterBids[bidderName2].Bids, 1, "Bidders number doesn't match")
assert.Len(t, bidCategory, 2, "Bidders category mapping doesn't match")
Expand Down Expand Up @@ -3287,7 +3288,7 @@ func TestBidRejectionErrors(t *testing.T) {
},
duration: 70,
expectedRejections: []string{
"bid rejected [bid ID: bid_id1] reason: Bid duration exceeds maximum allowed",
"bid rejected [bid ID: bid_id1] reason: bid duration exceeds maximum allowed",
},
},
{
Expand Down Expand Up @@ -5641,6 +5642,77 @@ func TestNilAuctionRequest(t *testing.T) {
assert.Nil(t, err)
}

func TestSelectNewDuration(t *testing.T) {
type testInput struct {
dur int
durRanges []int
}
type testOutput struct {
dur int
err error
}
testCases := []struct {
desc string
in testInput
expected testOutput
}{
{
desc: "nil duration range array, don't expect error",
in: testInput{
dur: 1,
durRanges: nil,
},
expected: testOutput{1, nil},
},
{
desc: "empty duration range array, don't expect error",
in: testInput{
dur: 1,
durRanges: []int{},
},
expected: testOutput{1, nil},
},
{
desc: "all duration range array elements less than duration, expect error",
in: testInput{
dur: 5,
durRanges: []int{-1, 0, 1, 2, 3},
},
expected: testOutput{5, errors.New("bid duration exceeds maximum allowed")},
},
{
desc: "all duration range array elements greater than duration, expect smallest element in durRanges and nil error",
in: testInput{
dur: 5,
durRanges: []int{9, math.MaxInt32, 8},
},
expected: testOutput{8, nil},
},
{
desc: "some array elements greater than duration, expect the value greater than dur that is closest in value.",
in: testInput{
dur: 5,
durRanges: []int{math.MaxInt32, -3, 7, 2},
},
expected: testOutput{7, nil},
},
{
desc: "an entry in the duration range array is equal to duration, expect its value in return.",
in: testInput{
dur: 5,
durRanges: []int{-3, math.MaxInt32, 5, 7},
},
expected: testOutput{5, nil},
},
}
for _, tc := range testCases {
newDur, err := findDurationRange(tc.in.dur, tc.in.durRanges)

assert.Equal(t, tc.expected.dur, newDur, tc.desc)
assert.Equal(t, tc.expected.err, err, tc.desc)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good and this change makes sense!
May you please modify existing applyCategoryMapping unit test to test this change in the main flow, for instance:
In TestCategoryMappingBidderNameNoCategories change next lines:
L3185: requestExt.Prebid.Targeting.DurationRangeSec = []int{30, 10, 25, 5, 20, 50}
then input bids (L3194 and 3195):

bid1_1 :=  ...  &openrtb_ext.ExtBidPrebidVideo{Duration: 17} ...
bid1_2 :=  ... &openrtb_ext.ExtBidPrebidVideo{Duration: 8}, ...

And then result asserts should be (L3217 and 3218):

assert.Equal(t, "10.00_20s_bidder1", bidCategory["bid_id1"], "Category mapping doesn't match")
assert.Equal(t, "12.00_10s_bidder2", bidCategory["bid_id2"], "Category mapping doesn't match")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


func TestSetSeatNonBid(t *testing.T) {
type args struct {
bidResponseExt *openrtb_ext.ExtBidResponse
Expand Down
Loading