single-request-bidder: updates to bidderFactory and converted rubiconBidAdapter

[snapwich]

Type of change

• Refactoring (no functional changes, no api changes)

Description of change

Here's my update to the single-request-bidder branch that includes a converted rubiconBidAdapter, some updates to the bidderFactory, and a few bug fixes.

Some of the pain points and problem areas in converting that I put fixes in for or want to highlight (we might also want to discuss if they are the right fixes):

1. Renamed spec.areParamsValid to spec.isBidRequestValid and pass the whole bidRequest rather than just the params. This was necessary because some of the validation is on the bid.sizes that comes from the adUnit, but are not available on the bid.params.
2. Passing in the bidderRequest as a second parameter to spec.buildRequests as our adapter uses some of that information that wasn't available otherwise (specifically the timeout and auctionStart properties which we use to do some timeout adjustment for our requests).
3. Had to bind the request object that spec.buildRequests creates to each of the onSuccess handlers so it can be sent in as the context to spec.interpretResponse. I needed to do this to get access to some of the information about the request that is used when formulating the responses but isn't necessarily available through the call to our endpoint. I don't know if I like this solution that much, but wasn't sure if there was a more elegant solution; and it seems like this might arise as a problem for others as well.
4. I couldn't find any way to port over our instance properties that are unique to an instance of an adapter. This isn't a frequent problem since there is only usually one instance of an adapter; however, it did prove problematic for testing which creates multiple instances. For instance, hasUserSyncFired which was a private property of RubiconAdapter had to be made global to the module and has to be reset manually in the tests by a new method call I added (which is only used for testing). That private user sync tracking variable was the only property that didn't have an elegant solution in our adapter, but this could be a common problem in other adapters.
5. Before the refactor, our adapter did some size processing and then validated that those sizes were correct before using them in the request. With this new adapter pattern, we have to do those size processing calculations twice, once in spec.isBidRequestValid to determine if the bidRequest is valid, and then again within spec.buildRequests so we can use those calculated sizes in our requests. I'm wondering if there is a better solutions there...

Besides those issues, all unit tests are passing and I performed a functional test of the adapter which had it still working as expected.

Other information

Addition to #1494

[dbemiller]

This should be on the spec now as a user sync, right? It hasn't been implemented yet, pending #1549... but I doc'd it.

 * @property {function(SyncOptions, Array): UserSync[]} [getUserSyncs] Given an array of all the responses
 *   from the server, determine which user syncs should occur. The argument array will contain every element
 *   which has been sent through to interpretResponse. The order of syncs in this array matters. The most
 *   important ones should come first, since publishers may limit how many are dropped on their page.

That should also help smooth over your resetUserSync() issue.

[dbemiller]

this is basically just a sneaky way of passing the request parameter implicitly.

This does look like a legitimate use-case, though. To make it more explicit, could we remove the .bind(request) and just make the spec signature interpretResponse(response, request)?

If you pull onSuccess and onFailure functions inside processRequest, the request will still be in scope.

[dbemiller]

This surfaces a serious lacking in the base class.

Ideally, I'd like to purge $$PREBID_GLOBAL$$ from the 1.0 adapters (see #1508, #1510). I have a plan to handle it (registerBidder((config) => spec))... but I don't think it's fair to ask you to make those changes, since they're with my code.

Apparently, my changes landed in master while i was at lunch today... so I'll spin up a new branch where we can hack away on this.

[dbemiller]

I chatted with @mkendall07 about this yesterday. I'm going to be focused on server-side stuff for the foreseeable future, so I won't actually have any time to work on it :/.

I opened up #1587 with the quick changes I'd make to the base adapter. I don't really care if you look those over and we merge that, or if you incorporate them into this PR. The only major change is replacing bind with a second argument.

I doubt anyone else will want to actually implement registerBidder((config) => spec). Instead, I'd suggest doing import config and using it directly. Although it means you're stuck with singleton configs in prebid, it's still worthwhile to purge $$PREBID_GLOBAL$$ from the code, for the reasons mentioned in #1508

[dbemiller]

Needs to be rebased off of master now

[dbemiller]

Closing this, since it duplicates #1624