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.

Sign up for GitHub

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

remove isResumable option #209

Merged
merged 3 commits into from
Dec 11, 2024
Merged

remove isResumable option #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6eae3bf
remove isResumable option
mifi Dec 11, 2024
4cb28f2
fix outdated internal unit tests
mifi Dec 11, 2024
e8930b2
fix fomatting
mifi Dec 11, 2024
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
55 changes: 6 additions & 49 deletions src/Transloadit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ export class Transloadit {
const {
params = {},
waitForCompletion = false,
isResumable = true,
chunkSize: requestedChunkSize = Infinity,
uploadConcurrency = 10,
timeout = 24 * 60 * 60 * 1000, // 1 day
Expand All @@ -170,13 +169,6 @@ export class Transloadit {
assemblyId,
} = opts

if (!isResumable) {
process.emitWarning(
'Parameter value isResumable = false is deprecated. All uploads will be resumable (using TUS) in the future',
'DeprecationWarning'
)
}

// Keep track of how long the request took
const startTimeMs = getHrTimeMs()

Expand Down Expand Up @@ -242,25 +234,15 @@ export class Transloadit {
method: 'post',
timeout,
params,
}

if (isResumable) {
requestOpts.fields = {
fields: {
tus_num_expected_upload_files: allStreams.length,
}
},
}

// upload as form multipart or tus?
const formUploadStreamsMap: Record<string, Stream> = isResumable ? {} : allStreamsMap

const result = await this._remoteJson<Assembly>(
requestOpts,
formUploadStreamsMap,
onUploadProgress
)
const result = await this._remoteJson<Assembly>(requestOpts)
checkResult(result)

if (isResumable && Object.keys(allStreamsMap).length > 0) {
if (Object.keys(allStreamsMap).length > 0) {
await sendTusRequest({
streamsMap: allStreamsMap,
assembly: result,
Expand Down Expand Up @@ -678,7 +660,6 @@ export class Transloadit {
private _appendForm(
form: FormData,
params: KeyVal,
streamsMap?: Record<string, Stream>,
fields?: Record<string, string | number>
): void {
const sigData = this.calcSignature(params)
Expand All @@ -694,13 +675,6 @@ export class Transloadit {
}

form.append('signature', signature)

if (streamsMap) {
Object.entries(streamsMap).forEach(([label, { stream, path }]) => {
const options = path ? undefined : { filename: label } // https://github.com/transloadit/node-sdk/issues/86
form.append(label, stream, options)
})
}
}

// Implements HTTP GET query params, handling the case where the url already
Expand Down Expand Up @@ -743,11 +717,7 @@ export class Transloadit {
// Responsible for making API calls. Automatically sends streams with any POST,
// PUT or DELETE requests. Automatically adds signature parameters to all
// requests. Also automatically parses the JSON response.
private async _remoteJson<T>(
opts: RequestOptions,
streamsMap?: Record<string, Stream>,
onProgress: CreateAssemblyOptions['onUploadProgress'] = () => {}
): Promise<T> {
private async _remoteJson<T>(opts: RequestOptions): Promise<T> {
const {
urlSuffix,
url: urlInput,
Expand Down Expand Up @@ -775,11 +745,9 @@ export class Transloadit {

if (method === 'post' || method === 'put' || method === 'delete') {
form = new FormData()
this._appendForm(form, params, streamsMap, fields)
this._appendForm(form, params, fields)
}

const isUploadingStreams = streamsMap && Object.keys(streamsMap).length > 0

const requestOpts: OptionsOfJSONResponseBody = {
retry: this._gotRetry,
body: form as FormData,
Expand All @@ -792,18 +760,8 @@ export class Transloadit {
responseType: 'json',
}

// For non-file streams transfer encoding does not get set, and the uploaded files will not get accepted
// https://github.com/transloadit/node-sdk/issues/86
// https://github.com/form-data/form-data/issues/394#issuecomment-573595015
if (isUploadingStreams) requestOpts.headers!['transfer-encoding'] = 'chunked'

try {
const request = got[method]<T>(url, requestOpts)
if (isUploadingStreams) {
request.on('uploadProgress', ({ transferred, total }) =>
onProgress({ uploadedBytes: transferred, totalBytes: total })
)
}
const { body } = await request
return body
} catch (err) {
Expand Down Expand Up @@ -852,7 +810,6 @@ export interface CreateAssemblyOptions {
[name: string]: Readable | intoStream.Input
}
waitForCompletion?: boolean
isResumable?: boolean
chunkSize?: number
uploadConcurrency?: number
timeout?: number
Expand Down
17 changes: 5 additions & 12 deletions test/integration/live-api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,22 +337,19 @@ describe('API integration', { timeout: 60000 }, () => {
expect(result.assembly_id).toMatch(promise.assemblyId)
})

async function testUploadProgress(isResumable: boolean) {
async function testUploadProgress() {
const client = createClient()

let progressCalled = false
function onUploadProgress({ uploadedBytes, totalBytes }: UploadProgress) {
// console.log(uploadedBytes)
expect(uploadedBytes).toBeDefined()
if (isResumable) {
expect(totalBytes).toBeDefined()
expect(totalBytes).toBeGreaterThan(0)
}
expect(totalBytes).toBeDefined()
expect(totalBytes).toBeGreaterThan(0)
progressCalled = true
}

const params: CreateAssemblyOptions = {
isResumable,
params: {
steps: {
resize: resizeOriginalStep,
Expand All @@ -368,12 +365,8 @@ describe('API integration', { timeout: 60000 }, () => {
expect(progressCalled).toBe(true)
}

it('should trigger progress callbacks when uploading files, resumable', async () => {
await testUploadProgress(true)
})

it('should trigger progress callbacks when uploading files, nonresumable', async () => {
await testUploadProgress(false)
it('should trigger progress callbacks when uploading files', async () => {
await testUploadProgress()
})

it('should return properly waitForCompletion is false', async () => {
Expand Down
18 changes: 2 additions & 16 deletions test/unit/test-transloadit-client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,6 @@ describe('Transloadit', () => {
it('should append all required fields to the request form', () => {
const client = new Transloadit({ authKey: 'foo_key', authSecret: 'foo_secret' })

const stream1 = new Readable()
const stream2 = new Readable()

const streamsMap = {
stream1: { stream: stream1 },
stream2: { stream: stream2 },
}

const form = new FormData()
const params = {}
const fields = {
Expand All @@ -170,7 +162,7 @@ describe('Transloadit', () => {
const formAppendSpy = vi.spyOn(form, 'append')

// @ts-expect-error This tests private internals
client._appendForm(form, params, streamsMap, fields)
client._appendForm(form, params, fields)

expect(calcSignatureSpy).toHaveBeenCalledWith(params)

Expand All @@ -181,8 +173,6 @@ describe('Transloadit', () => {
'signature',
'sha384:f146533532844d4f4e34221288be08e14a2779eeb802a35afa6a193762f58da95d2423a825aa4cb4c7420e25f75a5c90',
],
['stream1', stream1, { filename: 'stream1' }],
['stream2', stream2, { filename: 'stream2' }],
])
})
})
Expand Down Expand Up @@ -280,11 +270,7 @@ describe('Transloadit', () => {

await client.createAssembly()

expect(spy).toBeCalledWith(
expect.objectContaining({ timeout: 24 * 60 * 60 * 1000 }),
{},
expect.any(Function)
)
expect(spy).toBeCalledWith(expect.objectContaining({ timeout: 24 * 60 * 60 * 1000 }))
})

describe('_calcSignature', () => {
Expand Down
Loading