[Sugar-devel] [PATCH sugar] Journal detail view: don't choke on invalid 'keep' property

[Simon Schampijer]

Hi Sascha,

thanks for the patch.

On 02/11/11 23:28, Sascha Silbe wrote:
> Properties of data store entries can get corrupted, e.g. due to low level
> crashes or running out of battery (see OLPC#11372 [1] for a real-life
> example). In addition any activity can - accidentally or on purpose - write
> data store entries with arbitrary metadata.
>
> By comparing the 'keep' property as a string we can avoid the ValueError that
> might happen when trying to convert the property value to an integer.
>
> [1] https://dev.laptop.org/ticket/11372
>
> Reported-by: Gary Martin<garycmartin at googlemail.com>
> Signed-off-by: Sascha Silbe<silbe at activitycentral.com>
> ---
>
> Tested using the data store attached to the ticket referenced above.
>
>   src/jarabe/journal/expandedentry.py |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/jarabe/journal/expandedentry.py b/src/jarabe/journal/expandedentry.py
> index 495c7e9..caf6003 100644
> --- a/src/jarabe/journal/expandedentry.py
> +++ b/src/jarabe/journal/expandedentry.py
> @@ -147,7 +147,7 @@ class ExpandedEntry(hippo.CanvasBox):
>           self._metadata = metadata
>           self._object_id = object_id
>
> -        self._keep_icon.keep = (int(metadata.get('keep', 0)) == 1)
> +        self._keep_icon.keep = (str(metadata.get('keep', 0)) == '1')
>
>           self._icon = self._create_icon()
>           self._icon_box.clear()
> --
> 1.7.6.3

There is another place in the code where we retrieve the 'keep' 
property, I would suggest something like:

diff --git a/src/jarabe/journal/expandedentry.py 
b/src/jarabe/journal/expandedentry.py
index 4e99dc2..3617cbb 100644
--- a/src/jarabe/journal/expandedentry.py
+++ b/src/jarabe/journal/expandedentry.py
@@ -144,7 +144,7 @@ class ExpandedEntry(hippo.CanvasBox):
              return
          self._metadata = metadata

-        self._keep_icon.keep = (int(metadata.get('keep', 0)) == 1)
+        self._keep_icon.keep = self.get_keep()

          self._icon = self._create_icon()
          self._icon_box.clear()
@@ -419,7 +419,8 @@ class ExpandedEntry(hippo.CanvasBox):
          self._update_title_sid = None

      def get_keep(self):
-        return int(self._metadata.get('keep', 0)) == 1
+        # We can not rely on the keep metadata to be int, SL 1591
+        return str(self._metadata.get('keep', 0)) == '1'

      def _keep_icon_activated_cb(self, keep_icon):
          if self.get_keep():


Afaik, this patch is not needed to fix [1]. I think it would be better 
to note [2][3] as tickets it fixes. As well, we should check if there 
are other cases that fail and fix those, too like a sensible approach 
noted in [3].

[1] https://dev.laptop.org/ticket/11372
[2] http://bugs.sugarlabs.org/ticket/1591	
[3] http://bugs.sugarlabs.org/ticket/1561