Update Rubicon Adapter with SRA-GET support (HB-2145) #13

idettman · 2018-02-13T22:26:31Z

IMPORTANT! Do not merge, for code review only

Type of change

Feature

Description of change

Adds "Single Request Architecture" (SRA) support to the Rubicon Adapter.

// By default, SRA mode is turned off

// Enable SRA with the following config change
pbjs.setConfig({
  rubicon: {
    singleRequest: true
  }
});

test parameters for validating bids

const exampleBids = [
{
  code: 'div-1',
  sizes: [[728, 90], [970, 90]],
  bids: [
    {
      bidder: 'rubicon',
      params: {
        accountId: "1234",
        siteId: "1234",
        zoneId: "1001"
    },
    {
      bidder: 'rubicon',
      params: {
        accountId: "1234",
        siteId: "1234",
        zoneId: "1002",
    },
    {
      bidder: 'rubicon',
      params: {
        accountId: "1234",
        siteId: "9876", // Note: <-- this siteId doesn't match the other two bids, so a second request is sent for bids with that 'siteId' value
        zoneId: "1003"
    }]
}];

Other information

HB-2145

… test to query string param concatenation

…e for sinon 2.0 compatibility

…nction

harpere

reviewed the code changes, still need to review the tests.

harpere · 2018-02-16T01:07:29Z

modules/rubiconBidAdapter.js

-            ? memo + curr + '=' + encodeURIComponent(data[index + 1]) + '&' : memo,
-        ''
-      ).slice(0, -1); // remove trailing &
+    page_url = bidRequest.params.secure ? page_url.replace(/^http:/i, 'https:') : page_url;


may want to put all of this page_url logic in a function since it's used here and for video slots.

harpere · 2018-02-16T01:12:06Z

modules/rubiconBidAdapter.js

+      // bids are not grouped if single request mode is not enabled
+      requests = videoRequests.concat(bidRequests.filter(bidRequest => bidRequest.mediaType !== 'video').map(bidRequest => {
+        const bidParams = spec.createSlotParams(bidRequest);
+        const combinedSlotParams = spec.combineSlotUrlParams([bidParams]);


why are you calling combineSlotUrlParams() when this isn't single-request?

harpere · 2018-02-16T01:16:18Z

modules/rubiconBidAdapter.js

+          method: 'GET',
+          url: FASTLANE_ENDPOINT,
+          data: Object.keys(combinedSlotParams).reduce((paramString, key) => {
+            return (typeof combinedSlotParams[key] === 'string' || combinedSlotParams[key] !== '') ? `${paramString}${key}=${encodeURIComponent(combinedSlotParams[key])}&` : paramString;


what if the parameter is a number, like floor for example?

also, wouldn't this be an && rather than an ||?

harpere · 2018-02-16T01:22:55Z

modules/rubiconBidAdapter.js

+          method: 'GET',
+          url: FASTLANE_ENDPOINT,
+          data: Object.keys(combinedSlotParams).reduce((paramString, key) => {
+            return (typeof combinedSlotParams[key] === 'string' || combinedSlotParams[key] !== '') ? `${paramString}${key}=${encodeURIComponent(combinedSlotParams[key])}&` : paramString;


same comments as earlier

harpere · 2018-02-16T19:13:39Z

modules/rubiconBidAdapter.js

+    'dt.id': digiTrustId.id,
+    'dt.keyv': digiTrustId.keyv,
+    'dt.pref': 0
+  };


did this need to be changed? was it not working previously?

It was working previously, but the get param object properties were being stored in an array (opposed to plain object) to honor ordering of parameters, but since order does not matter, I'm skipping the step of using an array. I converted this to return an object for simpler key value assignment to the param object

harpere · 2018-02-20T18:47:24Z

test/spec/modules/rubiconBidAdapter_spec.js

@@ -422,6 +449,93 @@ describe('the rubicon adapter', () => {
            expect(window.DigiTrust.getUser.calledOnce).to.equal(true);
          });
        });
+
+        describe('singleRequest config', () => {


tests confirming requests are combined as expected are good, but should have some tests verifying the params are combined into the grouped request as expected, and that the bidRequest param is an array of all the grouped requests.

harpere · 2018-02-20T18:49:02Z

test/spec/modules/rubiconBidAdapter_spec.js

+            {p2: 'test'}
+          ])).to.deep.equal({p1: 'foo;foo;', p2: 'test;bar;test', p3: 'baz;;'});
+        });
+      });
    });

    describe('interpretResponse', () => {


should be some new tests for SRA responses.

…esponse bid could not be linked to returned ad data

harpere

new tests LGTM. thanks.

…con-project/Prebid.js into rubicon-adapter-sra-support

…rrays. Removed code/test for equal lengths between ads and bidRequests.

…ror status if only one bid was invalid

…e object.

…ync (prebid#9700) * collect EIDs for bid request * add ad slot positioning to payload * RPO-2012: Update local storage name-spacing for c_uid (#8) * Updates c_uid namespacing to be more specific for concert * fixes unit tests * remove console.log * RPO-2012: Add check for shared id (#9) * Adds check for sharedId * Updates cookie name * remove trailing comma * [RPO-3152] Enable Support for GPP Consent (#12) * Adds gpp consent integration to concert bid adapter * Update tests to check for gpp consent string param * removes user sync endpoint and tests * updates comment * cleans up consentAllowsPpid function * comment fix * rename variables for clarity * fixes conditional logic for consent allows function (#13) --------- Co-authored-by: antoin <antoin.campbell@voxmedia.com> Co-authored-by: Antoin <antoinfive@gmail.com>

) * collect EIDs for bid request * add ad slot positioning to payload * RPO-2012: Update local storage name-spacing for c_uid (#8) * Updates c_uid namespacing to be more specific for concert * fixes unit tests * remove console.log * RPO-2012: Add check for shared id (#9) * Adds check for sharedId * Updates cookie name * remove trailing comma * [RPO-3152] Enable Support for GPP Consent (#12) * Adds gpp consent integration to concert bid adapter * Update tests to check for gpp consent string param * removes user sync endpoint and tests * updates comment * cleans up consentAllowsPpid function * comment fix * rename variables for clarity * fixes conditional logic for consent allows function (#13) * [RPO-3262] Update getUid function to check for pubcid and sharedid (#14) * Update getUid function to check for pubcid and sharedid * updates adapter version --------- Co-authored-by: antoin <antoin.campbell@voxmedia.com> Co-authored-by: Antoin <antoinfive@gmail.com>

…ebid#10356) * collect EIDs for bid request * add ad slot positioning to payload * RPO-2012: Update local storage name-spacing for c_uid (#8) * Updates c_uid namespacing to be more specific for concert * fixes unit tests * remove console.log * RPO-2012: Add check for shared id (#9) * Adds check for sharedId * Updates cookie name * remove trailing comma * [RPO-3152] Enable Support for GPP Consent (#12) * Adds gpp consent integration to concert bid adapter * Update tests to check for gpp consent string param * removes user sync endpoint and tests * updates comment * cleans up consentAllowsPpid function * comment fix * rename variables for clarity * fixes conditional logic for consent allows function (#13) * [RPO-3262] Update getUid function to check for pubcid and sharedid (#14) * Update getUid function to check for pubcid and sharedid * updates adapter version * [RPO-3405] Add browserLanguage to request meta object --------- Co-authored-by: antoin <antoin.campbell@voxmedia.com> Co-authored-by: Antoin <antoinfive@gmail.com> Co-authored-by: Brett Bloxom <38990705+BrettBlox@users.noreply.github.com>

Isaac Dettman and others added 13 commits February 13, 2018 13:24

Modified code for easier code merge

2d26659

Reverted digitrust function in-lining

826261a

Added missing function jsdoc

0dc32d7

Added cpm sort to fix broken unit tests

d9c70e6

Fixed linting errors

3797107

Bug fix to map converted bids before combining, added empty/undefined…

4ffa40d

… test to query string param concatenation

Added referrer and page_url changes and spec sinon sandbox.stub updat…

85ffec7

…e for sinon 2.0 compatibility

Added referrer and page_url changes

df3d331

Fixed broken unit tests

da85489

Added request parameter tg_fl.eid for non-video requests

2752d7b

Removed spaces to correct linting error

0cc3a43

Replaced calls to JSON.stringify and parse with the 'clone' helper fu…

088c7ea

…nction

Updated unit tests for new sra get param 'tg_fl.eid'

06ce341

idettman requested a review from harpere February 16, 2018 18:59

harpere requested changes Feb 16, 2018

View reviewed changes

Isaac Dettman added 9 commits February 16, 2018 11:58

Updated with requested changes from pull request

36aad9f

Extracted code for page_url to a function for reuse

151f268

Added unit test for sra request limited to 10 slots

653b19e

Updated comment

a2cf89c

Added warning for bids dropped from SRA get request

8fb38e0

Changed log execution line ordering in max bids limit block

b9e747b

Updated until test description

25dd212

Added unit test for expectedQuery function

d914977

Fixed unit test for createSlotParams

595fa90

idettman changed the base branch from rubicon_adapter_sra_support to master February 20, 2018 17:52

idettman changed the title ~~SRA-GET support for Rubicon bid adapter (HB-2145)~~ Update Rubicon Adapter with SRA-GET support - release/1.5 (HB-2145) Feb 20, 2018

idettman changed the title ~~Update Rubicon Adapter with SRA-GET support - release/1.5 (HB-2145)~~ Update Rubicon Adapter with SRA-GET support - release/0.34.5 (HB-2145) Feb 20, 2018

harpere requested changes Feb 20, 2018

View reviewed changes

Isaac Dettman added 2 commits February 21, 2018 16:53

Add additional unit tests

b54dab6

Updated unit test for max of 10 slots sent per request

f8a917c

Isaac Dettman added 19 commits February 23, 2018 16:09

Updated unit tests for new sra get param 'tg_fl.eid'

72c1b9d

Updated with requested changes from pull request

40dd0ac

Extracted code for page_url to a function for reuse

d83635a

Added unit test for sra request limited to 10 slots

6129eee

Updated comment

148aec3

Added warning for bids dropped from SRA get request

3348ac0

Changed log execution line ordering in max bids limit block

182e17b

Updated until test description

a995782

Added unit test for expectedQuery function

6aaef71

Fixed unit test for createSlotParams

14be5bb

Add additional unit tests

e9f844a

Updated unit test for max of 10 slots sent per request

ffb5854

Updated with changed from media types update branch, added error if r…

101d5ce

…esponse bid could not be linked to returned ad data

Fix for video bid validation backwards compatibility

e4971b2

Merged code from updated media types branch

a5a7044

Temp commit before refactoring extracting inline code to functions

9025492

Added additional unit tests for interpret response

0123c7f

Fix for validate bids

28fa3d2

Rebased/merged with master

7637845

harpere approved these changes Feb 27, 2018

View reviewed changes

Isaac Dettman added 4 commits February 27, 2018 13:40

Merge branch 'rubicon-adapter-sra-support' of https://github.com/rubi…

419bcb3

…con-project/Prebid.js into rubicon-adapter-sra-support

Removed code/tests to validate matching order for ad and bidRequest a…

215eb29

…rrays. Removed code/test for equal lengths between ads and bidRequests.

Removed validation for root response obj status, it was getting an er…

5f862b5

…ror status if only one bid was invalid

Updated interpretResponse validation for response obj to test for typ…

fcf4d4f

…e object.

idettman changed the title ~~Update Rubicon Adapter with SRA-GET support - release/0.34.5 (HB-2145)~~ Update Rubicon Adapter with SRA-GET support (HB-2145) Mar 6, 2018

snapwich closed this Jun 8, 2018

idettman deleted the rubicon-adapter-sra-support branch March 18, 2019 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Rubicon Adapter with SRA-GET support (HB-2145) #13

Update Rubicon Adapter with SRA-GET support (HB-2145) #13

idettman commented Feb 13, 2018 •

edited

Loading

harpere left a comment

harpere Feb 16, 2018

harpere Feb 16, 2018

harpere Feb 16, 2018

harpere Feb 16, 2018

harpere Feb 16, 2018

harpere Feb 16, 2018

idettman Feb 16, 2018 •

edited

Loading

harpere Feb 20, 2018

harpere Feb 20, 2018

harpere left a comment

Update Rubicon Adapter with SRA-GET support (HB-2145) #13

Update Rubicon Adapter with SRA-GET support (HB-2145) #13

Conversation

idettman commented Feb 13, 2018 • edited Loading

IMPORTANT! Do not merge, for code review only

Type of change

Description of change

Other information

harpere left a 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

idettman Feb 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harpere left a comment

Choose a reason for hiding this comment

idettman commented Feb 13, 2018 •

edited

Loading

idettman Feb 16, 2018 •

edited

Loading