OMS: add video support #3779

przemkaczmarek · 2025-02-24T13:38:51Z

🔧 Type of changes

bid adapter update

✨ What's the context?

AntoxaAntoxic · 2025-03-05T07:59:54Z

src/main/java/org/prebid/server/proto/openrtb/ext/request/omx/ExtImpOms.java

+
+    String pid;
+
+    Integer publisherId;


add JsonProperty

also I would change the integration test payload to cover publisherId parameter

I have cheanged pid to publisherId but IMO its indifferent

the pid is declared as deprecated

I meant, it makes sense to add a publisherId to the test instead of deprecated pid, but pid is still can be used, it's deprecated, not removed, please revert this change

AntoxaAntoxic · 2025-03-05T08:14:51Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

+    public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest request) {
+        String uri = endpointUrl;
+
+        if (!request.getImp().isEmpty()) {
+            try {
+                final ExtImpOms impExt = parseImpExt(request.getImp().get(0));
+                if (impExt != null) {
+                    if (impExt.getPid() != null && !impExt.getPid().isEmpty()) {
+                        uri = String.format("%s?publisherId=%s", endpointUrl, impExt.getPid());
+                    } else if (impExt.getPublisherId() != null && impExt.getPublisherId() > 0) {
+                        uri = String.format("%s?publisherId=%d", endpointUrl, impExt.getPublisherId());
+                    }
+                }
+            } catch (PreBidException e) {
+                return Result.withError(BidderError.badInput(e.getMessage()));
+            }
+        }
+
+        return Result.withValue(BidderUtil.defaultRequest(request, uri, mapper));
+    }


I believe it can work this way

try { final ExtImpOms impExt = parseImpExt(request.getImp().getFirst()); final String publisherId = impExt.getPid() == null && impExt.getPublisherId() != null && impExt.getPublisherId() > 0 ? String.valueOf(impExt.getPublisherId()) : impExt.getPid(); final String url = "%s?publisherId=%s".formatted(endpointUrl, publisherId); return Result.withValue(BidderUtil.defaultRequest(request, url, mapper)); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); }

AntoxaAntoxic · 2025-03-05T08:16:39Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

@@ -59,7 +91,15 @@ private static List<BidderBid> bidsFromResponse(BidResponse bidResponse) {
                .map(SeatBid::getBid)
                .filter(Objects::nonNull)
                .flatMap(Collection::stream)
-                .map(bid -> BidderBid.of(bid, BidType.banner, bidResponse.getCur()))
+                .map(bid -> BidderBid.of(bid, getBidType(bid.getMtype()), bidResponse.getCur()))


the following is not ported

func getBidVideo(bidType openrtb_ext.BidType, bid *openrtb2.Bid) *openrtb_ext.ExtBidPrebidVideo { if bidType != openrtb_ext.BidTypeVideo { return nil } var primaryCategory string if len(bid.Cat) > 0 { primaryCategory = bid.Cat[0] } return &openrtb_ext.ExtBidPrebidVideo{ Duration: int(bid.Dur), PrimaryCategory: primaryCategory, } }

AntoxaAntoxic · 2025-03-10T14:31:06Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

+        return catNotEmpty || durationValid
+                ? ExtBidPrebidVideo.of(
+                durationValid ? duration : null,
+                catNotEmpty ? cat.getFirst() : null)
+                : null;


In Go they always create a video object

also a duration validation seems redundant (again, according to Go)

AntoxaAntoxic · 2025-03-10T14:32:58Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

+        final Integer markupType = bid.getMtype();
+        return switch (markupType) {
            case 1 -> BidType.banner;
            case 2 -> BidType.video;
-            case null, default -> throw new PreBidException("Unsupported mType " + mType);
+            case null, default -> BidType.banner;


return Objects.equals(bid.getMtype(), 2) ? BidType.video : BidType.banner;

AntoxaAntoxic · 2025-03-10T14:34:03Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

        };
    }
+
+    private static ExtBidPrebidVideo videoInfo(Bid bid) {
+        if (!Integer.valueOf(2).equals(bid.getMtype())) {


I'd use a bid type resolved on the previous step

AntoxaAntoxic · 2025-03-10T14:34:33Z

src/main/java/org/prebid/server/proto/openrtb/ext/request/omx/ExtImpOms.java

+
+    String pid;
+
+    Integer publisherId;


the pid is declared as deprecated

AntoxaAntoxic · 2025-03-10T14:35:51Z

src/main/java/org/prebid/server/proto/openrtb/ext/request/omx/ExtImpOms.java

+public class ExtImpOms {
+
+    String pid;
+    @JsonProperty("publisherId")


add an empty line

AntoxaAntoxic · 2025-03-10T15:30:23Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

+                final String publisherId = impExt.getPublisherId() != null && impExt.getPublisherId() > 0
+                        ? String.valueOf(impExt.getPublisherId()) : null;


why did you change it? I believe in the last commit it was correct

because U said that pid is deprecated

deprecated doesn't mean removed

so i should add only annotation @deprecated on pid filed ?

the initial point was about using publisherId in the integration test instead of pid, that's it, everything else was fine, the pid is still supported

In the integration test I used only publisherId.
I used pid only in unit test.
Should it look like this?

AntoxaAntoxic · 2025-03-10T15:35:41Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

-                : null;
+        return ExtBidPrebidVideo.of(
+                duration != null ? duration : 0,
+                CollectionUtils.isNotEmpty(cat) ? cat.getFirst() : ""


StringUtils.EMPTY

AntoxaAntoxic · 2025-03-10T15:36:42Z

src/main/java/org/prebid/server/proto/openrtb/ext/request/omx/ExtImpOms.java

@@ -6,7 +6,6 @@
 @Value(staticConstructor = "of")
 public class ExtImpOms {

-    String pid;


do not need to remove, since it's present in Go

AntoxaAntoxic · 2025-03-10T15:37:35Z

src/main/java/org/prebid/server/proto/openrtb/ext/request/omx/ExtImpOms.java

+
+    String pid;
+
+    Integer publisherId;


I meant, it makes sense to add a publisherId to the test instead of deprecated pid, but pid is still can be used, it's deprecated, not removed, please revert this change

AntoxaAntoxic · 2025-03-11T12:38:29Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

    }

    @Override
    public final Result<List<BidderBid>> makeBids(BidderCall<BidRequest> httpCall, BidRequest bidRequest) {
        try {
            final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class);
            return Result.withValues(extractBids(bidResponse));
-        } catch (DecodeException e) {
+        } catch (DecodeException | PreBidException e) {


Is PrebidException expected anywhere?

AntoxaAntoxic · 2025-03-11T12:39:17Z

src/main/java/org/prebid/server/bidder/oms/OmsBidder.java

+        final Integer duration = bid.getDur();
+
+        return ExtBidPrebidVideo.of(
+                duration != null ? duration : 0,


ObjectUtils.defaultIfNull

OMS: Video support #3770

d87670e

przemkaczmarek added the do not port label Feb 24, 2025

osulzhenko assigned przemkaczmarek Feb 25, 2025

osulzhenko linked an issue Feb 25, 2025 that may be closed by this pull request

Port PR from PBS-Go: OMS: Video support #3770

Open

osulzhenko changed the title ~~OMS: Video support #3770~~ OMS: add video support Feb 28, 2025

CTMBNara requested a review from AntoxaAntoxic March 4, 2025 17:19

AntoxaAntoxic requested changes Mar 5, 2025

View reviewed changes

fix comments

6459db5

osulzhenko requested a review from AntoxaAntoxic March 10, 2025 10:38

AntoxaAntoxic requested changes Mar 10, 2025

View reviewed changes

fix comments

f32d656

AntoxaAntoxic requested changes Mar 10, 2025

View reviewed changes

fix comments

ed5230c

AntoxaAntoxic reviewed Mar 11, 2025

View reviewed changes

fix comments

057d343

AntoxaAntoxic approved these changes Mar 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OMS: add video support #3779

OMS: add video support #3779

przemkaczmarek commented Feb 24, 2025

AntoxaAntoxic Mar 5, 2025

przemkaczmarek Mar 5, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025 •

edited

Loading

AntoxaAntoxic Mar 5, 2025

AntoxaAntoxic Mar 5, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

przemkaczmarek Mar 10, 2025 •

edited

Loading

AntoxaAntoxic Mar 10, 2025

przemkaczmarek Mar 10, 2025

AntoxaAntoxic Mar 11, 2025

przemkaczmarek Mar 11, 2025

AntoxaAntoxic Mar 11, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025

AntoxaAntoxic Mar 10, 2025 •

edited

Loading

AntoxaAntoxic Mar 11, 2025

AntoxaAntoxic Mar 11, 2025

		final String publisherId = impExt.getPublisherId() != null && impExt.getPublisherId() > 0
		? String.valueOf(impExt.getPublisherId()) : null;

OMS: add video support #3779

Are you sure you want to change the base?

OMS: add video support #3779

Conversation

przemkaczmarek commented Feb 24, 2025

🔧 Type of changes

✨ What's the context?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AntoxaAntoxic Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

przemkaczmarek Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AntoxaAntoxic Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AntoxaAntoxic Mar 10, 2025 •

edited

Loading

przemkaczmarek Mar 10, 2025 •

edited

Loading

AntoxaAntoxic Mar 10, 2025 •

edited

Loading