Skip to content

Commit

Permalink
Improves verified-caching
Browse files Browse the repository at this point in the history
Resolves brave#12272

Auditors:

Test Plan:
  • Loading branch information
NejcZdovc committed Dec 18, 2017
1 parent 36a7f61 commit 7d40cc2
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 41 deletions.
73 changes: 47 additions & 26 deletions app/browser/api/ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ let balanceTimeoutId = false
let runTimeoutId
let promotionTimeoutId
let togglePromotionTimeoutId
let verifiedTimeoutId = false

// Database
let v2RulesetDB
Expand Down Expand Up @@ -149,12 +150,24 @@ const paymentPresent = (state, tabId, present) => {
if (!balanceTimeoutId) {
module.exports.getBalance(state)
}

getPublisherTimestamp(true)
} else if (balanceTimeoutId) {
clearTimeout(balanceTimeoutId)
balanceTimeoutId = false
}
}

const getPublisherTimestamp = (updateList) => {
client.publisherTimestamp((err, result) => {
if (err) {
console.error('Error while retrieving publisher timestamp', err.toString())
return
}
appActions.onPublisherTimestamp(result.timestamp, updateList)
})
}

const addFoundClosed = (state) => {
if (balanceTimeoutId) {
clearTimeout(balanceTimeoutId)
Expand Down Expand Up @@ -185,13 +198,12 @@ const onBootStateFile = (state) => {
try {
clientprep()
client = ledgerClient(null, underscore.extend({roundtrip: roundtrip}, clientOptions), null)
client.publisherTimestamp((err, result) => {
if (err) {
console.error('Error while retrieving publisher timestamp', err.toString())
return
}
appActions.onPublisherTimestamp(result.timestamp)
})

getPublisherTimestamp()
if (verifiedTimeoutId) {
clearInterval(verifiedTimeoutId)
}
verifiedTimeoutId = setInterval(getPublisherTimestamp, 1 * ledgerUtil.milliseconds.hour)
} catch (ex) {
state = ledgerState.resetInfo(state)
bootP = false
Expand Down Expand Up @@ -689,17 +701,17 @@ const saveVisit = (state, publisherKey, options) => {
})
state = ledgerState.setPublisher(state, publisherKey, synopsis.publishers[publisherKey])
state = updatePublisherInfo(state)
state = checkVerifiedStatus(state, publisherKey)
state = module.exports.checkVerifiedStatus(state, publisherKey)

return state
}

const checkVerifiedStatus = (state, publisherKey) => {
const checkVerifiedStatus = (state, publisherKey, publisherTimestamp) => {
if (publisherKey == null) {
return state
}

const lastUpdate = parseInt(ledgerState.getLedgerValue(state, 'publisherTimestamp'))
const lastUpdate = parseInt(publisherTimestamp || ledgerState.getLedgerValue(state, 'publisherTimestamp'))
const lastPublisherUpdate = parseInt(ledgerState.getPublisherOption(state, publisherKey, 'verifiedTimestamp') || 0)

if (lastUpdate > lastPublisherUpdate) {
Expand Down Expand Up @@ -1831,13 +1843,12 @@ const onInitRead = (state, parsedData) => {
client = ledgerClient(parsedData.personaId,
underscore.extend(parsedData.options, {roundtrip: roundtrip}, options),
parsedData)
client.publisherTimestamp((err, result) => {
if (err) {
console.error('Error while retrieving publisher timestamp', err.toString())
return
}
appActions.onPublisherTimestamp(result.timestamp)
})

getPublisherTimestamp()
if (verifiedTimeoutId) {
clearInterval(verifiedTimeoutId)
}
verifiedTimeoutId = setInterval(getPublisherTimestamp, 1 * ledgerUtil.milliseconds.hour)

// Scenario: User enables Payments, disables it, waits 30+ days, then
// enables it again -> reconcileStamp is in the past.
Expand Down Expand Up @@ -2265,13 +2276,7 @@ const transitionWalletToBat = () => {
}))
appActions.onBitcoinToBatTransitioned()
ledgerNotifications.showBraveWalletUpdated()
client.publisherTimestamp((err, result) => {
if (err) {
console.error('Error while retrieving publisher timestamp', err.toString())
return
}
appActions.onPublisherTimestamp(result.timestamp)
})
getPublisherTimestamp()
}
})
} catch (ex) {
Expand Down Expand Up @@ -2495,6 +2500,21 @@ const onPromotionResponse = (state, status) => {
return state
}

const onPublisherTimestamp = (state, oldTimestamp, newTimestamp) => {
if (oldTimestamp === newTimestamp) {
return
}

const publishers = ledgerState.getPublishers(state)
if (publishers.isEmpty()) {
return
}

publishers.forEach((publisher, key) => {
module.exports.checkVerifiedStatus(state, key, newTimestamp)
})
}

const getMethods = () => {
const publicMethods = {
backupKeys,
Expand Down Expand Up @@ -2537,7 +2557,9 @@ const getMethods = () => {
claimPromotion,
onPromotionResponse,
getBalance,
getPromotion
getPromotion,
onPublisherTimestamp,
checkVerifiedStatus
}

let privateMethods = {}
Expand Down Expand Up @@ -2571,7 +2593,6 @@ const getMethods = () => {
},
getCurrentMediaKey: (key) => currentMediaKey,
synopsisNormalizer,
checkVerifiedStatus,
roundtrip,
observeTransactions,
onWalletRecovery
Expand Down
4 changes: 4 additions & 0 deletions app/browser/reducers/ledgerReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,11 @@ const ledgerReducer = (state, action, immutableAction) => {
}
case appConstants.APP_ON_PUBLISHER_TIMESTAMP:
{
const oldValue = ledgerState.getLedgerValue(state, 'publisherTimestamp')
state = ledgerState.setLedgerValue(state, 'publisherTimestamp', action.get('timestamp'))
if (action.get('updateList')) {
ledgerApi.onPublisherTimestamp(state, oldValue, action.get('timestamp'))
}
break
}
case appConstants.APP_SAVE_LEDGER_PROMOTION:
Expand Down
26 changes: 13 additions & 13 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,19 +186,19 @@ AppStore
},
info: {
addresses: {
BAT: string,
BTC: string,
CARD_ID: string,
ETH: string,
BAT: string,
BTC: string,
CARD_ID: string,
ETH: string,
LTC: string
},
balance: number, // confirmed balance in BAT.toFixed(2)
bravery: {
days: number,
days: number,
fee: {
amount: number,
currency: string
},
},
setting: string
},
converted: string,
Expand All @@ -210,9 +210,9 @@ AppStore
paymentId: string,
probi: number,
rates:{
BTC: string,
ETH: number,
EUR: number,
BTC: string,
ETH: number,
EUR: number,
USD: number
},
reconcileFrequency: number // duration between each reconciliation in days
Expand Down Expand Up @@ -265,7 +265,7 @@ AppStore
options: {
persist: boolean,
style: string
}
}
},
panel: {
optInMarkup: {
Expand All @@ -288,7 +288,7 @@ AppStore
options: {
persist: boolean,
style: string
}
}
},
panel: {
disclaimer: string,
Expand All @@ -313,7 +313,7 @@ AppStore
options: {
persist: boolean,
style: string
}
}
},
panel: {
disclaimer: string,
Expand Down Expand Up @@ -348,7 +348,7 @@ AppStore
options: {
exclude: boolean,
verified: boolean,
verifiedTimestamp: number, // timestamp of the last change
verifiedTimestamp: number, // timestamp of the last change
stickyP: boolean
},
pinPercentage: number,
Expand Down
5 changes: 3 additions & 2 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1817,10 +1817,11 @@ const appActions = {
})
},

onPublisherTimestamp: function (timestamp) {
onPublisherTimestamp: function (timestamp, updateList) {
dispatch({
actionType: appConstants.APP_ON_PUBLISHER_TIMESTAMP,
timestamp
timestamp,
updateList
})
},

Expand Down
53 changes: 53 additions & 0 deletions test/unit/app/browser/api/ledgerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -1546,4 +1546,57 @@ describe('ledger api unit tests', function () {
assert.deepStrictEqual(unit, newSeed)
})
})

describe('onPublisherTimestamp', function () {
let checkVerifiedStatusSpy

const stateWithData = defaultAppState
.setIn(['ledger', 'synopsis', 'publishers', 'clifton.io'], Immutable.fromJS({
visits: 1
}))

before(function () {
checkVerifiedStatusSpy = sinon.spy(ledgerApi, 'checkVerifiedStatus')
ledgerApi.setClient({
publisherInfo: function () {
return false
}
})
})

afterEach(function () {
checkVerifiedStatusSpy.reset()
})

after(function () {
checkVerifiedStatusSpy.restore()
ledgerApi.setClient(undefined)
})

it('publisher timestamp is the same', function () {
ledgerApi.onPublisherTimestamp(defaultAppState, 10, 10)
assert(checkVerifiedStatusSpy.notCalled)
})

it('publisher list is empty', function () {
ledgerApi.onPublisherTimestamp(defaultAppState, 10, 20)
assert(checkVerifiedStatusSpy.notCalled)
})

it('check publishers', function () {
ledgerApi.onPublisherTimestamp(stateWithData, 10, 20)
assert(checkVerifiedStatusSpy.withArgs(sinon.match.any, 'clifton.io', 20).calledOnce)
})

it('check mulitple publishers', function () {
const multiple = stateWithData
.setIn(['ledger', 'synopsis', 'publishers', 'brave.com'], Immutable.fromJS({
visits: 1
}))
ledgerApi.onPublisherTimestamp(multiple, 10, 20)

assert.equal(checkVerifiedStatusSpy.getCall(0).args[1], 'clifton.io')
assert.equal(checkVerifiedStatusSpy.getCall(1).args[1], 'brave.com')
})
})
})

0 comments on commit 7d40cc2

Please sign in to comment.