From dbd10943895654a8f563ca9d97a856d79054d136 Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Wed, 7 Sep 2016 18:57:03 +0200 Subject: [PATCH 1/7] Fix potential race condition --- .../OAuth2AccessTokenManagerSpecs.groovy | 35 +++++++++++++++++++ .../heimdall/OAuth2AccessTokenManager.java | 31 ++++++++++------ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy b/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy index e570264..f185923 100644 --- a/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy +++ b/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy @@ -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") @@ -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(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" diff --git a/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java b/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java index f772ace..cf653f7 100644 --- a/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java +++ b/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java @@ -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; @@ -20,6 +20,7 @@ public class OAuth2AccessTokenManager { // Members private final OAuth2AccessTokenStorage mStorage; + private Observable mTokenObservable; // Constructor @@ -71,17 +72,25 @@ public Single grantNewAccessToken(OAuth2Grant 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(); + } - mStorage.storeAccessToken(accessToken); - }); + return mTokenObservable.toSingle(); } /** From c297ae4fde3605af313fde5b8be5c3d2e9ee9d0e Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Wed, 7 Sep 2016 19:00:19 +0200 Subject: [PATCH 2/7] raise version number --- library/build.gradle | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/build.gradle b/library/build.gradle index a8a78da..cc2cce5 100644 --- a/library/build.gradle +++ b/library/build.gradle @@ -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' @@ -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 @@ -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" } From 49485b4029878d0ba6e934a8337771ff896e51d8 Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Wed, 7 Sep 2016 19:10:35 +0200 Subject: [PATCH 3/7] Update yml --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f17a99d..a6d3c9a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,8 +11,8 @@ env: android: components: - tools - - build-tools-23.0.2 - - android-23 + - build-tools-23.0.3 + - android-24 - android-22 - sys-img-armeabi-v7a-android-22 - extra-google-m2repository From 26f4a3c918f4d736dfbcd1c101b4437601b0d3f6 Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Wed, 7 Sep 2016 20:42:00 +0200 Subject: [PATCH 4/7] Update yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index a6d3c9a..5aa2e3a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,7 @@ android: - build-tools-23.0.3 - android-24 - android-22 + - android-23 - sys-img-armeabi-v7a-android-22 - extra-google-m2repository - extra-android-m2repository From 7378dbb6de869a775c1a4bb18af87bdc3328d65a Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Wed, 7 Sep 2016 20:51:36 +0200 Subject: [PATCH 5/7] Update sample --- sample/build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sample/build.gradle b/sample/build.gradle index 0d4860e..3c0b9bb 100644 --- a/sample/build.gradle +++ b/sample/build.gradle @@ -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" } From b94833ad46c9d2c966e7b69c54b271589ef771b2 Mon Sep 17 00:00:00 2001 From: Giulio Petek Date: Thu, 8 Sep 2016 10:20:04 +0200 Subject: [PATCH 6/7] Refactor to use single instead of casting back and forth --- .../heimdall/OAuth2AccessTokenManagerSpecs.groovy | 2 +- .../heimdall/OAuth2AccessTokenManager.java | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy b/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy index f185923..5e86dac 100644 --- a/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy +++ b/library/src/androidTest/groovy/de/rheinfabrik/heimdall/OAuth2AccessTokenManagerSpecs.groovy @@ -170,7 +170,7 @@ class OAuth2AccessTokenManagerGetValidAccessTokenSpecs extends AndroidSpecificat tokenManager.getValidAccessToken(grant).subscribe() then: "The refresh grant is asked for a new token TWICE" - 2 * grant.grantNewAccessToken() >> just(accessToken) + 2 * grant.grantNewAccessToken() >> just(accessToken) >> just(accessToken) } def "it should throw an IllegalArgumentException when the refreshAccessTokenGrant parameter is null"() { diff --git a/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java b/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java index cf653f7..9e16867 100644 --- a/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java +++ b/library/src/main/java/de/rheinfabrik/heimdall/OAuth2AccessTokenManager.java @@ -20,7 +20,7 @@ public class OAuth2AccessTokenManager { // Members private final OAuth2AccessTokenStorage mStorage; - private Observable mTokenObservable; + private Single mTokenSingle; // Constructor @@ -72,8 +72,8 @@ public Single grantNewAccessToken(OAuth2Grant grant, throw new IllegalArgumentException("Grant MUST NOT be null."); } - if (mTokenObservable == null) { - mTokenObservable = grant + if (mTokenSingle == null) { + mTokenSingle = grant .grantNewAccessToken() .doOnSuccess(accessToken -> { if (accessToken.expiresIn != null) { @@ -84,13 +84,11 @@ public Single grantNewAccessToken(OAuth2Grant grant, mStorage.storeAccessToken(accessToken); - mTokenObservable = null; - }) - .toObservable() - .cache(); + mTokenSingle = null; + }); } - return mTokenObservable.toSingle(); + return mTokenSingle; } /** From f9e66dbd8e459080e1aacafe339a4a42993a35bf Mon Sep 17 00:00:00 2001 From: Adam Harazim Date: Thu, 8 Sep 2016 10:36:58 +0200 Subject: [PATCH 7/7] added adb install timeout --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 5aa2e3a..1f42d35 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ jdk: oraclejdk8 env: global: - MALLOC_ARENA_MAX=2 + - ADB_INSTALL_TIMEOUT=10 matrix: - ANDROID_TARGET=android-22 ANDROID_ABI=armeabi-v7a