After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669158 - Readonly/Offline issues
Readonly/Offline issues
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: e-d-s backend
git master
Other Linux
: High major
: Unset
Assigned To: Jeremy Whiting
folks-maint
Depends on:
Blocks:
 
 
Reported: 2012-02-01 10:42 UTC by Alexander Larsson
Modified: 2012-07-16 22:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Down downgrade trust_level, always fully trust google contact address books (1.93 KB, patch)
2012-07-16 22:13 UTC, Jeremy Whiting
needs-work Details | Review

Description Alexander Larsson 2012-02-01 10:42:38 UTC
In order for gnome-contacts to robustly work with google contacts we need to
better handle the case where the network is offline or the non-authenticated state so we can:

a) Show a warning about this in the UI, and potentially launch the GOA auth dialog
b) Disable editing

Editing while offline is in a particularly bad state atm. It just waits 30 seconds and then we get a timeout error.

I've looked into this, and it seems that the "readonly" addressbook state in the google backend more or less is set based on the authentication, so as long as we're authenticated (via goa in my case) we're not readonly. Then the "online" state is kept separately from that. I'm not sure this is right, but we should either make offline always make the google addressbook readonly, or we should expose the online state in folks.

Another problem is that if an store is readonly then the backend marks it with PARTIAL trust. This means that the individual-aggregator stops handling linking for the store, instantly unlinking all e.g. TP personas from your primary stores contacts.
Comment 1 Travis Reitter 2012-02-01 19:45:19 UTC
(In reply to comment #0)
> In order for gnome-contacts to robustly work with google contacts we need to
> better handle the case where the network is offline or the non-authenticated
> state so we can:
> 
> a) Show a warning about this in the UI, and potentially launch the GOA auth
> dialog
> b) Disable editing
> 
> Editing while offline is in a particularly bad state atm. It just waits 30
> seconds and then we get a timeout error.
> 
> I've looked into this, and it seems that the "readonly" addressbook state in
> the google backend more or less is set based on the authentication, so as long
> as we're authenticated (via goa in my case) we're not readonly. Then the
> "online" state is kept separately from that. I'm not sure this is right, but we
> should either make offline always make the google addressbook readonly, or we
> should expose the online state in folks.

Just to further complicate this, I thought libgdata has a cache. Maybe it's only for data coming from the server, but maybe it also caches outgoing writes (and hence it could make sense to be writeable even when we're offline)? I'll let Philip clarify.

If edits can't be performed offline, we should just ensure that the PersonaStore maintains its trust = FULL while it's read-only.

If the Google address book is unavailable because we're offline, is it still possible to make edits to the local default address book? I think any UI should ensure that writability works regardless of whether you're online or not.

> Another problem is that if an store is readonly then the backend marks it with
> PARTIAL trust. This means that the individual-aggregator stops handling linking
> for the store, instantly unlinking all e.g. TP personas from your primary
> stores contacts.

As stated on IRC, I think this was on the (not-quite-right) assumptions that:

 1. PersonaStores we have full control over will always be writeable and
 2. read-only PersonaStores are from sources beyond our control (like Facebook or LDAP)

So, yes, I think part of the solution is to distinguish exactly what determines trust and not just tie it to writeability. That may prove more complicated (which is why we didn't do it). We may have to maintain specific whitelists based on the PersonaStores or Backends.
Comment 2 Alexander Larsson 2012-02-02 08:36:35 UTC
libgdata has a cache, but making changes is write-through, i.e. if offline they will always fail.

I don't see how you could support local writes in a sane way, as that would get you the whole "sync" model rather than the "link" model folks is built on. For instance, you need to merge your local changes when you go online again and handle conflicts, etc. If you don't do this but just add a new local persona and link that to the google persona then things will look very weird, some changes you make will be seen on your smartphone, some not.

As for the trust, I think in practice all EDS addressbook should have full trust. LDAP or similar global addressbooks while not directly under your control is at least manually set up to some global addressbook you share (and anyway, they only work during search anyway, so linking with them won't normally happen).
Comment 3 Philip Withnall 2012-02-03 21:23:16 UTC
(In reply to comment #1)
> Just to further complicate this, I thought libgdata has a cache. Maybe it's
> only for data coming from the server, but maybe it also caches outgoing writes
> (and hence it could make sense to be writeable even when we're offline)? I'll
> let Philip clarify.

EDS’ Google Contacts backend has a cache (libgdata doesn’t), but as Alex says, it’s write-through.

> If edits can't be performed offline, we should just ensure that the
> PersonaStore maintains its trust = FULL while it's read-only.

I see two (non-mutually-exclusive) solutions here:
 • Ensure that the trust level of a PersonaStore never drops; if it’s been FULL at some stage, there’s no reason to ever lower its trust.
 • Google address books should always have a FULL trust level, since you need to be authenticated to get any data out of them.

> If the Google address book is unavailable because we're offline, is it still
> possible to make edits to the local default address book? I think any UI should
> ensure that writability works regardless of whether you're online or not.

As Alex says, this introduces synchronisation problems and would be hell to implement properly.

> > Another problem is that if an store is readonly then the backend marks it with
> > PARTIAL trust. This means that the individual-aggregator stops handling linking
> > for the store, instantly unlinking all e.g. TP personas from your primary
> > stores contacts.
> 
> As stated on IRC, I think this was on the (not-quite-right) assumptions that:
> 
>  1. PersonaStores we have full control over will always be writeable and
>  2. read-only PersonaStores are from sources beyond our control (like Facebook
> or LDAP)
> 
> So, yes, I think part of the solution is to distinguish exactly what determines
> trust and not just tie it to writeability. That may prove more complicated
> (which is why we didn't do it). We may have to maintain specific whitelists
> based on the PersonaStores or Backends.

As above, I think those assumptions still hold (modulo address books being read-only because we’re offline). I think we can fix this case by just hard-coding Google address books to have FULL trust.

(In reply to comment #2)
> As for the trust, I think in practice all EDS addressbook should have full
> trust. LDAP or similar global addressbooks while not directly under your
> control is at least manually set up to some global addressbook you share (and
> anyway, they only work during search anyway, so linking with them won't
> normally happen).

I disagree. I can sometimes (don’t ask me how) get to browse my university’s LDAP server, which has thousands of vCards under other people’s control. If I can browse it, folks can link on it, and thus I don’t want to give it FULL trust.

(Sorry for the slow reply; it’s been a hectic few days.)
Comment 4 Travis Reitter 2012-02-06 16:39:38 UTC
(In reply to comment #3)

> > If edits can't be performed offline, we should just ensure that the
> > PersonaStore maintains its trust = FULL while it's read-only.
> 
> I see two (non-mutually-exclusive) solutions here:
>  • Ensure that the trust level of a PersonaStore never drops; if it’s been FULL
> at some stage, there’s no reason to ever lower its trust.
>  • Google address books should always have a FULL trust level, since you need
> to be authenticated to get any data out of them.

Both sound good to me.

> > If the Google address book is unavailable because we're offline, is it still
> > possible to make edits to the local default address book? I think any UI should
> > ensure that writability works regardless of whether you're online or not.
> 
> As Alex says, this introduces synchronisation problems and would be hell to
> implement properly.

Just to clarify, I mean that you should be able to edit an Individual at any given moment. If you want to add a field and the only personas attached to an Individual are read-only, we should attach a new EDS contact with the new field (and the appropriate linking fields so the Individual will be re-constructed in the same way on subsequent runs).

There are probably some assumptions we need to clean this up (eg, the trust issues raised above), but my phone lets me make edits at any time, and every time I've done so when offline/without a network, I've realized how frustrated I'd be if I had to wait.

It uses a similar scheme to what I've described above. The biggest downside is that it sometimes tends to duplicate information (but I think we could minimize that problem).
Comment 5 Alexander Larsson 2012-02-07 14:05:47 UTC
I don't think that is right. You don't want to edit a google contact, and then randomly (without telling you this) some edits (depending on if you're online or not) gets put in a local contact that you won't see later from your android phone.

To get full support for using google contacts I think we have to bite the bullet and store local modifications in the offline cache and apply then when we connect.

Its not a full sync though, because in practice we're gonna be online pretty often, so we merge often, which means the merge algorithm can be pretty stupid.
Comment 6 Philip Withnall 2012-02-07 15:36:38 UTC
(In reply to comment #5)
*snip*
> Its not a full sync though, because in practice we're gonna be online pretty
> often, so we merge often, which means the merge algorithm can be pretty stupid.

If this were to be implemented, I guess it should be in EDS rather than folks or GNOME Contacts. I wonder if manual merging might be better than automatic merging, in the case that the server copy of a vCard has been modified more recently than the version the local copy was based on.
Comment 7 Alexander Larsson 2012-02-28 11:21:10 UTC
Yeah, i agree that this should go inside eds.
This somewhat limits the complexity of the sync operation as we're not syncing between two different kinds of systems. Also we expect to be online often, and addressbooks are primary read-only so the two should not diverge too much.

Android does offline modification like this, but I'm not sure what happens on a merge conflict (i.e. change from both sides), i know I got a notification warning once during sync when i had deleted several items on the phone offline.
Comment 8 Philip Withnall 2012-03-24 23:13:24 UTC
As Alex has suggested, the EDS offline-modification-sync stuff has been added to GNOME's GSoC ideas page for this year (https://live.gnome.org/SummerOfCode2012/Ideas) so hopefully it'll get done by a student over the summer.

With that taken care of, what else remains to be done in this bug?
Comment 9 Jeremy Whiting 2012-07-06 16:04:40 UTC
Alexander,

Is there anything that needs to be done in folks for this bug? Or is it only an improvement for eds itself that needs to be made?
Comment 10 Philip Withnall 2012-07-06 18:08:12 UTC
(In reply to comment #9)
> Is there anything that needs to be done in folks for this bug? Or is it only an
> improvement for eds itself that needs to be made?

I think the following still needs doing, which should be fairly simple (see the quote below). Apart from that, I think it’s just the EDS stuff.

(In reply to comment #3)
> I see two (non-mutually-exclusive) solutions here:
>  • Ensure that the trust level of a PersonaStore never drops; if it’s been FULL
> at some stage, there’s no reason to ever lower its trust.
>  • Google address books should always have a FULL trust level, since you need
> to be authenticated to get any data out of them.
Comment 11 Jeremy Whiting 2012-07-16 19:18:57 UTC
Philip,

For the first item do we want to not enable trust level to drop from Partial to None? or just disallow dropping from Full to Partial?  This should be pretty simple to add at the folks/persona-store.vala level in the trust_level set method either way.

For the second item, is there an easy way to tell if an address book is a Google one?  Some flag on eds persona stores?
Comment 12 Philip Withnall 2012-07-16 20:17:06 UTC
(In reply to comment #11)
> Philip,
> 
> For the first item do we want to not enable trust level to drop from Partial to
> None? or just disallow dropping from Full to Partial?  This should be pretty
> simple to add at the folks/persona-store.vala level in the trust_level set
> method either way.

Don’t allow any drops in trust level (full → partial or partial → none), I think.

> For the second item, is there an easy way to tell if an address book is a
> Google one?  Some flag on eds persona stores?

Edsf.PersonaStore._is_google_contacts_address_book().
Comment 13 Jeremy Whiting 2012-07-16 22:13:37 UTC
Created attachment 218953 [details] [review]
Down downgrade trust_level, always fully trust google contact address books
Comment 14 Philip Withnall 2012-07-16 22:21:33 UTC
Review of attachment 218953 [details] [review]:

::: backends/eds/lib/edsf-persona-store.vala
@@ +2372,3 @@
         }
 
+      if (((!) this)._is_google_contacts_address_book())

The “(!)” shouldn’t be necessary for ‘this’. Missing a space before ‘()’.

This block should probably have a comment pointing to the bug and explaining why we trust Google address books.

::: folks/persona-store.vala
@@ +602,3 @@
+      get { return this._trust_level; }
+      
+      set {

The opening brace should be on a new line, and the whole block should probably have a comment referencing this bug.

@@ +605,3 @@
+        if (value > trust_level)
+          {
+            this._trust_level = value;

Missing a GObject.notify_property() call.
Comment 15 Jeremy Whiting 2012-07-16 22:31:13 UTC
Updated the commits as per your review and pushed to master.  Also added this bug number to NEWS file.