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.

Sign up for GitHub

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

Jump to bottom

Correct GetCpmStringValue's second return value #1520

Merged
merged 9 commits into from
Oct 15, 2020
Merged

Correct GetCpmStringValue's second return value #1520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0916618
First draft
Oct 2, 2020
b5f85fc
Correctinos after feedback
Oct 8, 2020
601ffd4
legacy endpoint issue
Oct 8, 2020
6a05aad
Rolledback bucketMax if statement
Oct 8, 2020
df76541
set the MaxDecimalFigures to 15
Oct 14, 2020
61365d0
Return error if precision is greater than 15
Oct 14, 2020
388a8a0
Merge branch 'master' into PBS-768
Oct 14, 2020
0ecbfed
Correct maximum precision corner case
Oct 15, 2020
69a2ded
Removed leftover assertion
Oct 15, 2020
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
8 changes: 3 additions & 5 deletions openrtb_ext/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,9 @@ func (pg *PriceGranularity) UnmarshalJSON(b []byte) error {
}
if pgraw.Precision < 0 {
return errors.New("Price granularity error: precision must be non-negative")
} else if pgraw.Precision > MaxDecimalFigures {
pgraw.Precision = MaxDecimalFigures
}
if pgraw.Precision > MaxDecimalFigures {
return errors.New("Price granularity error: precision of more than 15 significant figures is not supported")
}
if len(pgraw.Ranges) > 0 {
var prevMax float64 = 0
Expand All @@ -192,9 +193,6 @@ func (pg *PriceGranularity) UnmarshalJSON(b []byte) error {
}
// Enforce that we don't read "min" from the request
pgraw.Ranges[i].Min = prevMax
if pgraw.Ranges[i].Min < prevMax {
return errors.New("Price granularity error: overlapping granularity ranges")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we are ignoring "min" json field

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. That condition will never be met with the pgraw.Ranges[i].Min = prevMax line right before it.

prevMax = gr.Max
}
*pg = PriceGranularity(pgraw)
Expand Down
52 changes: 37 additions & 15 deletions openrtb_ext/request_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,43 @@ var validGranularityTests []granularityTestData = []granularityTestData{
}

func TestGranularityUnmarshalBad(t *testing.T) {
tests := [][]byte{
[]byte(`[]`),
[]byte(`{"precision": -1, "ranges": [{"max":20, "increment":0.5}]}`),
[]byte(`{"ranges":[{"max":20, "increment": -1}]}`),
[]byte(`{"ranges":[{"max":"20", "increment": "0.1"}]}`),
[]byte(`{"ranges":[{"max":20, "increment":0.1}. {"max":10, "increment":0.02}]}`),
[]byte(`{"ranges":[{"max":20, "min":10, "increment": 0.1}, {"max":10, "min":0, "increment":0.05}]}`),
[]byte(`{"ranges":[{"max":1.0, "increment": 0.07}, {"max" 1.0, "increment": 0.03}]}`),
testCases := []struct {
description string
jsonPriceGranularity []byte
}{
{
"Malformed",
[]byte(`[]`),
},
{
"Negative precision",
[]byte(`{"precision": -1, "ranges": [{"max":20, "increment":0.5}]}`),
},
{
"Precision greater than MaxDecimalFigures supported",
[]byte(`{"precision": 16, "ranges": [{"max":20, "increment":0.5}]}`),
},
{
"Negative increment",
[]byte(`{"ranges":[{"max":20, "increment": -1}]}`),
},
{
"Range with non float64 max value",
[]byte(`{"ranges":[{"max":"20", "increment": "0.1"}]}`),
},
{
"Ranges in decreasing order",
[]byte(`{"ranges":[{"max":20, "increment":0.1}. {"max":10, "increment":0.02}]}`),
},
{
"Max equal to previous max",
[]byte(`{"ranges":[{"max":1.0, "increment": 0.07}, {"max" 1.0, "increment": 0.03}]}`),
},
}
var resolved PriceGranularity
for _, b := range tests {
resolved = PriceGranularity{}
err := json.Unmarshal(b, &resolved)
if err == nil {
t.Errorf("Invalid granularity unmarshalled without error.\nJSON was: %s\n Resolved to: %v", string(b), resolved)
}

for _, test := range testCases {
resolved := PriceGranularity{}
err := json.Unmarshal(test.jsonPriceGranularity, &resolved)
assert.Errorf(t, err, "Invalid granularity unmarshalled without error.\nJSON was: %s\n Resolved to: %v. Test: %s", string(test.jsonPriceGranularity), resolved, test.description)
}
}