Field serialization adapters

[jone]

Closes #38

As discussed in #38 this PR introduces field serializer adapters for dexterity based content types in order to make the serialization better customizable and extendable and provide a dependency injection entry point.

Closes #38, #42, #4

IFieldSerializer

The field serializer adapter adapts the dexterity field, the context and the request. It's duty is to convert the value of the adapted field on the current context to a data structure which is supported by the JSON module.
This provides the entry point for customizing how a certain field or a type of fields should be represented in the JSON.

IJsonCompatible

The json compatible adapter converts values. It does not know anything about fields and has no context. The idea here is that is possible to recursively convert data. This is especially important for supporting list fields, object fields, data grid fields etc.

Discussion

I tried to make as few changes as possible but do a fully working implementation. I'd like to continue with supporting relations (#43), using the IJsonCompatible converters for Plone site representation too, and more.

Here some notes about this PR:

• I switch to using dexterity's iterSchemata and removed our custom code. See Question about iterating dexterity fields #42, Use Dexterity Schema and Behaviors to get the Object Schema. #4
  • non-field values, such as meta_type are no longer included
  • some fields such as effective did not use the field value but direct attribute access, which had a different result, which is fixed now
• I have moved image scaling mechanism to a field serializer and moved everything into a dict, acting as a value for this field. This seems more consistent to me.
• I could eliminate the custom field- and image-content-converters (ISerializeToJson) and use the general dexterity converter as logic was moved to field serializers.
• Richtext-Values are represented with only the output. We might change that with frames but I think that deserves a separate issue / PR. Discussion needed.

[jone]

How should richtext field be represented in the JSON output?

I see three possibilites:

1. Provide all infos of the richtext object, as above, so that the client knows which mimetype / encoding the value has.
2. Just return the raw output, so that we do not have a nested dict but a string value as with textline fields (e.g. title)
3. Same as 2 but convert the string to the outputMimeType (transforms). use .output ; same as the current implemntation in serializer.py.

What do you think is best?

[lukasgraf]

I don't think 2. really is an option. The client would be lacking necessary information it needs to deal with the data.

I'm leaning towards 1.

[jone]

I have currently implemented 1., but I think that 3. is better.
Example:

# 1. all infos:
{
 "text": {
   "encoding": "utf-8", 
   "mimeType": "text/plain", 
   "outputMimeType": "text/html", 
   "raw": "If you're seeing this instead of the web site you were expecting, the owner of this web site has just installed Plone. Do not contact the Plone Team or the Plone mailing lists about this."
  }
}

# 3. Converted to configured output type (`text/html` in this case):
{
  "text": "<p>If you're seeing this instead of the web site you were expecting, the owner of this web site has just installed Plone. Do not contact the Plone Team or the Plone mailing lists about this.</p>",
}

As an API client I don't care how the data is stored, but I want to have the data as it is meant to be used (which is text/html as declared in outputMimeType).
A client also may not have a transformation engine to convert from text/plain to text/html, which may be necessary in the above example in order to have the correct output mimetype.

I think it is better to either just return the transformed value (3.) or extend the current dict (1.) with the converted value.
We probably want to hide the internal data structure (raw value) from the public since it is not relevant, we want them to use the value converted to the output mime type, right?

[jone]

I think we should go with 3., which is:

{
  "text": "<p>If you're seeing this instead of the web site you were expecting, the owner of this web site has just installed Plone. Do not contact the Plone Team or the Plone mailing lists about this.</p>",
}

Discussion:

• encoding is not necessary, since the whole document is encoded with the same encoding (utf8).
• raw is internal, the client does not nedd to know what we store
• mimeType is internal, the client does not need to know how we store it
• outputMimeType should be defined in the schema (e.g. text is always text/html)
• we therefore only require the actual transformed value.

[davisagli]

If a client wants to fetch the value so it can write an updated value, then it needs the raw data and the stored mimetype, not the transformed data. I agree using the transformed data and outputMimeType is a good default, but we might need an option or different API to get values for editing instead (like the distinction between get and getRaw in Archetypes)

[tisto]

I would prefer to keep things simple and hide the internal encoding as an implementation detail for now. If a client really needs a different representation it should ask for a different format through the Accept HTTP header (even though we are talking about an embedded format here). We will have an "editing" frame that can provide more information if necessary. This is just the "view/content" frame, right?

[davisagli]

Okay, if there's a separate frame for editing that's perfect.

David Glick
(mobile)

On Nov 11, 2015, at 8:51 AM, Timo Stollenwerk notifications@github.com wrote:

In src/plone/restapi/tests/test_field_serializer.py:

•    folder = createContentInContainer(
  

•        self.portal, u'Folder',
  

•        effective_date=datetime(2017, 10, 22, 19, 45, 1))
  

•    value = self.serialize(folder, u'effective')
  

•    self.assertEquals(u'2017-10-22T19:45:01', value)
  

•    self.assertEquals(value, json.loads(json.dumps(value)))
  
• def test_richtext(self):

•    document = createContentInContainer(
  

•        self.portal, u'Document',
  

•        text=IRichText['text'].fromUnicode(u'<p>Some Text</p>'))
  

•    value = self.serialize(document, u'text')
  

•    self.assertEquals({'raw': u'<p>Some Text</p>',
  

•                       'mimeType': 'text/html',
  

•                       'outputMimeType': 'text/x-html-safe',
  

•                       'encoding': 'utf-8'}, value)
  

  I would prefer to keep things simple and hide the internal encoding as an implementation detail for now. If a client really needs a different representation it should ask for a different format through the Accept HTTP header (even though we are talking about an embedded format here). We will have an "editing" frame that can provide more information if necessary. This is just the "view/content" frame, right?

—
Reply to this email directly or view it on GitHub.

[jone]

Builds are broken because of a version conflict for setuptools, see #40

[jone]

I've finished this PR about field serializer.
@tisto @bloodbare @lukasgraf @buchi I'd appreciate your feedback!

[lukasgraf]

I would probably use the repr() of the value here:

'No converter for making'
' {0!r} ({1}) JSON compatible.'.format(value, type(value)))

[lukasgraf]

Awesome work! 🎉

Besides the thing with the first adapter lookup in lookup_field_serializer() these are really just minor points.

Other than this, looks good to me! 👍

[lukasgraf]

@jone could you also summarize these changes in a changelog entry?

[jone]

There is not yet a release, should we really write changelogs for the initial release? There is a git log 😉

[lukasgraf]

Ah, I got plone.restapi mixed up with plone.rest, which already has an (alpha) release - 👍 for no CL from me 😉

[tisto]

We don't need a changelog before we have an initial release...

[landscape-bot]

Repository health increased by 1% when pulling aad795a on jone-field-serializers into e824b94 on master.

• No new problems were introduced.
• 2 problems were fixed (including 0 errors and 2 code smells).

[jone]

@lukasgraf Thanks for the feedback!
I've updated most things or commented in the discussion 😜

[landscape-bot]

Repository health increased by 1% when pulling 245bb73 on jone-field-serializers into e824b94 on master.

• No new problems were introduced.
• 2 problems were fixed (including 0 errors and 2 code smells).

[lukasgraf]

If there are no objections, I'll be merging this one tomorrow.

[tisto]

@jone @lukasgraf I briefly looked over the pr and it looks good to me. Good works guys!!!

I'm ok with merging, we could squash the commits before we do so if you have time...

[jone]

👍 I've squashed everything into one commit.

[landscape-bot]

Repository health increased by 1% when pulling db0732f on jone-field-serializers into e824b94 on master.

• No new problems were introduced.
• 2 problems were fixed (including 0 errors and 2 code smells).