This fixes #106 (Stop health check when wrong password)

[llvasconcellos]

Stop health check when wrong password.

[capnfabs]

Nice. It might be better to call stop() or stopImpl() instead of quitting the Looper directly though.

[dancunningham]

It will still show the error message right? And if you change the password or reapply settings will it resume checking?

[llvasconcellos]

@dancunningham yes!

[capnfabs]

Is there a reason for defining this in the build.gradle instead of in the code where it was defined before?

[capnfabs]

Thanks for looking into this Leonardo!

As I mentioned to you in chat, I don't think the approach of disabling the sync account and re-enabling it again later is a good idea. Here's why:

• We'll still make enough requests that it would be possible to generate the auth errors. Other sources that generate requests are:

  • User syncing, which can happen independently of the actual sync process
  • Analytics / server-side logging, which happens on some button clicks and on every activity change

  Additionally, all health checks are restarted when a user switches to any activity except for the settings activity, including the BuendiaApiHealthCheck.

• The logic for whether requests should be started or stopped is now split across multiple places:

  • BuendiaApiHealthCheck
  • SyncAdapter, which checks for an AuthFailureError as the cause of the exception a few layers deep.
  • The settings code, which is responsible for restarting everything when settings change just in case it works.

Instead, I would advise overriding BuendiaApiHealthCheck#isApiUnavailable() to return a value depending on auth failure, which influences HealthMonitor#isApiUnavailable(). Then, you can make the other request sources check that value before querying the API. This means all authorisation checking logic is centralised in that single health check, which reduces the risk of bugs, and makes it easier to modify in the future.

[llvasconcellos]

Ok @capnfabs I'll look at this after this sprint. I'll leave the PR open though.

[llvasconcellos]

I'm still leaving it open...

[llvasconcellos]

I went for a different approach following @capnfabs advice.
#275