New Adapter: TradPlus #3361
New Adapter: TradPlus #3361
Conversation
| private ExtImpTradPlus parseAndValidateImpExt(ObjectNode extNode) { | ||
| final ExtImpTradPlus extImpTradPlus; | ||
| try { | ||
| extImpTradPlus = mapper.mapper().convertValue(extNode, EXT_TYPE_REFERENCE).getBidder(); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new PreBidException(e.getMessage()); | ||
| } | ||
| if (StringUtils.isBlank(extImpTradPlus.getAccountId())) { | ||
| throw new PreBidException("Invalid/Missing AccountID"); | ||
| } | ||
|
|
||
| return extImpTradPlus; |
There was a problem hiding this comment.
please split into two methods: parseImpExt and validateImpExt
| final ExtImpTradPlus extImpTradPlus = parseAndValidateImpExt(imp.getExt()); | ||
| extToImps.computeIfAbsent(extImpTradPlus, ext -> new ArrayList<>()).add(imp); | ||
| } catch (PreBidException e) { | ||
| return Result.withError(BidderError.badInput(e.getMessage())); |
There was a problem hiding this comment.
in order to confirm: is it required to fail all the requests because of a single one invalid extImp?
| .map(entry -> makeHttpRequest(entry.getKey(), entry.getValue(), bidRequest)) | ||
| .toList(); | ||
|
|
||
| return Result.of(httpRequests, Collections.emptyList()); |
There was a problem hiding this comment.
Result.withValues(...)
| private static List<Imp> removeFirstImpExt(List<Imp> imps) { | ||
| return IntStream.range(0, imps.size()) | ||
| .mapToObj(impIndex -> impIndex == 0 | ||
| ? imps.get(impIndex).toBuilder().ext(null).build() | ||
| : imps.get(impIndex)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
imps.set(0, imps.getFirst().toBuilder().ext(null).build()
| @Test | ||
| public void makeHttpRequestsShouldReturnErrorWhenSourceIdIsNull() { | ||
| // given | ||
| final BidRequest bidRequest = givenBidRequest(ExtImpTradPlus.of(null, "zoneId")); | ||
|
|
||
| // when | ||
| final Result<List<HttpRequest<BidRequest>>> result = target.makeHttpRequests(bidRequest); | ||
|
|
||
| // then | ||
| assertThat(result.getValue()).isEmpty(); | ||
| assertThat(result.getErrors()).hasSize(1) | ||
| .containsOnly(BidderError.badInput("Invalid/Missing AccountID")); | ||
| } |
There was a problem hiding this comment.
accoding to the json scheme null is impossible as an accountId value, so it make sense to check when accountId is blank " "
|
|
||
| private final TradPlusBidder target = new TradPlusBidder(ENDPOINT_TEMPLATE, jacksonMapper); | ||
|
|
||
| @Test |
There was a problem hiding this comment.
- check also headers, impIds
- Also the payload of the requests is never checked
- Also the imps grouping logic is not covered, as well as the clean-up of the very first impExt for the request
| BidRequest.builder() | ||
| .imp(singletonList(Imp.builder().id("123").build())) | ||
| .build(), | ||
| mapper.writeValueAsString( |
There was a problem hiding this comment.
hide the mapper.writeValueAsString inside the givenBidResponse since it's the duplicate code
| @Test | ||
| public void openrtb2AuctionShouldRespondWithBidsFromTradPlus() throws IOException, JSONException { | ||
| // given | ||
| WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/tradplus-exchange")) |
There was a problem hiding this comment.
please chech here that the marcos replacement for the url is working
|
@tradplus, could you please address the comments mentioned above? |
|
@tradplus There are conflicts in this PR. In addition to addressing the comments above, could you please merge the master branch as well? |
|
PR is inactive for more than 3-4 weeks. Therefore, closing this PR. @tradplus, reopen this PR after addressing open comments or making code changes. |
|
the issue has been fixed, please reopen this PR. |
|
A PR has been recreated：#3508. |
TradPlus is a leading, stable, and reliable ad monetization platform that is committed to providing global developers with fair, transparent and efficient monetization solutions. We also provide ADX and Saas ADX services. We serve 2,000+ global developers. Moreover, Our SDK has been certified by IAB Tech Lab and also joins Google Play SDK Index. And we have Information Security Management Certification.
Our advertising business spans the world. This is our official website: https://tradplusad.com/en. You can get more details about TradPlus.
this is TradPlus adapter version 1.0.
Thank!