Skip to content

Commit

Permalink
Fix GitHub batch
Browse files Browse the repository at this point in the history 
Properly handle multi level folder and incoming Buffers
  • Loading branch information
@JbIPS @lexoyo
JbIPS authored and lexoyo committed Mar 9, 2018
1 parent fd4fe61 commit 1d1e6be
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 29 deletions.
112 changes: 89 additions & 23 deletions lib/unifile-github.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ function move(src, dest, treeRes) {
* @param {string} [branch=master] - Branch containing file/folder
* @private
*/
function removeFile(path, treeRes) {
function removeFile(path, treeRes, done) {
const regex = new RegExp('^' + path + '$|^' + path + '(/)');
const filteredTree = treeRes.tree.filter(function(file) {
return !regex.test(file.path);
});
if(filteredTree.length === treeRes.tree.length)
throw new UnifileError(UnifileError.ENOENT, 'Not Found');
else return filteredTree;
done(new UnifileError(UnifileError.ENOENT, 'Not Found'));
else done(null, filteredTree);
}

const createBranch = Symbol('createBranch');
Expand Down Expand Up @@ -532,7 +532,7 @@ class GitHubConnector {
default: // Rename a file/folder
const fileSrc = splitPath.slice(2).join('/');
const fileDest = splitPathDest.slice(2).join('/');
return this[transformTree](session, splitPath[0], move.bind(undefined, fileSrc, fileDest),
return this[transformTree](session, splitPath[0], (tree, done) => done(null, move(fileSrc, fileDest, tree)),
'Move ' + fileSrc + ' to ' + fileDest, splitPath[1]);
}
}
Expand Down Expand Up @@ -626,13 +626,15 @@ class GitHubConnector {
if(sameBranch) fileActions.push(actionQueue.shift());
}

actionsChain = actionsChain.then(() => this[transformTree](session, splitPath[0], function(treeRes) {
actionsChain = actionsChain.then(() => this[transformTree](session, splitPath[0], (treeRes, done) => {
const newTrees = {};
const blobsWaiting = [];
for(const currentAction of fileActions) {
const path = getPathTokens(currentAction.path).slice(2).join('/');
switch (currentAction.name.toLowerCase()) {
case 'unlink':
case 'rmdir':
treeRes.tree = removeFile(path, treeRes);
treeRes.tree = removeFile(path, treeRes, done);
break;
case 'rename':
if(!currentAction.destination)
Expand All @@ -644,33 +646,91 @@ class GitHubConnector {
treeRes.tree = move(src, dest, treeRes);
break;
case 'mkdir':
treeRes.tree.push({
path: path + '/.gitkeep',
mode: '100644',
type: 'blob',
content: ''
});
newTrees[path] = {
tree: [],
blobs: []
};
break;
case 'writefile':
if(!currentAction.content)
return new Promise.reject(new UnifileError(
UnifileError.EINVAL,
'WriteFile actions should have a content'));
treeRes.tree.push({
path: path,
content: currentAction.content,
mode: '100644',
type: 'blob'
});
// Get the longest path matching this file parent
const closestParent = Object.keys(newTrees).filter((p) => path.includes(p)).sort().pop();
if(closestParent) {
newTrees[closestParent].blobs.push(this[createBlob](session, splitPath[0], currentAction.content));
newTrees[closestParent].tree.push({
path: path.replace(closestParent + '/', ''),
mode: '100644',
type: 'blob'
});
} else {
treeRes.tree = treeRes.tree.filter((node) => node.path !== path);
const newNode = {
path: path.replace(closestParent + '/', ''),
mode: '100644',
type: 'blob'
};
if(Buffer.isBuffer(currentAction.content)) {
blobsWaiting.push({
path,
blob: this[createBlob](session, splitPath[0], currentAction.content)
});
} else newNode.content = currentAction.content;
treeRes.tree.push(newNode);
}
break;
default:
console.warn(`Unsupported batch action: ${currentAction.name}`);
}
}
return treeRes.tree;

const treesToPlant = [];
Object.keys(newTrees).forEach((path) => {
if(newTrees[path].tree.length > 0)
treesToPlant.push(path);
else treeRes.tree.push({
path: path + '/.gitkeep',
mode: '100644',
type: 'blob',
content: ''
});
});
return Promise.all(blobsWaiting.map((b) => b.blob))
.then((blobShas) => {
blobShas.forEach((sha, idx) => {
treeRes.tree
.find((n) => n.path === blobsWaiting[idx].path).sha = sha.sha;
});
return Promise.all(treesToPlant.map((treePath) => {
const tree = {tree: newTrees[treePath].tree};
const apiPath = '/repos/' + session.account.login + '/' + splitPath[0];
return Promise.all(newTrees[treePath].blobs)
.then((shas) => {
tree.tree.forEach((t, index) => t.sha = shas[index].sha);
return this[callAPI](session, apiPath + '/git/trees', tree, 'POST');
})
.then((t) => {
treeRes.tree = treeRes.tree.filter((node) => node.path !== treePath);
treeRes.tree.push({
path: treePath,
type: 'tree',
mode: '040000',
sha: t.sha
});
});
}))
.then(() => {
done(null, treeRes.tree);
})
.catch((e) => {
done(new Error(`Could not create a new tree ${e}`));
});
});
}, message || 'Batch update', splitPath[1]))
.catch((err) => err.name !== 'BatchError', (err) => {
throw new BatchError(UnifileError.EIO, `Could not modify tree: ${err.message}`);
throw new BatchError(UnifileError.EIO, `Error while batch: ${err.message}`);
});
}
}
Expand Down Expand Up @@ -766,7 +826,7 @@ class GitHubConnector {
* Transform the git tree and commit the transformed tree
* @param {GHSession} session - GH session
* @param {string} repo - Name of the repository to commit
* @param {Function} transformer - Function to apply on tree. Get the tree as only param and return an array.
* @param {Function} transformer - Function to apply on tree. Get the tree as first param and wait for an array in the callback.
* @param {string} message - Commit message for the new tree
* @param {string} [branch=master] - Branch containing the tree
* @return {Promise} a Promise of the server response
Expand All @@ -783,7 +843,12 @@ class GitHubConnector {
return this[callAPI](session, apiPath + '/git/trees/' + head.object.sha, {recursive: 1}, 'GET');
})
.then((res) => {
return transformer(res);
return new Promise((resolve, reject) => {
transformer(res, (err, result) => {
if(err && err instanceof Error) reject(err);
else resolve(result);
});
});
})
.then((tree) => {
if(Array.isArray(tree) && tree.length > 0) {
Expand Down Expand Up @@ -900,7 +965,8 @@ class GitHubConnector {
const {code, message} = (() => {
const defaultMessage = JSON.parse(body).message;
switch (res.statusCode) {
case 401: // fallthrough
case 401:
return {code: UnifileError.EACCES, message: 'Bad credentials'};
case 403:
return {code: UnifileError.EACCES, message: defaultMessage};
case 404:
Expand Down
24 changes: 18 additions & 6 deletions test/unifile-github.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function checkSession(session) {

describe('GitHubConnector', function() {
this.slow(500);
this.timeout(30000);
this.timeout(60000);

const session = {};
const defaultConfig = {
Expand Down Expand Up @@ -905,10 +905,14 @@ describe('GitHubConnector', function() {

describe('batch()', function() {
let connector;
const content = new Date().toISOString();
const creation = [
{name: 'mkdir', path: 'tmp'},
{name: 'mkdir', path: 'tmp/test'},
{name: 'writeFile', path: 'tmp/test/a', content: 'aaa'},
{name: 'writeFile', path: 'tmp/test/a', content: Buffer.from('aaa')},
{name: 'mkdir', path: 'tmp/test/af'},
{name: 'writeFile', path: 'tmp/test/af/aa', content},
{name: 'writeFile', path: 'tmp/test/af/ab', content: Buffer.from(content)},
{name: 'mkdir', path: 'tmp/test/dir'},
{name: 'rename', path: 'tmp/test/a', destination: 'tmp/test/b'},
{name: 'mkdir', path: 'tmp2'},
Expand Down Expand Up @@ -945,12 +949,12 @@ describe('GitHubConnector', function() {

it('rejects the promise if a rename action does not have a destination', function() {
return connector.batch(session, [{name: 'rename', path: 'tmp/test/a'}])
.should.be.rejectedWith('Could not modify tree');
.should.be.rejectedWith('Error while batch');
});

it('rejects the promise if a writefile action does not have content', function() {
return connector.batch(session, [{name: 'writefile', path: 'tmp/test/a'}])
.should.be.rejectedWith('Could not modify tree');
.should.be.rejectedWith('Error while batch');
});

it('rejects the promise if a writefile is programmed on a repo/branch', function() {
Expand All @@ -964,9 +968,17 @@ describe('GitHubConnector', function() {
return Promise.all([
connector.readFile(session, 'tmp/test/b')
.then((content) => {
return expect(content.toString()).to.equal('aaa');
return expect(content.toString(), 'read tmp/test/b').to.equal('aaa');
}),
expect(connector.readdir(session, 'tmp3')).to.be.fulfilled
expect(connector.readdir(session, 'tmp3')).to.be.fulfilled,
connector.readFile(session, 'tmp/test/af/aa')
.then((fileContent) => {
return expect(fileContent.toString(), 'read tmp/test/af/aa').to.equal(content);
}),
connector.readFile(session, 'tmp/test/af/ab')
.then((fileContent) => {
return expect(fileContent.toString(), 'read tmp/test/af/aa').to.equal(content);
})
]);
})
.then(() => connector.batch(session, destruction))
Expand Down

0 comments on commit 1d1e6be

Please sign in to comment.