OMS: add video support by przemkaczmarek · Pull Request #3779 · prebid/prebid-server-java

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

🔧 Type of changes

bid adapter update

✨ What's the context?

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

+
+    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

+    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

                .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

+        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

+        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

    }
+
+    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

+
+    String pid;
+
+    Integer publisherId;


the pid is declared as deprecated

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

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


add an empty line

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

+                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

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


StringUtils.EMPTY

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

 @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

+
+    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

            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

+        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

Closed

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 suggested changes Mar 5, 2025

View reviewed changes

fix comments

6459db5

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

AntoxaAntoxic suggested changes Mar 10, 2025

View reviewed changes

fix comments

f32d656

AntoxaAntoxic suggested changes Mar 10, 2025

View reviewed changes

fix comments

ed5230c

AntoxaAntoxic reviewed Mar 11, 2025

View reviewed changes

fix comments

057d343

AntoxaAntoxic previously approved these changes Mar 12, 2025

View reviewed changes

CTMBNara requested changes Mar 17, 2025

View reviewed changes

fix comments

2d95af3

przemkaczmarek dismissed AntoxaAntoxic’s stale review via 2d95af3 March 19, 2025 10:56

osulzhenko requested a review from CTMBNara March 28, 2025 17:01

CTMBNara requested changes Apr 1, 2025

View reviewed changes

Comment thread src/main/java/org/prebid/server/bidder/oms/OmsBidder.java Outdated

fix comments

cef4cb6

osulzhenko requested a review from CTMBNara April 11, 2025 05:49

CTMBNara approved these changes Apr 17, 2025

View reviewed changes

CTMBNara merged commit 10a041a into master Apr 17, 2025
7 of 8 checks passed

CTMBNara deleted the OMS-Video-support-#3770 branch April 17, 2025 10:51

riteshghodrao pushed a commit to riteshghodrao/prebid-server-java that referenced this pull request Apr 25, 2026

OMS: add video support (prebid#3779)

67427a9

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

Conversation

przemkaczmarek commented Feb 24, 2025

🔧 Type of changes

✨ What's the context?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AntoxaAntoxic Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

przemkaczmarek Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AntoxaAntoxic Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AntoxaAntoxic Mar 10, 2025 •

edited

Loading

przemkaczmarek Mar 10, 2025 •

edited

Loading

AntoxaAntoxic Mar 10, 2025 •

edited

Loading