Missing previews in Journal, and UnicodeWarning in log

[quozl]

/usr/lib/python2.7/dist-packages/sugar3/datastore/datastore.py:113: \
UnicodeWarning: Unicode unequal comparison failed to convert both \
arguments to Unicode - interpreting them as being unequal
  if key not in self._properties or self._properties[key] != value:

[quozl]

Also occurs in Clock activity.

/usr/lib/python2.7/dist-packages/sugar3/datastore/datastore.py:113: \
UnicodeWarning: Unicode unequal comparison failed to convert both \
arguments to Unicode - interpreting them as being unequal
  if key not in self._properties or self._properties[key] != value:

Does not occur in Read if run in Python 3.

[quozl]

Reproduced by running Read in Python 2. Instrumented code with tracebacks. The warning occurs when the preview metadata is changed for the third time. The sequence of changes to preview are;

1566952256.173391 ERROR root: __setitem__ key='preview' value=''
    in `_initialise_journal_object`, file src/sugar3/actiivty/activity.py
1566952257.608579 ERROR root: __setitem__ key='preview' value=dbus.ByteArray('\x89PNG ...
    in `save`, file src/sugar3/actiivty/activity.py
1566952257.760859 ERROR root: __setitem__ key=dbus.String(u'preview') value=dbus.String(u'\\\\x89PNG\\\\r ...
    in `__object_updated_cb`, file src/sugar3/datastore/datastore.py

The warning happens when the updated signal is delivered after the activity has updated the preview metadata. The updated signal is handled by setting the data again, which is usually harmless because the data came from the activity itself.

The warning happens because the binary data in preview could not be converted from dbus.String; with a UnicodeDecodeError, because the data contains byte sequences which are not Unicode.

All other forms of metadata are textual. Only preview triggers this.

[quozl]

The binary metadata for the preview returned by get_properties has a type of dbus.String instead of the expected type dbus.ByteArray. Using dbus-monitor shows that two items of non-string metadata (timestamp and preview) are sent to the datastore, but when they come back from the datastore they have changed type to string. The datastore has stopped being type-transparent.

Looks like this happened in 6f4fcae ("Port to Python 3").

@pro-panda, it was about this time last year, but can you look at those lines and tell me why the metadata is being forced to string?

[quozl]

May be the cause of missing previews in Journal. Detail view shows traceback;

Traceback (most recent call last):
  File "/usr/lib/python3.7/dist-packages/jarabe/journal/detailview.py", line 63, in _update_view
    self._expanded_entry.set_metadata(self._metadata)
  File "/usr/lib/python3.7/dist-packages/jarabe/journal/expandedentry.py", line 336, in set_metadata
    self._preview_box.add(self._create_preview())
  File "/usr/lib/python3.7/dist-packages/jarabe/journal/expandedentry.py", line 378, in _create_preview
    pixbuf = get_preview_pixbuf(metadata.get('preview', ''))
  File "/usr/lib/python3.7/dist-packages/sugar3/graphics/objectchooser.py", line 87, in get_preview_pixbuf
    png_file = six.StringIO(preview_data)
TypeError: initial_value must be str or None, not bytes

[mdengler]

That 6f4fcae ("Port to Python 3") commit is bad in that respect. The metadata was stored as bytes before, and there is a big comment about how it has to be stored as bytes and how, coincidentally, str(bytes_data) didn't interfere, but would in the future. The future has come, and the current port/code is blindly converting everything to unicode. It should not be.

[quozl]

Thanks. I agree. I'll look at it in about four hours if nobody else does.

[rhl-bthr]

From memory, All values are converted to bytes in

sugar-datastore/src/carquinyol/metadatareader.c

Line 83 in 6f4fcae

PyObject *args = Py_BuildValue ("(y#)", value_buf, file_size);

and ultimately returning strings of format "b'<string>'"

[mdengler]

From memory, All values are converted to bytes in
[metadatareader.add_property]
and ultimately returning strings of format "b'<string>'"

Seems like there should be no strings (or StringIO) in the python code, then, and rather bytes (or BytesIO).

[martin-langhoff]

I agree with @mdengler 's comments. Py2 string handling was /mostly/ 8-bit clean, and Py3 string handling is most definitely not. IMHO datastore should not ever try to interpret them -- I'd remove all casts to string.

Unfortunately I don't have a test env handy to run Sugar components (and I'm about to have to dodge a hurricane). I do have some recent/current Fedora systems.

[quozl]

Think I've fixed it.

#18
sugarlabs/sugar-toolkit-gtk3#428