instacart / truetime-android

Android NTP time library. Get the true current time impervious to device clock time changes
https://tech.instacart.com/truetime/
Apache License 2.0
1.42k stars 194 forks source link

TrueTime caching not working #64

Closed IonutNegru87 closed 6 years ago

IonutNegru87 commented 7 years ago

I'm initializing the library in my application onCreate(). I enabled the cache via: .withSharedPreferences(this) but the method TrueTimeRx.isInitialized() always returns false between application restarts. Also if I try to call TrueTimeRx.now() it throws an exception.

Can anyone confirm if the cache really works? The logs do not state anything going wrong.

IonutNegru87 commented 7 years ago

Found an dirty fix, by calling TrueTimeRx.build().withSharedPreferences(getApplicationContext()).isInitialized() The issue was that there was no _sharedPreferences in the DiskCacheClient class.

EdwardvanRaak commented 7 years ago

@IonutNegru87 I believe you meant TrueTimeRx.build().withSharedPreferences(context).initialize() right? I had the same problem.

IonutNegru87 commented 7 years ago

@EdwardvanRaak Actually no. This is the method I'm using in my Application class:

  private void initTime() {
    //noinspection AccessStaticViaInstance
    if (TrueTimeRx.build().withSharedPreferences(getApplicationContext()).isInitialized()) {
      Timber.d("initTime(): TrueTime is already initialized per this boot - using cache!");
      return;
    }
    // This will make network calls to get the real time
    TrueTimeRx.build()
              .withLoggingEnabled(true)
              .withSharedPreferences(getApplicationContext())
              .initializeRx("time.google.com")
              .subscribeOn(Schedulers.io())
              .subscribe(date -> Timber.d("initTime(): TrueTime was initialized and we have a time: " + date),
                  throwable -> Timber.e(throwable,
                      "initTime(): Something went wrong when trying to initializeRx TrueTime"),
                  () -> Timber.d("initTime(): TrueTime finished!"));
  }

If I use TrueTimeRx.isInitialized() directly, it will return false due to the fact that it cannot access SharedPreferences without an Context. Also, If I initialize the library without this check it will trigger the entire initialization process without checking if it is already available.

kaushikgopal commented 7 years ago

couple of things:

  1. @IonutNegru87 : we don't need the context really, we just need the shared preferences object to exist (i.e. be not null). when you build truetime this is stored as a reference.

This is where the logic is handled:

https://github.com/instacart/truetime-android/blob/master/library/src/main/java/com/instacart/library/truetime/DiskCacheClient.java#L53

https://github.com/instacart/truetime-android/blob/master/library/src/main/java/com/instacart/library/truetime/DiskCacheClient.java#L87

i would confirm the output there.

  1. i would think, this is all that you would need to do
private TrueTimeRx trueTime  = TrueTimeRx.build()
            .withSharedPreferences(getApplicationContext())
            .withServerResponseDelayMax(250)
            .withLoggingEnabled(true);

 private void initTime() {
    if (trueTime.isInitialized()) {
      Timber.d("initTime(): TrueTime is already initialized per this boot - using cache!");
      return;
    }

    // This will make network calls to get the real time
    trueTime
              .initializeRx("time.google.com")
              .subscribeOn(Schedulers.io())
              .subscribe(date -> Timber.d("initTime(): TrueTime was initialized and we have a time: " + date),
                  throwable -> Timber.e(throwable,
                      "initTime(): Something went wrong when trying to initializeRx TrueTime"),
                  () -> Timber.d("initTime(): TrueTime finished!"));
  }

another thing you could do to debug, is actually check if the shared preferences holds the cached sntp time, after TT is being initialized.

nizamsp commented 6 years ago

Thanks for the thoughtful library! I am also facing the problem of Time not surviving across app restarts. It always needs internet and disk cache doesn't seem to work across app restarts. I checked the shared preferences file. It had no entries. I am using 2.2 version of truetime rx. Following is my init code for your reference.

Any help is much appreciated!

 public static void initialize(Context context) {
        if (TrueTime.build().withSharedPreferences(context).withLoggingEnabled(BuildConfig.DEBUG).isInitialized()) {
            Log.d("truetime", "initTime(): TrueTime is already initialized per this boot - using cache!");
            return;
        }

        if (!AppUtils.isInternetAvailable(context)) {
            return;
        }

//        TrueTimeRx.clearCachedInfo(context);

        TrueTimeRx.build()
                .withConnectionTimeout(NTP_CONNECTION_TIMEOUT_IN_MS)
                .withRetryCount(50)
                .withSharedPreferences(context)
                .withLoggingEnabled(BuildConfig.DEBUG)
                .initializeRx("time.google.com")
                .subscribeOn(Schedulers.io())
                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(new Subscriber<Date>() {
                    @Override
                    public void onCompleted() {

                    }

                    @Override
                    public void onError(Throwable e) {
                                                   Crashlytics.logException(e);
                                                                               }

                    @Override
                    public void onNext(Date date) {
                        Log.d("truetime", "successfully synced time: " + date.toString());
                    }
                });
    }

    public static synchronized Date getDateTime() {
        if (TrueTime.isInitialized()) {
            return TrueTime.now();
        }
        return new Date();
    }
nur4i commented 6 years ago

Caching doesn't work across phone restarts, both on TrueTime and TrueTimeRx versions. I'm using version 3.3 My simple code:

TrueTime.build().withSharedPreferences(context);
if (TrueTime.isInitialized()) {
// do smtn
} else{
    TrueTime.build().initialize()
}
EdwardvanRaak commented 6 years ago

@nur4i How can it "not work" if that's not even a feature of this library?

nur4i commented 6 years ago

@EdwardvanRaak Sorry, my bad, didn't fully understand features

kaushikgopal commented 6 years ago

@nur4i a reboot is explicitly one of those times that TrueTime should be "reset". Have a look at this part of the readme.

@nizamsp @IonutNegru87 : we have a PR #62 that another contributor is helping with. Once they have that PR merged in, i intend to take a more exhaustive look at this again. stay tuned.

kaushikgopal commented 6 years ago

so the caching changes are in version 3.4, that being said there's an edge case that still hasn't been accounted for. This is documented here -> https://github.com/instacart/truetime-android/issues/85

closing this issue out in favor of that one. let me know if we're missing anything