Skip to content

Commit

Permalink
Up front work to stream line image caching / fetching
Browse files Browse the repository at this point in the history
  • Loading branch information
whomwah committed Jan 8, 2020
1 parent 25d926c commit fef5e37
Show file tree
Hide file tree
Showing 38 changed files with 414 additions and 216 deletions.
10 changes: 9 additions & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@
"!src/app.js",
"!src/constants/*.js",
"!src/config/*.js"
]
],
"coverageThreshold": {
"global": {
"branches": 100,
"functions": 100,
"lines": 100,
"statements": 100
}
}
},
"standard": {
"parser": "babel-eslint",
Expand Down
2 changes: 1 addition & 1 deletion backend/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ app.use(morgan('combined', { stream: logger.stream }))
const server = http.createServer(app)
const socketio = io(server, { pingTimeout: 30000 })

const broadcastToAll = (key, message) => Broadcaster.toAllGeneric(socketio, key, message)
const broadcastToAll = (key, message) => Broadcaster.toAll(socketio, key, message)
const broadcastMopidyStateChange = (online) => Broadcaster.toAllMopidy(socketio, { online })
const allowSocketConnections = (mopidy) => {
Scheduler.scheduleAutoPlayback({ stop: () => mopidy.playback.stop() })
Expand Down
1 change: 1 addition & 0 deletions backend/src/constants/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export default {
GENERIC: 'message',
MOPIDY: 'mopidy',
SEARCH: 'search',
IMAGE: 'image',
INCOMING_CORE: 'INCOMING MOPIDY [CORE]',
INCOMING_CLIENT: 'INCOMING CLIENT',
INCOMING_MOPIDY: 'INCOMING MOPIDY',
Expand Down
22 changes: 6 additions & 16 deletions backend/src/handlers/mopidy/image-cache/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,20 @@ const expiresDate = (imgs) => {
return new Date(today.getTime() + (day * 30))
}

const addToCacheHandler = (encodedKey) => {
const handler = (data) => {
const uri = encodedKey.split('#')[1]

return Image.create({
expireAt: expiresDate(data[uri]),
uri: encodedKey,
data
})
}
return handler
}
const storeImage = (uri, data) => Image.create({ expireAt: expiresDate(data[uri]), uri, data })
const addToCacheHandler = (uri) => (data) => storeImage(uri, data)

const ImageCache = {
check: (key, data) => new Promise((resolve) => {
if (!isImageKey(key)) return resolve({ image: false })
const encodedKey = `${key}#${data[0][0]}`
const uri = data[0][0]

fetchFromCache(encodedKey).then((response) => {
fetchFromCache(uri).then((response) => {
if (response) {
logger.info('Using cache', { key: encodedKey })
logger.info('FOUND CACHED IMAGE', { key: uri })
return resolve({ image: response.data })
} else {
return resolve({ addToCache: addToCacheHandler(encodedKey) })
return resolve({ addToCache: addToCacheHandler(uri) })
}
})
})
Expand Down
2 changes: 1 addition & 1 deletion backend/src/handlers/mopidy/image-cache/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('ImageCache', () => {
ImageCache.check('mopidy::library.getImages', [['uri123']])
.then((result) => {
expect(result).toEqual({ image: 'cached123' })
expect(logger.info).toHaveBeenCalledWith('Using cache', { key: 'mopidy::library.getImages#uri123' })
expect(logger.info).toHaveBeenCalledWith('FOUND CACHED IMAGE', { key: 'uri123' })
done()
})
})
Expand Down
10 changes: 3 additions & 7 deletions backend/src/handlers/mopidy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ const isValidTrack = (key, data) => {
return Spotify.validateTrack(data.uri)
}

const sendToClient = (bcast, ws, payload, data) => {
bcast.to(ws, payload, data)
}

const logEvent = (headers, params, response, context) => {
EventLogger({ encoded_key: headers.encoded_key }, params, response, context)
}
Expand All @@ -39,7 +35,7 @@ const MopidyHandler = (payload, ws, bcast, mopidy) => {
payload.encoded_key, data
).then((obj) => {
if (obj.image) {
sendToClient(bcast, ws, payload, obj.image)
bcast.to(ws, payload, obj.image, MessageType.IMAGE)
} else {
const apiCall = StrToFunction(mopidy, key)
logEvent(payload, data, null, MessageType.OUTGOING_MOPIDY)
Expand All @@ -51,7 +47,7 @@ const MopidyHandler = (payload, ws, bcast, mopidy) => {
if (obj.addToCache) obj.addToCache(response)
}

sendToClient(bcast, ws, payload, response)
bcast.to(ws, payload, response)
}

(data ? apiCall(data) : apiCall())
Expand All @@ -61,7 +57,7 @@ const MopidyHandler = (payload, ws, bcast, mopidy) => {
})
}).catch((err) => {
payload.encoded_key = Mopidy.VALIDATION_ERROR
sendToClient(bcast, ws, payload, err.message)
bcast.to(ws, payload, err.message)
})
}

Expand Down
3 changes: 2 additions & 1 deletion backend/src/handlers/mopidy/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ describe('MopidyHandler', () => {
const mopidy = 'mopidy'
const payload = { encoded_key: 'mopidy::library.getImages', data: [['12345zsdf23456']] }
const trackMock = jest.fn().mockResolvedValue()
const cacheMock = jest.fn().mockResolvedValue({ image: 'image' })
const cacheMock = jest.fn().mockResolvedValue({ image: 'cachedImageData' })
jest.spyOn(Spotify, 'validateTrack').mockImplementation(trackMock)
jest.spyOn(ImageCache, 'check').mockImplementation(cacheMock)

Expand All @@ -215,6 +215,7 @@ describe('MopidyHandler', () => {
expect(broadcastMock).toHaveBeenCalledWith(
ws,
{ data: [['12345zsdf23456']], encoded_key: 'mopidy::library.getImages' },
'cachedImageData',
'image'
)
done()
Expand Down
4 changes: 2 additions & 2 deletions backend/src/utils/broadcaster/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const Broadcaster = {
})
},

toAllGeneric: (socket, key, message) => {
toAll: (socket, key, message) => {
const payload = Payload.encodeToJson(key, message)

try {
Expand All @@ -37,7 +37,7 @@ const Broadcaster = {
EventLogger({ encoded_key: 'state' }, null, payload, MessageType.OUTGOING_API_ALL)
socket.emit(MessageType.MOPIDY, payload)
} catch (e) {
logger.error('Broadcaster#toAll', { message: e.message })
logger.error('Broadcaster#toAllMopidy', { message: e.message })
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions backend/src/utils/broadcaster/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('Broadcaster', () => {
})
})

describe('#toAllGeneric', () => {
describe('#toAll', () => {
it('handles call', () => {
const sendMock = jest.fn()
const socketMock = {
Expand All @@ -98,7 +98,7 @@ describe('Broadcaster', () => {
const key = 'mopidy::playback.next'
const message = 'hello mum'

broadcaster.toAllGeneric(socketMock, key, message)
broadcaster.toAll(socketMock, key, message)
expect(sendMock.mock.calls.length).toEqual(1)
expect(sendMock.mock.calls[0][0]).toEqual('message')
expect(sendMock.mock.calls[0][1]).toEqual('{"key":"mopidy::playback.next","data":"hello mum"}')
Expand All @@ -116,7 +116,7 @@ describe('Broadcaster', () => {
const key = 'mopidy::playback.next'
const message = 'hello mum'

broadcaster.toAllGeneric(socketMock, key, message)
broadcaster.toAll(socketMock, key, message)
expect(sendMock.mock.calls[0]).toEqual(['message', '{"key":"mopidy::playback.next","data":"hello mum"}'])
expect(logger.error.mock.calls[0][0]).toEqual('Broadcaster#toAll')
expect(logger.error.mock.calls[0][1]).toEqual({ message: 'oops' })
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('Broadcaster', () => {

broadcaster.toAllMopidy(socketMock, message)
expect(sendMock.mock.calls[0]).toEqual(['mopidy', '{"online":false}'])
expect(logger.error.mock.calls[0][0]).toEqual('Broadcaster#toAll')
expect(logger.error.mock.calls[0][0]).toEqual('Broadcaster#toAllMopidy')
expect(logger.error.mock.calls[0][1]).toEqual({ message: 'oops' })
})
})
Expand Down
21 changes: 18 additions & 3 deletions backend/src/utils/track/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
import Track from 'services/mongodb/models/track'
import Image from 'services/mongodb/models/image'
import logger from 'config/winston'

export function findTracks (uris) {
return new Promise((resolve, reject) => {
Track.find({'_id': { $in: uris }})
.then(tracks => resolve(tracks))
Track.find({ _id: { $in: uris } })
.then(tracks => {
logger.info('FOUND CACHED TRACKS', { keys: uris })
return resolve(tracks)
})
.catch(err => reject(err))
})
}

export function findImages (uris) {
return new Promise((resolve, reject) => {
Image.find({ uri: { $in: uris } })
.then(images => {
logger.info('FOUND CACHED IMAGES', { keys: uris })
return resolve(images)
})
.catch(err => reject(err))
})
}
Expand All @@ -30,4 +45,4 @@ export function addTracks (uris, user) {
})
}

export default { findTracks, addTracks }
export default { findTracks, findImages, addTracks }
34 changes: 29 additions & 5 deletions backend/src/utils/track/index.spec.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
import { findTracks, addTracks } from './index'
import { findTracks, addTracks, findImages } from './index'
import Track from 'services/mongodb/models/track'
import Image from 'services/mongodb/models/image'
import logger from 'config/winston'
jest.mock('config/winston')
jest.mock('services/mongodb/models/track')
jest.mock('services/mongodb/models/image')

const userObject = {
_id: '999',
fullname: 'Fred Spanner'
}

describe('trackUtils', () => {
describe('#findTracks', () => {
afterEach(() => {
jest.clearAllMocks()
})
afterEach(() => {
jest.clearAllMocks()
})

describe('#findTracks', () => {
it('makes a call to findOne Track document', () => {
expect.assertions(1)
Track.find.mockResolvedValue([{ _id: '123' }])
Expand All @@ -36,6 +38,28 @@ describe('trackUtils', () => {
})
})

describe('#findImages', () => {
it('makes a call to findImages', () => {
expect.assertions(1)
Image.find.mockResolvedValue([{ _id: '123' }])
return findImages('123').then(() => {
expect(Image.find).toHaveBeenCalledWith({
uri: {
$in: '123'
}
})
})
})

it('handles errors', () => {
expect.assertions(1)
Image.find.mockRejectedValue(new Error('bang'))
return findImages('uri123').catch((error) => {
expect(error.message).toEqual('bang')
})
})
})

describe('#addTrack', () => {
const trackObject = { trackUri: '123' }
const fakeDate = new Date(1222222224332)
Expand Down
5 changes: 4 additions & 1 deletion backend/src/utils/transformer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,12 @@ const Transform = {
clearSetTimeout(recommendTimer)
if (data && data.length > 0) return resolve(TransformTrack(data[0].track))
return resolve()
case Mopidy.LIBRARY_GET_IMAGES:
const uri = Object.keys(data)[0]
const image = data[uri][0].uri
return resolve({ [uri]: image })
case Mopidy.TRACKLIST_CLEAR:
case Mopidy.CONNECTION_ERROR:
case Mopidy.LIBRARY_GET_IMAGES:
case Mopidy.MIXER_GET_VOLUME:
case Mopidy.MIXER_SET_VOLUME:
case Mopidy.PLAYBACK_GET_TIME_POSITION:
Expand Down
14 changes: 9 additions & 5 deletions backend/src/utils/transformer/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,17 @@ describe('Transformer', () => {
})

describe('event:volumeChanged', () => {
data = { volume: 99 }

it('returns the volume data passed in', () => {
data = { volume: 99 }
expect.assertions(1)
return Transformer.mopidyCoreMessage(h('mopidy::event:volumeChanged'), data)
.then(returnData => expect(returnData).toEqual(data.volume))
})
})

describe('event:playbackStateChanged', () => {
data = { new_state: 'playing' }

it('returns the playback state data passed in', () => {
data = { new_state: 'playing' }
expect.assertions(1)
return Transformer.mopidyCoreMessage(h('mopidy::event:playbackStateChanged'), data)
.then(returnData => expect(returnData).toEqual(data.new_state))
Expand All @@ -173,9 +171,15 @@ describe('Transformer', () => {

describe('library.getImages', () => {
it('returns the data passed in', () => {
data = {
'spotify123abc': [
{ uri: 'path/to/img/1' },
{ uri: 'path/to/img/2' }
]
}
expect.assertions(1)
return Transformer.message(h('mopidy::library.getImages'), data)
.then(returnData => expect(returnData).toEqual(data))
.then(returnData => expect(returnData).toEqual({'spotify123abc': 'path/to/img/1'}))
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@ export default function (json) {
length: json.length || json.duration_ms
}

if (json.image) {
payload.image = json.image
}

if (json.album) {
payload.album = {
uri: json.album.uri,
name: json.album.name,
year: json.album.date
}

if (json.album.images && json.album.images.length > 0) {
if (!payload.image && json.album.images && json.album.images.length > 0) {
payload.image = json.album.images[0].url
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ describe('TransformerTrack', () => {
it('transforms the track', () => {
const albumTrack = payload[0]
albumTrack.addedBy = 'duncan'
albumTrack.image = 'http://path/to/image'

expect(TransformerTrack(albumTrack)).toEqual({
track: {
uri: 'spotify:track:1yzSSn5Sj1azuo7RgwvDb3',
name: 'No Time for Caution',
image: 'http://path/to/image',
year: '2014',
length: 246000,
addedBy: 'duncan',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
import TransformTrack from 'utils/transformer/transformers/mopidy/track'
import { findTracks } from 'utils/track'
import { findTracks, findImages } from 'utils/track'

const Tracklist = (json) => {
return new Promise((resolve) => {
const trackUris = json.map(data => data.uri)
const imageUris = json.filter(data => data.album).map(data => data.album.uri)
const requests = [
findTracks(trackUris),
findImages(imageUris)
]

findTracks(trackUris).then(tracks => {
Promise.all(requests).then((responses) => {
const tracks = responses[0]
const images = responses[1]
const decoratedTracks = json.map(data => {
const trackData = tracks.find(track => track._id === data.uri)
const imageData = images.find(image => image.uri === (data.album && data.album.uri))

if (trackData) {
data.addedBy = trackData.addedBy.reverse().map(user => user[0])
}

if (imageData) {
data.image = imageData.data[data.album.uri][0].uri
}

return TransformTrack(data)
})

Expand Down
Loading

0 comments on commit fef5e37

Please sign in to comment.