Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable29] fix(files): Correctly parse external shares for files UI #47692

Merged
merged 2 commits into from
Sep 3, 2024
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
58 changes: 53 additions & 5 deletions apps/files_sharing/src/services/SharingService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
*/
import type { OCSResponse } from '@nextcloud/typings/ocs'
import { expect } from '@jest/globals'
import { Type } from '@nextcloud/sharing'
import { File, Folder } from '@nextcloud/files'
import { ShareType } from '@nextcloud/sharing'
import * as auth from '@nextcloud/auth'
import axios from '@nextcloud/axios'

import { getContents } from './SharingService'
import { File, Folder } from '@nextcloud/files'
import logger from './logger'

global.window.OC = {
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('SharingService filtering', () => {
data: [
{
id: '62',
share_type: Type.SHARE_TYPE_USER,
share_type: ShareType.User,
uid_owner: 'test',
displayname_owner: 'test',
permissions: 31,
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('SharingService filtering', () => {
})

test('Shared with others filtering', async () => {
const shares = await getContents(false, true, false, false, [Type.SHARE_TYPE_USER])
const shares = await getContents(false, true, false, false, [ShareType.User])

expect(axios.get).toHaveBeenCalledTimes(1)
expect(shares.contents).toHaveLength(1)
Expand All @@ -198,7 +198,7 @@ describe('SharingService filtering', () => {
})

test('Shared with others filtering empty', async () => {
const shares = await getContents(false, true, false, false, [Type.SHARE_TYPE_LINK])
const shares = await getContents(false, true, false, false, [ShareType.Link])

expect(axios.get).toHaveBeenCalledTimes(1)
expect(shares.contents).toHaveLength(0)
Expand Down Expand Up @@ -294,6 +294,25 @@ describe('SharingService share to Node mapping', () => {
tags: [window.OC.TAG_FAVORITE],
}

const remoteFile = {
mimetype: 'text/markdown',
mtime: 1688721600,
permissions: 19,
type: 'file',
file_id: 1234,
id: 4,
share_type: ShareType.User,
parent: null,
remote: 'http://exampe.com',
remote_id: '12345',
share_token: 'share-token',
name: '/test.md',
mountpoint: '/shares/test.md',
owner: 'owner-uid',
user: 'sharee-uid',
accepted: true,
}

test('File', async () => {
jest.spyOn(axios, 'get').mockReturnValueOnce(Promise.resolve({
data: {
Expand Down Expand Up @@ -353,6 +372,35 @@ describe('SharingService share to Node mapping', () => {
expect(folder.attributes.favorite).toBe(1)
})

test('Remote file', async () => {
jest.spyOn(axios, 'get').mockReturnValueOnce(Promise.resolve({
data: {
ocs: {
data: [remoteFile],
},
},
}))

const shares = await getContents(false, true, false, false)

expect(axios.get).toHaveBeenCalledTimes(1)
expect(shares.contents).toHaveLength(1)

const file = shares.contents[0] as File
expect(file).toBeInstanceOf(File)
expect(file.fileid).toBe(1234)
expect(file.source).toBe('http://localhost/remote.php/dav/files/test/shares/test.md')
expect(file.owner).toBe('owner-uid')
expect(file.mime).toBe('text/markdown')
expect(file.mtime?.getTime()).toBe(remoteFile.mtime * 1000)
// not available for remote shares
expect(file.size).toBe(undefined)
expect(file.permissions).toBe(0)
expect(file.root).toBe('/files/test')
expect(file.attributes).toBeInstanceOf(Object)
expect(file.attributes.favorite).toBe(0)
})

test('Empty', async () => {
jest.spyOn(logger, 'error').mockImplementationOnce(() => {})
jest.spyOn(axios, 'get').mockReturnValueOnce(Promise.resolve({
Expand Down
31 changes: 20 additions & 11 deletions apps/files_sharing/src/services/SharingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
/* eslint-disable camelcase, n/no-extraneous-import */
import type { AxiosPromise } from 'axios'
import type { AxiosPromise } from '@nextcloud/axios'
import type { OCSResponse } from '@nextcloud/typings/ocs'

import { Folder, File, type ContentsWithRoot, Permission } from '@nextcloud/files'
Expand All @@ -40,10 +39,16 @@ const ocsEntryToNode = async function(ocsEntry: any): Promise<Folder | File | nu
try {
// Federated share handling
if (ocsEntry?.remote_id !== undefined) {
const mime = (await import('mime')).default
// This won't catch files without an extension, but this is the best we can do
ocsEntry.mimetype = mime.getType(ocsEntry.name)
ocsEntry.item_type = ocsEntry.mimetype ? 'file' : 'folder'
if (!ocsEntry.mimetype) {
const mime = (await import('mime')).default
// This won't catch files without an extension, but this is the best we can do
ocsEntry.mimetype = mime.getType(ocsEntry.name)
}
ocsEntry.item_type = ocsEntry.type || (ocsEntry.mimetype ? 'file' : 'folder')

// different naming for remote shares
ocsEntry.item_mtime = ocsEntry.mtime
ocsEntry.file_target = ocsEntry.file_target || ocsEntry.mountpoint

// Need to set permissions to NONE for federated shares
ocsEntry.item_permissions = Permission.NONE
Expand All @@ -60,14 +65,15 @@ const ocsEntryToNode = async function(ocsEntry: any): Promise<Folder | File | nu

// If this is an external share that is not yet accepted,
// we don't have an id. We can fallback to the row id temporarily
const fileid = ocsEntry.file_source || ocsEntry.id
// local shares (this server) use `file_source`, but remote shares (federated) use `file_id`
const fileid = ocsEntry.file_source || ocsEntry.file_id || ocsEntry.id

// Generate path and strip double slashes
const path = ocsEntry?.path || ocsEntry.file_target || ocsEntry.name
const path = ocsEntry.path || ocsEntry.file_target || ocsEntry.name
const source = generateRemoteUrl(`dav/${rootPath}/${path}`.replaceAll(/\/\//gm, '/'))

let mtime = ocsEntry.item_mtime ? new Date((ocsEntry.item_mtime) * 1000) : undefined
// Prefer share time if more recent than item mtime
let mtime = ocsEntry?.item_mtime ? new Date((ocsEntry.item_mtime) * 1000) : undefined
if (ocsEntry?.stime > (ocsEntry?.item_mtime || 0)) {
mtime = new Date((ocsEntry.stime) * 1000)
}
Expand All @@ -88,7 +94,8 @@ const ocsEntryToNode = async function(ocsEntry: any): Promise<Folder | File | nu
'owner-id': ocsEntry?.uid_owner,
'owner-display-name': ocsEntry?.displayname_owner,
'share-types': ocsEntry?.share_type,
favorite: ocsEntry?.tags?.includes(window.OC.TAG_FAVORITE) ? 1 : 0,
'share-attributes': ocsEntry?.attributes || '[]',
favorite: ocsEntry?.tags?.includes((window.OC as Nextcloud.v29.OC & { TAG_FAVORITE: string }).TAG_FAVORITE) ? 1 : 0,
},
})
} catch (error) {
Expand Down Expand Up @@ -159,6 +166,8 @@ const getDeletedShares = function(): AxiosPromise<OCSResponse<any>> {
/**
* Group an array of objects (here Nodes) by a key
* and return an array of arrays of them.
* @param nodes Nodes to group
* @param key The attribute to group by
*/
const groupBy = function(nodes: (Folder | File)[], key: string) {
return Object.values(nodes.reduce(function(acc, curr) {
Expand All @@ -168,7 +177,7 @@ const groupBy = function(nodes: (Folder | File)[], key: string) {
}

export const getContents = async (sharedWithYou = true, sharedWithOthers = true, pendingShares = false, deletedshares = false, filterTypes: number[] = []): Promise<ContentsWithRoot> => {
const promises = [] as AxiosPromise<OCSResponse<any>>[]
const promises = [] as AxiosPromise<OCSResponse<unknown>>[]

if (sharedWithYou) {
promises.push(getSharedWithYou(), getRemoteShares())
Expand Down
Loading
Loading