Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fixes undefined synopsis for media publishers #12706

Merged
merged 1 commit into from
Jan 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 15 additions & 17 deletions app/browser/api/ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line checks if data is corrupted and if that is the case we don't use cache, but fetch is again

return module.exports.saveVisit(state, publisherKey, {
duration,
revisited,
Expand Down Expand Up @@ -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()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We always check for synopsis as well. If synopsis or publisher in the state are missing, we initialize it again, so that we don't have corrupted data.

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)) {
Expand All @@ -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()
Expand Down
123 changes: 99 additions & 24 deletions test/unit/app/browser/api/ledgerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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 () {
Expand All @@ -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())
})
Expand Down Expand Up @@ -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())
})
Expand Down