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

feat: uploadFile support for File type #93

Merged
merged 3 commits into from
Feb 1, 2021
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
20 changes: 15 additions & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {
} from './types'
import { BeeError } from './utils/error'
import { prepareWebsocketData } from './utils/data'
import { fileArrayBuffer, isFile } from './utils/file'

/**
* The Bee class provides a way of interacting with the Bee APIs based on the provided url
Expand Down Expand Up @@ -62,18 +63,27 @@ export class Bee {
/**
* Upload single file to a Bee node
*
* @param data Data to be uploaded
* @param name Name of the uploaded file
* @param data Data or file to be uploaded
* @param name Name of the uploaded file (optional)
* @param options Aditional options like tag, encryption, pinning, content-type
*
* @returns reference is a content hash of the file
*/
uploadFile(
data: string | Uint8Array | Readable,
async uploadFile(
data: string | Uint8Array | Readable | File,
name?: string,
options?: file.FileUploadOptions,
): Promise<Reference> {
return file.upload(this.url, data, name, options)
if (isFile(data)) {
const fileData = await fileArrayBuffer(data)
const fileName = name || data.name
const contentType = data.type
const fileOptions = options !== undefined ? { contentType, ...options } : { contentType }

return file.upload(this.url, fileData, fileName, fileOptions)
} else {
return file.upload(this.url, data, name, options)
}
}

/**
Expand Down
14 changes: 1 addition & 13 deletions src/modules/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { makeTar } from '../utils/tar'
import { safeAxios } from '../utils/safeAxios'
import { extractUploadHeaders, readFileHeaders } from '../utils/headers'
import { BeeArgumentError } from '../utils/error'
import { fileArrayBuffer } from '../utils/file'

const dirsEndpoint = '/dirs'
const bzzEndpoint = '/bzz'
Expand Down Expand Up @@ -80,19 +81,6 @@ async function buildCollectionRelative(
return collection
}

function fileArrayBuffer(file: File): Promise<ArrayBuffer> {
if (file.arrayBuffer) {
return file.arrayBuffer()
}

// workaround for Safari where arrayBuffer is not supported on Files
return new Promise(resolve => {
const fr = new FileReader()
fr.onload = () => resolve(fr.result as ArrayBuffer)
fr.readAsArrayBuffer(file)
})
}

/*
* This is a workaround for fixing the type definitions
* regarding the missing `webkitRelativePath` property which is
Expand Down
4 changes: 2 additions & 2 deletions src/modules/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ function extractFileUploadHeaders(options?: FileUploadOptions): FileUploadHeader
*/
export async function upload(
url: string,
data: string | Uint8Array | Readable,
data: string | Uint8Array | Readable | ArrayBuffer,
name?: string,
options?: FileUploadOptions,
): Promise<string> {
const response = await safeAxios<{ reference: string }>({
method: 'post',
url: url + endpoint,
data: await prepareData(data),
data: prepareData(data),
headers: {
'content-type': 'application/octet-stream',
...extractFileUploadHeaders(options),
Expand Down
4 changes: 3 additions & 1 deletion src/utils/data.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { Readable } from 'stream'
import type { Data } from 'ws'

export function prepareData(data: string | Uint8Array | Readable): Uint8Array | Readable {
export function prepareData(data: string | ArrayBuffer | Uint8Array | Readable): Uint8Array | Readable {
if (typeof data === 'string') {
return new TextEncoder().encode(data)
} else if (data instanceof ArrayBuffer) {
return new Uint8Array(data)
}

return data
Expand Down
40 changes: 40 additions & 0 deletions src/utils/file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Compatibility functions for working with File API objects
*
* https://developer.mozilla.org/en-US/docs/Web/API/File
*/

export function isFile(file: unknown): file is File {
// browser
if (typeof File === 'function') {
return file instanceof File
}

// node.js
const f = file as File

return (
typeof f === 'object' &&
typeof f.name === 'string' &&
(typeof f.stream === 'function' || typeof f.arrayBuffer === 'function')
)
}

/**
* Compatibility helper for browsers where the `arrayBuffer function is
* missing from `File` objects.
*
* @param file A File object
*/
export function fileArrayBuffer(file: File): Promise<ArrayBuffer> {
if (file.arrayBuffer) {
return file.arrayBuffer()
}

// workaround for Safari where arrayBuffer is not supported on Files
return new Promise(resolve => {
const fr = new FileReader()
fr.onload = () => resolve(fr.result as ArrayBuffer)
fr.readAsArrayBuffer(file)
})
}
Comment on lines +1 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW if we wanted to not include this in browser bundle (which we probably in future should do to minimize bundle size) we can define file.browser.ts which exports both of those functions with much less code and in package.json in the browser field, we can swap them when the compilation target is browser.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea, but I am not sure how to use it correctly.

Currently my approach is to target the browser and optimize for that because that's where the bundle size matters and the node.js version gets polyfilled. In this case both function is only necessary in the browser because node.js doesn't even implement the File Web API and I just added a workaround to isFile so that the tests pass with mocked objects.

If I move only the necessary functionality to file.browser.ts tests will fail, so eventually I will have to duplicate all the functions but then the browser ones won't be tested automatically, and even worse the compiler won't catch if you forget to duplicate a function in the .browser.ts version and it will cause a runtime error in the browser.

Do you have any idea how to solve this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me try and create example tomorrow. I think I know how to go about this but not sure. Feel free to merge as is.

52 changes: 52 additions & 0 deletions test/bee-class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,58 @@ describe('Bee class', () => {
expect(file.name).toEqual(name)
expect(file.data).toEqual(content)
})

it('should work with file object', async () => {
const content = new Uint8Array([1, 2, 3])
const name = 'hello.txt'
const type = 'text/plain'
const file = ({
arrayBuffer: () => content,
name,
type,
} as unknown) as File

const hash = await bee.uploadFile(file)
const downloadedFile = await bee.downloadFile(hash)

expect(downloadedFile.data).toEqual(content)
expect(downloadedFile.name).toEqual(name)
expect(downloadedFile.contentType).toEqual(type)
})

it('should work with file object and name overridden', async () => {
const content = new Uint8Array([1, 2, 3])
const name = 'hello.txt'
const file = ({
arrayBuffer: () => content,
name,
} as unknown) as File
const nameOverride = 'hello-override.txt'

const hash = await bee.uploadFile(file, nameOverride)
const downloadedFile = await bee.downloadFile(hash)

expect(downloadedFile.data).toEqual(content)
expect(downloadedFile.name).toEqual(nameOverride)
})

it('should work with file object and content-type overridden', async () => {
const content = new Uint8Array([1, 2, 3])
const name = 'hello.txt'
const type = 'text/plain'
const file = ({
arrayBuffer: () => content,
name,
type,
} as unknown) as File
const contentTypeOverride = 'text/plain+override'

const hash = await bee.uploadFile(file, undefined, { contentType: contentTypeOverride })
const downloadedFile = await bee.downloadFile(hash)

expect(downloadedFile.data).toEqual(content)
expect(downloadedFile.contentType).toEqual(contentTypeOverride)
})
})

describe('collections', () => {
Expand Down