Remove plone.jsonserializer dependency - #114
Conversation
0be6dac to
452f43b
Compare
mauritsvanrees
left a comment
There was a problem hiding this comment.
Seems okay.
But I am missing the big picture. What is wrong with plone.jsonserializer? If it is too big, then why do we replace it with the even bigger plone.restapi?
| from plone.app.textfield import RichText | ||
| from plone.app.textfield import RichTextValue | ||
| except ImportError: | ||
| self.skipTest("plone.app.textfield not available") |
There was a problem hiding this comment.
Instead of self.skipTest I would prefer marking the test with something like @unittest.skipUnless(RichText, "plone.app.testfield not available"). And then try/except that import at the top, falling back to RichText = None. Or similar with @unittest.skipIf.
There was a problem hiding this comment.
sorry, will do that in the next PR...
|
The idea is to get rid of a package which is
|
mauritsvanrees
left a comment
There was a problem hiding this comment.
Seems reasonable.
With plone.jsonserializer it was possible to register new adapters though. I don't know if anyone ever did that. Maybe someone did, to fix serializing RelationValues.
So I wonder if we can offer a simple hook for this. We can create a register_converter function. I tried it here by extracting the first converter into a separate function:
_converters = []
_marker = object()
def register_converter(method):
_converters.append(method)
def dict_converter(value, schema_or_field, default):
# dict + schema interface: convert each field value
if not isinstance(value, dict) or IField.providedBy(schema_or_field):
return default
if not value:
return {}
result = {}
for key, val in value.items():
if key not in schema_or_field:
continue
result[str(key)] = schema_compatible(val, schema_or_field[key])
return result
register_converter(dict_converter)
def schema_compatible(value, schema_or_field):
"""Convert a value to zope.schema compatible data.
Replacement for plone.jsonserializer's schema_compatible function.
"""
if value is None:
return value
for converter in _converters:
result = converter(value, schema_or_field, default=_marker)
if result is not _marker:
return result
# I extracted the next one into a separate function:
# # dict + schema interface: convert each field value
# if isinstance(value, dict) and not IField.providedBy(schema_or_field):
# if not value:
# return {}
# result = {}
# for key, val in value.items():
# if key not in schema_or_field:
# continue
# result[str(key)] = schema_compatible(val, schema_or_field[key])
# return result
I think our current converters can stay inline, and always be called first. We could iterate over the registered converters at the end, right before falling back to simply return value.
What do you think?
But this could be done as an improvement later, so let me approve your current PR.
|
Good idea. Lets do this after merging this. Maybe we can also implement more restapi logic here (lookup registered deserializers instead of converting it on. our own) |
Since
plone.jsonserializeris very old and not actively maintained we should remove the dependency here.json_compatibleis already there inplone.restapischema_compatibleis unfortunately not that easy to replace withplone.restapis schema deserializer, so I simply took over the factory fromplone.jsonserializerfor now.fixes #92