Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: handle propfind response from Nginx dav ext #59

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ tests =
typing_extensions==3.10.0.2
pytest==6.2.5
pytest-cov==2.12.1
respx==0.17.1
cheroot==8.5.2
WsgiDAV==3.1.1
colorama==0.4.4
Expand Down
20 changes: 16 additions & 4 deletions src/webdav4/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,12 @@ def propfind(
headers=headers,
)
http_resp = self.with_retry(call)
return parse_multistatus_response(http_resp)
msr = parse_multistatus_response(http_resp)
if not data and len(msr.responses) == 1:
_, response = next(iter(msr.responses.items()))
if response.has_propstat and not response.properties.status_ok():
raise ResourceNotFound(path)
return msr

def get_props(
self,
Expand Down Expand Up @@ -547,11 +552,17 @@ def exists(self, path: str) -> bool:

def isdir(self, path: str) -> bool:
"""Checks whether the resource with the given path is a directory."""
return bool(self.get_props(path).collection)
try:
return bool(self.get_props(path).collection)
except ResourceNotFound:
return False

def isfile(self, path: str) -> bool:
"""Checks whether the resource with the given path is a file."""
return not self.isdir(path)
try:
return not self.get_props(path).collection
except ResourceNotFound:
return False

def content_length(self, path: str) -> Optional[int]:
"""Returns content-length of the resource with the given path."""
Expand Down Expand Up @@ -586,7 +597,8 @@ def open(
chunk_size: int = None,
) -> Iterator[Union[TextIO, BinaryIO]]:
"""Returns file-like object to a resource."""
if self.isdir(path):
props = self.get_props(path)
if props.collection:
raise IsACollectionError(path, "Cannot open a collection")
assert mode in {"r", "rt", "rb"}

Expand Down
25 changes: 24 additions & 1 deletion src/webdav4/multistatus.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Parsing propfind response."""

from contextlib import suppress
from http.client import responses
from typing import TYPE_CHECKING, Any, Dict, Optional, Union
from xml.etree.ElementTree import Element, ElementTree, SubElement
Expand Down Expand Up @@ -88,6 +89,27 @@ def extract_text(prop_name: str) -> Optional[str]:

self.display_name = extract_text("display_name")

status_line = (
prop(response_xml, "status", relative=True)
if response_xml
else None
)
self.status_code: Optional[int] = None
if status_line:
_, code_str, *_ = status_line.split()
with suppress(ValueError):
self.status_code = int(code_str)

self.reason_phrase = (
responses[self.status_code] if self.status_code else None
)

def status_ok(self) -> bool:
"""Check if the propstat:status is ok."""
if not self.status_code:
return True
return 200 <= self.status_code < 300

def as_dict(self, raw: bool = False) -> Dict[str, Any]:
"""Returns all properties that it supports parsing.

Expand Down Expand Up @@ -144,7 +166,8 @@ def __init__(self, response_xml: Element) -> None:
code = None
if status_line:
_, code_str, *_ = status_line.split()
code = int(code_str)
with suppress(ValueError):
code = int(code_str)

self.status_code = code
self.reason_phrase = (
Expand Down
61 changes: 44 additions & 17 deletions tests/test_client.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Tests for webdav client."""
import textwrap
from datetime import datetime, timezone
from http import HTTPStatus
from io import DEFAULT_BUFFER_SIZE, BytesIO
Expand All @@ -8,6 +9,7 @@
from unittest.mock import MagicMock, patch

import pytest
from respx import MockRouter

from webdav4.client import (
BadGatewayError,
Expand Down Expand Up @@ -792,23 +794,8 @@ def test_isdir_isfile(storage_dir: TmpDir, client: Client):
assert client.isfile("/data/foo")
assert not client.isdir("/data/foo")

with pytest.raises(ResourceNotFound) as exc_info:
client.isdir("/data/file")

assert exc_info.value.path == "/data/file"
assert (
str(exc_info.value)
== "The resource /data/file could not be found in the server"
)

with pytest.raises(ResourceNotFound) as exc_info:
client.isdir("/data/file")

assert exc_info.value.path == "/data/file"
assert (
str(exc_info.value)
== "The resource /data/file could not be found in the server"
)
assert not client.isfile("/data/file")
assert not client.isdir("/data/file")


def test_exists(storage_dir: TmpDir, client: Client):
Expand Down Expand Up @@ -969,3 +956,43 @@ def test_client_retries(client: Client, server_address: URL):
)
client.copy("/container1", "/container2")
assert func.call_count == 3


def test_handle_nginx_propfind_responses_correctly(respx_mock: MockRouter):
"""Add support for Propfind response from nginx."""
from httpx import Response

path = "not-existing-file"
content = textwrap.dedent(
f"""\
<?xml version="1.0" encoding="utf-8" ?>
<D:multistatus xmlns:D="DAV:">
<D:response>
<D:href>/{path}</D:href>
<D:propstat>
<D:prop></D:prop>
<D:status>HTTP/1.1 404 Not Found</D:status>
</D:propstat>
</D:response>
</D:multistatus>"""
)
base_url = "https://example.org"
client = Client(base_url)
route = respx_mock.request(Method.PROPFIND, f"{base_url}/{path}").mock(
return_value=Response(HTTPStatus.MULTI_STATUS, content=content)
)

assert not client.exists(path)
with pytest.raises(ResourceNotFound):
client.propfind(path)

assert not client.isdir(path)
assert not client.isfile(path)

with pytest.raises(ResourceNotFound):
client.info(path)

with pytest.raises(ResourceNotFound):
assert client.ls(path)

assert route.called
11 changes: 11 additions & 0 deletions tests/test_multistatus.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ def test_dav_properties_empty(args: Tuple[Element]):
assert props.display_name is None
assert props.collection is None
assert props.resource_type is None
assert props.status_code is None
assert props.reason_phrase is None
assert props.status_ok()


def test_dav_properties():
Expand All @@ -88,6 +91,7 @@ def test_dav_properties():
<d:getetag>"8db748065bfed5c0731e9c7ee5f9bf4c"</d:getetag>
<d:getcontenttype>text/plain</d:getcontenttype>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>"""
elem = fromstring(content)
Expand Down Expand Up @@ -116,6 +120,8 @@ def test_dav_properties():
assert props.display_name == "foo"
assert props.collection is False
assert props.resource_type == "file"
assert props.status_code == 200
assert props.reason_phrase == "OK"

assert props.as_dict() == {
"created": datetime(2020, 1, 2, 3, 4, 5, tzinfo=tzutc()),
Expand All @@ -127,6 +133,7 @@ def test_dav_properties():
"display_name": "foo",
"type": "file",
}
assert props.status_ok()


def test_dav_properties_partial():
Expand Down Expand Up @@ -166,6 +173,10 @@ def test_dav_properties_partial():
assert props.collection is True
assert props.resource_type == "directory"

assert props.status_code is None
assert props.reason_phrase is None
assert props.status_ok()


@pytest.mark.parametrize(
"href, absolute",
Expand Down