Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/s2s client side fallback #1485

Merged
merged 15 commits into from
Aug 22, 2017
Merged

Feature/s2s client side fallback #1485

merged 15 commits into from
Aug 22, 2017

Conversation

protonate
Copy link
Collaborator

Type of change

  • Feature

Description of change

@mkendall07
Copy link
Member

@protonate looks like tests are failing

dbemiller
dbemiller reviewed Aug 17, 2017
View reviewed changes
.gitignore Outdated
@@ -6,6 +6,7 @@ build
test/app
gpt.html
gpt-each-bidder3.html
nate_dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: You can stick "personal" ignores in your ~/.gitignore_global. It'll apply to all your git repos that way.

Great place for stuff like .DS_Store too.

mkendall07
mkendall07 reviewed Aug 18, 2017
View reviewed changes
src/config.js Outdated
@@ -12,7 +12,7 @@ const utils = require('./utils');
const DEFAULT_DEBUG = false;
const DEFAULT_BIDDER_TIMEOUT = 3000;
const DEFAULT_PUBLISHER_DOMAIN = window.location.origin;
const DEFAULT_COOKIESYNC_DELAY = 100;
const DEFAULT_COOKIESYNC_DELAY = 500;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did this change?

@mkendall07
Copy link
Member

tests still failing.

protonate added 8 commits August 18, 2017 14:23
add a storagemanager to access local storage
a89330f 
filter s2s bidders by already synced
return true/false from sync operations
update storage item with bidder name on success
remove bidder from storage time if fail
set storage item if not exists
b447463
use Array.includes
12d96f4
ignore dev test folder
3347436
storagemanager uses JSON parse and stringify
1794df7
pass resolve and reject callbacks to insert cookie and insert pixel f…
b69b4cc 
…unctions with binding to current bidder
just get storage, tinker with cookie sync delay
35dd026
Set synced flag on fireSyncs feel good
eaa9964 
* set bidder synced flag on fireSyncs
* set storage item correctly
* remove onload callbacks
* remove personal .gitignore
* unwind changes no longer needed
@protonate protonate force-pushed the feature/s2s-client-side-fallback branch from 6d1a898 to eaa9964 Compare August 18, 2017 21:23
dbemiller
dbemiller reviewed Aug 21, 2017
View reviewed changes
src/cookie.js Outdated
function setBidderSynced(bidder) {
setStorageItem(S2S.SYNCED_BIDDERS_KEY, getStorageItem(S2S.SYNCED_BIDDERS_KEY)
.concat([bidder])
.filter(utils.uniques));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use an object? {'appnexus': 1, 'rubicon": 1}

Uniqueness comes for free, and your lookup in adaptermanager should be constant instead of linear.

src/constants.json
@@ -56,6 +56,7 @@
"DEFAULT_ENDPOINT" : "https://prebid.adnxs.com/pbs/v1/auction",
"SRC" : "s2s",
"ADAPTER" : "prebidServer",
"SYNC_ENDPOINT" : "https://prebid.adnxs.com/pbs/v1/cookie_sync"
"SYNC_ENDPOINT" : "https://prebid.adnxs.com/pbs/v1/cookie_sync",
"SYNCED_BIDDERS_KEY": "pbjsSyncs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkendall07 pointed out in another one of my PRs that this constants file does more harm than good, now that we can export const SYNCED_BIDDERS_KEY = "pbjsSyncs" somewhere in the code. I think he's got a good point.

src/storagemanager.js Outdated
* Only html5 localStorage implemented currently.
*
* @param key
* @param item
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param {string} key and @param {*} item?

@dbemiller dbemiller mentioned this pull request Aug 21, 2017
Merged
8 tasks
dbemiller
dbemiller previously approved these changes Aug 21, 2017
View reviewed changes
@dbemiller dbemiller added the LGTM label Aug 21, 2017
mkendall07
mkendall07 requested changes Aug 21, 2017
View reviewed changes
src/adaptermanager.js
@@ -86,7 +88,7 @@ exports.callBids = ({adUnits, cbTimeout}) => {

if (_s2sConfig.enabled) {
// these are called on the s2s adapter
let adaptersServerSide = _s2sConfig.bidders;
let adaptersServerSide = _s2sConfig.bidders.filter(bidder => syncedBidders.includes(bidder));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this results in empty array, don't call prebidServer.

handle first run case with uids and uuid2 cookies but no localStorage…
e9942a1 
… flags

* improve storagemanager with `add` and `remove` higher level functions
* handle null return from StorageManager.get
@mkendall07 mkendall07 merged commit 6974777 into master Aug 22, 2017
@mkendall07 mkendall07 deleted the feature/s2s-client-side-fallback branch August 22, 2017 16:13
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Aug 24, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
3d8e268 
…built

* 'master' of https://github.com/prebid/Prebid.js: (83 commits)
  feat(strAdapt): check if tagJS is already present (prebid#1500)
  Updated karma-mocha, and simplified the test framework for runs which dont include --watch. (prebid#1520)
  Revert "drop specific code for index adapter (prebid#1487)" (prebid#1529)
  Override default asset params when set on ad unit (prebid#1524)
  Adding new kv to xhb Adapter (prebid#1513)
  removing for...of loops because IE cannot handle them properly (prebid#1523)
  Increment pre version
  Prebid 0.27.0 Release
  Move unit test file to appropriate location (prebid#1516)
  Support 'cta' native asset (prebid#1505)
  Add adapter parameter types (prebid#1504)
  Register bid adapter (prebid#1514)
  Match port when testing legacy browser (prebid#1511)
  modify 'featureforward' adapter to be an indepandant adapter (prebid#1288)
  Feature/s2s client side fallback (prebid#1485)
  Make bid.vastUrl use the cache URL if the bid didnt already have one. (prebid#1506)
  OpenX Adapter Update: (prebid#1438)
  Add session id feature for roxot analytics adapter (prebid#1498)
  Update platformioBidAdapter (prebid#1493)
  Adding VAST Payload support for video bids (prebid#1407)
  ...
ptomasroos pushed a commit to happypancake/Prebid.js that referenced this pull request Aug 25, 2017
@ptomasroos
Feature/s2s client side fallback (prebid#1485)
4fe0fad 
* add a storagemanager to access local storage
filter s2s bidders by already synced
return true/false from sync operations
update storage item with bidder name on success
remove bidder from storage time if fail

* set storage item if not exists

* use Array.includes

* ignore dev test folder

* storagemanager uses JSON parse and stringify

* pass resolve and reject callbacks to insert cookie and insert pixel functions with binding to current bidder

* just get storage, tinker with cookie sync delay

* Set synced flag on fireSyncs feel good

* set bidder synced flag on fireSyncs
* set storage item correctly
* remove onload callbacks
* remove personal .gitignore
* unwind changes no longer needed

* fix test

* update to use factory pattern, and test

* rename storage manager functions to `get` and `set`

* remove localStorage ref

* handle first run case with uids and uuid2 cookies but no localStorage flags

* improve storagemanager with `add` and `remove` higher level functions
* handle null return from StorageManager.get

* don't send auction on first request

* only fire sync if needed
philipwatson pushed a commit to mbrtargeting/Prebid.js that referenced this pull request Sep 18, 2017
@philipwatson
Feature/s2s client side fallback (prebid#1485)
4739c12 
* add a storagemanager to access local storage
filter s2s bidders by already synced
return true/false from sync operations
update storage item with bidder name on success
remove bidder from storage time if fail

* set storage item if not exists

* use Array.includes

* ignore dev test folder

* storagemanager uses JSON parse and stringify

* pass resolve and reject callbacks to insert cookie and insert pixel functions with binding to current bidder

* just get storage, tinker with cookie sync delay

* Set synced flag on fireSyncs feel good

* set bidder synced flag on fireSyncs
* set storage item correctly
* remove onload callbacks
* remove personal .gitignore
* unwind changes no longer needed

* fix test

* update to use factory pattern, and test

* rename storage manager functions to `get` and `set`

* remove localStorage ref

* handle first run case with uids and uuid2 cookies but no localStorage flags

* improve storagemanager with `add` and `remove` higher level functions
* handle null return from StorageManager.get

* don't send auction on first request

* only fire sync if needed
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017
@jbAdyoulike
Feature/s2s client side fallback (prebid#1485)
ef16c86 
* add a storagemanager to access local storage
filter s2s bidders by already synced
return true/false from sync operations
update storage item with bidder name on success
remove bidder from storage time if fail

* set storage item if not exists

* use Array.includes

* ignore dev test folder

* storagemanager uses JSON parse and stringify

* pass resolve and reject callbacks to insert cookie and insert pixel functions with binding to current bidder

* just get storage, tinker with cookie sync delay

* Set synced flag on fireSyncs feel good

* set bidder synced flag on fireSyncs
* set storage item correctly
* remove onload callbacks
* remove personal .gitignore
* unwind changes no longer needed

* fix test

* update to use factory pattern, and test

* rename storage manager functions to `get` and `set`

* remove localStorage ref

* handle first run case with uids and uuid2 cookies but no localStorage flags

* improve storagemanager with `add` and `remove` higher level functions
* handle null return from StorageManager.get

* don't send auction on first request

* only fire sync if needed
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@dluxemburg
Feature/s2s client side fallback (prebid#1485)
8a155d1 
* add a storagemanager to access local storage
filter s2s bidders by already synced
return true/false from sync operations
update storage item with bidder name on success
remove bidder from storage time if fail

* set storage item if not exists

* use Array.includes

* ignore dev test folder

* storagemanager uses JSON parse and stringify

* pass resolve and reject callbacks to insert cookie and insert pixel functions with binding to current bidder

* just get storage, tinker with cookie sync delay

* Set synced flag on fireSyncs feel good

* set bidder synced flag on fireSyncs
* set storage item correctly
* remove onload callbacks
* remove personal .gitignore
* unwind changes no longer needed

* fix test

* update to use factory pattern, and test

* rename storage manager functions to `get` and `set`

* remove localStorage ref

* handle first run case with uids and uuid2 cookies but no localStorage flags

* improve storagemanager with `add` and `remove` higher level functions
* handle null return from StorageManager.get

* don't send auction on first request

* only fire sync if needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbemiller dbemiller dbemiller approved these changes

@mkendall07 mkendall07 mkendall07 approved these changes

Assignees
No one assigned
Labels
LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@protonate @mkendall07 @dbemiller