From 2b0967f30e4f96ed169c3293d1716bb8271eb8d4 Mon Sep 17 00:00:00 2001 From: Jingdan Date: Fri, 30 Mar 2018 11:34:31 +0800 Subject: [PATCH] fix(Browser): multipartUpload InvalidPartOrderError by doneParts repeat (#414) * fix(Browser): multipartUpload InvalidPartOrderError issues * fix(Browser): multipartUpload InvalidPartOrderError issues * fix: multipart doneParts repeat bug --- lib/browser/managed_upload.js | 25 +++++++++++++++++++------ lib/common/multipart.js | 7 ++++--- test/node/callback.test.js | 2 +- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/lib/browser/managed_upload.js b/lib/browser/managed_upload.js index f7c47f0bb..d3dd28930 100644 --- a/lib/browser/managed_upload.js +++ b/lib/browser/managed_upload.js @@ -5,6 +5,7 @@ const is = require('is-type-of'); const util = require('util'); const path = require('path'); const mime = require('mime'); +const copy = require('copy-to'); const proto = exports; @@ -113,8 +114,15 @@ proto._resumeMultipart = function* _resumeMultipart(checkpoint, options) { file, fileSize, partSize, uploadId, doneParts, name, } = checkpoint; + const internalDoneParts = []; + + if (doneParts.length > 0) { + copy(doneParts).to(internalDoneParts); + } + const partOffs = this._divideParts(fileSize, partSize); const numParts = partOffs.length; + let multipartFinish = false; const uploadPartJob = function* uploadPartJob(self, partNo) { if (!self.isCancel()) { @@ -126,12 +134,16 @@ proto._resumeMultipart = function* _resumeMultipart(checkpoint, options) { }; const result = yield self._uploadPart(name, uploadId, partNo, data); - if (!self.isCancel()) { - doneParts.push({ + if (!self.isCancel() && !multipartFinish) { + checkpoint.doneParts.push({ + number: partNo, + etag: result.res.headers.etag, + }); + + internalDoneParts.push({ number: partNo, etag: result.res.headers.etag, }); - checkpoint.doneParts = doneParts; if (options && options.progress) { yield options.progress(doneParts.length / numParts, checkpoint, result.res); @@ -148,7 +160,7 @@ proto._resumeMultipart = function* _resumeMultipart(checkpoint, options) { }; const all = Array.from(new Array(numParts), (x, i) => i + 1); - const done = doneParts.map(p => p.number); + const done = internalDoneParts.map(p => p.number); const todo = all.filter(p => done.indexOf(p) < 0); const defaultParallel = 5; const parallel = options.parallel || defaultParallel; @@ -168,7 +180,8 @@ proto._resumeMultipart = function* _resumeMultipart(checkpoint, options) { } // start uploads jobs - const errors = yield this._thunkPool(jobs, parallel); + const errors = this._thunkPool(jobs, parallel); + multipartFinish = true; // check errors after all jobs are completed if (errors && errors.length > 0) { @@ -183,7 +196,7 @@ proto._resumeMultipart = function* _resumeMultipart(checkpoint, options) { throw this._makeCancelEvent(); } } - return yield this.completeMultipartUpload(name, uploadId, doneParts, options); + return yield this.completeMultipartUpload(name, uploadId, internalDoneParts, options); }; diff --git a/lib/common/multipart.js b/lib/common/multipart.js index 7ecace9f2..ffd1d1f41 100644 --- a/lib/common/multipart.js +++ b/lib/common/multipart.js @@ -160,10 +160,11 @@ proto.uploadPart = function* uploadPart(name, uploadId, partNo, file, start, end * } */ proto.completeMultipartUpload = function* completeMultipartUpload(name, uploadId, parts, options) { - parts.sort((a, b) => a.number - b.number); + const completeParts = parts.concat().sort((a, b) => a.number - b.number) + .filter((item, index, arr) => !index || item.number !== arr[index - 1].number); let xml = '\n\n'; - for (let i = 0; i < parts.length; i++) { - const p = parts[i]; + for (let i = 0; i < completeParts.length; i++) { + const p = completeParts[i]; xml += '\n'; xml += `${p.number}\n`; xml += `${p.etag}\n`; diff --git a/test/node/callback.test.js b/test/node/callback.test.js index c14bf8601..9f632e7cc 100644 --- a/test/node/callback.test.js +++ b/test/node/callback.test.js @@ -7,7 +7,7 @@ const config = require('../config').oss; const { callbackServer } = require('../const'); const mm = require('mm'); -describe('test/multipart.test.js', () => { +describe('test/callback.test.js', () => { const { prefix } = utils; before(function* () {