From a1bbea0b57712a84a927aab89c5db76cf6546a28 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Wed, 21 Feb 2018 15:50:30 +0100 Subject: [PATCH] Improves time logging --- app/browser/api/ledger.js | 13 ++- app/common/cache/ledgerVideoCache.js | 19 +++- app/common/lib/ledgerUtil.js | 92 +++++++++++++++++-- docs/state.md | 1 + test/unit/app/browser/api/ledgerTest.js | 12 +-- .../app/common/cache/ledgerVideoCacheTest.js | 41 ++++++++- test/unit/app/common/lib/ledgerUtilTest.js | 55 +++++++++-- 7 files changed, 199 insertions(+), 34 deletions(-) diff --git a/app/browser/api/ledger.js b/app/browser/api/ledger.js index 6d07f712242..be5914854ca 100644 --- a/app/browser/api/ledger.js +++ b/app/browser/api/ledger.js @@ -2406,7 +2406,7 @@ const onMediaRequest = (state, xhr, type, tabId) => { } const mediaKey = ledgerUtil.getMediaKey(mediaId, type) - let duration = ledgerUtil.getMediaDuration(parsed, type) + let duration = ledgerUtil.getMediaDuration(state, parsed, mediaKey, type) if (duration == null || mediaKey == null) { return state @@ -2428,9 +2428,14 @@ const onMediaRequest = (state, xhr, type, tabId) => { currentMediaKey = mediaKey } + const stateData = ledgerUtil.generateMediaCacheData(state, parsed, type) const cache = ledgerVideoCache.getDataByVideoId(state, mediaKey) if (!cache.isEmpty()) { + if (!stateData.isEmpty()) { + state = ledgerVideoCache.mergeCacheByVideoId(state, mediaKey, stateData) + } + const publisherKey = cache.get('publisher') const publisher = ledgerState.getPublisher(state, publisherKey) if (!publisher.isEmpty() && publisher.has('providerName')) { @@ -2442,6 +2447,10 @@ const onMediaRequest = (state, xhr, type, tabId) => { } } + if (!stateData.isEmpty()) { + state = ledgerVideoCache.setCacheByVideoId(state, mediaKey, stateData) + } + const options = underscore.extend({roundtrip: module.exports.roundtrip}, clientOptions) const mediaProps = { mediaId, @@ -2519,7 +2528,7 @@ const onMediaPublisher = (state, mediaKey, response, duration, revisited) => { .set('publisher', publisherKey) // Add to cache - state = ledgerVideoCache.setCacheByVideoId(state, mediaKey, cacheObject) + state = ledgerVideoCache.mergeCacheByVideoId(state, mediaKey, cacheObject) state = module.exports.saveVisit(state, publisherKey, { duration, diff --git a/app/common/cache/ledgerVideoCache.js b/app/common/cache/ledgerVideoCache.js index c7f0ae3fcb1..adab8a5f561 100644 --- a/app/common/cache/ledgerVideoCache.js +++ b/app/common/cache/ledgerVideoCache.js @@ -33,7 +33,24 @@ const setCacheByVideoId = (state, key, data) => { return state.setIn(['cache', 'ledgerVideos', key], data) } +const mergeCacheByVideoId = (state, key, data) => { + state = validateState(state) + + if (key == null || data == null) { + return state + } + + data = makeImmutable(data) + + if (data.isEmpty()) { + return state + } + + return state.mergeIn(['cache', 'ledgerVideos', key], data) +} + module.exports = { getDataByVideoId, - setCacheByVideoId + setCacheByVideoId, + mergeCacheByVideoId } diff --git a/app/common/lib/ledgerUtil.js b/app/common/lib/ledgerUtil.js index 3ae25ef25f9..bac62633060 100644 --- a/app/common/lib/ledgerUtil.js +++ b/app/common/lib/ledgerUtil.js @@ -13,6 +13,7 @@ const queryString = require('querystring') // State const siteSettingsState = require('../state/siteSettingsState') const ledgerState = require('../state/ledgerState') +const ledgerVideoCache = require('../cache/ledgerVideoCache') // Constants const settings = require('../../../js/constants/settings') @@ -224,7 +225,15 @@ const getMediaId = (data, type) => { } case ledgerMediaProviders.TWITCH: { - if (data.event === 'minute-watched' && data.properties) { + if ( + ( + data.event === 'minute-watched' || + data.event === 'video-play' || + data.event === 'player_click_playpause' || + data.event === 'vod_seek' + ) && + data.properties + ) { id = data.properties.channel let vod = data.properties.vod @@ -294,7 +303,7 @@ const getMediaData = (xhr, type) => { return result } -const getMediaDuration = (data, type) => { +const getMediaDuration = (state, data, mediaKey, type) => { let duration = 0 switch (type) { case ledgerMediaProviders.YOUTUBE: @@ -304,7 +313,7 @@ const getMediaDuration = (data, type) => { } case ledgerMediaProviders.TWITCH: { - duration = getTwitchDuration(data) + duration = getTwitchDuration(state, data, mediaKey) break } } @@ -312,6 +321,36 @@ const getMediaDuration = (data, type) => { return duration } +const generateMediaCacheData = (state, parsed, type) => { + let data = Immutable.Map() + + if (parsed == null) { + return data + } + + switch (type) { + case ledgerMediaProviders.TWITCH: + { + data = generateTwitchCacheData(state, parsed) + break + } + } + + return data +} + +const generateTwitchCacheData = (state, parsed) => { + if (parsed == null) { + return Immutable.Map() + } + + return Immutable.fromJS({ + event: parsed.event, + minutesLogged: parsed.properties.minutes_logged, + time: parsed.properties.time + }) +} + const getMediaFavicon = (providerName) => { let image = null @@ -337,17 +376,51 @@ const getMediaFavicon = (providerName) => { return image } -const getTwitchDuration = (data) => { +const getTwitchDuration = (state, data, mediaKey) => { + if (data == null) { + return 0 + } + + const previousData = ledgerVideoCache.getDataByVideoId(state, mediaKey) + + if (previousData.isEmpty() && data.event === 'video-play') { + return milliseconds.second * 10 + } + + if (!data.properties) { + return 0 + } + + const oldMedia = ledgerVideoCache.getDataByVideoId(state, mediaKey) let time = 0 + const currentTime = parseFloat(data.properties.time) + const oldTime = parseFloat(previousData.get('time')) - if (data == null) { - return time + if ( + data.event === 'player_click_playpause' && + oldMedia.get('event') !== 'player_click_playpause' + ) { + // User paused a video + time = currentTime - oldTime + } else if (previousData.get('event') === 'video-play') { + // From video play event to x event + time = currentTime - oldTime - 10 + } else if (data.event === 'minute-watched') { + // Minute watched event + time = currentTime - oldTime + } + + if (time < 0) { + time = 0 } - if (data.properties && data.properties.minutes_logged) { - time = 60 * 1000 // 1 min + if (time > 120) { + time = 120 } + // we get seconds back, so we need to convert it into ms + time = time * 1000 + return time } @@ -432,7 +505,8 @@ const getMethods = () => { getMediaKey, milliseconds, defaultMonthlyAmounts, - getMediaFavicon + getMediaFavicon, + generateMediaCacheData } let privateMethods = {} diff --git a/docs/state.md b/docs/state.md index 0e72d368459..aa5f8bbc5b9 100644 --- a/docs/state.md +++ b/docs/state.md @@ -93,6 +93,7 @@ AppStore ledgerVideos: { [mediaKey]: { publisher: string // publisher key + beatData: object // data that we get from a heartbeat } } } diff --git a/test/unit/app/browser/api/ledgerTest.js b/test/unit/app/browser/api/ledgerTest.js index ef4d114fb16..54f79f3dcd8 100644 --- a/test/unit/app/browser/api/ledgerTest.js +++ b/test/unit/app/browser/api/ledgerTest.js @@ -803,11 +803,7 @@ describe('ledger api unit tests', function () { cache: { ledgerVideos: { 'youtube_kLiLOkzLetE': { - publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg', - faviconName: 'Brave', - providerName: 'Youtube', - faviconURL: 'data:image/jpeg;base64,...', - publisherURL: 'https://brave.com' + publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' } } }, @@ -846,11 +842,7 @@ describe('ledger api unit tests', function () { cache: { ledgerVideos: { 'youtube_kLiLOkzLetE': { - publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg', - faviconName: 'Brave', - providerName: 'Youtube', - faviconURL: 'data:image/jpeg;base64,...', - publisherURL: 'https://brave.com' + publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' } } }, diff --git a/test/unit/app/common/cache/ledgerVideoCacheTest.js b/test/unit/app/common/cache/ledgerVideoCacheTest.js index feadd27153c..71b4cd6907c 100644 --- a/test/unit/app/common/cache/ledgerVideoCacheTest.js +++ b/test/unit/app/common/cache/ledgerVideoCacheTest.js @@ -17,6 +17,10 @@ const stateWithData = Immutable.fromJS({ ledgerVideos: { 'youtube_kLiLOkzLetE': { publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' + }, + 'twitch_test': { + publisher: 'twitch#author:test', + time: 1234 } } } @@ -52,9 +56,40 @@ describe('ledgerVideoCache unit test', function () { const state = ledgerVideoCache.setCacheByVideoId(baseState, 'youtube_kLiLOkzLetE', Immutable.fromJS({ publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' })) - const expectedState = state.setIn(['cache', 'ledgerVideos', 'youtube_kLiLOkzLetE'], Immutable.fromJS({ - publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' - })) + const expectedState = state + .setIn(['cache', 'ledgerVideos', 'youtube_kLiLOkzLetE'], Immutable.fromJS({ + publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg' + })) + assert.deepEqual(state.toJS(), expectedState.toJS()) + }) + }) + + describe('mergeCacheByVideoId', function () { + it('null case', function () { + const state = ledgerVideoCache.mergeCacheByVideoId(baseState) + assert.deepEqual(state.toJS(), baseState.toJS()) + }) + + it('old data is missing', function () { + const state = ledgerVideoCache.mergeCacheByVideoId(baseState, 'twitch_test', {someData: 'test'}) + const expectedState = baseState + .setIn(['cache', 'ledgerVideos', 'twitch_test'], Immutable.fromJS({someData: 'test'})) + assert.deepEqual(state.toJS(), expectedState.toJS()) + }) + + it('new data is null', function () { + const state = ledgerVideoCache.mergeCacheByVideoId(stateWithData, 'twitch_test') + assert.deepEqual(state.toJS(), stateWithData.toJS()) + }) + + it('old and new data are present', function () { + const state = ledgerVideoCache.mergeCacheByVideoId(stateWithData, 'twitch_test', {someData: 'test'}) + const expectedState = stateWithData + .setIn(['cache', 'ledgerVideos', 'twitch_test'], Immutable.fromJS({ + publisher: 'twitch#author:test', + time: 1234, + someData: 'test' + })) assert.deepEqual(state.toJS(), expectedState.toJS()) }) }) diff --git a/test/unit/app/common/lib/ledgerUtilTest.js b/test/unit/app/common/lib/ledgerUtilTest.js index d2e058f5397..f57b4f767c3 100644 --- a/test/unit/app/common/lib/ledgerUtilTest.js +++ b/test/unit/app/common/lib/ledgerUtilTest.js @@ -438,6 +438,36 @@ describe('ledgerUtil unit test', function () { }, ledgerMediaProviders.TWITCH) assert.equal(result, 'tchannel_vod_12343234') }) + + it('event is video-play', function () { + const result = ledgerUtil.getMediaId({ + event: 'video-play', + properties: { + channel: 'tchannel' + } + }, ledgerMediaProviders.TWITCH) + assert.equal(result, 'tchannel') + }) + + it('event is player_click_playpause', function () { + const result = ledgerUtil.getMediaId({ + event: 'player_click_playpause', + properties: { + channel: 'tchannel' + } + }, ledgerMediaProviders.TWITCH) + assert.equal(result, 'tchannel') + }) + + it('event is vod_seek', function () { + const result = ledgerUtil.getMediaId({ + event: 'vod_seek', + properties: { + channel: 'tchannel' + } + }, ledgerMediaProviders.TWITCH) + assert.equal(result, 'tchannel') + }) }) }) @@ -558,15 +588,6 @@ describe('ledgerUtil unit test', function () { const result = ledgerUtil.getTwitchDuration({}) assert.equal(result, 0) }) - - it('minutes_logged is present', function () { - const result = ledgerUtil.getTwitchDuration({ - properties: { - minutes_logged: 1 - } - }) - assert.equal(result, 60000) - }) }) describe('getMediaProvider', function () { @@ -664,4 +685,20 @@ describe('ledgerUtil unit test', function () { assert.equal(result, 'twitch.svg') }) }) + + describe('getMediaDuration', function () { + // TODO add test + }) + + describe('generateMediaCacheData', function () { + // TODO add test + }) + + describe('generateTwitchCacheData', function () { + // TODO add test + }) + + describe('getTwitchDuration', function () { + // TODO add test + }) })