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 629081 - Add API to allow specific backends to be disabled
Add API to allow specific backends to be disabled
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: libfolks
git master
Other Linux
: Normal enhancement
: gnome-3.0
Assigned To: folks-maint
folks-maint
Depends on: 628970
Blocks: 628868 629078 629862 630433
 
 
Reported: 2010-09-08 15:53 UTC by Philip Withnall
Modified: 2010-11-04 00:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Squashed diff of the 629081-disable-backends branch (7.57 KB, patch)
2010-09-16 15:58 UTC, Philip Withnall
none Details | Review
Squashed diff of the 629081-disable-backends branch (updated) (8.29 KB, patch)
2010-09-16 17:02 UTC, Philip Withnall
none Details | Review
Squashed diff of the 629081-disable-backends branch (updated again) (8.32 KB, patch)
2010-09-17 09:53 UTC, Philip Withnall
none Details | Review

Description Philip Withnall 2010-09-08 15:53:59 UTC
This is necessary so that, for example, once a Pidgin backend is added (bug #628868), I can disable it once I've imported the Pidgin meta-contact data.

This API would best live in BackendStore, storing the list of blacklisted backends in a key file in ~/.local/share/folks.
Comment 1 Philip Withnall 2010-09-13 13:58:53 UTC
(Mass changing milestones; please search for this phrase to delete the bug spam.)
Comment 2 Philip Withnall 2010-09-16 15:58:13 UTC
Created attachment 170421 [details] [review]
Squashed diff of the 629081-disable-backends branch

http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629081-disable-backends

Here's a patch which adds support for a key file for the BackendStore, specifying options for each backend (currently only “enabled”). This doesn't attempt to support dynamically adding and removing backends at runtime; only changing their enabled status in the key file.

The next step is to add support for runtime changes in the set of backends, but I'd like the work in bug #629331 to land before starting on that, as that branch goes some way towards solving the problem.
Comment 3 Philip Withnall 2010-09-16 17:02:39 UTC
Created attachment 170428 [details] [review]
Squashed diff of the 629081-disable-backends branch (updated) 

http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629081-disable-backends

When writing the key file tests (bug #629862), I found that (obviously) the key file has to be loaded before BackendStore.disable_backend() can be called, which necessitates splitting it out into a BackendStore.prepare() method. That's what this updated branch does.
Comment 4 Philip Withnall 2010-09-17 09:53:52 UTC
Created attachment 170471 [details] [review]
Squashed diff of the 629081-disable-backends branch (updated again)

http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629081-disable-backends

This one actually works.
Comment 5 Travis Reitter 2010-09-21 15:44:45 UTC
This generally looks good - though, as discussed on IRC, we'll need to make load_backends() re-load the now-enabled backends and unload the now-disabled backends, to allow "online" backend loading and unloading.

This will affect the API for clients as well, so I'm going to make this block bug #629078
Comment 6 Travis Reitter 2010-09-21 16:54:55 UTC
bug #629331 comment #5 points out that we can't simply have BackendStore.load_backends() return early if it's already been called (by the time we merge this branch).

So this point will need to be addressed in the final branch, before it's merged.
Comment 7 Travis Reitter 2010-09-23 17:04:24 UTC
At least for the purposes of testing, we'll need the ability to enable exactly 1 specific backend (as opposed to disabling only the backends we're aware of).

See bug #630433.
Comment 8 Philip Withnall 2010-09-23 23:06:50 UTC
(In reply to comment #7)
> At least for the purposes of testing, we'll need the ability to enable exactly
> 1 specific backend (as opposed to disabling only the backends we're aware of).

We should just have a utility method in the test suite library which does something like the following:

  yield backend_store.prepare ();
  yield backend_store.load_backends ();
  foreach (Backend backend in backend_store.enabled_backends)
    {
      if (backend.name != the_backend_i_want)
        yield backend_store.disable_backend (backend.name);
    }

That would be called before the tests are run.

I don't think we should expose a method on the BackendStore to enable a single backend, as it's a fairly unusual thing to want to do, and we should be encouraging applications to use personas from all backends equally.
Comment 9 Travis Reitter 2010-11-04 00:43:35 UTC
After way too much work, this has finally been merged (there were some unrelated commits in between some of these):

commit 037df05b1a069d484ff84de2aa5f86b3da259f1f
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Tue Nov 2 09:53:29 2010 -0700

    Make BackendStore.load_backends() unload disabled backends.
    
    This also allows this function to be called multiple times (previously,
    subsequent calls were ignored).
    
    Fixes bgo#629081.

 NEWS                               |    8 ++
 backends/key-file/kf-backend.vala  |   16 +++
 backends/telepathy/tp-backend.vala |   27 ++++++
 folks/backend-store.vala           |  183 +++++++++++++++++++++++-------------
 folks/backend.vala                 |   16 +++
 tests/folks/backend-loading.vala   |  131 ++++++++++++++++++++++++++
 6 files changed, 316 insertions(+), 65 deletions(-)

commit ba5b502380b2fec8e1c4f3989769c9db8fcf66a7
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Mon Nov 1 19:08:43 2010 -0700

    Add a test of backend disabling.
    
    Helps bgo#629081.

 tests/folks/backend-loading.vala |   55 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

commit d65fc45442d2ed3fe714291ffa43393e60ed47da
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Fri Oct 29 16:49:14 2010 -0700

    Add initial BackendStore test.
    
    Helps bgo#629081.

 configure.ac                     |    1 +
 tests/Makefile.am                |    1 +
 tests/folks/Makefile.am          |   47 ++++++++++++++++++++++
 tests/folks/backend-loading.vala |   79 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+), 0 deletions(-)

commit 2623a7558b31b310ea0c6436dbab75e0b9eaf3c5
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Wed Nov 3 17:02:01 2010 -0700

    Clean up the way we load modules.
    
    Helps bgo#629081.

 folks/backend-store.vala |   29 ++++++++++++++++++++---------
 1 files changed, 20 insertions(+), 9 deletions(-)

commit 1ed00cda2a890412bf301073f6daa6578e28509b
Author: Philip Withnall <philip.withnall@collabora.co.uk>
Date:   Thu Sep 16 16:32:09 2010 +0100

    Add BackendStore.[enable|disable]_backend() methods
    
    These update the BackendStore's key file and save it asynchronously, but
    don't load or unload the backends.
    
    Helps bgo#629081.

commit 0854baf3f2ce4eaeb20580fe104f6d689e3fcb7e
Author: Philip Withnall <philip.withnall@collabora.co.uk>
Date:   Thu Sep 16 15:56:42 2010 +0100

    Add support for disabling backends in the BackendStore using a key file
    
    Using a key file, ~/.local/share/folks/backends.ini, specific backends
    can now be disabled.
    
    The key file is of the format:
      [telepathy]
      enabled=true
      [key-file]
      enabled=false
    
    Helps: bgo#629081

 NEWS                     |    6 +++
 folks/backend-store.vala |  112 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletions(-)