From e60d93a05a23538ca47241e4a9f4cfaeaf3681bd Mon Sep 17 00:00:00 2001 From: Ryan Harg <3821-ryan_harg@users.noreply.dev.funkwhale.audio> Date: Fri, 30 Jul 2021 08:57:49 +0000 Subject: [PATCH] #7: Add unit tests to OAuthDatasource --- .gitlab-ci.yml | 73 +++++++++----- app/build.gradle.kts | 67 ++++++++++++- .../funkwhale/ffa/activities/LoginActivity.kt | 15 ++- .../ffa/activities/SplashActivity.kt | 8 +- .../ffa/playback/OAuth2Datasource.kt | 14 ++- .../funkwhale/ffa/playback/QueueManager.kt | 26 ++--- .../ffa/repositories/AlbumsRepository.kt | 6 +- .../repositories/ArtistTracksRepository.kt | 6 +- .../ffa/repositories/ArtistsRepository.kt | 6 +- .../ffa/repositories/FavoritesRepository.kt | 17 +++- .../ffa/repositories/HttpUpstream.kt | 5 +- .../repositories/PlaylistTracksRepository.kt | 6 +- .../ffa/repositories/PlaylistsRepository.kt | 18 ++-- .../ffa/repositories/RadiosRepository.kt | 6 +- .../ffa/repositories/SearchRepository.kt | 18 +++- .../ffa/repositories/TracksRepository.kt | 6 +- .../java/audio/funkwhale/ffa/utils/Data.kt | 63 +++++++------ .../audio/funkwhale/ffa/utils/Extensions.kt | 9 +- .../java/audio/funkwhale/ffa/utils/OAuth.kt | 94 ++++++++++++++----- .../ffa/playback/OAuthDatasourceTest.kt | 75 +++++++++++++++ .../audio/funkwhale/util/MockKJUnitRunner.kt | 58 ++++++++++++ 21 files changed, 465 insertions(+), 131 deletions(-) create mode 100644 app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt create mode 100644 app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f801726..51dd459 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,51 +1,82 @@ image: jangrewe/gitlab-ci-android -stages: -- build -- deploy +variables: + JACOCO_CSV_LOCATION: '$CI_PROJECT_DIR/app/build/reports/jacoco/testDebugUnitTestCoverage/testDebugUnitTestCoverage.csv' +stages: + - test + - visualize + - build + - deploy -.build: +.gradle-default: stage: build before_script: - - export GRADLE_USER_HOME=$(pwd)/.gradle - - chmod +x ./gradlew - - mkdir -p .android && touch .android/repositories.cfg + - export GRADLE_USER_HOME=$(pwd)/.gradle + - chmod +x ./gradlew + - mkdir -p .android && touch .android/repositories.cfg script: - - echo "Overwrite me" + - echo "Overwrite me" cache: key: ${CI_PROJECT_ID} paths: - - .gradle/ + - .gradle/ + +.build: + extends: .gradle-default + artifacts: + paths: + - app/build/outputs/apk/debug/app-debug.apk +test: + extends: .gradle-default + stage: test + script: + - ./gradlew test testDebugUnitTestCoverage + - awk -F"," '{ instructions += $4 + $5; covered += $5 } END { print covered, "/", instructions, " instructions covered"; print 100*covered/instructions, "% covered" }' $JACOCO_CSV_LOCATION artifacts: + reports: + junit: app/build/test-results/test**/TEST-*.xml paths: - - app/build/outputs/apk/debug/app-debug.apk + - app/build/reports/jacoco/testDebugUnitTestCoverage/testDebugUnitTestCoverage.xml + +coverage: + stage: visualize + image: gjrtimmer/jacoco2cobertura:1.0.8 + script: + # convert report from jacoco to cobertura, use relative project path + - 'python /opt/cover2cover.py app/build/reports/jacoco/testDebugUnitTestCoverage/testDebugUnitTestCoverage.xml $CI_PROJECT_DIR/app/src/main/java > app/build/reports/cobertura.xml' + needs: [ "test" ] + dependencies: + - test + artifacts: + reports: + cobertura: app/build/reports/cobertura.xml build-develop: extends: .build script: - - echo -n $SIGNING_KEY_STORE | base64 -d > app/android.keystore - - ./gradlew assembleDebug -Psigning.store=android.keystore -Psigning.store_passphrase=$SIGNING_KEY_PASS -Psigning.key_passphrase=$SIGNING_KEY_PASS + - echo -n $SIGNING_KEY_STORE | base64 -d > app/android.keystore + - ./gradlew assembleDebug -Psigning.store=android.keystore -Psigning.store_passphrase=$SIGNING_KEY_PASS -Psigning.key_passphrase=$SIGNING_KEY_PASS only: - - develop + - develop build-bleeding-edge: extends: .build script: - - ./gradlew assembleDebug + - ./gradlew assembleDebug except: - - develop + - develop deploy-develop: stage: deploy - only: - - develop + only: + - develop script: - - eval `ssh-agent -s` - - ssh-add <(echo "$SSH_PRIVATE_KEY") - - scp -o StrictHostKeyChecking=no app/build/outputs/apk/debug/app-debug.apk fdroid@apps.funkwhale.audio:/srv/fdroid/fdroid/develop/repo/audio.funkwhale.ffa.dev-$CI_COMMIT_SHORT_SHA.apk - - ssh -o StrictHostKeyChecking=no fdroid@apps.funkwhale.audio 'docker run --rm -u $(id -u):$(id -g) -v /srv/fdroid/fdroid/develop:/repo registry.gitlab.com/fdroid/docker-executable-fdroidserver:master update' + - eval `ssh-agent -s` + - ssh-add <(echo "$SSH_PRIVATE_KEY") + - scp -o StrictHostKeyChecking=no app/build/outputs/apk/debug/app-debug.apk fdroid@apps.funkwhale.audio:/srv/fdroid/fdroid/develop/repo/audio.funkwhale.ffa.dev-$CI_COMMIT_SHORT_SHA.apk + - ssh -o StrictHostKeyChecking=no fdroid@apps.funkwhale.audio 'docker run --rm -u $(id -u):$(id -g) -v /srv/fdroid/fdroid/develop:/repo registry.gitlab.com/fdroid/docker-executable-fdroidserver:master update' tags: - shell diff --git a/app/build.gradle.kts b/app/build.gradle.kts index dd6db60..87040e1 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -9,6 +9,7 @@ plugins { id("org.jlleitschuh.gradle.ktlint") version "8.1.0" id("com.gladed.androidgitversion") version "0.4.14" id("com.github.triplet.play") version "2.4.2" + jacoco } val props = Properties().apply { @@ -18,6 +19,10 @@ val props = Properties().apply { } } +jacoco { + toolVersion = "0.8.7" +} + androidGitVersion { codeFormat = "MMNNPPBBB" format = "%tag%%-count%%-commit%%-branch%" @@ -75,6 +80,10 @@ android { } } + testOptions { + unitTests.isReturnDefaultValues = true + } + buildTypes { getByName("debug") { isDebuggable = true @@ -84,6 +93,8 @@ android { signingConfig = signingConfigs.getByName("debug") } + isTestCoverageEnabled = true + resValue("string", "debug.hostname", props.getProperty("debug.hostname", "")) resValue("string", "debug.username", props.getProperty("debug.username", "")) resValue("string", "debug.password", props.getProperty("debug.password", "")) @@ -160,6 +171,60 @@ dependencies { implementation("com.google.code.gson:gson:2.8.7") implementation("com.squareup.picasso:picasso:2.71828") implementation("jp.wasabeef:picasso-transformations:2.4.0") - implementation("net.openid:appauth:0.9.1") + testImplementation("junit:junit:4.13.2") + testImplementation("io.mockk:mockk:1.12.0") + androidTestImplementation("io.mockk:mockk-android:1.12.0") + testImplementation("androidx.test:core:1.4.0") + testImplementation("io.strikt:strikt-core:0.31.0") +} + +project.afterEvaluate { + android.applicationVariants.forEach { variant -> + val testTaskName = "test${variant.name.capitalize()}UnitTest" + tasks.create(name = "${testTaskName}Coverage") { + + dependsOn(testTaskName) + + group = "Reporting" + description = "Generate Jacoco coverage reports after running tests." + + reports { + xml.required.set(true) + csv.required.set(true) + html.required.set(true) + } + + val excludes = listOf( + "**/R.class", + "**/R$*.class", + "**/BuildConfig.*", + "**/Manifest*.*", + "**/*Test*.*", + "android/**/*.*" + ) + + val javaClasses = + fileTree(baseDir = variant.javaCompileProvider.get().destinationDirectory).matching { + setExcludes(excludes) + } + val kotlinClasses = + fileTree(baseDir = "$buildDir/tmp/kotlin-classes/${variant.name}").matching { + setExcludes(excludes) + } + classDirectories.setFrom(files(listOf(javaClasses, kotlinClasses))) + + val sourceDirectories = files( + listOf( + "$project.projectDir/src/main/java", + "$project.projectDir/src/${variant.name}/java", + "$project.projectDir/src/main/kotlin", + "$project.projectDir/src/${variant.name}/kotlin" + ) + ) + + sourceDirectories.setFrom(files(sourceDirectories)) + executionData.setFrom(files("${project.buildDir}/jacoco/$testTaskName.exec")) + } + } } diff --git a/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt b/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt index b2f4196..b4f4038 100644 --- a/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt +++ b/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt @@ -13,6 +13,7 @@ import audio.funkwhale.ffa.databinding.ActivityLoginBinding import audio.funkwhale.ffa.fragments.LoginDialog import audio.funkwhale.ffa.utils.AppContext import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.Userinfo import audio.funkwhale.ffa.utils.log import com.github.kittinunf.fuel.Fuel @@ -28,13 +29,13 @@ data class FwCredentials(val token: String, val non_field_errors: List?) class LoginActivity : AppCompatActivity() { private lateinit var binding: ActivityLoginBinding - + private lateinit var oAuth: OAuth override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) binding = ActivityLoginBinding.inflate(layoutInflater) - + oAuth = OAuthFactory.instance() setContentView(binding.root) limitContainerWidth() } @@ -45,7 +46,7 @@ class LoginActivity : AppCompatActivity() { data?.let { when (requestCode) { 0 -> { - OAuth.exchange(this, data, + oAuth.exchange(this, data, { PowerPreference .getFileByName(AppContext.PREFS_CREDENTIALS) @@ -114,11 +115,9 @@ class LoginActivity : AppCompatActivity() { private fun authedLogin(hostname: String) { PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).setString("hostname", hostname) - - OAuth.init(hostname) - - OAuth.register { - OAuth.authorize(this) + oAuth.init(hostname) + oAuth.register { + oAuth.authorize(this) } } diff --git a/app/src/main/java/audio/funkwhale/ffa/activities/SplashActivity.kt b/app/src/main/java/audio/funkwhale/ffa/activities/SplashActivity.kt index 26e147d..8aba3a8 100644 --- a/app/src/main/java/audio/funkwhale/ffa/activities/SplashActivity.kt +++ b/app/src/main/java/audio/funkwhale/ffa/activities/SplashActivity.kt @@ -7,16 +7,21 @@ import androidx.appcompat.app.AppCompatActivity import audio.funkwhale.ffa.FFA import audio.funkwhale.ffa.utils.AppContext import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.Settings class SplashActivity : AppCompatActivity() { + private lateinit var oAuth: OAuth + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + oAuth = OAuthFactory.instance() + getSharedPreferences(AppContext.PREFS_CREDENTIALS, Context.MODE_PRIVATE) .apply { - when (OAuth.isAuthorized(this@SplashActivity) || Settings.isAnonymous()) { + when (oAuth.isAuthorized(this@SplashActivity) || Settings.isAnonymous()) { true -> Intent(this@SplashActivity, MainActivity::class.java).apply { flags = Intent.FLAG_ACTIVITY_NO_ANIMATION startActivity(this) @@ -30,5 +35,4 @@ class SplashActivity : AppCompatActivity() { } } } - } diff --git a/app/src/main/java/audio/funkwhale/ffa/playback/OAuth2Datasource.kt b/app/src/main/java/audio/funkwhale/ffa/playback/OAuth2Datasource.kt index e189045..64b5786 100644 --- a/app/src/main/java/audio/funkwhale/ffa/playback/OAuth2Datasource.kt +++ b/app/src/main/java/audio/funkwhale/ffa/playback/OAuth2Datasource.kt @@ -11,7 +11,8 @@ import com.google.android.exoplayer2.upstream.TransferListener class OAuthDatasource( private val context: Context, - private val http: HttpDataSource + private val http: HttpDataSource, + private val oauth: OAuth ) : DataSource { override fun addTransferListener(transferListener: TransferListener?) { @@ -19,7 +20,10 @@ class OAuthDatasource( } override fun open(dataSpec: DataSpec?): Long { - OAuth.tryRefreshAccessToken(context) + oauth.tryRefreshAccessToken(context) + http.apply { + setRequestProperty("Authorization", "Bearer ${oauth.state().accessToken}") + } return http.open(dataSpec) } @@ -34,15 +38,15 @@ class OAuthDatasource( override fun close() { http.close() } - } class OAuth2DatasourceFactory( private val context: Context, - private val http: DefaultHttpDataSourceFactory + private val http: DefaultHttpDataSourceFactory, + private val oauth: OAuth ) : DataSource.Factory { override fun createDataSource(): DataSource { - return OAuthDatasource(context, http.createDataSource()) + return OAuthDatasource(context, http.createDataSource(), oauth) } } diff --git a/app/src/main/java/audio/funkwhale/ffa/playback/QueueManager.kt b/app/src/main/java/audio/funkwhale/ffa/playback/QueueManager.kt index baac829..e39d963 100644 --- a/app/src/main/java/audio/funkwhale/ffa/playback/QueueManager.kt +++ b/app/src/main/java/audio/funkwhale/ffa/playback/QueueManager.kt @@ -9,7 +9,7 @@ import audio.funkwhale.ffa.utils.Command import audio.funkwhale.ffa.utils.CommandBus import audio.funkwhale.ffa.utils.Event import audio.funkwhale.ffa.utils.EventBus -import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.QueueCache import audio.funkwhale.ffa.utils.Settings import audio.funkwhale.ffa.utils.Track @@ -17,6 +17,7 @@ import audio.funkwhale.ffa.utils.mustNormalizeUrl import com.github.kittinunf.fuel.gson.gsonDeserializerOf import com.google.android.exoplayer2.source.ConcatenatingMediaSource import com.google.android.exoplayer2.source.ProgressiveMediaSource +import com.google.android.exoplayer2.upstream.DataSource import com.google.android.exoplayer2.upstream.DefaultHttpDataSourceFactory import com.google.android.exoplayer2.upstream.FileDataSource import com.google.android.exoplayer2.upstream.cache.CacheDataSource @@ -32,19 +33,9 @@ class QueueManager(val context: Context) { companion object { fun factory(context: Context): CacheDataSourceFactory { - val http = DefaultHttpDataSourceFactory( - Util.getUserAgent(context, context.getString(R.string.app_name)) - ) - .apply { - defaultRequestProperties.apply { - if (!Settings.isAnonymous()) { - set("Authorization", "Bearer ${OAuth.state().accessToken}") - } - } - } val playbackCache = - CacheDataSourceFactory(FFA.get().exoCache, OAuth2DatasourceFactory(context, http)) + CacheDataSourceFactory(FFA.get().exoCache, createDatasourceFactory(context)) return CacheDataSourceFactory( FFA.get().exoDownloadCache, @@ -55,6 +46,17 @@ class QueueManager(val context: Context) { null ) } + + private fun createDatasourceFactory(context: Context): DataSource.Factory { + val http = DefaultHttpDataSourceFactory( + Util.getUserAgent(context, context.getString(R.string.app_name)) + ) + return if (!Settings.isAnonymous()) { + OAuth2DatasourceFactory(context, http, OAuthFactory.instance()) + } else { + http + } + } } init { diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/AlbumsRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/AlbumsRepository.kt index 8ff809d..99b279e 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/AlbumsRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/AlbumsRepository.kt @@ -4,6 +4,7 @@ import android.content.Context import audio.funkwhale.ffa.utils.Album import audio.funkwhale.ffa.utils.AlbumsCache import audio.funkwhale.ffa.utils.AlbumsResponse +import audio.funkwhale.ffa.utils.OAuthFactory import com.github.kittinunf.fuel.gson.gsonDeserializerOf import com.google.gson.reflect.TypeToken import java.io.BufferedReader @@ -11,6 +12,8 @@ import java.io.BufferedReader class AlbumsRepository(override val context: Context?, artistId: Int? = null) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId: String by lazy { if (artistId == null) "albums" else "albums-artist-$artistId" @@ -25,7 +28,8 @@ class AlbumsRepository(override val context: Context?, artistId: Int? = null) : context!!, HttpUpstream.Behavior.Progressive, url, - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) } diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistTracksRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistTracksRepository.kt index ef8b8f8..71aa263 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistTracksRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistTracksRepository.kt @@ -1,6 +1,7 @@ package audio.funkwhale.ffa.repositories import android.content.Context +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.Track import audio.funkwhale.ffa.utils.TracksCache @@ -12,13 +13,16 @@ import java.io.BufferedReader class ArtistTracksRepository(override val context: Context?, private val artistId: Int) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "tracks-artist-$artistId" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.AtOnce, "/api/v1/tracks/?playable=true&artist=$artistId", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = TracksCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistsRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistsRepository.kt index 9aed863..03d6e26 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistsRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/ArtistsRepository.kt @@ -4,6 +4,7 @@ import android.content.Context import audio.funkwhale.ffa.utils.Artist import audio.funkwhale.ffa.utils.ArtistsCache import audio.funkwhale.ffa.utils.ArtistsResponse +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import com.github.kittinunf.fuel.gson.gsonDeserializerOf import com.google.gson.reflect.TypeToken @@ -11,13 +12,16 @@ import java.io.BufferedReader class ArtistsRepository(override val context: Context?) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "artists" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.Progressive, "/api/v1/artists/?playable=true&ordering=name", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = ArtistsCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/FavoritesRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/FavoritesRepository.kt index e30c805..b87fccc 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/FavoritesRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/FavoritesRepository.kt @@ -5,7 +5,7 @@ import audio.funkwhale.ffa.FFA import audio.funkwhale.ffa.utils.Cache import audio.funkwhale.ffa.utils.FavoritedCache import audio.funkwhale.ffa.utils.FavoritedResponse -import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.Settings import audio.funkwhale.ffa.utils.Track @@ -28,13 +28,16 @@ import java.io.BufferedReader class FavoritesRepository(override val context: Context?) : Repository() { + private var oAuth = OAuthFactory.instance() + override val cacheId = "favorites.v2" override val upstream = HttpUpstream>( context!!, HttpUpstream.Behavior.AtOnce, "/api/v1/tracks/?favorites=true&playable=true&ordering=title", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = TracksCache(data) @@ -67,7 +70,7 @@ class FavoritesRepository(override val context: Context?) : Repository() { + + private val oAuth = OAuthFactory.instance() + override val cacheId = "favorited" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.Single, "/api/v1/favorites/tracks/all/?playable=true", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = FavoritedCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/HttpUpstream.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/HttpUpstream.kt index e983f3c..2a1ecf2 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/HttpUpstream.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/HttpUpstream.kt @@ -23,7 +23,8 @@ class HttpUpstream>( val context: Context?, val behavior: Behavior, private val url: String, - private val type: Type + private val type: Type, + private val oAuth: OAuth ) : Upstream { enum class Behavior { @@ -110,7 +111,7 @@ class HttpUpstream>( return if (http.refresh()) { val request = Fuel.get(mustNormalizeUrl(url)).apply { if (!Settings.isAnonymous()) { - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oAuth.state().accessToken}") } } diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistTracksRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistTracksRepository.kt index 9c94261..5fd8df4 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistTracksRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistTracksRepository.kt @@ -1,6 +1,7 @@ package audio.funkwhale.ffa.repositories import android.content.Context +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.PlaylistTrack import audio.funkwhale.ffa.utils.PlaylistTracksCache @@ -15,13 +16,16 @@ import java.io.BufferedReader class PlaylistTracksRepository(override val context: Context?, playlistId: Int) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "tracks-playlist-$playlistId" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.Single, "/api/v1/playlists/$playlistId/tracks/?playable=true", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = PlaylistTracksCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistsRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistsRepository.kt index 703a6ed..4ff8758 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistsRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/PlaylistsRepository.kt @@ -1,7 +1,7 @@ package audio.funkwhale.ffa.repositories import android.content.Context -import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.Playlist import audio.funkwhale.ffa.utils.PlaylistsCache @@ -30,7 +30,8 @@ class PlaylistsRepository(override val context: Context?) : Repository() {}.type + object : TypeToken() {}.type, + OAuthFactory.instance() ) override fun cache(data: List) = PlaylistsCache(data) @@ -41,13 +42,16 @@ class PlaylistsRepository(override val context: Context?) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "tracks-playlists-management" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.AtOnce, "/api/v1/playlists/?scope=me&ordering=name", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = PlaylistsCache(data) @@ -62,7 +66,7 @@ class ManagementPlaylistsRepository(override val context: Context?) : val request = Fuel.post(mustNormalizeUrl("/api/v1/playlists/")).apply { if (!Settings.isAnonymous()) { authorize(context) - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oAuth.state().accessToken}") } } @@ -85,7 +89,7 @@ class ManagementPlaylistsRepository(override val context: Context?) : val request = Fuel.post(mustNormalizeUrl("/api/v1/playlists/$id/add/")).apply { if (!Settings.isAnonymous()) { authorize(context) - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oAuth.state().accessToken}") } } @@ -106,7 +110,7 @@ class ManagementPlaylistsRepository(override val context: Context?) : val request = Fuel.post(mustNormalizeUrl("/api/v1/playlists/$id/remove/")).apply { if (!Settings.isAnonymous()) { authorize(context) - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oAuth.state().accessToken}") } } @@ -125,7 +129,7 @@ class ManagementPlaylistsRepository(override val context: Context?) : val request = Fuel.post(mustNormalizeUrl("/api/v1/playlists/$id/move/")).apply { if (!Settings.isAnonymous()) { authorize(context) - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oAuth.state().accessToken}") } } diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/RadiosRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/RadiosRepository.kt index 0126d2e..4b705c9 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/RadiosRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/RadiosRepository.kt @@ -1,6 +1,7 @@ package audio.funkwhale.ffa.repositories import android.content.Context +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.Radio import audio.funkwhale.ffa.utils.RadiosCache @@ -11,13 +12,16 @@ import java.io.BufferedReader class RadiosRepository(override val context: Context?) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "radios" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.Progressive, "/api/v1/radios/radios/?ordering=name", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = RadiosCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/SearchRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/SearchRepository.kt index 449c3fc..88f57c1 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/SearchRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/SearchRepository.kt @@ -8,6 +8,7 @@ import audio.funkwhale.ffa.utils.AlbumsResponse import audio.funkwhale.ffa.utils.Artist import audio.funkwhale.ffa.utils.ArtistsCache import audio.funkwhale.ffa.utils.ArtistsResponse +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.Track import audio.funkwhale.ffa.utils.TracksCache import audio.funkwhale.ffa.utils.TracksResponse @@ -22,6 +23,8 @@ import java.io.BufferedReader class TracksSearchRepository(override val context: Context?, var query: String) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId: String? = null override val upstream: Upstream @@ -29,7 +32,8 @@ class TracksSearchRepository(override val context: Context?, var query: String) context, HttpUpstream.Behavior.AtOnce, "/api/v1/tracks/?playable=true&q=$query", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = TracksCache(data) @@ -61,13 +65,17 @@ class TracksSearchRepository(override val context: Context?, var query: String) class ArtistsSearchRepository(override val context: Context?, var query: String) : Repository() { + + private val oAuth = OAuthFactory.instance() + override val cacheId: String? = null override val upstream: Upstream get() = HttpUpstream( context, HttpUpstream.Behavior.AtOnce, "/api/v1/artists/?playable=true&q=$query", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = ArtistsCache(data) @@ -77,13 +85,17 @@ class ArtistsSearchRepository(override val context: Context?, var query: String) class AlbumsSearchRepository(override val context: Context?, var query: String) : Repository() { + + private val oAuth = OAuthFactory.instance() + override val cacheId: String? = null override val upstream: Upstream get() = HttpUpstream( context, HttpUpstream.Behavior.AtOnce, "/api/v1/albums/?playable=true&q=$query", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = AlbumsCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/repositories/TracksRepository.kt b/app/src/main/java/audio/funkwhale/ffa/repositories/TracksRepository.kt index 6c63456..1eafcae 100644 --- a/app/src/main/java/audio/funkwhale/ffa/repositories/TracksRepository.kt +++ b/app/src/main/java/audio/funkwhale/ffa/repositories/TracksRepository.kt @@ -2,6 +2,7 @@ package audio.funkwhale.ffa.repositories import android.content.Context import audio.funkwhale.ffa.FFA +import audio.funkwhale.ffa.utils.OAuthFactory import audio.funkwhale.ffa.utils.OtterResponse import audio.funkwhale.ffa.utils.Track import audio.funkwhale.ffa.utils.TracksCache @@ -19,13 +20,16 @@ import java.io.BufferedReader class TracksRepository(override val context: Context?, albumId: Int) : Repository() { + private val oAuth = OAuthFactory.instance() + override val cacheId = "tracks-album-$albumId" override val upstream = HttpUpstream>( context, HttpUpstream.Behavior.AtOnce, "/api/v1/tracks/?playable=true&album=$albumId&ordering=disc_number,position", - object : TypeToken() {}.type + object : TypeToken() {}.type, + oAuth ) override fun cache(data: List) = TracksCache(data) diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/Data.kt b/app/src/main/java/audio/funkwhale/ffa/utils/Data.kt index c4630fb..d414001 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/Data.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/Data.kt @@ -19,25 +19,33 @@ object RefreshError : Throwable() class HTTP(val context: Context?) { suspend fun refresh(): Boolean { - val body = mapOf( - "username" to PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) - .getString("username"), - "password" to PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) - .getString("password") - ).toList() - - val result = Fuel.post(mustNormalizeUrl("/api/v1/token"), body) - .awaitObjectResult(gsonDeserializerOf(FwCredentials::class.java)) - - return result.fold( - { data -> - PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) - .setString("access_token", data.token) - - true - }, - { false } - ) + context?.let { + val body = mapOf( + "username" to PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) + .getString("username"), + "password" to PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) + .getString("password") + ).toList() + + val result = Fuel.post(mustNormalizeUrl("/api/v1/token"), body).apply { + if (!Settings.isAnonymous()) { + authorize(it) + header("Authorization", "Bearer ${OAuthFactory.instance().state().accessToken}") + } + } + .awaitObjectResult(gsonDeserializerOf(FwCredentials::class.java)) + + return result.fold( + { data -> + PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS) + .setString("access_token", data.token) + + true + }, + { false } + ) + } + throw IllegalStateException("Illegal state: context is null") } suspend inline fun get(url: String): Result { @@ -46,7 +54,7 @@ class HTTP(val context: Context?) { val request = Fuel.get(mustNormalizeUrl(url)).apply { if (!Settings.isAnonymous()) { authorize(it) - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${OAuthFactory.instance().state().accessToken}") } } @@ -65,18 +73,13 @@ class HTTP(val context: Context?) { url: String ): Result { context?.let { - return if (refresh()) { - val request = Fuel.get(mustNormalizeUrl(url)).apply { - if (!Settings.isAnonymous()) { - authorize(context) - header("Authorization", "Bearer ${OAuth.state().accessToken}") - } + val request = Fuel.get(mustNormalizeUrl(url)).apply { + if (!Settings.isAnonymous()) { + authorize(context) + header("Authorization", "Bearer ${OAuthFactory.instance().state().accessToken}") } - - request.awaitObjectResult(gsonDeserializerOf(T::class.java)) - } else { - Result.Failure(FuelError.wrap(RefreshError)) } + request.awaitObjectResult(gsonDeserializerOf(T::class.java)) } throw IllegalStateException("Illegal state: context is null") } diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt b/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt index 91138c9..15d2cda 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt @@ -80,16 +80,17 @@ fun Request.authorize(context: Context): Request { return runBlocking { this@authorize.apply { if (!Settings.isAnonymous()) { - OAuth.state().let { state -> + val oauth = OAuthFactory.instance() + oauth.state().let { state -> val old = state.accessToken - val auth = ClientSecretPost(OAuth.state().clientSecret) + val auth = ClientSecretPost(oauth.state().clientSecret) val done = CompletableDeferred() - state.performActionWithFreshTokens(OAuth.service(context), auth) { token, _, _ -> + state.performActionWithFreshTokens(oauth.service(context), auth) { token, _, _ -> if (token != old && token != null) { state.save() } - header("Authorization", "Bearer ${OAuth.state().accessToken}") + header("Authorization", "Bearer ${oauth.state().accessToken}") done.complete(true) } done.await() diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt index 4019be1..4b174ce 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt @@ -29,13 +29,64 @@ fun AuthState.save() { } } -object OAuth { +interface OAuth { + + fun exchange(context: Activity, authorization: Intent, success: () -> Unit, error: () -> Unit) + + fun init(hostname: String) + + fun register(callback: () -> Unit) + + fun authorize(context: Activity) + + fun isAuthorized(context: Context): Boolean + + fun tryRefreshAccessToken(context: Context, overrideNeedsTokenRefresh: Boolean = false): Boolean + + fun tryState(): AuthState? + + fun state(): AuthState + + fun service(context: Context): AuthorizationService +} + +object OAuthFactory { + + private val oAuth: OAuth + + init { + oAuth = DefaultOAuth() + } + + fun instance() = oAuth +} + +class DefaultOAuth : OAuth { + + companion object { + + private val REDIRECT_URI = + Uri.parse("urn:/audio.funkwhale.funkwhale-android/oauth/callback") + } + data class App(val client_id: String, val client_secret: String) - private val REDIRECT_URI = - Uri.parse("urn:/audio.funkwhale.funkwhale-android/oauth/callback") + override fun tryState(): AuthState? { + + val savedState = PowerPreference + .getFileByName(AppContext.PREFS_CREDENTIALS) + .getString("state") + + return if (savedState != null && savedState.isNotEmpty()) { + return AuthState.jsonDeserialize(savedState) + } else { + null + } + } + + override fun state(): AuthState = tryState()!! - fun isAuthorized(context: Context): Boolean { + override fun isAuthorized(context: Context): Boolean { val state = tryState() return if (state != null) { state.isAuthorized || tryRefreshAccessToken(context) @@ -46,9 +97,10 @@ object OAuth { } } - fun state(): AuthState = tryState()!! - - fun tryRefreshAccessToken(context: Context, overrideNeedsTokenRefresh: Boolean = false): Boolean { + override fun tryRefreshAccessToken( + context: Context, + overrideNeedsTokenRefresh: Boolean + ): Boolean { tryState()?.let { state -> val shouldRefreshAccessToken = overrideNeedsTokenRefresh || state.needsTokenRefresh if (shouldRefreshAccessToken && state.refreshToken != null) { @@ -71,26 +123,13 @@ object OAuth { } } - fun tryState(): AuthState? { - - val savedState = PowerPreference - .getFileByName(AppContext.PREFS_CREDENTIALS) - .getString("state") - - return if (savedState != null && savedState.isNotEmpty()) { - return AuthState.jsonDeserialize(savedState) - } else { - null - } - } - - fun init(hostname: String) { + override fun init(hostname: String) { AuthState(config(hostname)).save() } - fun service(context: Context) = AuthorizationService(context) + override fun service(context: Context): AuthorizationService = AuthorizationService(context) - fun register(callback: () -> Unit) { + override fun register(callback: () -> Unit) { state().authorizationServiceConfiguration?.let { config -> runBlocking { @@ -133,7 +172,7 @@ object OAuth { ) } - fun authorize(context: Activity) { + override fun authorize(context: Activity) { val intent = service(context).run { authorizationRequest()?.let { getAuthorizationRequestIntent(it) @@ -143,7 +182,12 @@ object OAuth { context.startActivityForResult(intent, 0) } - fun exchange(context: Activity, authorization: Intent, success: () -> Unit, error: () -> Unit) { + override fun exchange( + context: Activity, + authorization: Intent, + success: () -> Unit, + error: () -> Unit + ) { state().let { state -> state.apply { update( diff --git a/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt b/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt new file mode 100644 index 0000000..1dd0b7c --- /dev/null +++ b/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt @@ -0,0 +1,75 @@ +package audio.funkwhale.ffa.playback + +import android.content.Context +import android.net.Uri +import audio.funkwhale.ffa.utils.OAuth +import audio.funkwhale.util.MockKJUnitRunner +import com.google.android.exoplayer2.upstream.DataSpec +import com.google.android.exoplayer2.upstream.HttpDataSource +import com.google.android.exoplayer2.upstream.TransferListener +import io.mockk.every +import io.mockk.impl.annotations.InjectMockKs +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.verify +import org.junit.Test +import org.junit.runner.RunWith +import strikt.api.expectThat +import strikt.assertions.isEqualTo + +@RunWith(MockKJUnitRunner::class) +class OAuthDatasourceTest { + + @InjectMockKs + private lateinit var datasource: OAuthDatasource + + @MockK + private lateinit var context: Context + + @MockK + private lateinit var http: HttpDataSource + + @MockK + private lateinit var oAuth: OAuth + + private var dataSpec: DataSpec = DataSpec(Uri.EMPTY) + + @Test + fun `open() should set accessToken and delegate to http dataSource`() { + every { http.open(any()) } returns 0 + every { oAuth.tryRefreshAccessToken(any(), any()) } returns true + every { oAuth.state().accessToken } returns "accessToken" + + datasource.open(dataSpec) + verify { http.open(dataSpec) } + verify { http.setRequestProperty("Authorization", "Bearer accessToken") } + } + + @Test + fun `close() should delegate to http dataSource`() { + datasource.close() + verify { http.close() } + } + + @Test + fun `addTransferListener() should delegate to http dataSource`() { + val transferListener = mockk() + datasource.addTransferListener(transferListener) + verify { http.addTransferListener(transferListener) } + } + + @Test + fun `read() should delegate to http dataSource`() { + every { http.read(any(), any(), any()) } returns 0 + datasource.read("123".encodeToByteArray(), 1, 2) + verify { http.read("123".encodeToByteArray(), 1, 2) } + } + + @Test + fun `getUri() should delegate to http dataSource`() { + every { http.uri } returns Uri.EMPTY + val result = datasource.uri + verify { http.uri } + expectThat(result).isEqualTo(Uri.EMPTY) + } +} diff --git a/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt b/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt new file mode 100644 index 0000000..3965737 --- /dev/null +++ b/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt @@ -0,0 +1,58 @@ +package audio.funkwhale.util + +import io.mockk.MockKAnnotations +import io.mockk.clearAllMocks +import org.junit.Test +import org.junit.runner.Description +import org.junit.runner.Runner +import org.junit.runner.notification.Failure +import org.junit.runner.notification.RunNotifier +import java.lang.reflect.Method + +class MockKJUnitRunner(private val testClass: Class<*>) : Runner() { + + private val methodDescriptions: MutableMap = mutableMapOf() + + init { + // Build method/descriptions map + testClass.methods + .map { method -> + val annotation: Annotation? = method.getAnnotation(Test::class.java) + method to annotation + } + .filter { (_, annotation) -> + annotation != null + } + .map { (method, annotation) -> + val desc = Description.createTestDescription(testClass, method.name, annotation) + method to desc + } + .forEach { (method, desc) -> methodDescriptions[method] = desc } + } + + override fun getDescription(): Description { + val description = Description.createSuiteDescription( + testClass.name, *testClass.annotations + ) + methodDescriptions.values.forEach { description.addChild(it) } + return description + } + + override fun run(notifier: RunNotifier?) { + val testObject = testClass.newInstance() + MockKAnnotations.init(testObject, relaxUnitFun = true) + + methodDescriptions + .onEach { (_, _) -> clearAllMocks() } + .onEach { (_, desc) -> notifier!!.fireTestStarted(desc) } + .forEach { (method, desc) -> + try { + method.invoke(testObject) + } catch (e: Throwable) { + notifier!!.fireTestFailure(Failure(desc, e.cause)) + } finally { + notifier!!.fireTestFinished(desc) + } + } + } +}