multiformat size validation checks

[jsnellbaker]

Type of change

• Feature

Description of change

Add new checks to validate/support declaring size attributes within the mediaType.banner|video|native objects and using these values in place of the adUnits.sizes value. Summary of changes/expected usage (for testing) for each mediaType is listed below:

• mediaTypes.banner.sizes accepts an array of sizes (either [300, 250] or [[300, 250],[300, 600]]). If the mediaTypes.banner object is present, but sizes is left out - the banner object gets rejected and is removed from the request. At this point, the request would fall back to original behavior (where it assumes a request with no mediaTypes object is a banner); note it will use the adUnits.sizes value in this case. As an additional note - banner width/height sizes must be defined within the bid response object. This validation already exists within the bidderFactory.js file, so no additional changes were made in support of this requirement.

• mediaTypes.video.playerSize accepts a single array for the WxH of the player (ie [640, 480]). This is an optional field for the mediaTypes.video object, but when it's declared it can only contain one size. If this setting is present, this size will replace the original adUnits.sizes to ensure the player ignores the original value. If the playerSize is misconfigured, we will reject the mediaTypes.video.playerSize attribute from the request (the rest of the video object remains). We are not validating video bid response to have a defined width/height.

• mediaTypes.native.image.sizes and/or mediaTypes.native.image.aspect_ratios each accept an array of sizes (either single or multiple like banner). Either of these attributes can be present, both can be present, or neither can be present within the mediaTypes.native.image object. If they are present, we are simply validating they are using an array declaration. If they are not configured correctly, then the mediaTypes.native.image.sizes|aspect_ratios attribute is removed from the request (the rest of the native.image object remains).

• mediaTypes.native.icon.sizes can accept an array of sizes (either single or multiple). This is an optional field. The same logic done for the mediaTypes.native.image applies here as well.

• For both mediaTypes.native.image and mediaTypes.native.icon objects, we are validating the bid response has a corresponding defined height/width (if the image/icon objects exists). If the dimensions are not present in the supplied object, the bid is rejected. Currently the value in adUnits.sizes is supplied in the request, but it's basically ignored in the subsequent bid response. This functionality hasn't been altered.

• Added a deprecation warning message when the adUnits.sizes field is defined/used in the original request object. The message currently states:
  Usage of adUnits.sizes will eventually be deprecated. Please define size dimensions within the corresponding area of the mediaTypes. (eg mediaTypes.banner.sizes).

  Other information

  RAD-1954 and related sub-tasks for original requirements.

[mkendall07]

Great start! Left a few comments on code structure. Also please add unit tests

[mkendall07]

generally recommend combining conditions to reduce the number of nested if conditions. For example you could combine into if (adUnit.mediaTypes && adUnit.mediaTypes.banner)

Also since you are doing a lot of testing of on adUnit.mediaTypes - you can also assign this to a var:
const mediaTypes = adUnit.mediaTypes; and that will shorten the conditions:
if (mediaTypes && mediaTypes.banner) {...}

[jsnellbaker]

Hey @mkendall07 In regards to the condition breakdown, I made them so heavily layered to avoid have the checks run when they shouldn't. As an example if I were to combine `adUnit.mediaTypes && adUnit.mediaTypes.banner), this condition would fail for anyone trying to serve a video or native ad (as the banner doesn't exist) and they would see the subsequent error/warning messages in the console log for something they weren't intentionally using. The same applied when I tried to combine some of the inner checks with the parent object (eg adUnit.mediaTypes.native.image && adUnit.mediaTypes.native.image.size). If they were to intentionally not use the image asset, they would get the warning message because half the condition would fail.

If there's another way to handle this type of setup while still supporting ideal failure conditions, please let me know.

I'll look into establishing variables though to help lessen the length of these statements.

[jsnellbaker]

Had a talk with Matt separately. Will make some changes where applicable.

[mkendall07]

could also do a assignment here for video to make it easier to read the logic.

[mkendall07]

when you delete something (video in this case) make sure you add that into the warn message. I.E. "mediaType.video has been removed from this request. "

[jsnellbaker]

I'll update the messages accordingly.

[jsnellbaker]

@mkendall07 I submitted some changes in lieu of the provided feedback (including the unit tests). Please let me know of any additional feedback. Thanks.

[matthewlane]

Looks great @jsnellbaker, really good PR notes too! During testing I found a section of the appnexus adapter that should be updated: right now, this section always sets values for native image and icons. Those values can be undefined if the server doesn't respond with anything, but since the appnexus adapter always sets an image and icon object on the native bid response, this causes the new validation in native.js to fail. In that section of the appnexus adapter, changing bid.native.icon/image to be set conditionally only if they are on the nativeAd object might do the trick

[jsnellbaker]

@matthewlane Thanks for the review/feedback. I made an update to the appnexusBidAdapter.js file along what you suggested. Can you take a look and let me know your thoughts? Thanks.