Skip to content

Commit

Permalink
Merge pull request #5073 from owncloud/lazy-avatars
Browse files Browse the repository at this point in the history
background loading for avatars
  • Loading branch information
kulmann authored May 11, 2021
2 parents 47c2dda + d453b6c commit bc0774a
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 31 deletions.
9 changes: 9 additions & 0 deletions changelog/unreleased/enhancement-lazy-file-avatar-loading
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Enhancement: Lazy file avatar loading

We've changed the way how large file lists get rendered.
In some cases where we had a long list of files, the loading of avatars could lead to long waiting times till the first paint happens.

Now we first render the list of files, load the associated avatars in the background and then update the ui.

https://github.com/owncloud/web/pull/5073
https://github.com/owncloud/web/issues/4973
101 changes: 82 additions & 19 deletions packages/web-app-files/src/helpers/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,25 @@ export function attachIndicators(resource, sharesTree) {
* @param {Boolean} allowSharePerm Asserts whether the reshare permission is available
* @param {String} server The url of the backend
* @param {String} token The access token of the authenticated user
* @param {Object} client The ownCloud SDK client
* @param {Function} updateFn The closure action that gets called on update
*/
export async function aggregateResourceShares(
export function aggregateResourceShares(
shares,
incomingShares = false,
allowSharePerm,
server,
token
token,
client,
updateFn
) {
if (incomingShares) {
return Promise.all(
_(shares)
.orderBy(['file_target', 'permissions'], ['asc', 'desc'])
.map(
async share =>
await buildSharedResource(share, incomingShares, allowSharePerm, server, token)
)
)
return _.chain(shares)
.orderBy(['file_target', 'permissions'], ['asc', 'desc'])
.map(share =>
buildSharedResource(share, incomingShares, allowSharePerm, server, token, client, updateFn)
)
.value()
}

shares.sort((a, b) => a.path.localeCompare(b.path))
Expand All @@ -132,7 +134,7 @@ export async function aggregateResourceShares(
prev.sharedWith.push({
username: share.share_with,
displayName: share.share_with_displayname,
avatar: await getAvatarSrc(share.share_with, server, token)
avatar: undefined
})
} else if (share.share_type === shareTypes.link) {
prev.sharedWith.push({
Expand All @@ -149,7 +151,7 @@ export async function aggregateResourceShares(
{
username: share.share_with,
displayName: share.share_with_displayname,
avatar: await getAvatarSrc(share.share_with, server, token)
avatar: undefined
}
]
} else if (share.share_type === shareTypes.link) {
Expand All @@ -165,19 +167,19 @@ export async function aggregateResourceShares(
resources.push(share)
}

return Promise.all(
resources.map(
async share => await buildSharedResource(share, incomingShares, allowSharePerm, server, token)
)
return resources.map(share =>
buildSharedResource(share, incomingShares, allowSharePerm, server, token, client, updateFn)
)
}

export async function buildSharedResource(
export function buildSharedResource(
share,
incomingShares = false,
allowSharePerm,
server,
token
token,
client,
updateFn
) {
const resource = {
id: share.item_source,
Expand All @@ -194,9 +196,10 @@ export async function buildSharedResource(
{
username: share.uid_owner,
displayName: share.displayname_owner,
avatar: await getAvatarSrc(share.uid_file_owner, server, token)
avatar: undefined
}
]

resource.status = share.state
resource.name = path.basename(share.file_target)
resource.path = share.file_target
Expand Down Expand Up @@ -225,6 +228,49 @@ export async function buildSharedResource(
resource.icon = isFolder ? 'folder' : getFileIcon(resource.extension)
resource.sdate = share.stime * 1000

updateResource(async () => {
const avatars = new Map()
;['sharedWith', 'owner'].forEach(k => {
;(resource[k] || []).forEach((obj, i) => {
if (!_.has(obj, 'avatar')) {
return
}
avatars.set(`${k}.[${i}].avatar`, obj.username)
})
})

if (!avatars.size) {
return
}

await Promise.all(
Array.from(avatars).map(avatar =>
(async () => {
let url
try {
url = await getAvatarSrc(avatar[1], server, token, client)
} catch (e) {
avatars.delete(avatar[0])
return
}

avatars.set(avatar[0], url)
})()
)
)

if (!avatars.size) {
return
}

const cResource = _.cloneDeep(resource)
avatars.forEach((value, key) => {
_.set(cResource, key, value)
})

return cResource
}, updateFn)

return resource
}

Expand Down Expand Up @@ -348,3 +394,20 @@ export function buildDeletedResource(resource) {
indicators: []
}
}

export const updateResource = (task = async () => {}, cb = () => {}) => {
;(async () => {
let val
try {
val = await task()
} catch (e) {
return
}

if (!val) {
return
}

cb(val)
})()
}
41 changes: 35 additions & 6 deletions packages/web-app-files/src/helpers/user.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,46 @@
export async function getAvatarSrc(userId, server, token) {
const headers = new Headers()
const url = server + 'remote.php/dav/avatars/' + userId + '/128.png'
const urlCache = new Map()
const ttl = 60 * 1000
const addUrl = (userId, url) => {
urlCache.set(userId, {
ts: Date.now() + ttl,
url
})

return url
}
const getUrl = userId => {
if (urlCache.has(userId)) {
const { ts, url } = urlCache.get(userId)
if (Date.now() <= ts) {
return url
}
}
}

export async function getAvatarSrc(userId, server, token, client) {
const cachedUrl = getUrl(userId)

if (cachedUrl) {
return cachedUrl
}

const url = server + 'remote.php/dav/avatars/' + userId + '/128.png'
const headers = new Headers()
headers.append('Authorization', 'Bearer ' + token)

const response = await fetch(url, {
method: 'HEAD',
headers
})

if (response.status === 200) {
return await this.$client.signUrl(url)
if (response.status !== 200) {
throw new Error(response.statusText)
}

if (!client || typeof client.signUrl !== 'function') {
return addUrl(userId, url)
}

return ''
const signedUrl = await client.signUrl(url)
return addUrl(userId, signedUrl)
}
2 changes: 1 addition & 1 deletion packages/web-app-files/src/views/SharedViaLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default {
return
}
resources = await aggregateResourceShares(
resources = aggregateResourceShares(
resources,
false,
!this.isOcis,
Expand Down
6 changes: 4 additions & 2 deletions packages/web-app-files/src/views/SharedWithMe.vue
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,14 @@ export default {
return
}
resources = await aggregateResourceShares(
resources = aggregateResourceShares(
resources,
true,
!this.isOcis,
this.configuration.server,
this.getToken
this.getToken,
this.$client,
this.UPDATE_RESOURCE
)
this.LOAD_FILES({ currentFolder: rootFolder, files: resources })
Expand Down
13 changes: 10 additions & 3 deletions packages/web-app-files/src/views/SharedWithOthers.vue
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,12 @@ export default {
methods: {
...mapActions('Files', ['setHighlightedFile', 'loadIndicators', 'loadPreviews']),
...mapMutations('Files', ['LOAD_FILES', 'SELECT_RESOURCES', 'CLEAR_CURRENT_FILES_LIST']),
...mapMutations('Files', [
'LOAD_FILES',
'SELECT_RESOURCES',
'CLEAR_CURRENT_FILES_LIST',
'UPDATE_RESOURCE'
]),
...mapMutations(['SET_QUOTA']),
async loadResources() {
Expand All @@ -153,12 +158,14 @@ export default {
return
}
resources = await aggregateResourceShares(
resources = aggregateResourceShares(
resources,
false,
!this.isOcis,
this.configuration.server,
this.getToken
this.getToken,
this.$client,
this.UPDATE_RESOURCE
)
this.LOAD_FILES({ currentFolder: rootFolder, files: resources })
Expand Down
63 changes: 63 additions & 0 deletions packages/web-app-files/tests/helpers/resources.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { buildSharedResource } from '../../src/helpers/resources'

describe('buildSharedResource', () => {
const resourceFactory = (incomingShares = false) => {
let doResolve
let pending = true
let fulfilled = false

const resourceUpdater = new Promise(resolve => (doResolve = resolve))
resourceUpdater.pending = () => pending
resourceUpdater.fulfilled = () => fulfilled

const resource = buildSharedResource(
{
file_target: '/path.pdf',
path: '/path.pdf',
displayname_owner: 'displayname_owner',
sharedWith: [
{
username: 'sharedWith',
avatar: undefined
}
]
},
incomingShares,
'',
'',
'',
'',
r => {
fulfilled = true
pending = false
doResolve(r)
}
)

return { resource, resourceUpdater }
}

it('fetches avatars for outgoing shares in the background', async () => {
const { resource, resourceUpdater } = resourceFactory()
expect(resource.sharedWith[0].avatar).toBeFalsy()
expect(resourceUpdater.pending()).toBeTruthy()
expect(resourceUpdater.fulfilled()).toBeFalsy()

const updatedResource = await resourceUpdater
expect(updatedResource.sharedWith[0].avatar).toBeTruthy()
expect(resourceUpdater.pending()).toBeFalsy()
expect(resourceUpdater.fulfilled()).toBeTruthy()
})

it('fetches avatars for incoming shares in the background', async () => {
const { resource, resourceUpdater } = resourceFactory(true)
expect(resource.owner[0].avatar).toBeFalsy()
expect(resourceUpdater.pending()).toBeTruthy()
expect(resourceUpdater.fulfilled()).toBeFalsy()

const updatedResource = await resourceUpdater
expect(updatedResource.owner[0].avatar).toBeTruthy()
expect(resourceUpdater.pending()).toBeFalsy()
expect(resourceUpdater.fulfilled()).toBeTruthy()
})
})
28 changes: 28 additions & 0 deletions packages/web-app-files/tests/helpers/user.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { getAvatarSrc } from '../../src/helpers/user'

describe('getAvatarSrc', () => {
it('throws an error', async () => {
fetch.mockResponse(new Error(), { status: 404 })
await expect(getAvatarSrc('userId', 'server', 'token')).rejects.toBeTruthy()
})

it('returns a signed url', async () => {
fetch.mockResponse('', { status: 200 })
const client = { signUrl: jest.fn() }
await getAvatarSrc('userId', 'server', 'token', client)
expect(client.signUrl.mock.calls.length).toBe(1)
})

it('returns the url if no valid client is present', async () => {
fetch.mockResponse('', { status: 200 })
const url = await getAvatarSrc('userId', 'server', 'token')
expect(url).toBe('serverremote.php/dav/avatars/userId/128.png')
})

it('fetches avatarSrc from cache if url exists there', async () => {
fetch.mockResponse('', { status: 200 })
const client = { signUrl: jest.fn() }
await getAvatarSrc('userId', 'server', 'token', client)
expect(client.signUrl.mock.calls.length).toBe(0)
})
})

0 comments on commit bc0774a

Please sign in to comment.