Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent refresh race conditions #40

Merged
merged 7 commits into from
Sep 8, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ env:
android:
components:
- tools
- build-tools-23.0.2
- android-23
- build-tools-23.0.3
- android-24
- android-22
- android-23
- sys-img-armeabi-v7a-android-22
- extra-google-m2repository
- extra-android-m2repository
Expand Down
10 changes: 5 additions & 5 deletions library/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ buildscript {
}

dependencies {
classpath 'com.android.tools.build:gradle:1.2.3'
classpath 'com.android.tools.build:gradle:2.1.3'
classpath 'me.tatarka:gradle-retrolambda:2.5.0'
classpath 'org.codehaus.groovy:gradle-groovy-android-plugin:0.3.6'
classpath 'com.github.dcendents:android-maven-plugin:1.2'
Expand All @@ -20,8 +20,8 @@ buildscript {
group = 'com.github.rheinfabrik'

android {
compileSdkVersion 23
buildToolsVersion "23.0.2"
compileSdkVersion 24
buildToolsVersion "23.0.3"

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
Expand All @@ -37,8 +37,8 @@ android {
defaultConfig {
minSdkVersion 9
targetSdkVersion 23
versionCode 104
versionName "1.0.4"
versionCode 105
versionName "1.0.5"
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import de.rheinfabrik.heimdall.grants.OAuth2Grant
import de.rheinfabrik.heimdall.grants.OAuth2RefreshAccessTokenGrant
import spock.lang.Title

import java.util.concurrent.TimeUnit

import static rx.Single.just

@Title("Tests for the constructor of the OAuth2AccessTokenManager class")
Expand Down Expand Up @@ -138,6 +140,39 @@ class OAuth2AccessTokenManagerGetValidAccessTokenSpecs extends AndroidSpecificat

// Scenarios

def "it should only request a new valid token ONCE"() {

given: "An expired OAuth2AccessToken"
OAuth2AccessToken accessToken = Mock(OAuth2AccessToken)
accessToken.refreshToken = "rt"
accessToken.isExpired() >> true

and: "A mock storage emitting that token"
OAuth2AccessTokenStorage storage = Mock(OAuth2AccessTokenStorage)
storage.getStoredAccessToken() >> just(accessToken).delay(1, TimeUnit.SECONDS)

and: "An OAuth2AccessTokenManager with that storage"
OAuth2AccessTokenManager tokenManager = new OAuth2AccessTokenManager<OAuth2AccessToken>(storage)

and: "A mock grant"
OAuth2RefreshAccessTokenGrant grant = Mock(OAuth2RefreshAccessTokenGrant)

when: "I ask for a valid access token"
tokenManager.getValidAccessToken(grant).subscribe()

and: "I ask again"
tokenManager.getValidAccessToken(grant).subscribe()

and: "I wait 2 seconds"
sleep(2000)

and: "I ask again"
tokenManager.getValidAccessToken(grant).subscribe()

then: "The refresh grant is asked for a new token TWICE"
2 * grant.grantNewAccessToken() >> just(accessToken)
}

def "it should throw an IllegalArgumentException when the refreshAccessTokenGrant parameter is null"() {

given: "A null grant"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import de.rheinfabrik.heimdall.grants.OAuth2Grant;
import de.rheinfabrik.heimdall.grants.OAuth2RefreshAccessTokenGrant;
import rx.Observable;
import rx.Single;
import rx.functions.Func1;

import static rx.Single.error;

Expand All @@ -20,6 +20,7 @@ public class OAuth2AccessTokenManager<TAccessToken extends OAuth2AccessToken> {
// Members

private final OAuth2AccessTokenStorage<TAccessToken> mStorage;
private Observable<TAccessToken> mTokenObservable;

// Constructor

Expand Down Expand Up @@ -71,17 +72,25 @@ public Single<TAccessToken> grantNewAccessToken(OAuth2Grant<TAccessToken> grant,
throw new IllegalArgumentException("Grant MUST NOT be null.");
}

return grant
.grantNewAccessToken()
.doOnSuccess(accessToken -> {
if (accessToken.expiresIn != null) {
Calendar expirationDate = (Calendar) calendar.clone();
expirationDate.add(Calendar.SECOND, accessToken.expiresIn);
accessToken.expirationDate = expirationDate;
}
if (mTokenObservable == null) {
mTokenObservable = grant
.grantNewAccessToken()
.doOnSuccess(accessToken -> {
if (accessToken.expiresIn != null) {
Calendar expirationDate = (Calendar) calendar.clone();
expirationDate.add(Calendar.SECOND, accessToken.expiresIn);
accessToken.expirationDate = expirationDate;
}

mStorage.storeAccessToken(accessToken);

mTokenObservable = null;
})
.toObservable()
.cache();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cache here would mean that every time we subscribe every element will be replayed right?

}

mStorage.storeAccessToken(accessToken);
});
return mTokenObservable.toSingle();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and then we the cached element is turned into a single which only emits one item and then "completes" aka calls onSuccess - don't get the combination. couldn't we just do this:

Token token = null;
doOnSuccess(
   ...
   token = accessToken
)

Observable.just(token)

}

/**
Expand Down
6 changes: 3 additions & 3 deletions sample/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ buildscript {
}

android {
compileSdkVersion 23
buildToolsVersion "23.0.2"
compileSdkVersion 24
buildToolsVersion "23.0.3"

defaultConfig {
applicationId "de.rheinfabrik.heimdall"
minSdkVersion 15
targetSdkVersion 23
targetSdkVersion 24
versionCode 1
versionName "1.0"
}
Expand Down