From 1f86d946cbc29e0f5b1586330abbba18d48d10f1 Mon Sep 17 00:00:00 2001 From: CJ Koral Date: Fri, 6 Jul 2018 16:05:16 -0600 Subject: [PATCH 1/5] Add full support for text and numeric list field --- requirements.txt | 1 + swimlane/core/fields/base/cursor.py | 2 +- swimlane/core/fields/list.py | 231 ++++++++++++++++++++++++++++ tests/fields/test_valueslist.py | 2 +- 4 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 swimlane/core/fields/list.py diff --git a/requirements.txt b/requirements.txt index 4ec47507..c8508b1f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,3 +5,4 @@ pyuri>=0.3,<0.4 requests[security]>=2,<3 six>=1.10,<1.11 sortedcontainers>=1.5,<1.6 +shortid==0.1.2 \ No newline at end of file diff --git a/swimlane/core/fields/base/cursor.py b/swimlane/core/fields/base/cursor.py index 77d9de65..1385e89f 100644 --- a/swimlane/core/fields/base/cursor.py +++ b/swimlane/core/fields/base/cursor.py @@ -22,7 +22,7 @@ def __init__(self, field, initial_elements=None): def __repr__(self): # pylint: disable=missing-format-attribute - return '<{self.__class__.__name__}: {self._record}["{self._field.name}"] ({length})>'.format( + return '<{self.__class__.__name__}: {self._record!r}["{self._field.name}"] ({length})>'.format( self=self, length=len(self) ) diff --git a/swimlane/core/fields/list.py b/swimlane/core/fields/list.py new file mode 100644 index 00000000..b11f916f --- /dev/null +++ b/swimlane/core/fields/list.py @@ -0,0 +1,231 @@ +from numbers import Number + +import six +from shortid import ShortId + +from swimlane.core.fields.base import FieldCursor +from swimlane.exceptions import ValidationError +from .base import CursorField + +SID = ShortId() + + +class _ListFieldCursor(FieldCursor): + """Base class for Text and Numeric FieldCursors emulating a basic list""" + + def _validate_list(self, target): + """Validate a list against field validation rules""" + # Check list length restrictions + min_items = self._field.field_definition.get('minItems') + max_items = self._field.field_definition.get('maxItems') + + if min_items is not None: + if len(target) < min_items: + raise ValidationError( + self._record, + "Field '{}' must have a minimum of {} item(s)".format(self._field.name, min_items) + ) + + if max_items is not None: + if len(target) > max_items: + raise ValidationError( + self._record, + "Field '{}' can only have a maximum of {} item(s)".format(self._field.name, max_items) + ) + + # Individual item validation + for item in target: + self._validate_item(item) + + def _validate_item(self, item): + """Validate individual item against field rules. Defaults to no-op""" + + def __getattr__(self, item): + """Fallback to any builtin list methods on self._elements for any undefined methods called on cursor + + List methods are wrapped in a function ensuring the updated value passes field validation before actually + applying to the internal value + + Wrapper function creates a copy of the current elements in the cursor, calls whatever list method was executed + on the copy, then validates the updated list against field rules. + + If validation fails, raise ValidationError, otherwise update the cursor elements and field value to the modified + list copy + """ + try: + # Check for method on list class + func = getattr(list, item) + + # Create a copy of elements if function exists to be the actual target of the method call + elements_copy = self._elements[:] + + # Wrap in function adding validation after the method is executed + def wrapper(*args, **kwargs): + # Execute method against the copied elements + result = func(elements_copy, *args, **kwargs) + + # Validate elements copy after any potential modification + self._validate_list(elements_copy) + + # Update internal cursor elements to modified copy and sync with field + self._elements = elements_copy + self._sync_field() + + # Return in case of methods retrieving values instead of modifying state + return result + + return wrapper + except AttributeError: + # Raise separate AttributeError with correct class reference instead of list + raise AttributeError("{} object has no attribute {}".format(self.__class__, item)) + + +class TextListFieldCursor(_ListFieldCursor): + """Cursor for Text ListField""" + + def _validate_item(self, item): + """Validate char/word count""" + if not isinstance(item, six.string_types): + raise ValidationError( + self._record, + "Text list field items must be strings, not '{}'".format(item.__class__) + ) + + words = item.split(' ') + + item_length_type = self._field.field_definition.get('itemLengthType') + item_max_length = self._field.field_definition.get('itemMaxLength') + item_min_length = self._field.field_definition.get('itemMinLength') + if item_length_type is not None: + # Min/max word count + if item_length_type == 'words': + if item_max_length is not None: + if len(words) > item_max_length: + raise ValidationError( + self._record, + "Field '{}' items cannot contain more than {} words".format( + self._field.name, + item_max_length + ) + ) + if item_min_length is not None: + if len(words) < item_min_length: + raise ValidationError( + self._record, + "Field '{}' items must contain at least {} words".format( + self._field.name, + item_min_length + ) + ) + + # Min/max char count of full item + else: + if item_max_length is not None: + if len(item) > item_max_length: + raise ValidationError( + self._record, + "Field '{}' items cannot contain more than {} characters".format( + self._field.name, + item_max_length + ) + ) + if item_min_length is not None: + if len(item) < item_min_length: + raise ValidationError( + self._record, + "Field '{}' items must contain at least {} characters".format( + self._field.name, + item_min_length + ) + ) + + +class NumericListFieldCursor(_ListFieldCursor): + """Cursor for Numeric ListField""" + + def _validate_item(self, item): + if not isinstance(item, Number): + raise ValidationError( + self._record, + "Numeric list field items must be numbers, not '{}'".format(item.__class__) + ) + + # range restrictions + item_max = self._field.field_definition.get('itemMax') + item_min = self._field.field_definition.get('itemMin') + + if item_max is not None: + if item > item_max: + raise ValidationError( + self._record, + "Field '{}' items cannot be greater than {}".format( + self._field.name, + item_max + ) + ) + + if item_min is not None: + if item < item_min: + raise ValidationError( + self._record, + "Field '{}' items cannot be less than {}".format( + self._field.name, + item_min + ) + ) + + +class ListField(CursorField): + """Text and Numeric List field""" + + field_type = ( + 'Core.Models.Fields.List.ListField, Core', + 'Core.Models.Fields.ListField, Core', + ) + + _type_map = { + 'numeric': { + 'list_item_type': 'Core.Models.Record.ListItem`1[[System.Double, mscorlib]], Core', + 'cursor_class': NumericListFieldCursor + }, + 'text': { + 'list_item_type': 'Core.Models.Record.ListItem`1[[System.String, mscorlib]], Core', + 'cursor_class': TextListFieldCursor + } + } + + def __init__(self, *args, **kwargs): + super(ListField, self).__init__(*args, **kwargs) + self.cursor_class = self._type_map[self.input_type]['cursor_class'] + + def set_swimlane(self, value): + """Convert from list of dicts with values to list of values""" + value = [d['value'] for d in value or []] + + return super(ListField, self).set_swimlane(value) + + def set_python(self, value): + """Validate using cursor for consistency between direct set of values vs modification of cursor values""" + if not isinstance(value, (list, type(None))): + raise ValidationError( + self.record, + "Field '{}' must be set to a list, not '{}'".format( + self.name, + value.__class__ + ) + ) + value = value or [] + self.cursor._validate_list(value) + return super(ListField, self).set_python(value) + + def cast_to_swimlane(self, value): + value = super(ListField, self).cast_to_swimlane(value) + return [self._build_list_item(item) for item in value] + + def _build_list_item(self, item_value): + """Return a dict with random ID and $type for API representation of value""" + return { + '$type': self._type_map[self.input_type]['list_item_type'], + 'id': SID.generate(), + 'value': item_value + } diff --git a/tests/fields/test_valueslist.py b/tests/fields/test_valueslist.py index aa915874..f65e7bed 100644 --- a/tests/fields/test_valueslist.py +++ b/tests/fields/test_valueslist.py @@ -78,7 +78,7 @@ def test_values_list_multi_select_field(mock_record): def test_cursor_repr(mock_record): - assert repr(mock_record['Values List']) == '' + assert repr(mock_record['Values List']) == '["Values List"] (2)>' def test_get_report(mock_record): From acb89ecfb603be62cfd5719be4b8703435e929f1 Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Wed, 11 Jul 2018 15:57:48 -0600 Subject: [PATCH 2/5] Fix cursor lost ref to field instance after saving record --- swimlane/core/fields/base/cursor.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/swimlane/core/fields/base/cursor.py b/swimlane/core/fields/base/cursor.py index 1385e89f..7539137a 100644 --- a/swimlane/core/fields/base/cursor.py +++ b/swimlane/core/fields/base/cursor.py @@ -17,6 +17,7 @@ def __init__(self, field, initial_elements=None): self._elements = initial_elements or self._elements + self.__field_name = field.name self.__record_ref = weakref.ref(field.record) self.__field_ref = weakref.ref(field) @@ -40,7 +41,13 @@ def _record(self): @property def _field(self): - return self.__field_ref() + field = self.__field_ref() + # Occurs when a record is saved and reinitialized, creating new Field instances and losing the existing weakref + # Update weakref to point to new Field instance of the same name + if field is None: + field = self._record.get_field(self.__field_name) + self.__field_ref = weakref.ref(field) + return field class CursorField(Field): From 5267d179259dc2f4dc8f5d6d0821d4fdfaccc868 Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Wed, 11 Jul 2018 16:01:40 -0600 Subject: [PATCH 3/5] Fix clearing list field value --- swimlane/core/fields/list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swimlane/core/fields/list.py b/swimlane/core/fields/list.py index b11f916f..4c761314 100644 --- a/swimlane/core/fields/list.py +++ b/swimlane/core/fields/list.py @@ -220,7 +220,7 @@ def set_python(self, value): def cast_to_swimlane(self, value): value = super(ListField, self).cast_to_swimlane(value) - return [self._build_list_item(item) for item in value] + return [self._build_list_item(item) for item in value] or None def _build_list_item(self, item_value): """Return a dict with random ID and $type for API representation of value""" From dc90b775100305aaab626ebcb2a0ca95c32600bf Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Wed, 11 Jul 2018 17:40:22 -0600 Subject: [PATCH 4/5] Add list field example doc --- docs/examples/fields.rst | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/docs/examples/fields.rst b/docs/examples/fields.rst index 5d182df2..dfb32324 100644 --- a/docs/examples/fields.rst +++ b/docs/examples/fields.rst @@ -324,6 +324,53 @@ Field can be set directly to any iterable, overwriting current selection entirel assert len(record['Values List']) == 2 +ListField +--------- + +Text and numeric list field. Uses a :class:`TextListFieldCursor` or a :class:`NumericListFieldCursor` depending on the +field type to enforce min/max item count restrictions, min/max character/word limits, and numeric range restrictions. + +Cursor works exactly like a normal primitive Python :class:`list` with added validation around any methods modifying the +list or its items, and when overriding the field value entirely. + +.. code-block:: python + + # Cursor behaving like a list + text_list_cursor = record['Text List Field'] + + # Iteration + for value in text_list_cursor: + print(value) + + # Modification + text_list_cursor.reverse() + text_list_cursor.insert(0, 'new value') + + # Index/slice + assert text_list_cursor[0] == 'new value' + + # Contains + assert 'new value' in text_list_cursor + + # Type validation + # Failing validation will not modify the field value + original_values = list(text_list_cursor) + try: + text_list_cursor.append(123) + except ValidationError: + assert len(original_values) == len(text_list_cursor) + + # Replacement + # Can be set directly to a new list of values + record['Text List Field'] = ['new', 'values'] + + # Any invalid values will abort the entire operation + try: + record['Text List Field'] = ['text', 456] + except ValidationError: + assert list(record['Text List Field']) == ['new', 'values'] + + UserGroupField -------------- From 09557a25bcd9d12a22dd5dc45f92c02e11257e0a Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Wed, 11 Jul 2018 18:04:52 -0600 Subject: [PATCH 5/5] Add full test coverage for list field --- tests/conftest.py | 23 ++++++++++ tests/fields/test_basic.py | 3 +- tests/fields/test_list.py | 90 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/fields/test_list.py diff --git a/tests/conftest.py b/tests/conftest.py index 8ac60b21..38d6a710 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,6 +47,29 @@ def mock_app(mock_swimlane): 'required': False, 'visualize': False, 'visualizeMode': 0}, + {u'supportsMultipleOutputMappings': True, u'name': u'Numeric List', u'itemStep': 0, + u'$type': u'Core.Models.Fields.List.ListField, Core', u'required': False, u'inputType': u'numeric', + u'readOnly': False, u'fieldType': u'list', u'key': u'numeric-list', u'id': u'azvbz'}, + {u'supportsMultipleOutputMappings': True, u'name': u'Numeric List Range Limit', u'itemStep': 0, + u'minItems': 1, u'$type': u'Core.Models.Fields.List.ListField, Core', u'required': False, + u'id': u'adxvq', + u'key': u'numeric-list-range-limit', u'readOnly': False, u'itemMin': 5.0, u'fieldType': u'list', + u'maxItems': 3, u'itemMax': 10.0, u'inputType': u'numeric'}, + {u'supportsMultipleOutputMappings': True, u'name': u'Text List', u'itemStep': 0, + u'$type': u'Core.Models.Fields.List.ListField, Core', u'required': False, u'inputType': u'text', + u'readOnly': False, u'itemLengthType': u'none', u'fieldType': u'list', u'key': u'text-list', + u'id': u'ajvrh'}, {u'supportsMultipleOutputMappings': True, u'key': u'text-list-char-limit', + u'name': u'Text List Char Limit', u'itemMinLength': 3, u'itemStep': 0, + u'minItems': 1, + u'$type': u'Core.Models.Fields.List.ListField, Core', u'required': False, + u'id': u'aiprb', u'itemMaxLength': 5, u'readOnly': False, + u'itemLengthType': u'characters', u'fieldType': u'list', u'maxItems': 3, + u'inputType': u'text'}, + {u'supportsMultipleOutputMappings': True, u'key': u'text-list-word-limit', + u'name': u'Text List Word Limit', u'itemMinLength': 2, u'itemStep': 0, u'minItems': 1, + u'$type': u'Core.Models.Fields.List.ListField, Core', u'required': False, u'id': u'aeo2v', + u'itemMaxLength': 3, u'readOnly': False, u'itemLengthType': u'words', u'fieldType': u'list', + u'maxItems': 3, u'inputType': u'text'}, {'$type': 'Core.Models.Fields.TextField, Core', 'fieldType': 'text', 'helpTextType': 'none', diff --git a/tests/fields/test_basic.py b/tests/fields/test_basic.py index 50d72f43..11a8f39b 100644 --- a/tests/fields/test_basic.py +++ b/tests/fields/test_basic.py @@ -6,6 +6,7 @@ from swimlane.core.fields import resolve_field_class, _FIELD_TYPE_MAP, Field, _build_field_type_map from swimlane.core.fields.base import ReadOnly, FieldCursor +from swimlane.core.fields.list import ListField from swimlane.exceptions import ValidationError from swimlane.utils import get_recursive_subclasses @@ -76,7 +77,7 @@ class Invalid(Base): @pytest.mark.parametrize( 'field_class', - [cls for cls in _FIELD_TYPE_MAP.values() if not issubclass(cls, ReadOnly)] + [cls for cls in _FIELD_TYPE_MAP.values() if not (issubclass(cls, ReadOnly) or cls == ListField)] ) def test_all_fields_empty_value(mock_record, field_class): """Test setting fields to empty value works for all field classes""" diff --git a/tests/fields/test_list.py b/tests/fields/test_list.py new file mode 100644 index 00000000..133582c1 --- /dev/null +++ b/tests/fields/test_list.py @@ -0,0 +1,90 @@ +import pytest + +from swimlane.exceptions import ValidationError + + +def test_getattr_fallback(mock_record): + """Verify cursor __getattr__ falls back to AttributeError for unknown cursor + list methods""" + with pytest.raises(AttributeError): + getattr(mock_record['Text List'], 'unknown_method') + + +def test_set_validation(mock_record): + """Test directly setting a ListField value for validation""" + + mock_record['Text List'] = ['text'] + + with pytest.raises(ValidationError): + mock_record['Text List'] = [123] + + with pytest.raises(ValidationError): + mock_record['Text List'] = 123 + + with pytest.raises(ValidationError): + mock_record['Text List'] = 'text' + + +def test_modification_validation(mock_record): + """Test calling list methods on cursor respects validation""" + + mock_record['Text List'].append('text') + + with pytest.raises(ValidationError): + mock_record['Text List'].append(123) + + +def test_numeric_range(mock_record): + """Test item numeric range restrictions""" + + key = 'Numeric List Range Limit' + + mock_record[key] = [5] + + with pytest.raises(ValidationError): + mock_record[key] = [3] + + with pytest.raises(ValidationError): + mock_record[key] = [12] + + +def test_list_length_validation(mock_record): + """List length validation check""" + key = 'Numeric List Range Limit' + + mock_record[key] = [5, 6, 7] + + with pytest.raises(ValidationError): + mock_record[key].append(8) + + with pytest.raises(ValidationError): + mock_record[key] = [] + + +def test_item_type_validation(mock_record): + """Validate correct item type for text/numeric values""" + key = 'Numeric List Range Limit' + + with pytest.raises(ValidationError): + mock_record[key] = ['text'] + + +def test_min_max_word_validation(mock_record): + """Validate against min/max word restrictions""" + key = 'Text List Word Limit' + + with pytest.raises(ValidationError): + mock_record[key] = ['word ' * 10] + + with pytest.raises(ValidationError): + mock_record[key] = ['word'] + + +def test_min_max_char_validation(mock_record): + """Min/max characters restriction validation""" + key = 'Text List Char Limit' + + with pytest.raises(ValidationError): + mock_record[key] = ['defg', 'hijkl', 'mno pqr'] + + with pytest.raises(ValidationError): + mock_record[key] = ['']