generate bid.adId uniquely instead of using bidRequest.bidId

[jsnellbaker]

Type of change

• Refactoring (no functional changes, no api changes)
• Other

Description of change

This PR modifies the manner in which bid.adId is generated in bidfactory.js. Previously it was directly copied from the bidRequest.bidId and if that wasn't available a new string was generated.

The new logic is:

• we always generate a new string value for the bid.adId
• the bidRequest.bidId is set to the bid.requestId

Reason for change

The reason for this change is to help allow multiple bid responses be associated to the same bid object. When the bid.adId was set equal to the bidRequest.bidId, it implied a 1:1 relationship between the two objects. Given some of the upcoming changes related to #3379, we want prebid be able to support having multiple bid responses associated to a single bidRequest.bids object. While the rest of core would support this type of association, the bid.adId would be copied into the multiple bid response objects, which would cause issues when trying to render the bid (as it would grab the first bid that matched rather than the bid that won). By generating unique strings, it addresses this problem.

Additional context/information

The bid.requestId is meant to be the tie between the bid response object and the bidRequest.bids object. The bid.requestId is a typically populated from adapter files when they're creating the initial bid object (ie when they're reading their adserver's bid response). So the bid.requestId we're setting in the bidfactory.js is meant to be a back-up in the rare case some adapters don't set the value (like in serverbidServerBidAdapter.js).

The changes to the function calls for getBidRequest() are to line up with the above logic that bid.requestId is the lookup field.

The change in the getBidRequest() function is to prevent some false positive results for the unit tests; an undefined id value would match the inner some check if one of those sub-fields (eg bid_id) didn't exist in the sample/test request object.

[harpere]

one comment that we might want to address. Otherwise LGTM.

[harpere]

I noticed that the currency file calls bidfactory.createBid() when conversions fail. There might be some (minor) repercussions since the adId will no longer be maintained in the new bid. Might want to at least change this line in the currency module, from bidId: bid.adId to bidId: bid.requestId to maintain the requestId - though that isn't maintained currently.

[jsnellbaker]

Thanks for the feedback regarding the currency file. I pushed an update for that reference.

[jaiminpanchal27]

LGTM.
Just one thought, we need to make sure that we only accept multiple bid responses for longform mediaType.

[jsnellbaker]

As an update in regards to previous comment - based on some internal discussion, we will not be limiting this to only adpod (formerly longform) video mediaTypes.

A 1:1 relationship between request.bids and the bid responses was not a strictly enforced requirement from the code-perspective; so we're not going enforce it now as part of this change.