Merge branch 'development'

coldbox-modules · Feb 7, 2023 · a6917ed · a6917ed
2 parents e1977bd + 6e5bbc4
commit a6917ed
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 16 deletions.
diff --git a/box.json b/box.json
@@ -1,6 +1,6 @@
 {
     "name":"Amazon S3 SDK",
-    "version":"5.5.1",
+    "version":"5.5.2",
     "slug":"s3sdk",
     "location":"https://downloads.ortussolutions.com/ortussolutions/coldbox-modules/s3sdk/@build.version@/[email protected]@.zip",
     "author":"Ortus Solutions, Corp",

diff --git a/changelog.md b/changelog.md
@@ -6,7 +6,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ----
-## v5.1.3 => Unreleased
+## v5.5.2 => 2023-FEB-07
+### Fixed
+* Multi-part upload concurrency fixes
+## v5.5.1 => 2023-FEB-03
+### Added
+* Support for multi-part file uploads to conserve memory usage
+
+## v5.4.1 => 2023-FEB-02
+
+## v5.3.1 => 2023-FEB-02
+## v5.2.0 => 2023-JAN-26
 ### Added
 * Add support for server side encryption
 * Add retry support for S3 connection failures

diff --git a/models/AmazonS3.cfc b/models/AmazonS3.cfc
@@ -645,46 +645,47 @@ component accessors="true" singleton {
 			arguments.contentType = getFileMimeType( arguments.filepath );
 		}
 
-		var jFiles = createObject( "java", "java.nio.file.Files" );
-		var jPath  = createObject( "java", "java.nio.file.Paths" ).get(
-			// Java is less lax on slashes than CF, so getCanonicalPath() cleans that up
-			javacast( "string", getCanonicalPath( arguments.filePath ) ),
-			javacast( "java.lang.String[]", [] )
-		);
-		var byteChannel = jFiles.newByteChannel( jPath, [] );
-		var byteCount   = byteChannel.size();
+		var byteCount   = getFileInfo( arguments.filepath ).size;
 
 		if ( byteCount <= variables.multiPartByteThreshold ) {
 			arguments.data = fileReadBinary( arguments.filepath );
 			return putObject( argumentCollection = arguments );
 		} else {
+
+			var jFiles = createObject( "java", "java.nio.file.Files" );
+			var jPath  = createObject( "java", "java.nio.file.Paths" ).get(
+				// Java is less lax on slashes than CF, so getCanonicalPath() cleans that up
+				javacast( "string", getCanonicalPath( arguments.filePath ) ),
+				javacast( "java.lang.String[]", [] )
+			);
+
+			var parts = [];
 			try {
 				var multipartResponse = createMultiPartUpload( argumentCollection = arguments );
 				var uploadId          = xmlParse( multipartResponse.response ).InitiateMultipartUploadResult.UploadId.xmlText;
 				var partNumber        = 1;
-				var parts             = [];
 				var numberOfUploads   = ceiling( byteCount / variables.multiPartByteThreshold );
 				for ( var i = 1; i <= numberOfUploads; i++ ) {
 					var remaining = byteCount - ( ( i - 1 ) * variables.multiPartByteThreshold );
 					parts.append( {
 						"uploadId"    : uploadId,
 						"partNumber"  : i,
-						"offset"      : i * variables.multiPartByteThreshold,
+						"offset"      : (i-1) * variables.multiPartByteThreshold,
 						"limit"       : remaining <= variables.multiPartByteThreshold ? remaining : variables.multiPartByteThreshold,
-						"byteChannel" : byteChannel,
-						"timeout"     : arguments.HTTPTimeout
+						"timeout"     : arguments.HTTPTimeout,
+						"channel"	  : jFiles.newByteChannel( jPath, [] )
 					} );
 				}
 				try {
 					parts = variables.asyncManager.allApply( parts, function( part ){
-						var channel = part.byteChannel;
-						channel.position( part.offset );
+						var channel = part.channel.position( part.offset );
 						var buffer = createObject( "java", "java.nio.ByteBuffer" ).allocate( part.limit );
 						channel.read( buffer );
 
 						return {
 							"partNumber" : part.partNumber,
 							"size"       : part.limit,
+							"channel"	 : part.channel,
 							"response"   : s3Request(
 								method     = "PUT",
 								resource   = bucketName & "/" & uri,
@@ -693,6 +694,9 @@ component accessors="true" singleton {
 								parameters = {
 									"uploadId"   : part.uploadId,
 									"partNumber" : part.partNumber
+								},
+								headers = {
+									"content-type" : "binary/octet-stream"
 								}
 							)
 						};
@@ -748,6 +752,12 @@ component accessors="true" singleton {
 				);
 				arguments.data = fileReadBinary( arguments.filepath );
 				return putObject( argumentCollection = arguments );
+			} finally {
+				parts.each( (p)=>{
+					try{
+						p.channel.close()
+					} catch( any e ) {}
+				} );
 			}
 		}
 	}

diff --git a/test-harness/tests/specs/AmazonS3Spec.cfc b/test-harness/tests/specs/AmazonS3Spec.cfc
@@ -122,6 +122,15 @@ component extends="coldbox.system.testing.BaseTestCase" {
 					expect( md ).notToBeEmpty();
 					expect( md[ "Content-Length" ] ).toBe( fileSize );
 					expect( md[ "Content-Type" ] ).toBe( "text/plain" );
+
+					// Download the uploaded file
+					s3.downloadObject(
+						testBucket,
+						"big_file.txt",
+						expandPath( "/tests/tmp/big_file2.txt" )
+					);
+					// And confirm a hash of both file contents still matches
+					expect( hash( fileRead( expandPath( "/tests/tmp/big_file2.txt" ) ) ) ).toBe( hash( fileRead( expandPath( "/tests/tmp/big_file.txt" ) ) ) )
 				} );
 
 				it(