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

fix: Only request vendorlist once #209

Merged
merged 6 commits into from
Oct 2, 2018
Merged

fix: Only request vendorlist once #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3682721
fix: Only request vendorlist once
Fumler Sep 6, 2018
e77dab6
refactor: Remove console.log from test
Fumler Sep 6, 2018
9a0693c
refactor: Simplify checking for cached vendorlist
Fumler Sep 7, 2018
9fff479
remove comments and add semicolon
phogel Sep 10, 2018
24ed51d
refactor: Change tests to make sure we do not have cached version
Fumler Sep 27, 2018
4dc66cd
fix: Resolve promise instead of return
Fumler Oct 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/scripts/core/core_vendor_information.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,25 @@ export const DEFAULT_VENDOR_LIST = {
};

export let cachedVendorList;
export let pendingVendorlistPromise = null;

export function loadVendorList() {
return new Promise(function (resolve) {
if (cachedVendorList) {
resolve(cachedVendorList);
} else if (pendingVendorlistPromise) {
resolve(pendingVendorlistPromise);
} else {
let iabVendorListUrl = getIabVendorListUrl();
fetchJsonData(iabVendorListUrl)
.then(response => {
pendingVendorlistPromise = fetchJsonData(iabVendorListUrl)
Fumler marked this conversation as resolved.
Show resolved Hide resolved
pendingVendorlistPromise.then(response => {
cachedVendorList = response;
pendingVendorlistPromise = null;
sortVendors(cachedVendorList);
resolve(cachedVendorList);
})
.catch(error => {
pendingVendorlistPromise = null;
logError(`OIL getVendorList failed and returned error: ${error}. Falling back to default vendor list!`);
resolve(getVendorList());
});
Expand Down Expand Up @@ -67,6 +72,7 @@ export function getVendorList() {

export function clearVendorListCache() {
cachedVendorList = undefined;
pendingVendorlistPromise = null;
}

export function getVendorsToDisplay() {
Expand Down
35 changes: 32 additions & 3 deletions test/specs/core/core_vendor_information.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import {
getVendors,
getLimitedVendors,
getVendorsToDisplay,
loadVendorList
loadVendorList,
cachedVendorList,
pendingVendorlistPromise
} from '../../../src/scripts/core/core_vendor_information';
import VENDOR_LIST from '../../fixtures/vendorlist/simple_vendor_list.json';
import { resetOil } from '../../test-utils/utils_reset';
Expand Down Expand Up @@ -52,6 +54,33 @@ describe('core_vendor_information', () => {
});
});

it('should wait for cached vendor list if request is already started', (done) => {
let fetchSpy = spyOn(CoreUtils, 'fetchJsonData').and.returnValue(new Promise((resolve) => resolve(VENDOR_LIST)));
spyOn(CoreConfig, 'getIabVendorListUrl').and.returnValue("https://iab.vendor.list.url");

expect(pendingVendorlistPromise).toBeNull();
expect(cachedVendorList).toBeUndefined();
loadVendorList().then((retrievedVendorList) => {
expect(retrievedVendorList.vendorListVersion).toEqual(VENDOR_LIST.vendorListVersion);
expect(retrievedVendorList).toEqual(VENDOR_LIST);
expect(cachedVendorList).toBeDefined();
});
expect(cachedVendorList).toBeUndefined();
expect(pendingVendorlistPromise).toBeDefined();
loadVendorList().then((retrievedVendorList) => {
expect(retrievedVendorList.vendorListVersion).toEqual(VENDOR_LIST.vendorListVersion);
expect(retrievedVendorList).toEqual(VENDOR_LIST);
});
expect(cachedVendorList).toBeUndefined();
loadVendorList().then((retrievedVendorList) => {
expect(retrievedVendorList.vendorListVersion).toEqual(VENDOR_LIST.vendorListVersion);
expect(retrievedVendorList).toEqual(VENDOR_LIST);
done();
});
expect(fetchSpy.calls.count()).toBe(1);

});

it('should use default vendor list if vendor list fetching fails', (done) => {
spyOn(CoreUtils, 'fetchJsonData').and.returnValue(new Promise((resolve, reject) => reject(new Error("something went wrong"))));
spyOn(CoreConfig, 'getIabVendorListUrl').and.returnValue("https://iab.vendor.list.url");
Expand Down Expand Up @@ -273,7 +302,7 @@ describe('core_vendor_information', () => {
});

describe('getLimitedVendors', function() {

it('returns regular vendors when no whitelist or blacklist exists', function() {
spyOn(CoreConfig, 'getShowLimitedVendors').and.returnValue(true);
expect(getLimitedVendors().length).toEqual(DEFAULT_VENDOR_LIST.maxVendorId);
Expand All @@ -294,7 +323,7 @@ describe('core_vendor_information', () => {
});

describe('getVendorsToDisplay', function() {

it('should return full vendor list when configuration parameter show_limited_vendors_only is false', function() {
spyOn(CoreConfig, 'getShowLimitedVendors').and.returnValue(false);
let result = getVendorsToDisplay();
Expand Down