Skip to content

Commit

Permalink
Fix _empty_path AttributeError for path fieldtype
Browse files Browse the repository at this point in the history
On Python 3.11 and lower, the returned path object from .parent did not
have _empty_path variable set due to how pathlib creates the path object

Fixes #131
  • Loading branch information
yunzheng committed Aug 10, 2024
1 parent beabaa6 commit 439f394
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 10 deletions.
24 changes: 16 additions & 8 deletions flow/record/fieldtypes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@

UTC = timezone.utc

PY_311 = sys.version_info >= (3, 11, 0)
PY_312 = sys.version_info >= (3, 12, 0)
PY_311_OR_HIGHER = sys.version_info >= (3, 11, 0)
PY_312_OR_HIGHER = sys.version_info >= (3, 12, 0)

TYPE_POSIX = 0
TYPE_WINDOWS = 1
Expand Down Expand Up @@ -288,7 +288,7 @@ def __new__(cls, *args, **kwargs):
# - Python 3.10 and older requires "T" between date and time in fromisoformat()
#
# There are other incompatibilities, but we don't care about those for now.
if not PY_311:
if not PY_311_OR_HIGHER:
# Convert Z to +00:00 so that fromisoformat() works correctly on Python 3.10 and older
if arg[-1] == "Z":
arg = arg[:-1] + "+00:00"
Expand Down Expand Up @@ -633,6 +633,8 @@ def _is_windowslike_path(path: Any):


class path(pathlib.PurePath, FieldType):
_empty_path = False

def __new__(cls, *args):
# This is modelled after pathlib.PurePath's __new__(), which means you
# will never get an instance of path, only instances of either
Expand All @@ -647,7 +649,7 @@ def __new__(cls, *args):
for path_part in args:
if isinstance(path_part, pathlib.PureWindowsPath):
cls = windows_path
if not PY_312:
if not PY_312_OR_HIGHER:
# For Python < 3.12, the (string) representation of a
# pathlib.PureWindowsPath is not round trip equivalent if a path
# starts with a \ or / followed by a drive letter, e.g.: \C:\...
Expand All @@ -670,7 +672,7 @@ def __new__(cls, *args):
# This handles any custom PurePath based implementations that have a windows
# like path separator (\).
cls = windows_path
if not PY_312:
if not PY_312_OR_HIGHER:
args = tuple(str(arg) for arg in args)
elif _is_posixlike_path(path_part):
# This handles any custom PurePath based implementations that don't have a
Expand All @@ -680,7 +682,7 @@ def __new__(cls, *args):
continue
break

if PY_312:
if PY_312_OR_HIGHER:
obj = super().__new__(cls)
else:
obj = cls._from_parts(args)
Expand All @@ -693,8 +695,8 @@ def __new__(cls, *args):
def __eq__(self, other: Any) -> bool:
if isinstance(other, str):
return str(self) == other or self == self.__class__(other)
if self._empty_path:
return isinstance(other, self.__class__) and other._empty_path
elif isinstance(other, self.__class__) and (self._empty_path or other._empty_path):
return self._empty_path == other._empty_path
return super().__eq__(other)

def __str__(self) -> str:
Expand All @@ -705,6 +707,12 @@ def __str__(self) -> str:
def __repr__(self) -> str:
return repr(str(self))

@property
def parent(self):
if self._empty_path:
return self
return super().parent

def _pack(self):
path_type = TYPE_WINDOWS if isinstance(self, windows_path) else TYPE_POSIX
return (str(self), path_type)
Expand Down
42 changes: 40 additions & 2 deletions tests/test_fieldtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import flow.record.fieldtypes
from flow.record import RecordDescriptor, RecordReader, RecordWriter, fieldtypes
from flow.record.fieldtypes import (
PY_312,
PY_312_OR_HIGHER,
TYPE_POSIX,
TYPE_WINDOWS,
_is_posixlike_path,
Expand Down Expand Up @@ -543,7 +543,7 @@ def custom_pure_path(sep, altsep):
# The flavour property of Path's is replaced by a link to e.g.
# posixpath or ntpath.
# See also: https://github.com/python/cpython/issues/88302
if PY_312:
if PY_312_OR_HIGHER:

class CustomFlavour:
def __new__(cls, *args, **kwargs):
Expand Down Expand Up @@ -1154,13 +1154,15 @@ def test_empty_path(path_cls) -> None:
assert p1._empty_path
assert str(p1) == ""
assert p1 != path_cls(".")
assert path_cls(".") != p1

# initialize without any arguments
p2 = path_cls()
assert p2 == ""
assert p2._empty_path
assert str(p2) == ""
assert p2 != path_cls(".")
assert path_cls(".") != p2

assert p1 == p2

Expand Down Expand Up @@ -1213,5 +1215,41 @@ def test_empty_path_serialization(tmp_path) -> None:
assert record.value == ""


def test_empty_windows_path_parent() -> None:
# test that the parent of an empty path is also an empty path
path = fieldtypes.windows_path("")
assert path.parent == ""
assert path.parent.parent == ""
assert path._empty_path
assert path.parent._empty_path
assert list(path.parents) == []

path = fieldtypes.windows_path("c:/windows/temp")
assert path.parent == "c:/windows"
assert path.parent.parent == "c:/"
assert path.parent.parent.parent == "c:/"
assert not path.parent._empty_path
assert not path._empty_path
assert list(path.parents) == ["c:/windows", "c:/"]


def test_empty_posix_path_parent() -> None:
# test that the parent of an empty path is also an empty path
path = fieldtypes.posix_path("")
assert path.parent == ""
assert path.parent.parent == ""
assert path._empty_path
assert path.parent._empty_path
assert list(path.parents) == []

path = fieldtypes.posix_path("/var/log")
assert path.parent == "/var"
assert path.parent.parent == "/"
assert path.parent.parent.parent == "/"
assert not path.parent._empty_path
assert not path._empty_path
assert list(path.parents) == ["/var", "/"]


if __name__ == "__main__":
__import__("standalone_test").main(globals())

0 comments on commit 439f394

Please sign in to comment.