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.41k stars 194 forks source link

TrueTime with Dagger @Singleton? #77

Closed scottybe closed 6 years ago

scottybe commented 6 years ago

Hi. The documentation notes "Preferable use dependency injection (like Dagger) and create a TrueTime @Singleton object".

I'd like to initialize TrueTimeRX when my app starts, then subscribe to it later in a specific activity. Is this the correct way to create the singleton for that purpose in Dagger 2?

@Singleton
public class MyTrueTime {
    public final Observable trueTime;

    @Inject
    public MyTrueTime(String url) {
        this.trueTime = TrueTimeRx.build()
            .initializeRx(url);
    }
}

Thanks!

ZimM-LostPolygon commented 6 years ago

Sound about right, I do it similarly.


@Singleton
public class AccurateTimeProvider {
    private static final Logger Log = LoggerFactory.getLogger("AccurateTimeProvider");
    private final TrueTimeRx mTrueTime;
    private boolean mIsInitializing;

    @Inject
    public AccurateTimeProvider(@ApplicationContext Context context) {
        mTrueTime =
            TrueTimeRx
                .build()
                .withLoggingEnabled(false)
                .withConnectionTimeout(15 * 1000)
                .withServerResponseDelayMax(400)
                .withRootDispersionMax(130)
                .withSharedPreferences(context);
    }

    public boolean isInitialized() {
        return TrueTimeRx.isInitialized();
    }

    @SuppressLint("CheckResult")
    public Date now() {
        if (TrueTimeRx.isInitialized())
            return TrueTimeRx.now();

        if (!mIsInitializing) {
            mIsInitializing = true;
            mTrueTime
                .initializeRx("pool.ntp.org")
                .subscribeOn(Schedulers.io())
                .subscribe(
                    date -> {
                        mIsInitializing = false;
                        Log.trace("TrueTime was initialized and we have a time: " + date);
                    },
                    throwable -> {
                        mIsInitializing = false;
                        Log.warn("NTP failed: ", throwable);
                    }
                );
        }

        return Calendar.getInstance().getTime();
    }
}
scottybe commented 6 years ago

Thanks for the example, but why does AccurateTimeProvider.now() return getTime() from Calendar?

I ended up initializing a TrueTimeRx Observable in my Application singleton, then subscribing to it in various Activities that need it.

By the way, I noticed that the .subscribe function was only called once when the internal clock it was off .3 seconds. Is that the expected behavior?

ZimM-LostPolygon commented 6 years ago

It only uses getTime() as a fallback, so we can have at least some time until TrueTime is initialized. Not sure about subscribe behavior, though.

scottybe commented 6 years ago

Ah, of course. Thanks again.

kaushikgopal commented 6 years ago

💯 I use a very similar approach for Instacart.

We use Dagger 2 and it's scoped for the Application @AppScope (so basically a @Singleton).

Internally, we have some additional stuff we tack on to the TrueTime (like helper methods to proceed with a business logic only post TrueTime.init etc.) so i have an ISTrueTime which much like both of your examples has an internal reference to a truetime var.

aside: having an ISTrueTime/MyTrueTime/AccurateTimeProvider is additionally crucial for testing. I chose to have TrueTime as a Singleton (trade off was api cleanliness vs testability) and so using this approach where the time provider is provided through Dagger is crucial for testing.

A bunch of our espresso tests require TrueTime and so in these cases being able to mock it out is ✔️ .

thiagotalma commented 6 years ago

@kaushikgopal @ZimM-LostPolygon How about sharing your implementations by enriching the app sample?

The app is too simplistic.

ZimM-LostPolygon commented 6 years ago

@thiagotalma Why would you want that? Sample apps are supposed to be simple, and my code is trivial.

thiagotalma commented 6 years ago

Readme suggests using singleton and dagger. It would be good for novices to have the sample app implement these suggestions in addition to the simple implementation.

kaushikgopal commented 6 years ago

I understand where you're coming from, but the trouble with going down that line is we're basically demoing dagger at that point :).

Basically i would have to add dagger in the sample app as a first step, which in itself –as ZimM alluded to– complicates things.

I'm happy to send this to you separately (if you still desire it) but probably won't add it to the sample app for now.

thiagotalma commented 6 years ago

I'm happy to send this to you separately (if you still desire it) but probably won't add it to the sample app for now.

Would be great! It would be an invaluable contribution to my Android learning. I already have enough road round with PHP but on mobile I'm still crawling.

thiagotalma commented 6 years ago

@kaushikgopal Your help would still be very helpful 😉