fix(Browser): multipartUpload InvalidPartOrderError by doneParts repe…

…at (#414) * fix(Browser): multipartUpload InvalidPartOrderError issues * fix(Browser): multipartUpload InvalidPartOrderError issues * fix: multipart doneParts repeat bug
ali-sdk · Mar 30, 2018 · 2b0967f · 2b0967f
1 parent e8a78b5
commit 2b0967f
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git 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
@@ -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 = '<?xml version="1.0" encoding="UTF-8"?>\n<CompleteMultipartUpload>\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 += '<Part>\n';
     xml += `<PartNumber>${p.number}</PartNumber>\n`;
     xml += `<ETag>${p.etag}</ETag>\n`;

diff --git 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* () {