-
Notifications
You must be signed in to change notification settings - Fork 187
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
Sovrn: Accept Imp.ext Bidfloor either as a number or string #3484
Conversation
private static BigDecimal resolveBidFloor(BigDecimal impBidFloor, JsonNode extBidFloor) { | ||
final BigDecimal fromExt = parseBidFloor(extBidFloor); | ||
return !BidderUtil.isValidPrice(impBidFloor) && BidderUtil.isValidPrice(fromExt) ? fromExt : impBidFloor; | ||
} | ||
|
||
private static BigDecimal parseBidFloor(JsonNode extBidFloor) { | ||
if (extBidFloor != null) { | ||
if (extBidFloor.isNumber()) { | ||
return extBidFloor.decimalValue(); | ||
} else if (extBidFloor.isTextual()) { | ||
try { | ||
return new BigDecimal(extBidFloor.textValue()); | ||
} catch (NumberFormatException e) { | ||
return BigDecimal.ZERO; | ||
} | ||
} | ||
} | ||
return BigDecimal.ZERO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can revert this entirely as well as reverting the type change in the ExtImpSovrn, since both a string and a number will be successfully deserialized to BigDecimal
the only change you need is changing the json scheme, everything else should work fine
in order to test whether it works you can play with the integration test SovrnTest
by changing values in the request json to double-check it works as intended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for making it simpler.
@@ -16,7 +15,7 @@ public class ExtImpSovrn { | |||
@JsonProperty("tagId") | |||
String legacyTagId; | |||
|
|||
BigDecimal bidfloor; | |||
JsonNode bidfloor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
🔧 Type of changes
✨ What's the context?
purpose of this commit is to fix this issue : prebid/prebid-server#3514
🧠 Rationale behind the change
🧪 Test plan
🏎 Quality check