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

[Sascha Silbe]

Excerpts from Simon Schampijer's message of 2012-01-10 14:26:46 +0100:

> There is another place in the code where we retrieve the 'keep' 
> property, I would suggest something like:
[...]
> @@ -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'

Good catch. I didn't trigger this one because I didn't try marking a
corrupted entry as favourite. The breakage isn't as bad either; instead
of failing to show the details view, it just won't mark a corrupted
entry as favourite (might even be a good thing to do on purpose ;) ).


> Afaik, this patch is not needed to fix [1].

It doesn't break the entire Journal, but it's triggered by the same
corrupted data store. Referencing the real-world case gives useful
background; I don't claim to fix the cited ticket.

> I think it would be better to note [2][3] as tickets it fixes.

Interesting, so we even had a patch [4] to fix this already (though it
may have been incomplete). Added SL#1591 [2] to the summary.

My patch doesn't fix SL#1561 [3] as I haven't done an exhaustive search
of all metadata users. While it would be useful, I don't consider it
worth the enormous effort. A better approach would be to construct a
data store with various variants of bad data. That would be
automatically testable, rather than requiring a regular code audit.


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

Sascha

[4] https://bugs.sugarlabs.org/attachment/ticket/1591/keep.patch
-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20120117/7cc9cd0d/attachment-0001.pgp>