Skip to content

Improve serialization of VFS URL query parameters #165

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

Closed
wants to merge 15 commits into from
Closed
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
25 changes: 25 additions & 0 deletions __tests__/utils/fetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as fetch from '../../src/utils/fetch';

describe('utils.fetch#encodeQueryData', () => {
test('should create valid query string', () => {
const result1 = fetch.encodeQueryData({
a: 1,
b: true,
c: null,
d: 'foo',
e: undefined
});

const result2 = fetch.encodeQueryData({
a: 1,
b: {
c: {
d: 'foo'
}
}
});

expect(result1).toEqual('a=1&b=true&c=null&d=foo&e=undefined');
expect(result2).toEqual('a=1&b=%7B%22c%22%3A%7B%22d%22%3A%22foo%22%7D%7D');
});
});
21 changes: 16 additions & 5 deletions __tests__/utils/vfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ describe('utils.vfs#transformReaddir', () => {
size: 666
}];

const check = (options = {}) => vfs.transformReaddir({path: root}, input, options);
const checkMap = (options = {}, key = 'filename') => check(options).map(iter => iter[key]);
const check = (options = {}, capability = {}) => vfs.transformReaddir({path: root}, input, capability, options);
const checkMap = (options = {}, key = 'filename', capability = {}) => check(options, capability).map(iter => iter[key]);

test('Should add parent directory', () => {
expect(check({
Expand All @@ -88,7 +88,7 @@ describe('utils.vfs#transformReaddir', () => {
test('Should remove dotfiles', () => {
expect(checkMap({
showHiddenFiles: false
})).toEqual(['..', 'directory', 'xdirectory', 'file', 'xfile']);
})).toEqual(['..', 'directory', 'file', 'xdirectory', 'xfile']);
});

test('Should sort by descending order', () => {
Expand All @@ -98,7 +98,7 @@ describe('utils.vfs#transformReaddir', () => {
});

return expect(result)
.toEqual(['..', 'xdirectory', 'directory', 'xfile', 'file']);
.toEqual(['..', 'xfile', 'xdirectory', 'file', 'directory']);
});

test('Should sort by ascending order', () => {
Expand All @@ -108,7 +108,7 @@ describe('utils.vfs#transformReaddir', () => {
});

return expect(result)
.toEqual(['..', 'directory', 'xdirectory', 'file', 'xfile']);
.toEqual(['..', 'directory', 'file', 'xdirectory', 'xfile']);
});

test('Should sort by specified column', () => {
Expand All @@ -123,6 +123,17 @@ describe('utils.vfs#transformReaddir', () => {

expect(every).toEqual(true);
});

test('Should not sort', () => {
const result = checkMap({}, 'filename', {
foo: {
sort: true
}
});

return expect(result)
.toEqual(['..', 'directory', 'xdirectory', 'file', 'xfile']);
});
});

describe('utils.vfs#getFileIcon', () => {
Expand Down
2 changes: 1 addition & 1 deletion __tests__/vfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('VFS', () => {
.toBeInstanceOf(ArrayBuffer);
});

test('writefile - blob', () => {
test('#writefile - blob', () => {
return expect(call('writefile', 'null:/filename', new Blob()))
.resolves
.toBe(-1);
Expand Down
1 change: 1 addition & 0 deletions src/adapters/vfs/null.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* @param {object} [options] Adapter options
*/
const nullAdapter = ({
capabilities: (path, options) => Promise.resolve({}),
readdir: (path, options) => Promise.resolve([]),
readfile: (path, type, options) => Promise.resolve({body: new ArrayBuffer(), mime: 'application/octet-stream'}),
writefile: (path, data, options) => Promise.resolve(-1),
Expand Down
15 changes: 11 additions & 4 deletions src/adapters/vfs/system.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* @license Simplified BSD License
*/

const getters = ['exists', 'stat', 'readdir', 'readfile'];
const getters = ['capabilities', 'exists', 'stat', 'readdir', 'readfile'];

const requester = core => (fn, body, type, options = {}) =>
core.request(`/vfs/${fn}`, {
Expand Down Expand Up @@ -57,9 +57,16 @@ const methods = (core, request) => {
.then(({body}) => body);

return {
capabilities: ({path}, options) => request('capabilities', {
path,
options
}, 'json').then(({body}) => {
return body;
}),

readdir: ({path}, options) => request('readdir', {
path,
options: {}
options,
}, 'json').then(({body}) => body),

readfile: ({path}, type, options) =>
Expand Down Expand Up @@ -89,7 +96,7 @@ const methods = (core, request) => {
stat: passthrough('stat'),

url: ({path}, options) => Promise.resolve(
core.url(`/vfs/readfile?path=${encodeURIComponent(path)}`)
core.url(`/vfs/readfile?path.s=${encodeURIComponent(path)}`)
),

search: ({path}, pattern, options) =>
Expand All @@ -102,7 +109,7 @@ const methods = (core, request) => {
download: ({path}, options = {}) => {
const json = encodeURIComponent(JSON.stringify({download: true}));

return Promise.resolve(`/vfs/readfile?options=${json}&path=` + encodeURIComponent(path))
return Promise.resolve(`/vfs/readfile?options=${json}&path.s=` + encodeURIComponent(path))
.then(url => {
return (options.target || window).open(url);
});
Expand Down
1 change: 1 addition & 0 deletions src/filesystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import merge from 'deepmerge';
* Filesystem Adapter Methods
* TODO: typedef
* @typedef {Object} FilesystemAdapterMethods
* @property {Function} capabilities
* @property {Function} readdir
* @property {Function} readfile
* @property {Function} writefile
Expand Down
27 changes: 19 additions & 8 deletions src/utils/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,22 @@
* @license Simplified BSD License
*/

/*
* Creates URL request path
*/
const encodeQueryData = data => Object.keys(data)
.filter(k => typeof data[k] !== 'object')
.map(k => encodeURIComponent(k) + '=' + encodeURIComponent(data[k]))
.join('&');

// /*
// * Creates URL request path
// */
export const encodeQueryData = (data) => {
const replacer = (k, v)=>(v === undefined ? null : v);
const pairs = Object.entries(data).map(([key, val]) => {
const isNull = val === null;
if (typeof val === 'object' && !isNull) {
return `${encodeURIComponent(key)}=${encodeURIComponent(JSON.stringify(val, replacer))}`;
} else {
return `${encodeURIComponent(key)}=${encodeURIComponent(val)}`;
}
});
return pairs.join('&');
};

const bodyTypes = [
window.ArrayBuffer,
Expand Down Expand Up @@ -65,7 +74,9 @@ const createFetchOptions = (url, options, type) => {
}

if (fetchOptions.body && fetchOptions.method.toLowerCase() === 'get') {
url += '?' + encodeQueryData(fetchOptions.body);
if(encodeQueryData(fetchOptions.body) !== '') {
url += '?' + encodeQueryData(fetchOptions.body);
}
delete fetchOptions.body;
}

Expand Down
55 changes: 33 additions & 22 deletions src/utils/vfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ const sortDefault = (k, d) => (a, b) =>
? (d === 'asc' ? 1 : 0)
: (d === 'asc' ? 0 : 1));

/*
* Sort by educated guess
*/
const sortAscii = (k, d) => (a, b) => d === 'asc'
? (a[k] < b[k]) ? -1 : 1
: (a[k] < b[k]) ? 1 : -1;

/*
* Sorts an array of files
*/
Expand All @@ -86,6 +93,8 @@ const sortFn = t => {
return sortString;
} else if (t === 'date') {
return sortDate;
} else if (t === 'ascii') {
return sortAscii;
}

return sortDefault;
Expand Down Expand Up @@ -175,14 +184,20 @@ export const humanFileSize = (bytes, si = false) => {
* Transforms a readdir result
* @param {object} root The path to the readdir root
* @param Object[] files An array of readdir results
* @param Object[] capabilityCache An object of mount point capabilities
* @param {object} options Options
* @param {Boolean} [options.showHiddenFiles=false] Show hidden files
* @param {Function} [options.filter] A filter
* @param {string} [options.sortBy='filename'] Sort by this attribute
* @param {string} [options.sortDir='asc'] Sort in this direction
* @return {Object[]}
*/
export const transformReaddir = ({path}, files, options = {}) => {
export const transformReaddir = ({path}, files, capabilityCache, options = {}) => {
const mountPoint = path => path.split(':/')[0];
let mountPointSort = false;
if(capabilityCache[mountPoint(path)] !== undefined) {
mountPointSort = capabilityCache[mountPoint(path)].sort;
}
options = {
showHiddenFiles: false,
sortBy: 'filename',
Expand All @@ -191,48 +206,44 @@ export const transformReaddir = ({path}, files, options = {}) => {
};

let {sortDir, sortBy, filter} = options;

if (typeof filter !== 'function') {
filter = () => true;
}

if (['asc', 'desc'].indexOf(sortDir) === -1) {
sortDir = 'asc';
}

const filterHidden = options.showHiddenFiles
? () => true
: file => file.filename.substr(0, 1) !== '.';

const sorter = sortMap[sortBy]
? sortMap[sortBy]
: sortFn('string');

const modify = (file) => ({
...file,
humanSize: humanFileSize(file.size)
});

// FIXME: Optimize this to one chain!
let sortedSpecial = [];
let sortedFiles = [];

const sortedSpecial = createSpecials(path)
.sort(sorter(sortBy, sortDir))
sortedSpecial = createSpecials(path)
.map(modify);

const sortedDirectories = files.filter(file => file.isDirectory)
.sort(sorter(sortBy, sortDir))
.filter(filterHidden)
sortedFiles = files.filter(filterHidden)
.filter(filter)
.map(modify);

const sortedFiles = files.filter(file => !file.isDirectory)
.sort(sorter(sortBy, sortDir))
.filter(filterHidden)
.filter(filter)
.map(modify);
if(!mountPointSort) {
if (['asc', 'desc'].indexOf(sortDir) === -1) {
sortDir = 'asc';
}
const sorter = sortMap[sortBy]
? sortMap[sortBy]
: sortFn('ascii');
sortedSpecial = sortedSpecial
.sort(sorter(sortBy, sortDir));
sortedFiles = sortedFiles
.sort(sorter(sortBy, sortDir));
}

return [
...sortedSpecial,
...sortedDirectories,
...sortedFiles
];
};
Expand Down
32 changes: 30 additions & 2 deletions src/vfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ import {
* @property {object} [stat]
*/

// Cache the capability of each mount point
let capabilityCache = {};

// Makes sure our input paths are object(s)
const pathToObject = path => ({
id: null,
Expand All @@ -69,11 +72,36 @@ const pathToObject = path => ({
// Handles directory listing result(s)
const handleDirectoryList = (path, options) => result =>
Promise.resolve(result.map(stat => createFileIter(stat)))
.then(result => transformReaddir(pathToObject(path), result, {
.then(result => transformReaddir(pathToObject(path), result, capabilityCache, {
showHiddenFiles: options.showHiddenFiles !== false,
filter: options.filter
}));

const filterOptions = (ignore, options) => Object.fromEntries(
Object
.entries(options)
.filter(([k]) => !ignore.includes(k))
);

/**
* Get vfs capabilities
*
* @param {string|VFSFile} path The path of a file
* @param {VFSMethodOptions} [options] Options
* @return {Promise<object[]>} An object of capabilities
*/
export const capabilities = (adapter, mount) => (path, options) => {
const cached = capabilityCache[mount.name];
if (cached) {
return Promise.resolve(cached);
}
return adapter.capabilities(pathToObject(path), options, mount)
.then(capabilities => {
capabilityCache[mount.name] = capabilities;
return capabilities;
});
};

/**
* Read a directory
*
Expand All @@ -82,7 +110,7 @@ const handleDirectoryList = (path, options) => result =>
* @return {Promise<object[]>} A list of files
*/
export const readdir = (adapter, mount) => (path, options = {}) =>
adapter.readdir(pathToObject(path), options, mount)
adapter.readdir(pathToObject(path), filterOptions(['filter'], options), mount)
.then(handleDirectoryList(path, options));

/**
Expand Down