Skip to content

Commit

Permalink
Fix a TypeError when field names arent native strings.
Browse files Browse the repository at this point in the history
  • Loading branch information
jamadden committed Jul 31, 2018
1 parent 5732c74 commit 5585c0c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 7 deletions.
3 changes: 2 additions & 1 deletion CHANGES.rst
Expand Up @@ -6,7 +6,8 @@
1.0.0a7 (unreleased)
====================

- Nothing changed yet.
- Avoid a ``TypeError`` from ``validate_named_field_value`` when
external objects have unicode keys.


1.0.0a6 (2018-07-31)
Expand Down
4 changes: 3 additions & 1 deletion src/nti/externalization/internalization/_fields.pxd
Expand Up @@ -40,7 +40,7 @@ cdef _FieldProperty_orig_set
@cython.freelist(1000)
cdef class SetattrSet(object):
cdef ext_self
cdef str field_name
cdef str field_name # setattr() wants native strings
cdef value

@cython.final
Expand All @@ -58,5 +58,7 @@ cdef _handle_SchemaNotProvided(field_name, field, value)
cdef _handle_WrongType(field_name, field, value)
cdef _handle_WrongContainedType(field_name, field, value)

cdef str _as_native_str(s)

cpdef validate_field_value(self, field_name, field, value)
cpdef validate_named_field_value(self, iface, field_name, value)
14 changes: 10 additions & 4 deletions src/nti/externalization/internalization/fields.py
Expand Up @@ -273,7 +273,7 @@ def validate_field_value(self, field_name, field, value):
set. If the value needs to be adapted to the schema type for validation to work,
this method will attempt that.
:param string field_name: The name of the field we are setting. This
:param str field_name: The name of the field we are setting. This
implementation currently only uses this for informative purposes.
:param field: The schema field to use to validate (and set) the value.
:type field: :class:`zope.schema.interfaces.IField`
Expand All @@ -283,7 +283,6 @@ def validate_field_value(self, field_name, field, value):
:return: A callable of no arguments to call to actually set the value (necessary
in case the value had to be adapted).
"""
__traceback_info__ = field_name, value
field = field.bind(self)
try:
if isinstance(value, text_type) and IFromUnicode_providedBy(field):
Expand All @@ -305,7 +304,7 @@ def validate_field_value(self, field_name, field, value):
if value is not None:
# First time through we get to set it, but we must bypass
# the field
_do_set = SetattrSet(self, str(field_name), value)
_do_set = SetattrSet(self, _as_native_str(field_name), value)
else:
_do_set = noop
else:
Expand All @@ -320,19 +319,26 @@ def validate_named_field_value(self, iface, field_name, value):
validate that the given ``value`` is appropriate to set. See :func:`validate_field_value`
for details.
:param string field_name: The name of a field contained in
:param str field_name: The name of a field contained in
`iface`. May name a regular :class:`zope.interface.Attribute`,
or a :class:`zope.schema.interfaces.IField`; if the latter,
extra validation will be possible.
:return: A callable of no arguments to call to actually set the value.
"""
field_name = _as_native_str(field_name)
field = iface[field_name]
if IField_providedBy(field):
return validate_field_value(self, field_name, field, value)

return SetattrSet(self, field_name, value)


def _as_native_str(s):
if isinstance(s, str):
return s
return s.encode('ascii')


from nti.externalization._compat import import_c_accel # pylint:disable=wrong-import-position,wrong-import-order
import_c_accel(globals(), 'nti.externalization.internalization._fields')
52 changes: 51 additions & 1 deletion src/nti/externalization/internalization/tests/test_fields.py
Expand Up @@ -24,8 +24,9 @@
from hamcrest import same_instance

from nti.externalization.internalization.fields import validate_named_field_value
from nti.externalization.internalization.fields import validate_field_value

# pylint:disable=inherit-non-class
# pylint:disable=inherit-non-class,blacklisted-name

class IThing(interface.Interface):
pass
Expand Down Expand Up @@ -93,3 +94,52 @@ def test_before_object_assigned_event_not_fired_invalid_value(self):

events = eventtesting.getEvents(IFieldUpdatedEvent)
assert_that(events, has_length(0))


class TestValidateFieldValue(unittest.TestCase):

def _callFUT(self, ext_self, iface, field_name, value):
return validate_field_value(ext_self, field_name, iface[field_name], value)

def test_unicode_field_name_field_non_property(self):
from zope.schema import TextLine
class IFoo(interface.Interface):
field = TextLine(title=u'text')

class Foo(object):
pass

foo = Foo()
self._callFUT(foo, IFoo, u'field', u'text')()
assert_that(foo, has_attr('field', u'text'))

def test_unicode_field_name_field_non_property_readonly(self):
from zope.schema import TextLine
class IFoo(interface.Interface):
field = TextLine(title=u'text', readonly=True)
field.setTaggedValue('_ext_allow_initial_set', True)

class Foo(object):
pass

foo = Foo()
self._callFUT(foo, IFoo, u'field', u'text')()
assert_that(foo, has_attr('field', u'text'))


class TestValidateNamedFieldValue(TestValidateFieldValue):

def _callFUT(self, ext_self, iface, field_name, value):
return validate_named_field_value(ext_self, iface, field_name, value)

def test_unicode_field_name_non_field(self):
class IFoo(interface.Interface):
field = interface.Attribute("An attribute")

class Foo(object):
pass

foo = Foo()
self._callFUT(foo, IFoo, u'field', 42)()

assert_that(foo, has_attr('field', 42))

0 comments on commit 5585c0c

Please sign in to comment.