fix(webhdfs): fix WebHDFS provider to handle requests without Range header (#90)

* fix(webhdfs): fix WebHDFS provider to handle requests without Range header
* test(webHDFS): handle GET file requests without Range header
* test(webhdfs): fix async mock misuse and align HTTP error behavior
* tests(webhdfs): add missing tests
* tests(webhdfs): address unawaited coroutine warnings

---------

Co-authored-by: SilviaSWR <sbogaart@pic.es>

Author: SilviaSWR

asgi_webdav/provider/webhdfs.py

@@ -261,10 +261,13 @@ async def _do_get(self, request: DAVRequest) -> tuple[
                 return status_response, dav_property.basic_data, None, None
 
             # Read file's content
-            response_content_range = get_response_content_range(
-                request_ranges=request.ranges,
-                file_size=dav_property.basic_data.content_length,
-            )
+            if not request.ranges:  # No range header, return the whole content
+                response_content_range = None
+            else:
+                response_content_range = get_response_content_range(
+                    request_ranges=request.ranges,
+                    file_size=dav_property.basic_data.content_length,
+                )
             if response_content_range is None:
                 response_content_range = DAVResponseContentRange(
                     DAVRangeType.RANGE,

tests/test_provider_webhdfs.py

@@ -63,7 +63,7 @@ async def test_get_url_path_with_home(mock_provider):
 async def test_do_filestatus_success(mock_provider, fake_request):
     mock_response = AsyncMock()
     mock_response.status_code = 200
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_response.json = MagicMock(
         return_value={
@@ -86,6 +86,23 @@ async def test_do_filestatus_success(mock_provider, fake_request):
     assert file_status["type"] == "FILE"
 
 
+@pytest.mark.asyncio
+async def test_do_filestatus_missing_filestatus(mock_provider, fake_request):
+    mock_response = AsyncMock()
+    mock_response.status_code = 200
+    mock_response.raise_for_status = MagicMock()
+    mock_response.json = MagicMock(return_value={"FileStatus": {}})
+
+    mock_provider.client.get.return_value = mock_response
+
+    status, data = await mock_provider._do_filestatus(
+        fake_request, DAVPath("/test.txt")
+    )
+
+    assert status == 200
+    assert data == {}
+
+
 @pytest.mark.asyncio
 async def test_do_get_file(mock_provider, fake_request):
     fake_status = {
@@ -102,7 +119,13 @@ async def test_do_get_file(mock_provider, fake_request):
             ),
         )
     )
-    mock_provider._dav_response_data_generator = AsyncMock(return_value=AsyncMock())
+
+    async def fake_generator():
+        yield b"data"
+
+    mock_provider._dav_response_data_generator = MagicMock(
+        return_value=fake_generator()
+    )
 
     fake_request.ranges = [DAVRequestRange(DAVRangeType.RANGE, 0, 100, 200)]
     status, basic_data, generator, _ = await mock_provider._do_get(fake_request)
@@ -112,11 +135,44 @@ async def test_do_get_file(mock_provider, fake_request):
     assert generator is not None
 
 
+@pytest.mark.asyncio
+async def test_do_get_file_non_range(mock_provider, fake_request):
+    fake_status = {
+        "type": "FILE",
+        "length": 100,
+        "modificationTime": 1234567890,
+    }
+
+    mock_provider._get_dav_property_d0 = AsyncMock(
+        return_value=(
+            200,
+            await mock_provider._create_dav_property_obj(
+                fake_request, DAVPath("/testfile.txt"), fake_status
+            ),
+        )
+    )
+
+    async def fake_generator():
+        yield b"data"
+
+    mock_provider._dav_response_data_generator = MagicMock(
+        return_value=fake_generator()
+    )
+
+    fake_request.ranges = None
+    status, basic_data, generator, _ = await mock_provider._do_get(fake_request)
+
+    assert status == 200
+    assert basic_data.content_length == 100
+    assert generator is not None
+
+
 @pytest.mark.asyncio
 async def test_do_delete_success(mock_provider, fake_request):
     mock_provider._precheck_source = AsyncMock(return_value=(True, True, False))
     mock_provider.client.delete.return_value = AsyncMock(
-        status_code=204, raise_for_status=AsyncMock()
+        status_code=204,
+        raise_for_status=MagicMock(),
     )
 
     result = await mock_provider._do_delete(fake_request)
@@ -130,6 +186,23 @@ async def test_do_delete_not_found(mock_provider, fake_request):
     assert result == 404
 
 
+@pytest.mark.asyncio
+async def test_do_delete_http_error(mock_provider, fake_request):
+    mock_provider._precheck_source = AsyncMock(return_value=(True, True, False))
+
+    mock_response = AsyncMock()
+    mock_response.raise_for_status = MagicMock(
+        side_effect=httpx.HTTPStatusError(
+            "error", request=MagicMock(), response=MagicMock(status_code=500)
+        )
+    )
+
+    mock_provider.client.delete.return_value = mock_response
+
+    response = await mock_provider._do_delete(fake_request)
+    assert response == 424
+
+
 def test_get_url():
     root = WebHDFSProvider(
         config=Config(),
@@ -175,17 +248,20 @@ def test_get_url():
 async def test_do_filestatus_failure(mock_provider, fake_request):
     mock_response = AsyncMock()
     mock_response.status_code = 404
-    mock_response.raise_for_status.side_effect = httpx.HTTPStatusError(
-        message="Error", request=MagicMock(), response=MagicMock(status_code=404)
+    mock_response.raise_for_status = MagicMock(
+        side_effect=httpx.HTTPStatusError(
+            message="Error",
+            request=MagicMock(),
+            response=MagicMock(status_code=404),
+        )
     )
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
 
     mock_provider.client.get.return_value = mock_response
 
     url_path = DAVPath("/notfound.txt")
-
-    response = await mock_provider._do_filestatus(fake_request, url_path)
-    assert response == (404, {})
+    with pytest.raises(httpx.HTTPStatusError):
+        await mock_provider._do_filestatus(fake_request, url_path)
 
 
 @pytest.mark.asyncio
@@ -214,6 +290,30 @@ async def test_do_get_collection(mock_provider, fake_request):
     assert response_content_range is None
 
 
+@pytest.mark.asyncio
+async def test_do_get_invalid_range(mock_provider, fake_request):
+    fake_status = {
+        "type": "FILE",
+        "length": 100,
+        "modificationTime": 0,
+    }
+
+    mock_provider._get_dav_property_d0 = AsyncMock(
+        return_value=(
+            200,
+            await mock_provider._create_dav_property_obj(
+                fake_request, DAVPath("/file.txt"), fake_status
+            ),
+        )
+    )
+
+    fake_request.ranges = [DAVRequestRange(DAVRangeType.RANGE, 200, 300, 100)]
+
+    status, _, _, _ = await mock_provider._do_get(fake_request)
+
+    assert status in (200, 416)
+
+
 @pytest.mark.asyncio
 async def test_dav_response_data_generator(mock_provider, fake_request):
     async def fake_aiter_bytes():
@@ -244,26 +344,60 @@ async def fake_aiter_bytes():
     assert result == [(b"chunk1", True), (b"chunk2", False)]
 
 
+@pytest.mark.asyncio
+async def test_dav_response_generator_empty(mock_provider, fake_request):
+    async def fake_aiter():
+        if False:
+            yield b""
+
+    fake_response = MagicMock()
+    fake_response.aiter_bytes = fake_aiter
+    fake_response.raise_for_status = MagicMock()
+
+    class AsyncContextManager:
+        async def __aenter__(self):
+            return fake_response
+
+        async def __aexit__(self, exc_type, exc_val, exc_tb):
+            return None
+
+    mock_provider.client.stream = MagicMock(return_value=AsyncContextManager())
+
+    gen = mock_provider._dav_response_data_generator(
+        fake_request, DAVPath("/file.txt"), None, None
+    )
+
+    result = [chunk async for chunk, more in gen]
+    assert result == [None]
+
+
 @pytest.mark.asyncio
 async def test_do_put(mock_provider, fake_request):
-    mock_response = MagicMock()
-    mock_response.status_code = 200
-    mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_provider._precheck_source = AsyncMock(return_value=(True, False, False))
 
-    mock_provider.client.get.return_value = mock_response
+    first_put = AsyncMock()
+    first_put.status_code = 307
+    first_put.headers = {
+        "location": "http://fake-hdfs:9870/webhdfs/v1/file_put_location"
+    }
+    first_put.raise_for_status = MagicMock()
 
-    response = await mock_provider._do_put(fake_request)
+    second_put = AsyncMock()
+    second_put.status_code = 201
+    second_put.raise_for_status = MagicMock()
 
-    assert response == 204
+    mock_provider.client.put = AsyncMock(side_effect=[first_put, second_put])
+
+    response = await mock_provider._do_put(fake_request)
+    assert response == 201
 
 
 @pytest.mark.asyncio
 async def test_do_copy(mock_provider, fake_request):
     mock_response = MagicMock()
     mock_response.status_code = 200
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_provider.client.get.return_value = mock_response
 
@@ -276,7 +410,7 @@ async def test_do_mkcol(mock_provider, fake_request):
     mock_response = MagicMock()
     mock_response.status_code = 200
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_provider.client.get.return_value = mock_response
 
@@ -290,7 +424,7 @@ async def test_do_propfind(mock_provider, fake_request):
     mock_response = MagicMock()
     mock_response.status_code = 200
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_provider.client.get.return_value = mock_response
 
@@ -340,12 +474,29 @@ async def test_do_propfind(mock_provider, fake_request):
         assert resp_val.extra_not_found == exp_val.extra_not_found
 
 
+@pytest.mark.asyncio
+async def test_do_propfind_with_extra_keys(mock_provider, fake_request):
+    fake_request.propfind_only_fetch_basic = False
+    fake_request.propfind_extra_keys = ["custom:key"]
+
+    mock_response = AsyncMock()
+    mock_response.status_code = 200
+    mock_response.raise_for_status = MagicMock()
+    mock_response.json = MagicMock(return_value={"FileStatus": {}})
+
+    mock_provider.client.get.return_value = mock_response
+
+    result = await mock_provider._do_propfind(fake_request)
+
+    assert isinstance(result, dict)
+
+
 @pytest.mark.asyncio
 async def test_do_head(mock_provider, fake_request):
     mock_response = MagicMock()
     mock_response.status_code = 200
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_provider.client.get.return_value = mock_response
 
@@ -359,7 +510,7 @@ async def test_do_precheck_destination(mock_provider, fake_request):
     mock_response = MagicMock()
     mock_response.status_code = 200
     mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    mock_response.raise_for_status = MagicMock()
 
     mock_provider.client.get.return_value = mock_response
 
@@ -370,13 +521,32 @@ async def test_do_precheck_destination(mock_provider, fake_request):
 
 @pytest.mark.asyncio
 async def test_do_move(mock_provider, fake_request):
-    mock_response = MagicMock()
-    mock_response.status_code = 200
-    mock_response.json = MagicMock(return_value={"FileStatus": {}})
-    mock_response.raise_for_status = AsyncMock()
+    fake_request.overwrite = True
 
-    mock_provider.client.get.return_value = mock_response
+    mock_provider._precheck_destination = AsyncMock(return_value=(True, False, True))
+
+    mock_delete_response = MagicMock()
+    mock_delete_response.status_code = 204
+    mock_delete_response.raise_for_status = MagicMock()
+
+    mock_provider.client.delete = AsyncMock(return_value=mock_delete_response)
+
+    mock_put_response = MagicMock()
+    mock_put_response.status_code = 201
+    mock_put_response.raise_for_status = MagicMock()
+
+    mock_provider.client.put = AsyncMock(return_value=mock_put_response)
+
+    result = await mock_provider._do_move(fake_request)
+    assert result == 204
+
+
+@pytest.mark.asyncio
+async def test_do_move_destination_exists_no_overwrite(mock_provider, fake_request):
+    fake_request.overwrite = False
+
+    mock_provider._precheck_destination = AsyncMock(return_value=(True, True, True))
 
-    response = await mock_provider._do_move(fake_request)
+    result = await mock_provider._do_move(fake_request)
 
-    assert response == 204
+    assert result == 403