-
Notifications
You must be signed in to change notification settings - Fork 224
Teal: new adapter #4350
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
base: master
Are you sure you want to change the base?
Teal: new adapter #4350
Conversation
| }; | ||
| private final String endpointUrl; |
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.
Add an empty line between these lines
|
|
||
| this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl)); | ||
| this.mapper = Objects.requireNonNull(mapper); | ||
| } |
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.
public TealBidder(String endpointUrl, JacksonMapper mapper) {
this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl));
this.mapper = Objects.requireNonNull(mapper);
}
| return !StringUtils.isBlank(parameter); | ||
| } |
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.
Inline this method
| final ExtImpTeal params; | ||
| try { | ||
| params = parseImpExt(request.getImp().getFirst()); | ||
| } catch (PreBidException e) { | ||
| return Result.withError(BidderError.badInput(e.getMessage())); | ||
| } | ||
| final String account = params.getAccount(); | ||
| if (!isParameterValid(account)) { | ||
| return Result.withError(BidderError.badInput("account parameter failed validation")); | ||
| } | ||
|
|
||
| final List<Imp> modifiedImps = new ArrayList<>(); | ||
| final List<BidderError> errors = new ArrayList<>(); | ||
|
|
||
| for (Imp imp : request.getImp()) { | ||
| final ExtImpTeal impParams; | ||
| try { | ||
| impParams = parseImpExt(imp); | ||
| } catch (PreBidException e) { | ||
| errors.add(BidderError.badInput(e.getMessage())); | ||
| continue; | ||
| } | ||
|
|
||
| final String placement = impParams.getPlacement(); | ||
| final boolean passedValidation = placement == null || isParameterValid(placement); | ||
|
|
||
| if (placement != null && !passedValidation) { | ||
| errors.add(BidderError.badInput("placement parameter failed validation")); | ||
| continue; | ||
| } | ||
|
|
||
| if (placement != null) { | ||
| final ObjectNode ext = Optional.ofNullable(imp.getExt()).orElse(mapper.mapper().createObjectNode()); | ||
| final ObjectNode prebid = ext.has("prebid") && ext.get("prebid").isObject() | ||
| ? (ObjectNode) ext.get("prebid") | ||
| : ext.putObject("prebid"); | ||
| prebid.putObject("storedrequest").put("id", placement); | ||
| modifiedImps.add(imp.toBuilder().ext(ext).build()); | ||
| } else { | ||
| modifiedImps.add(imp); | ||
| } | ||
| } | ||
|
|
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.
Since the 'account' property is marked as 'required' in the 'teal.json' file, it will be present in every imp object, so we can move the validation and reading of this parameter.
If 'required' was added by mistake and the parameter is expected only in the first imp, then move the account validation so that it appears after the loop.
@Override
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) {
final List<Imp> modifiedImps = new ArrayList<>();
final List<BidderError> errors = new ArrayList<>();
String account = null;
for (Imp imp : request.getImp()) {
final ExtImpTeal extImpTeal;
try {
extImpTeal = parseImpExt(imp);
validateImpExt(extImpTeal);
} catch (PreBidException e) {
errors.add(BidderError.badInput(e.getMessage()));
continue;
}
account = account == null ? extImpTeal.getAccount() : account;
modifiedImps.add(modifyImp(imp, extImpTeal.getPlacement()));
}
if (modifiedImps.isEmpty()) {
return Result.withErrors(errors);
}
final BidRequest modifiedRequest = modifyBidRequest(request, account, modifiedImps);
return Result.of(
Collections.singletonList(BidderUtil.defaultRequest(modifiedRequest, endpointUrl, mapper)),
errors);
}
private ExtImpTeal parseImpExt(Imp imp) {
try {
return mapper.mapper().convertValue(imp.getExt(), TYPE_REFERENCE).getBidder();
} catch (IllegalArgumentException e) {
throw new PreBidException("Error parsing imp.ext for impression " + imp.getId());
}
}
private static void validateImpExt(ExtImpTeal extImpTeal) {
if (StringUtils.isBlank(extImpTeal.getAccount())) {
throw new PreBidException("account parameter failed validation");
}
final String placement = extImpTeal.getPlacement();
if (placement != null && StringUtils.isBlank(placement)) {
throw new PreBidException("placement parameter failed validation");
}
}
private static Imp modifyImp(Imp imp, String placement) {
if (placement == null) {
return imp;
}
final ObjectNode modifiedExt = imp.getExt().deepCopy();
getOrCreate(getOrCreate(modifiedExt, "prebid"), "storedrequest")
.put("id", placement);
return imp.toBuilder().ext(modifiedExt).build();
}
private static ObjectNode getOrCreate(ObjectNode parent, String field) {
final JsonNode child = parent.get(field);
return child != null && child.isObject()
? (ObjectNode) child
: parent.putObject(field);
}
| private BidRequest enrichRequest(BidRequest request, String account, List<Imp> modifiedImps) { | ||
| final ExtRequest ext = Optional.ofNullable(request.getExt()).orElse(ExtRequest.empty()); | ||
| final ObjectNode bids = mapper.mapper().createObjectNode(); | ||
| bids.put("pbs", 1); | ||
| ext.addProperty("bids", bids); | ||
| return request.toBuilder() | ||
| .site(modifySite(request.getSite(), account)) | ||
| .app(modifyApp(request.getApp(), account)) | ||
| .imp(modifiedImps) | ||
| .ext(ext) | ||
| .build(); | ||
| } |
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.
private BidRequest modifyBidRequest(BidRequest request, String account, List<Imp> modifiedImps) {
final ExtRequest ext = ObjectUtils.defaultIfNull(request.getExt(), ExtRequest.empty());
ext.addProperty("bids", mapper.mapper().createObjectNode().put("pbs", 1));
return request.toBuilder()
.site(modifySite(request.getSite(), account))
.app(modifyApp(request.getApp(), account))
.imp(modifiedImps)
.ext(ext)
.build();
}
| try { | ||
| final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class); | ||
| return Result.of(extractBids(httpCall.getRequest().getPayload(), bidResponse), Collections.emptyList()); | ||
| } catch (DecodeException | PreBidException e) { |
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.
Remove PreBidException
| .flatMap(Collection::stream) | ||
| .map(bid -> BidderBid.of(bid, getBidType(bid.getImpid(), bidRequest.getImp()), bidResponse.getCur())) |
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.
add .filter(Objects::nonNull) between these lines
| .filter(Objects::nonNull) | ||
| .flatMap(Collection::stream) | ||
| .map(bid -> BidderBid.of(bid, getBidType(bid.getImpid(), bidRequest.getImp()), bidResponse.getCur())) | ||
| .collect(Collectors.toList()); |
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.
.collect(Collectors.toList()); -> .toList()
| @JsonProperty("account") | ||
| String account; | ||
|
|
||
| @JsonProperty("placement") | ||
| String placement; |
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.
No need for JsonProperty annotations here
| BidderDeps tealBidderDeps(BidderConfigurationProperties tealConfigurationProperties, | ||
| @NotBlank @Value("${external-url}") String externalUrl, | ||
| JacksonMapper mapper) { |
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.
Align parameters
|
@CTMBNara many thanks for your thorough review. I hope each of your points is now resolved - thank you for providing the code. To confirm, the account property is required. |
🔧 Type of changes
✨ What's the context?
Bid adapter for Teal SSP
🧠 Rationale behind the change
Enable publishers to access Teal demand via Prebid Server.
Needs its own adapter as parameters and custom ext fields required.
🔎 New Bid Adapter Checklist
🧪 Test plan
Has been tested on Teal's own fork of Prebid Server Java
🏎 Quality check