From b1cc0bc57f9c71cc2e9379f52c733314f5bd9f13 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Thu, 18 Jan 2018 08:16:35 +0100 Subject: [PATCH] Fixes undefined synopsis for media publishers Resolves #12591 Auditors: Test Plan: --- app/browser/api/ledger.js | 32 +++--- test/unit/app/browser/api/ledgerTest.js | 123 +++++++++++++++++++----- 2 files changed, 114 insertions(+), 41 deletions(-) diff --git a/app/browser/api/ledger.js b/app/browser/api/ledger.js index e71373e91a5..52c08780838 100644 --- a/app/browser/api/ledger.js +++ b/app/browser/api/ledger.js @@ -2400,7 +2400,7 @@ const onMediaRequest = (state, xhr, type, tabId) => { if (!cache.isEmpty()) { const publisherKey = cache.get('publisher') const publisher = ledgerState.getPublisher(state, publisherKey) - if (!publisher.isEmpty()) { + if (!publisher.isEmpty() && publisher.has('providerName')) { return module.exports.saveVisit(state, publisherKey, { duration, revisited, @@ -2449,19 +2449,13 @@ const onMediaPublisher = (state, mediaKey, response, duration, revisited) => { const publisherURL = response.get('publisherURL') const providerName = response.get('providerName') - if (publisher.isEmpty()) { - revisited = false + if (!synopsis.publishers[publisherKey] || publisher.isEmpty()) { synopsis.initPublisher(publisherKey) if (!synopsis.publishers[publisherKey]) { return state } - synopsis.publishers[publisherKey].faviconName = faviconName - synopsis.publishers[publisherKey].faviconURL = faviconURL - synopsis.publishers[publisherKey].publisherURL = publisherURL - synopsis.publishers[publisherKey].providerName = providerName - state = ledgerState.setPublisher(state, publisherKey, synopsis.publishers[publisherKey]) if (!getSetting(settings.PAYMENTS_SITES_AUTO_SUGGEST)) { @@ -2473,15 +2467,19 @@ const onMediaPublisher = (state, mediaKey, response, duration, revisited) => { savePublisherOption(publisherKey, 'exclude', exclude) }) } - } else { - synopsis.publishers[publisherKey].faviconName = faviconName - synopsis.publishers[publisherKey].faviconURL = faviconURL - synopsis.publishers[publisherKey].publisherURL = publisherURL - synopsis.publishers[publisherKey].providerName = providerName - state = ledgerState.setPublishersProp(state, publisherKey, 'faviconName', faviconName) - state = ledgerState.setPublishersProp(state, publisherKey, 'faviconURL', faviconURL) - state = ledgerState.setPublishersProp(state, publisherKey, 'publisherURL', publisherURL) - state = ledgerState.setPublishersProp(state, publisherKey, 'providerName', providerName) + } + + synopsis.publishers[publisherKey].faviconName = faviconName + synopsis.publishers[publisherKey].faviconURL = faviconURL + synopsis.publishers[publisherKey].publisherURL = publisherURL + synopsis.publishers[publisherKey].providerName = providerName + state = ledgerState.setPublishersProp(state, publisherKey, 'faviconName', faviconName) + state = ledgerState.setPublishersProp(state, publisherKey, 'faviconURL', faviconURL) + state = ledgerState.setPublishersProp(state, publisherKey, 'publisherURL', publisherURL) + state = ledgerState.setPublishersProp(state, publisherKey, 'providerName', providerName) + + if (publisher.isEmpty()) { + revisited = false } const cacheObject = Immutable.Map() diff --git a/test/unit/app/browser/api/ledgerTest.js b/test/unit/app/browser/api/ledgerTest.js index 4ddede1b971..acc9d92cae5 100644 --- a/test/unit/app/browser/api/ledgerTest.js +++ b/test/unit/app/browser/api/ledgerTest.js @@ -591,7 +591,8 @@ describe('ledger api unit tests', function () { })) .setIn(['ledger', 'synopsis', 'publishers', publisherKey], Immutable.fromJS({ visits: 1, - duration: 1000 + duration: 1000, + providerName: 'YouTube' })) beforeEach(function () { @@ -698,10 +699,26 @@ describe('ledger api unit tests', function () { ignoreMinTime: true }).calledOnce) }) + + it('state for this publisher is corrupted, so we need to fetch it again', function () { + const badState = defaultAppState + .setIn(['cache', 'ledgerVideos', videoId], Immutable.fromJS({ + publisher: publisherKey + })) + .setIn(['ledger', 'synopsis', 'publishers', publisherKey], Immutable.fromJS({ + options: { + excluded: false + } + })) + + ledgerApi.onMediaRequest(badState, xhr, ledgerMediaProviders.YOUTUBE, 10) + assert(mediaSpy.calledOnce) + assert(saveVisitSpy.notCalled) + }) }) describe('onMediaPublisher', function () { - let saveVisitSpy, verifiedPStub + let saveVisitSpy, verifiedPStub, setPublisherSpy const expectedState = Immutable.fromJS({ cache: { @@ -730,12 +747,18 @@ describe('ledger api unit tests', function () { migrations: {} }) - before(function () { - verifiedPStub = sinon.stub(ledgerApi, 'verifiedP', (state, publisherKey, fn) => state) + const response = Immutable.fromJS({ + publisher: publisherKey, + faviconName: 'Brave', + faviconURL: 'data:image/jpeg;base64,...', + publisherURL: 'https://brave.com', + providerName: 'Youtube' }) - after(function () { - verifiedPStub.restore() + before(function () { + verifiedPStub = sinon.stub(ledgerApi, 'verifiedP', (state, publisherKey, fn) => state) + saveVisitSpy = sinon.spy(ledgerApi, 'saveVisit') + setPublisherSpy = sinon.spy(ledgerState, 'setPublisher') }) beforeEach(function () { @@ -748,33 +771,92 @@ describe('ledger api unit tests', function () { options: { exclude: true }, - providerName: 'Youtube' + providerName: 'YouTube' } } }) - saveVisitSpy = sinon.spy(ledgerApi, 'saveVisit') + }) + + after(function () { + verifiedPStub.restore() + saveVisitSpy.restore() + setPublisherSpy.restore() }) afterEach(function () { ledgerApi.setSynopsis(undefined) - saveVisitSpy.restore() + verifiedPStub.reset() + saveVisitSpy.reset() + setPublisherSpy.reset() }) it('null case', function () { const result = ledgerApi.onMediaPublisher(defaultAppState) assert.deepEqual(result.toJS(), defaultAppState.toJS()) + assert(setPublisherSpy.notCalled) + assert(saveVisitSpy.notCalled) }) - it('create publisher if new and add cache', function () { - const response = Immutable.fromJS({ - publisher: publisherKey, - faviconName: 'Brave', - faviconURL: 'data:image/jpeg;base64,...', - publisherURL: 'https://brave.com', - providerName: 'Youtube' + it('we do not have publisher in the synopsis', function () { + ledgerApi.setSynopsis({ + initPublisher: () => { + ledgerApi.setSynopsis({ + initPublisher: () => {}, + addPublisher: () => {}, + publishers: { + [publisherKey]: { + exclude: false, + options: { + exclude: true + }, + providerName: 'YouTube' + } + } + }) + }, + addPublisher: () => {}, + publishers: { } }) + const newState = Immutable.fromJS({ + cache: { + ledgerVideos: { + 'youtube_kLiLOkzLetE': { + publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg', + faviconName: 'Brave', + providerName: 'Youtube', + faviconURL: 'data:image/jpeg;base64,...', + publisherURL: 'https://brave.com' + } + } + }, + ledger: { + synopsis: { + publishers: { + 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg': { + options: { + exclude: true + }, + faviconName: 'old Brave', + faviconURL: 'data:image/jpeg;base64,...', + publisherURL: 'https://brave.io', + providerName: 'Youtube' + } + } + } + }, + migrations: {} + }) + + const state = ledgerApi.onMediaPublisher(newState, videoId, response, 1000, false) + assert(saveVisitSpy.calledOnce) + assert(setPublisherSpy.calledTwice) + assert.deepEqual(state.toJS(), expectedState.toJS()) + }) + + it('create publisher if new and add cache', function () { const state = ledgerApi.onMediaPublisher(defaultAppState, videoId, response, 1000, false) + assert(setPublisherSpy.calledTwice) assert(saveVisitSpy.calledOnce) assert.deepEqual(state.toJS(), expectedState.toJS()) }) @@ -810,15 +892,8 @@ describe('ledger api unit tests', function () { migrations: {} }) - const response = Immutable.fromJS({ - publisher: publisherKey, - faviconName: 'Brave', - faviconURL: 'data:image/jpeg;base64,...', - publisherURL: 'https://brave.com', - providerName: 'Youtube' - }) - const state = ledgerApi.onMediaPublisher(newState, videoId, response, 1000, false) + assert(setPublisherSpy.calledOnce) assert(saveVisitSpy.calledOnce) assert.deepEqual(state.toJS(), expectedState.toJS()) })