[android] Introduce AccountsManager to support SKU tokens in API requests #14404
Conversation
|
@tobrun Per chat, while this PR isn't still ready to be merged (it requires updating some dependencies), the rest of the code is ready for a first review. |
| return now; | ||
| } | ||
|
|
||
| private @NonNull SharedPreferences getSharedPreferences() { |
There was a problem hiding this comment.
Annotations should be placed before private keyword, couple more occurrences below
There was a problem hiding this comment.
Weird, it wasn't caught by lint or checkstyle -- I'll update it.
There was a problem hiding this comment.
technically it's correct and not aware of any lint rules enforcing this though I think it's a best practice.
| } | ||
|
|
||
| static long getNow() { | ||
| return Calendar.getInstance(MapboxConstants.MAPBOX_LOCALE).getTimeInMillis(); |
There was a problem hiding this comment.
Better to use System.currentTimeMillis() for this use-case. Calendar adds overhead and relies on that same api. This algorithm doesn't need to be localized, we only need a reference point in time.
There was a problem hiding this comment.
This method will eventually be removed to use the underlying accounts library which, in turn, uses System.currentTimeMillis(). I'll still change it here for the time being.
| @@ -87,6 +90,11 @@ public static void setAccessToken(String accessToken) { | |||
| FileSource.getInstance(getApplicationContext()).setAccessToken(accessToken); | |||
| } | |||
|
|
|||
| public static String getSkuToken() { | |||
| validateMapbox(); | |||
| return INSTANCE.accounts.getSkuToken(); | |||
There was a problem hiding this comment.
If a developer doesn't provide an access token, eg. when using their own tile endpoint, this will throw a null pointer exception when the SDK tries to make a request.
There was a problem hiding this comment.
This method is only called for Mapbox API URLs so it's safe to remove the validation.
| @@ -87,6 +90,11 @@ public static void setAccessToken(String accessToken) { | |||
| FileSource.getInstance(getApplicationContext()).setAccessToken(accessToken); | |||
| } | |||
|
|
|||
| public static String getSkuToken() { | |||
| private static final String PREFERENCE_TIMESTAMP = "com.mapbox.mapboxsdk.accounts.timestamp"; | ||
| private static final String PREFERENCE_SKU_TOKEN = "com.mapbox.mapboxsdk.accounts.skutoken"; | ||
|
|
||
| static final long ONE_HOUR_MILLIS = 60 * 60 * 1_000L; |
There was a problem hiding this comment.
nit could be DateUtils.HOUR_IN_MILLIS instead
|
@zugaldia @tobrun folks, telem release with changes to turnstile event has been published on maven and github: https://github.com/mapbox/mapbox-events-android/releases/tag/telem-4.4.0 |
LukasPaczos
force-pushed
the
az-sku-token
branch
from
48e3bf5 to
7cf465f
Compare
| @@ -87,6 +90,11 @@ public static void setAccessToken(String accessToken) { | |||
| FileSource.getInstance(getApplicationContext()).setAccessToken(accessToken); | |||
| } | |||
|
|
|||
| public static String getSkuToken() { | |||
| validateMapbox(); | |||
| return INSTANCE.accounts.getSkuToken(); | |||
This PR proposes a new
AccountsManagerobject, instantiated within the regularMapboxobject, that manages the state and rotation of SKU tokens. SKU tokens are added to Mapbox API requests only.It relies on Android's
SharedPreferencesfor storage, and proposes a centralizedMapboxConstants.MAPBOX_SHARED_PREFERENCESto be shared withFileSourcevalues.Before being able to merge this PR we need to add two extra dependencies:
MapboxAccountslibrary to generate the content of the SKU tokens./cc: @friedbunny