fix: Fix uploading large files via multiple chunks

Previously `aiohttp-tus` wrongly writes data between the chunks as,

```python
handler.seek(offset)
handler.write(chunk)
```

in `wb` mode truncates all content before `offset` with `\0` bytes.

But, when `r+b` mode used, everything went fine. However, using
`r+b` mode needs to allow explicitly pass mode for initial save. And
for better results initial save attempts to store whole file size
to the disc to attempt avoid insufficient disk space errors on upload.

CHANGELOG.rst

@@ -2,6 +2,11 @@
 ChangeLog
 =========
 
+1.0.0rc1 (In Development)
+=========================
+
+- Fix upload large files via multiple chunks
+
 1.0.0rc0 (2020-03-26)
 =====================
 

Makefile

@@ -24,7 +24,7 @@ SPHINXBUILD ?= $(POETRY) run sphinx-build
 TOX ?= tox
 
 # Example constants
-AIOHTTP_PORT = 8300
+AIOHTTP_PORT ?= 8300
 
 all: install
 

aiohttp_tus/__init__.py

@@ -2,4 +2,4 @@
 
 __all__ = ("setup_tus",)
 __license__ = "BSD-3-Clause"
-__version__ = "1.0.0rc0"
+__version__ = "1.0.0rc1"

aiohttp_tus/data.py

@@ -132,12 +132,29 @@ def from_metadata(
             metadata_header=data["metadata_header"],
         )
 
+    def initial_save(
+        self, *, config: Config, match_info: web.UrlMappingMatchInfo
+    ) -> Tuple[Path, int]:
+        return self.save(
+            config=config,
+            match_info=match_info,
+            chunk=b"\0",
+            mode="wb",
+            offset=self.file_size - 1 if self.file_size > 0 else 0,
+        )
+
     def save(
-        self, *, config: Config, match_info: web.UrlMappingMatchInfo, chunk: bytes
+        self,
+        *,
+        config: Config,
+        match_info: web.UrlMappingMatchInfo,
+        chunk: bytes,
+        mode: str = None,
+        offset: int = None,
     ) -> Tuple[Path, int]:
         path = get_resource_path(config=config, match_info=match_info, uid=self.uid)
-        with open(path, "wb+") as handler:
-            handler.seek(self.offset)
+        with open(path, mode if mode is not None else "r+b") as handler:
+            handler.seek(offset if offset is not None else self.offset)
             chunk_size = handler.write(chunk)
         return (path, chunk_size)
 

aiohttp_tus/views.py

@@ -85,7 +85,7 @@ async def start_upload(request: web.Request) -> web.Response:
     # Save resource and its metadata
     match_info = request.match_info
     try:
-        resource.save(config=config, match_info=match_info, chunk=b"\0")
+        resource.initial_save(config=config, match_info=match_info)
         resource.save_metadata(config=config, match_info=match_info)
     # In case if file system is not able to store given files - abort the upload
     except IOError:

pyproject.toml

@@ -15,7 +15,7 @@ show_missing = true
 
 [tool.poetry]
 name = "aiohttp-tus"
-version = "1.0.0rc0"
+version = "1.0.0rc1"
 description = "tus.io protocol implementation for aiohttp.web applications"
 authors = ["Igor Davydenko <iam@igordavydenko.com>"]
 license = "BSD-3-Clause"

tests/test_tus.py

@@ -1,6 +1,5 @@
-import tempfile
+import shutil
 from functools import partial
-from pathlib import Path
 
 try:
     from contextlib import asynccontextmanager
@@ -31,7 +30,7 @@
 
 
 @pytest.fixture
-def aiohttp_test_client(aiohttp_client):
+def aiohttp_test_client(tmp_path, aiohttp_client):
     @asynccontextmanager
     async def factory(
         *,
@@ -42,18 +41,20 @@ async def factory(
         on_upload_done: ResourceCallback = None,
         decorator: Decorator = None,
     ) -> TestClient:
-        with tempfile.TemporaryDirectory(prefix="aiohttp_tus") as temp_path:
-            base_path = Path(temp_path)
-            app = setup_tus(
-                web.Application(),
-                upload_path=base_path / upload_suffix if upload_suffix else base_path,
-                upload_url=upload_url,
-                upload_resource_name=upload_resource_name,
-                allow_overwrite_files=allow_overwrite_files,
-                on_upload_done=on_upload_done,
-                decorator=decorator,
-            )
+        upload_path = tmp_path / "aiohttp_tus"
+        app = setup_tus(
+            web.Application(),
+            upload_path=upload_path / upload_suffix if upload_suffix else upload_path,
+            upload_url=upload_url,
+            upload_resource_name=upload_resource_name,
+            allow_overwrite_files=allow_overwrite_files,
+            on_upload_done=on_upload_done,
+            decorator=decorator,
+        )
+        try:
             yield await aiohttp_client(app)
+        finally:
+            shutil.rmtree(upload_path, ignore_errors=True)
 
     return factory
 
@@ -224,17 +225,23 @@ async def test_upload(
         assert expected_upload_path.read_bytes() == TEST_FILE_PATH.read_bytes()
 
 
-async def test_upload_large_file(aiohttp_test_client, loop):
-    upload = partial(
-        tus.upload, file_name=TEST_SCREENSHOT_NAME, chunk_size=TEST_CHUNK_SIZE
-    )
+@pytest.mark.parametrize(
+    "chunk_size", (TEST_CHUNK_SIZE, TEST_CHUNK_SIZE * 2, TEST_CHUNK_SIZE * 4)
+)
+async def test_upload_large_file(aiohttp_test_client, loop, chunk_size):
+    upload = partial(tus.upload, file_name=TEST_SCREENSHOT_NAME, chunk_size=chunk_size)
 
     async with aiohttp_test_client(upload_url=TEST_UPLOAD_URL) as client:
         with open(TEST_SCREENSHOT_PATH, "rb") as handler:
             await loop.run_in_executor(
                 None, upload, handler, get_upload_url(client, TEST_UPLOAD_URL)
             )
 
+        config: Config = client.app[APP_TUS_CONFIG_KEY]["/uploads"]
+        expected_upload_path = config.resolve_upload_path({}) / TEST_SCREENSHOT_NAME
+        assert expected_upload_path.exists()
+        assert expected_upload_path.read_bytes() == TEST_SCREENSHOT_PATH.read_bytes()
+
 
 async def test_upload_resource_name(aiohttp_test_client, loop):
     upload = partial(tus.upload, file_name=TEST_FILE_NAME)