Skip to content
This repository has been archived by the owner on Jul 7, 2024. It is now read-only.
/ Orion Public archive

Allow to wrap multiple uploads under the same directory #95

Merged
merged 4 commits into from
May 5, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
169 changes: 109 additions & 60 deletions app/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,72 +52,121 @@ export function initIPFSClient () {
}

/**
* This function will allow the user to add a file to the IPFS repo.
* ```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs, what does this do? Where could this be used?

* Link {
* hash: string;
* path: string;
* size: number;
* }
* ```
*
* @param {Link[]} links
* @returns {Link} the wrapper, which is of type Link
*/
export function wrapFiles (links) {
const wrapperDAG = {
Data: Buffer.from('\u0008\u0001'),
// The object.put API expects `Name` not `path`
Links: links.map(link => ({
Name: link.path,
Hash: link.hash,
Size: link.size
}))
}

return IPFS_CLIENT.object.put(wrapperDAG)
.then(res => {
// res is of type DAGNode
// https://github.com/ipld/js-ipld-dag-pb#nodetojson
res = res.toJSON()
const wrapper = {
hash: res.multihash,
path: '',
size: res.size
}

return wrapper
})
}

/**
* This function will allow the user to add a file or multiple files to the IPFS repo.
* Accepts a string or a string array.
* Wraps the files in a directory.
*
* ```
* Wrapper {
* hash: string;
* path: string;
* size: number;
* }
* ```
*
* @param {string|string[]} filePath
* @returns {Wrapper} wrapper
*/
export function addFileFromFSPath (filePath, _queryGateways = queryGateways) {
export function addFileOrFilesFromFSPath (filePath, _queryGateways = queryGateways) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will simplify a lot if you could switch to always array (you can have an array of 1 element).
This will remove some conditions in the code and simplify the logic, but it will require on the implementation of the function to use arrays.

Don't mix types of variables

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!IPFS_CLIENT) return Promise.reject(ERROR_IPFS_UNAVAILABLE)

const options = { recursive: true }
/**
* Add the file/directory from fs
*/
return IPFS_CLIENT.util.addFromFs(filePath, options)
.then(files => {
/**
* If it was a directory it will be last
* Example result:
* [{
* hash: "QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtuEfL"
* path: "ipfs-test-dir/wrappedtext.txt"
* size: 15
* }, {
* hash: "QmcysLdK6jV4QAgcdxVZFzTt8TieH4bkyW6kniPKTr2RXp"
* path: "ipfs-test-dir"
* size: 9425451
* }]
*
*/
const rootFile = files[files.length - 1]
const wrapperDag = {
Data: Buffer.from('\u0008\u0001'),
Links: [{
Name: rootFile.path,
Hash: rootFile.hash,
Size: rootFile.size
}]
}

return IPFS_CLIENT.object.put(wrapperDag)
.then(res => {
// res is of type DAGNode
// https://github.com/ipld/js-ipld-dag-pb#nodetojson
res = res.toJSON()
const wrapper = {
hash: res.multihash,
path: '',
size: res.size
const promises = []
if (Array.isArray(filePath)) {
Copy link
Member

@koalalorenzo koalalorenzo May 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly this :D (see previous comment)

filePath.map(path => promises.push(IPFS_CLIENT.util.addFromFs(path, options)))
} else {
/**
* Add the file/directory from fs
*/
promises.push(IPFS_CLIENT.util.addFromFs(filePath, options))
}

return Promise.all(promises)
.then(fileUploadResults => {
// IPFS_CLIENT.util.addFromFs always returns an array
// (because it can upload an dir recursively),
// which is why we expect an array of arrays

const rootFiles = fileUploadResults.map(result => {
/**
* If it was a directory it will be last
* Example result:
* [{
* hash: "QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtuEfL"
* path: "ipfs-test-dir/wrappedtext.txt"
* size: 15
* }, {
* hash: "QmcysLdK6jV4QAgcdxVZFzTt8TieH4bkyW6kniPKTr2RXp"
* path: "ipfs-test-dir"
* size: 9425451
* }]
*
*/
return result[result.length - 1]
})

return wrapFiles(rootFiles)
.then(wrapper => Promise.all([
// This value is needed further in the chain
wrapper,
// Pin the wrapper directory
IPFS_CLIENT.pin.add(wrapper.hash),
// Unpin the initial uploads
...rootFiles.map(rootFile => IPFS_CLIENT.pin.rm(rootFile.hash))
]))
/**
* Query the gateways and return the wrapper dir
*/
.then(results => {
const wrapper = results[0]

if (!Settings.getSync('skipGatewayQuery')) {
// Query all the uploaded files
fileUploadResults.forEach(files => files.forEach(file => _queryGateways(file.hash)))
// Query the wrapper
_queryGateways(wrapper.hash)
}
files.push(wrapper)

return Promise.all([
/**
* Pin the wrapper directory
*/
IPFS_CLIENT.pin.add(wrapper.hash),
/**
* Unpin the initial upload
*/
IPFS_CLIENT.pin.rm(rootFile.hash)
])
/**
* Return all items + the wrapper dir
*/
.then(() => {
if (!Settings.getSync('skipGatewayQuery')) {
files.forEach(file => _queryGateways(file.hash))
}
return Promise.resolve(files)
})

return Promise.resolve(wrapper)
})
})
}
Expand Down
28 changes: 9 additions & 19 deletions app/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,12 @@ describe('api.js', () => {
})
})

describe('addFileFromFSPath', () => {
describe('addFileOrFilesFromFSPath', () => {
it('should reject when IPFS is not started', () => {
// arrange
api.setClientInstance(null)
// act
return api.addFileFromFSPath('./fake-path')
return api.addFileOrFilesFromFSPath('./fake-path')
.catch(err => {
// assert
expect(err).toBe(ERROR_IPFS_UNAVAILABLE)
Expand Down Expand Up @@ -228,7 +228,7 @@ describe('api.js', () => {
})
const queryGatewaysMock = jest.fn()
// act
return api.addFileFromFSPath('./textfiles', queryGatewaysMock)
return api.addFileOrFilesFromFSPath('./textfiles', queryGatewaysMock)
.then(result => {
// assert
expect(addFromFsMock).toHaveBeenCalledWith('./textfiles', { recursive: true })
Expand All @@ -243,21 +243,11 @@ describe('api.js', () => {
expect(pinAddMock).toHaveBeenCalledWith('QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu003')
expect(pinRmMock).toHaveBeenCalledWith('QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu002')
expect(queryGatewaysMock).not.toHaveBeenCalled()
expect(result).toEqual([
{
hash: 'QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu001',
path: 'textfiles/foo.txt',
size: 30
}, {
hash: 'QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu002',
path: 'textfiles',
size: 50
}, {
hash: 'QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu003',
path: '',
size: 60
}
])
expect(result).toEqual({
hash: 'QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu003',
path: '',
size: 60
})
})
})

Expand Down Expand Up @@ -304,7 +294,7 @@ describe('api.js', () => {
})
const queryGatewaysMock = jest.fn()
// act
return api.addFileFromFSPath('./textfiles', queryGatewaysMock)
return api.addFileOrFilesFromFSPath('./textfiles', queryGatewaysMock)
.then(result => {
// assert
expect(queryGatewaysMock).toHaveBeenCalledWith('QmRgutAxd8t7oGkSm4wmeuByG6M51wcTso6cubDdQtu001')
Expand Down
60 changes: 44 additions & 16 deletions app/windows/Storage/fileIntegration.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,48 @@
import { remote } from 'electron'
import Settings from 'electron-settings'
import { addFileFromFSPath, unpinObject } from '../../api'
import { addFileOrFilesFromFSPath, unpinObject } from '../../api'
import DetailsWindow from '../Details/window'

const { app, dialog, shell } = remote

/**
* Returns `true` if the user wants to wrap all files under a single dir,
* `false` otherwise
*
* @returns {boolean}
*/
function askWhetherToWrapAllFiles () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done :) Having this in a method will be useful!

const buttons = ['No', 'Yes']
const successMessageOption = {
type: 'info',
title: 'Before we do that...',
message: 'Would you like to wrap all these files under the same directory?',
cancelId: 0,
defaultId: 1,
buttons
}

const btnId = dialog.showMessageBox(app.mainWindow, successMessageOption)
return btnId === 1
}

/**
* This function will add the files from a list of their paths, and show
* message when it was a success or a failure.
*
* ToDo: add loading messages/feedback
*/
export function addFilesPaths (paths) {
let promises

if (paths.length > 1 && askWhetherToWrapAllFiles()) {
// If the user says yes to wrapping all files, simply pass the paths array
promises = [addFileOrFilesFromFSPath(paths)]
} else {
// User wants to wrap each file
promises = paths.map(path => addFileOrFilesFromFSPath(path))
}

const buttons = ['Close', 'Open in the browser']
const successMessageOption = {
type: 'info',
Expand All @@ -25,31 +57,27 @@ export function addFilesPaths (paths) {
title: 'Adding the file failed'
}

const promises = paths.map(path => addFileFromFSPath(path))
return Promise.all(promises)
.then(results => {
// building the lines of the text messages, containing the file name
// followed by its hash
const textLines = results.map(result => {
// we need the hash of our wrapper
const wrapper = result[result.length - 1]
// we need the path of our upload (if it's a directory it will be second to last)
const root = result[result.length - 2]
// Array of wrappers expected
// If the user upload only one file (or wrapped everything), open the Property Window (Details)
if (results.length === 1) {
const wrapper = results[0]
return DetailsWindow.create(app, wrapper.hash)
}

return `${wrapper.hash} ${root.path}`
})
// the old fashion way: show an alert with the `Open in browser` button
// building the lines of the text messages, containing only the wrappers
const textLines = results.map(wrapper => wrapper.hash)

// ToDo: improve this, maybe show a custom window with more details.
// it is ugly!!!
successMessageOption.message += `This includes: \n${textLines.join('\n')}`
successMessageOption.message += `This includes the following hashes: \n${textLines.join('\n')}`
const btnId = dialog.showMessageBox(app.mainWindow, successMessageOption)

// if(btnId === buttons.indexOf('Open on the browser'))
if (btnId === 1) {
openInBrowser(results.map(result => {
const wrapper = result[result.length - 1]
return wrapper.hash
}))
openInBrowser(results.map(wrapper => wrapper.hash))
}
})
.catch((err) => {
Expand Down