Refactor writeObject to only use MultipartUpload when required

Signed-off-by: Bernd Rederlechner <Bernd.Rederlechner@t-systems.com>

Co-authored-by: Julius Härtl <jus@bitgrid.net>

2 files changed, 105 insertions, 0 deletions

diff --git a/tests/lib/Files/ObjectStore/ObjectStoreTest.php b/tests/lib/Files/ObjectStore/ObjectStoreTest.php
index 4ec44eb410d..a245f0ae366 100644
--- a/tests/lib/Files/ObjectStore/ObjectStoreTest.php
+++ b/tests/lib/Files/ObjectStore/ObjectStoreTest.php
@@ -26,11 +26,27 @@ use Test\TestCase;
 
 abstract class ObjectStoreTest extends TestCase {
 
+	/** @var string[] */
+	private $cleanup = [];
+
 	/**
 	 * @return \OCP\Files\ObjectStore\IObjectStore
 	 */
 	abstract protected function getInstance();
 
+	protected function cleanupAfter(string $urn) {
+		$this->cleanup[] = $urn;
+	}
+
+	public function tearDown(): void {
+		parent::tearDown();
+
+		$instance = $this->getInstance();
+		foreach ($this->cleanup as $urn) {
+			$instance->deleteObject($urn);
+		}
+	}
+
 	protected function stringToStream($data) {
 		$stream = fopen('php://temp', 'w+');
 		fwrite($stream, $data);
@@ -110,6 +126,9 @@ abstract class ObjectStoreTest extends TestCase {
 	}
 
 	public function testCopy() {
+		$this->cleanupAfter('source');
+		$this->cleanupAfter('target');
+
 		$stream = $this->stringToStream('foobar');
 
 		$instance = $this->getInstance();
diff --git a/tests/lib/Files/ObjectStore/S3Test.php b/tests/lib/Files/ObjectStore/S3Test.php
index 25bee9cbdd8..a7a95d53375 100644
--- a/tests/lib/Files/ObjectStore/S3Test.php
+++ b/tests/lib/Files/ObjectStore/S3Test.php
@@ -60,6 +60,12 @@ class NonSeekableStream extends Wrapper {
  * @group PRIMARY-s3
  */
 class S3Test extends ObjectStoreTest {
+	public function setUp(): void {
+		parent::setUp();
+		$s3 = $this->getInstance();
+		$s3->deleteObject('multiparttest');
+	}
+
 	protected function getInstance() {
 		$config = \OC::$server->getConfig()->getSystemValue('objectstore');
 		if (!is_array($config) || $config['class'] !== S3::class) {
@@ -70,6 +76,8 @@ class S3Test extends ObjectStoreTest {
 	}
 
 	public function testUploadNonSeekable() {
+		$this->cleanupAfter('multiparttest');
+
 		$s3 = $this->getInstance();
 
 		$s3->writeObject('multiparttest', NonSeekableStream::wrap(fopen(__FILE__, 'r')));
@@ -80,6 +88,8 @@ class S3Test extends ObjectStoreTest {
 	}
 
 	public function testSeek() {
+		$this->cleanupAfter('seek');
+
 		$data = file_get_contents(__FILE__);
 
 		$instance = $this->getInstance();
@@ -94,4 +104,80 @@ class S3Test extends ObjectStoreTest {
 		fseek($read, 100, SEEK_CUR);
 		$this->assertEquals(substr($data, 210, 100), fread($read, 100));
 	}
+
+	public function assertNoUpload($objectUrn) {
+		$s3 = $this->getInstance();
+		$s3client = $s3->getConnection();
+		$uploads = $s3client->listMultipartUploads([
+			'Bucket' => $s3->getBucket(),
+			'Prefix' => $objectUrn,
+		]);
+		$this->assertArrayNotHasKey('Uploads', $uploads);
+	}
+
+	public function testEmptyUpload() {
+		$s3 = $this->getInstance();
+
+		$emptyStream = fopen("php://memory", "r");
+		fwrite($emptyStream, null);
+
+		$s3->writeObject('emptystream', $emptyStream);
+
+		$this->assertNoUpload('emptystream');
+		$this->assertTrue($s3->objectExists('emptystream'));
+
+		$thrown = false;
+		try {
+			self::assertFalse($s3->readObject('emptystream'));
+		} catch (\Exception $e) {
+			// An exception is expected here since 0 byte files are wrapped
+			// to be read from an empty memory stream in the ObjectStoreStorage
+			$thrown = true;
+		}
+		self::assertTrue($thrown, 'readObject with range requests are not expected to work on empty objects');
+
+		$s3->deleteObject('emptystream');
+	}
+
+	/** File size to upload in bytes */
+	public function dataFileSizes() {
+		return [
+			[1000000], [2000000], [5242879], [5242880], [5242881], [10000000]
+		];
+	}
+
+	/** @dataProvider dataFileSizes */
+	public function testFileSizes($size) {
+		$this->cleanupAfter('testfilesizes');
+		$s3 = $this->getInstance();
+
+		$sourceStream = fopen('php://memory', 'wb+');
+		$writeChunkSize = 1024;
+		$chunkCount = $size / $writeChunkSize;
+		for ($i = 0; $i < $chunkCount; $i++) {
+			fwrite($sourceStream, str_repeat('A',
+				($i < $chunkCount - 1) ? $writeChunkSize : $size - ($i * $writeChunkSize)
+			));
+		}
+		rewind($sourceStream);
+		$s3->writeObject('testfilesizes', $sourceStream);
+
+		$this->assertNoUpload('testfilesizes');
+		self::assertTrue($s3->objectExists('testfilesizes'));
+
+		$result = $s3->readObject('testfilesizes');
+
+		// compare first 100 bytes
+		self::assertEquals(str_repeat('A', 100), fread($result, 100));
+
+		// compare 100 bytes
+		fseek($result, $size - 100);
+		self::assertEquals(str_repeat('A', 100), fread($result, 100));
+
+		// end of file reached
+		fseek($result, $size);
+		self:self::assertTrue(feof($result));
+
+		$this->assertNoUpload('testfilesizes');
+	}
 }