New Adapter: ReVantage

[v0idxyz]

🔧 Type of changes

• new bid adapter
• bid adapter update
• new feature
• new analytics adapter
• new module
• module update
• bugfix
• documentation
• configuration
• dependency update
• tech debt (test coverage, refactorings, etc.)

✨ What's the context?

What's the context for the changes?

🧠 Rationale behind the change

Why did you choose to make these changes? Were there any trade-offs you had to consider?

🔎 New Bid Adapter Checklist

• verify email contact works
• NO fully dynamic hostnames
• geographic host parameters are NOT required
• direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
• if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
• cover an adapter configuration with an integration test

🧪 Test plan

How do you know the changes are safe to ship to production?

🏎 Quality check

• Are your changes following our code style guidelines?
• Are there any breaking changes in your code?
• Does your test coverage exceed 90%?
• Are there any erroneous console logs, debuggers or leftover code in your changes?

[CTMBNara]

rewriteImpExt -> updateImp, rewrittenImp -> updatedImp

[CTMBNara]

No need for this checks. At this stage, neither the wrapper nor the wrapper.getBidder() can be null

[CTMBNara]

        final ObjectNode newExt = mapper.mapper().createObjectNode();
        newExt.put("feedId", feedId);

        final ObjectNode bidderNode = rewritten.putObject("bidder");

        final String placementId = ext.getPlacementId();
        if (StringUtils.isNotBlank(placementId)) {
            bidderNode.put("placementId", placementId);
        }

        final String publisherId = ext.getPublisherId();
        if (StringUtils.isNotBlank(publisherId)) {
            bidderNode.put("publisherId", publisherId);
        }

        return imp.toBuilder().ext(newExt).build();

[CTMBNara]

Use BidderUtil.defaultRequest instead

[CTMBNara]

List.of -> Collections.emptyList

[CTMBNara]

Refactor this method:

    private static BidType resolveBidType(Bid bid, List<Imp> imps) {
        return bidTypeFromMtype(bid.getMtype())
                .or(() -> bidTypeFromExt(bid.getExt()))
                .or(() -> bidTypeFromAdm(bid.getAdm()))
                .or(() -> bidTypeFromImp(bid.getImpid(), imps))
                .orElseThrow(() -> new PreBidException(
                        "Cannot determine media type for bid %s on imp %s".formatted(bid.getId(), bid.getImpid())));
    }

    private static Optional<BidType> bidTypeFromMtype(Integer mType) {
        return Optional.ofNullable(switch (mType) {
            case 1 -> BidType.banner;
            case 2 -> BidType.video;
            case null, default -> null;
        });
    }

    private static Optional<BidType> bidTypeFromExt(ObjectNode bidExt) {
        return Optional.ofNullable(bidExt)
                .map(ext -> ext.get("mediaType"))
                .filter(JsonNode::isTextual)
                .map(JsonNode::asText)
                .map(String::toLowerCase)
                .map(mediaType -> switch (mediaType) {
                    case "banner" -> BidType.banner;
                    case "video" -> BidType.video;
                    default -> null;
                });
    }

    private static Optional<BidType> bidTypeFromAdm(String adm) {
        if (StringUtils.isBlank(adm)) {
            return Optional.empty();
        }

        final String trimmed = adm.trim().toUpperCase();
        return trimmed.startsWith("<VAST") || trimmed.startsWith("<?XML")
                ? Optional.of(BidType.video)
                : Optional.empty();
    }

    private static Optional<BidType> bidTypeFromImp(String impId, List<Imp> imps) {
        for (Imp imp : imps) {
            if (!Objects.equals(imp.getId(), impId)) {
                continue;
            }

            final boolean hasBanner = imp.getBanner() != null;
            final boolean hasVideo = imp.getVideo() != null;
            if (hasVideo && !hasBanner) {
                return Optional.of(BidType.video);
            }
            if (hasBanner) {
                return Optional.of(BidType.banner);
            }

            break;
        }

        return Optional.empty();
    }

[CTMBNara]

No need for @Validated

[bukrain]

nitpick
I think we don't need to throw an exception here, we could just add to the errors list, when we don't have the feedId.
Something like

Suggested change

	try {
	final ExtImpRevantage ext = parseImpExt(imp);
	final String feedId = StringUtils.trimToNull(ext.getFeedId());
	if (feedId == null) {
	throw new PreBidException("imp %s: missing required param feedId".formatted(imp.getId()));
	}
	final Imp rewrittenImp = rewriteImpExt(imp, feedId, ext);
	impsByFeed.computeIfAbsent(feedId, k -> new ArrayList<>()).add(rewrittenImp);
	} catch (PreBidException e) {
	errors.add(BidderError.badInput(e.getMessage()));
	}
	}
	final ExtImpRevantage ext = parseImpExt(imp);
	final String feedId = StringUtils.trimToNull(ext.getFeedId());
	if (feedId == null) {
	errors.add(BidderError.badInput("imp %s: missing required param feedId".formatted(imp.getId())));
	continue;
	}
	final Imp rewrittenImp = rewriteImpExt(imp, feedId, ext);
	impsByFeed.computeIfAbsent(feedId, k -> new ArrayList<>()).add(rewrittenImp);

[bukrain]

As null is expected in the body, maybe it would make sense to first check if it's not null, since this test, doesn't seem to pass.
Also can the body of the response be null when makeBids is called? Would expect for it to be an empty string "".
@CTMBNara what do you think?

[bukrain]

nitpick
I would suggest to add a test cases for the adm that starts with <?XML and also for the case that adm is empty that it would be marked as an mtype of banner.