Add SyncAdapter support

[zsiegel]

Pull request for issue discussed in #52

[emilsjolander]

Something about this should be added in the README as well. Also i am unsure about including this in the sample application just because it increases the complexity of the sample by quite a bit without showing of a lot of sprinkles specific stuff.

Otherwise i think it is a great solution to the issue! :+1:

[zsiegel]

Ok so I have refactored things a bit.

You questioned why we needed to keep references to the ContentObservers. After re-thinking this a bit we don't and shouldn't be. The Sprinkles instance the user creates will now simple return those ContentObserver objects and they will manage them.

One side effect of this was I had to make the Utils class public and then scoped the getNotificationURI() method as public as well. We could just expose this on the Model class instead as a static method. either way would be fine with me.

I have also updated the README.md file with an example of how to use the observers and have cleaned up the sample code.

[zsiegel]

I have refactored how the user observes their models and updated the README accordingly.

The user will now create their own SprinklesContentObserver as @emilsjolander suggested and then they can call register() and unregister() as they see fit.

[zsiegel]

Updated the README. I left the super.***() calls for brevity but I can see how people might just copy and paste the methods and get screwed up.

[emilsjolander]

i like it :+1: Make sure to update the tests and i will get to merging this and releasing it sometime tomorrow!

[zsiegel]

Im not really sure of a good way to test the register() and unregister() methods.

Unfortunately the the ContentResolver has no API for getting the currently registered observers.

[emilsjolander]

Yeah i know, i don't have a great idea. One way would be to check that the ContentResolver is notified when saving a model.

[zsiegel]

I think I have an idea using mocks. Let me test a few things out.

Zachary Siegel zsiegel87@gmail.com On Feb 26, 2014 11:59 AM, "Emil Sjölander" notifications@github.com wrote:

Yeah i know, i don't have a great idea. One way would be to check that the ContentResolver is notified when saving a model.

Reply to this email directly or view it on GitHubhttps://github.com/emilsjolander/sprinkles/pull/54#issuecomment-36155425 .

[zsiegel]

So my thought was to mock the ContentObserver object then do a Model.save() and verify that the onChange() method was called.

Without having a SyncAdapter, ContentProvider, and AccountAuthenticator wired into the manifest that Robolectric reads during the tests the SprinklesContentObserver will never get its onChange() methods called.

Therefore making even testing the register() and unregister() methods essentially moot.

Any other ideas?

[emilsjolander]

Nope, i don't have any great ideas. Could we maybe just Mock a ContentResolver and assert that calling register() and unregister() on the SprinklesContentObserver actually delegates the method to the ContentResolver? I don't know how much this gives us though :/

[zsiegel]

All of my attempts at trying to mock/spy the Sprinkles object with Mockito fail because of the fact that the init method is static and Mockito does not play nice with these kind of objects.

I think we are just going to have to live with this unless we want to investigate using something like PowerMock which can allegedly handle situations like this.

[emilsjolander]

Yeah i'm ok with skipping the tests on this if we open a separate issue for it so that we have it documented and on the todo list :+1: I can create that issue after i have merge these changes.

[emilsjolander]

I'm thinking that people may want to attach other content observers that do not have to do with a sync adapter. So how about we implement it as such that SprinklesContentObserver is just a wrapper around a ContentObserver supplied by the user? if you make this change don't forget to also update the readme :)

Good job otherwise, i like it :+1:

[zsiegel]

Ok so I updated the SprinklesContentObserver so that it could also take a ContentObserver supplied by the user.

The SprinklesContentObserver then just forwards those method calls to the user supplied ContentObserver. I also added some testing using Mockito to verify that those calls are being forwarded to the supplied observer.

updated the README and fixed the formatting as well. Intellij must have done some funky things on a previous check in.

thanks.

[emilsjolander]

Do you have any good reason to a constructor specific for notifying a SyncAdapter? I would think that it's better not to make those kind of assumptions in this library and just let the user create their own ContentObserver which notifies the correct SyncAdapter.

[zsiegel]

The user needs a handle to the content resolver used by Sprinkles though right? Doesn't that mean we would need to expose that somehow?

Im thinking of this looking at this line in the SprinklesContentObserver

Sprinkles.sInstance.mContext.getContentResolver().registerContentObserver(Utils.getNotificationUri(clazz), notifyDescendants, this);

[emilsjolander]

Why would they? They would just wrap the sync functionality in a ContentObserver like this

class AccountSyncObserver extends ContentObserver {

    Account mAccount;
    String mAuthority;

    AccountSyncObserver(Account account, String authority) {
        this.mAccount = account;
        this.mAuthority = authority;
    }

    @Override
    public void onChange(boolean selfChange) {
        super.onChange(selfChange);
        sync();
    }

    private void sync() {
        Bundle extras = new Bundle();
        extras.putBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, true);
        ContentResolver.requestSync(mAccount, mAuthority, extras);
    }

}

And than just wrap is in a SprinklesContentObserver like this

SprinklesContentObserver observer = new SprinklesContentObserver(new AccountSyncObserver(acc, auth))

Or am i missing something?

[zsiegel]

Ahh ok now I see what you are getting at. This makes sense to me. Ill update the code and wording of the README to reflect this.

Essentially we are providing a new hook into the database. Which if the user chooses allows them the ability to launch any code (such as a SyncAdapter)

[emilsjolander]

I fixed a test that was failing and refactor some small things but nothing much. Good work, it took a while but we got a great API out of it. Will push to maven central now :+1: