From 08f2d2705b5e8df4a4b52707abf4b9a6db5b838e Mon Sep 17 00:00:00 2001
From: svalouch <svalouch@valouch.com>
Date: Mon, 2 Nov 2020 15:08:19 +0100
Subject: [PATCH] types: enhance and fix PropertySource

Enhancement: support bytes data type
Fixes: ZFS returns the 'inherited' properties as 'inherited from XYZ', which was not caught by the
parser and an exception was raised. With this change, such values are supported, too.
---
 src/simplezfs/types.py | 18 ++++++++++++------
 tests/test_types.py    | 41 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/src/simplezfs/types.py b/src/simplezfs/types.py
index 2648580..aad6a57 100644
--- a/src/simplezfs/types.py
+++ b/src/simplezfs/types.py
@@ -5,7 +5,7 @@ Type declarations
 
 import os
 from enum import Enum, unique
-from typing import NamedTuple, Optional
+from typing import NamedTuple, Optional, Union
 
 from .validation import validate_dataset_path, validate_pool_name
 
@@ -114,7 +114,7 @@ class PropertySource(str, Enum):
     NONE = 'none'
 
     @staticmethod
-    def from_string(value: str) -> 'PropertySource':
+    def from_string(value: Union[bytes, str]) -> 'PropertySource':
         '''
         Helper to convert a string to an instance.
 
@@ -122,14 +122,20 @@ class PropertySource(str, Enum):
         :returns: The enum value.
         :raises ValueError: If the supplied value is not found in the enumeration.
         '''
-        if not isinstance(value, str):
+        if not isinstance(value, bytes) and not isinstance(value, str):
             raise ValueError('only string types allowed')
-        val = value.lower()
+
+        if isinstance(value, bytes):
+            val = value.decode('utf-8').lower()
+        else:
+            val = value.lower()
+
         if val == 'default':
             return PropertySource.DEFAULT
         if val == 'local':
             return PropertySource.LOCAL
-        if val == 'inherited':
+        # support single word and "inherited from YXZ"
+        if val == 'inherited' or val.startswith('inherited from '):
             return PropertySource.INHERITED
         if val == 'temporary':
             return PropertySource.TEMPORARY
@@ -137,7 +143,7 @@ class PropertySource(str, Enum):
             return PropertySource.RECEIVED
         if val in ('none', '-'):
             return PropertySource.NONE
-        raise ValueError(f'Value {value} is not a valid PropertySource')
+        raise ValueError(f'Value {val} is not a valid PropertySource')
 
 
 class Property(NamedTuple):
diff --git a/tests/test_types.py b/tests/test_types.py
index ce1855d..c1debdd 100644
--- a/tests/test_types.py
+++ b/tests/test_types.py
@@ -3,7 +3,7 @@ from unittest.mock import patch
 import pytest
 
 from simplezfs.exceptions import ValidationError
-from simplezfs.types import Dataset, DatasetType
+from simplezfs.types import Dataset, DatasetType, PropertySource
 from simplezfs.validation import validate_dataset_path
 
 
@@ -39,3 +39,42 @@ class TestTypesDataset:
     def test_from_string_invalid(self, identifier):
         with pytest.raises(ValidationError):
             Dataset.from_string(identifier)
+
+
+class TestTypesPropertySource:
+    '''
+    Tests for simplezfs.types.PropertySource.
+    '''
+
+    @pytest.mark.parametrize('value,sourcetype', [
+        ('default', PropertySource.DEFAULT),
+        ('DEFault', PropertySource.DEFAULT),
+        ('local', PropertySource.LOCAL),
+        ('LOCAL', PropertySource.LOCAL),
+        ('inherited', PropertySource.INHERITED),
+        ('INHERITED', PropertySource.INHERITED),
+        ('inherited from somesource', PropertySource.INHERITED),
+        ('INHERITED FROM somesource', PropertySource.INHERITED),
+        ('temporary', PropertySource.TEMPORARY),
+        ('TEMPORARY', PropertySource.TEMPORARY),
+        (b'TEMPORARY', PropertySource.TEMPORARY),
+        ('received', PropertySource.RECEIVED),
+        ('RECEIVED', PropertySource.RECEIVED),
+        ('none', PropertySource.NONE),
+        ('NONE', PropertySource.NONE),
+        ('-', PropertySource.NONE),
+    ])
+    def test_from_string_valid(self, value: str, sourcetype: PropertySource) -> None:
+        '''
+        Happy path, test that it accepts known-good values.
+        '''
+        assert PropertySource.from_string(value) == sourcetype
+
+    @pytest.mark.parametrize('value', [' default', 'DEFAULT ', 'asdf', '', 'DEFAULT FROM asdf', 'inherited somesrc',
+                                       123])
+    def test_from_string_invalid(self, value: str) -> None:
+        '''
+        Test that it successfully reports an error when receiving invalid input.
+        '''
+        with pytest.raises(ValueError):
+            PropertySource.from_string(value)