From 667d668d7069dcde29cacda5356d90132a3578d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Sat, 22 Feb 2020 19:02:13 +0100 Subject: [PATCH] Do not refresh transaction when it is not being affected. (#246) * Do not refresh transaction when it is not being affected. * Use correct null-aware comparison for HttpTransaction. --- build.gradle | 1 + library/build.gradle | 1 + .../internal/data/entity/HttpTransaction.kt | 33 +++++++ .../HttpTransactionDatabaseRepository.kt | 3 +- .../RecordedThrowableDatabaseRepository.kt | 3 +- .../chucker/internal/support/LiveDataUtils.kt | 34 +++++++ .../java/com/chuckerteam/chucker/TestUtils.kt | 36 ++++++++ .../LiveDataDistinctUntilChangedTest.kt | 88 +++++++++++++++++++ 8 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 library/src/main/java/com/chuckerteam/chucker/internal/support/LiveDataUtils.kt create mode 100644 library/src/test/java/com/chuckerteam/chucker/internal/support/LiveDataDistinctUntilChangedTest.kt diff --git a/build.gradle b/build.gradle index c23cc5415..fc60e5660 100644 --- a/build.gradle +++ b/build.gradle @@ -9,6 +9,7 @@ buildscript { materialComponentsVersion = '1.1.0-rc02' roomVersion = '2.2.3' lifecycleVersion = '2.2.0' + androidXCore = '2.1.0' // Publishing androidMavenGradleVersion = '2.1' diff --git a/library/build.gradle b/library/build.gradle index 67854cc8c..844461541 100644 --- a/library/build.gradle +++ b/library/build.gradle @@ -75,6 +75,7 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-params:$junitVersion" testImplementation "io.mockk:mockk:$mockkVersion" testImplementation "com.squareup.okhttp3:mockwebserver:$okhttp3Version" + testImplementation "androidx.arch.core:core-testing:$androidXCore" } apply from: rootProject.file('gradle/gradle-mvn-push.gradle') diff --git a/library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt b/library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt index b263af001..a731f532b 100644 --- a/library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt +++ b/library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt @@ -214,4 +214,37 @@ internal class HttpTransaction( scheme = url.scheme() return this } + + // Not relying on 'equals' because comparison be long due to request and response sizes + // and it would be unwise to do this every time 'equals' is called. + @Suppress("ComplexMethod") + fun hasTheSameContent(other: HttpTransaction?): Boolean { + if (this === other) return true + if (other == null) return false + + return (id == other.id) && + (requestDate == other.requestDate) && + (responseDate == other.responseDate) && + (tookMs == other.tookMs) && + (protocol == other.protocol) && + (method == other.method) && + (url == other.url) && + (host == other.host) && + (path == other.path) && + (scheme == other.scheme) && + (requestContentLength == other.requestContentLength) && + (requestContentType == other.requestContentType) && + (requestHeaders == other.requestHeaders) && + (requestBody == other.requestBody) && + (isRequestBodyPlainText == other.isRequestBodyPlainText) && + (responseCode == other.responseCode) && + (responseMessage == other.responseMessage) && + (error == other.error) && + (responseContentLength == other.responseContentLength) && + (responseContentType == other.responseContentType) && + (responseHeaders == other.responseHeaders) && + (responseBody == other.responseBody) && + (isResponseBodyPlainText == other.isResponseBodyPlainText) && + responseImageData?.contentEquals(other.responseImageData ?: byteArrayOf()) != false + } } diff --git a/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/HttpTransactionDatabaseRepository.kt b/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/HttpTransactionDatabaseRepository.kt index 8577a8151..385061db1 100644 --- a/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/HttpTransactionDatabaseRepository.kt +++ b/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/HttpTransactionDatabaseRepository.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.LiveData import com.chuckerteam.chucker.internal.data.entity.HttpTransaction import com.chuckerteam.chucker.internal.data.entity.HttpTransactionTuple import com.chuckerteam.chucker.internal.data.room.ChuckerDatabase +import com.chuckerteam.chucker.internal.support.distinctUntilChanged import java.util.concurrent.Executor import java.util.concurrent.Executors @@ -19,7 +20,7 @@ internal class HttpTransactionDatabaseRepository(private val database: ChuckerDa } override fun getTransaction(transactionId: Long): LiveData { - return transcationDao.getById(transactionId) + return transcationDao.getById(transactionId).distinctUntilChanged { old, new -> old.hasTheSameContent(new) } } override fun getSortedTransactionTuples(): LiveData> { diff --git a/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/RecordedThrowableDatabaseRepository.kt b/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/RecordedThrowableDatabaseRepository.kt index ce9674108..bd628594d 100644 --- a/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/RecordedThrowableDatabaseRepository.kt +++ b/library/src/main/java/com/chuckerteam/chucker/internal/data/repository/RecordedThrowableDatabaseRepository.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.LiveData import com.chuckerteam.chucker.internal.data.entity.RecordedThrowable import com.chuckerteam.chucker.internal.data.entity.RecordedThrowableTuple import com.chuckerteam.chucker.internal.data.room.ChuckerDatabase +import com.chuckerteam.chucker.internal.support.distinctUntilChanged import java.util.concurrent.Executor import java.util.concurrent.Executors @@ -14,7 +15,7 @@ internal class RecordedThrowableDatabaseRepository( private val executor: Executor = Executors.newSingleThreadExecutor() override fun getRecordedThrowable(id: Long): LiveData { - return database.throwableDao().getById(id) + return database.throwableDao().getById(id).distinctUntilChanged() } override fun deleteAllThrowables() { diff --git a/library/src/main/java/com/chuckerteam/chucker/internal/support/LiveDataUtils.kt b/library/src/main/java/com/chuckerteam/chucker/internal/support/LiveDataUtils.kt new file mode 100644 index 000000000..5b79194f6 --- /dev/null +++ b/library/src/main/java/com/chuckerteam/chucker/internal/support/LiveDataUtils.kt @@ -0,0 +1,34 @@ +package com.chuckerteam.chucker.internal.support + +import android.annotation.SuppressLint +import androidx.arch.core.executor.ArchTaskExecutor +import androidx.lifecycle.LiveData +import androidx.lifecycle.MediatorLiveData +import java.util.concurrent.Executor + +// Unlike built-in extension operation is performed on a provided thread pool. +// This is needed in our case since we compare requests and responses which can be big +// and result in frame drops. +internal fun LiveData.distinctUntilChanged( + executor: Executor = ioExecutor(), + areEqual: (old: T, new: T) -> Boolean = { old, new -> old == new } +): LiveData { + val distinctMediator = MediatorLiveData() + var old = uninitializedToken + distinctMediator.addSource(this) { new -> + executor.execute { + @Suppress("UNCHECKED_CAST") + if (old === uninitializedToken || !areEqual(old as T, new)) { + old = new + distinctMediator.postValue(new) + } + } + } + return distinctMediator +} + +private val uninitializedToken: Any? = Any() + +// It is lesser evil than providing a custom executor. +@SuppressLint("RestrictedApi") +private fun ioExecutor() = ArchTaskExecutor.getIOThreadExecutor() diff --git a/library/src/test/java/com/chuckerteam/chucker/TestUtils.kt b/library/src/test/java/com/chuckerteam/chucker/TestUtils.kt index f0bfb7398..fa0eeca84 100644 --- a/library/src/test/java/com/chuckerteam/chucker/TestUtils.kt +++ b/library/src/test/java/com/chuckerteam/chucker/TestUtils.kt @@ -1,5 +1,7 @@ package com.chuckerteam.chucker +import androidx.lifecycle.LiveData +import androidx.lifecycle.Observer import java.io.File import okio.Buffer import okio.Okio @@ -9,3 +11,37 @@ fun getResourceFile(file: String): Buffer { writeAll(Okio.buffer(Okio.source(File("./src/test/resources/$file")))) } } + +fun LiveData.test(test: LiveDataRecord.() -> Unit) { + val observer = RecordingObserver() + observeForever(observer) + LiveDataRecord(observer).test() + removeObserver(observer) + observer.records.clear() +} + +class LiveDataRecord internal constructor( + private val observer: RecordingObserver +) { + fun expectData(): T { + if (observer.records.isEmpty()) { + throw AssertionError("Expected data but was empty.") + } + return observer.records.removeAt(0) + } + + fun expectNoData() { + if (observer.records.isNotEmpty()) { + val data = observer.records[0] + throw AssertionError("Expected no data but was $data.") + } + } +} + +internal class RecordingObserver : Observer { + val records = mutableListOf() + + override fun onChanged(data: T) { + records += data + } +} diff --git a/library/src/test/java/com/chuckerteam/chucker/internal/support/LiveDataDistinctUntilChangedTest.kt b/library/src/test/java/com/chuckerteam/chucker/internal/support/LiveDataDistinctUntilChangedTest.kt new file mode 100644 index 000000000..adca4cff7 --- /dev/null +++ b/library/src/test/java/com/chuckerteam/chucker/internal/support/LiveDataDistinctUntilChangedTest.kt @@ -0,0 +1,88 @@ +package com.chuckerteam.chucker.internal.support + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.lifecycle.MutableLiveData +import androidx.lifecycle.distinctUntilChanged +import com.chuckerteam.chucker.test +import junit.framework.TestCase.assertEquals +import org.junit.Rule +import org.junit.Test + +class LiveDataDistinctUntilChangedTest { + @get:Rule val instantExecutorRule = InstantTaskExecutorRule() + + @Test + fun initialUpstreamData_isEmittedDownstream() { + val upstream = MutableLiveData(null) + + upstream.distinctUntilChanged().test { + assertEquals(null, expectData()) + } + } + + @Test + fun emptyUpstream_isNotEmittedDownstream() { + val upstream = MutableLiveData() + + upstream.distinctUntilChanged().test { + expectNoData() + } + } + + @Test + fun newDistinctData_isEmittedDownstream() { + val upstream = MutableLiveData() + + upstream.distinctUntilChanged().test { + upstream.value = 1 + assertEquals(1, expectData()) + + upstream.value = 2 + assertEquals(2, expectData()) + + upstream.value = null + assertEquals(null, expectData()) + + upstream.value = 2 + assertEquals(2, expectData()) + } + } + + @Test + fun newIndistinctData_isNotEmittedDownstream() { + val upstream = MutableLiveData() + + upstream.distinctUntilChanged().test { + upstream.value = null + assertEquals(null, expectData()) + + upstream.value = null + expectNoData() + + upstream.value = "" + assertEquals("", expectData()) + + upstream.value = "" + expectNoData() + } + } + + @Test + fun customFunction_canBeUsedToDistinguishData() { + val upstream = MutableLiveData>() + + upstream.distinctUntilChanged { old, new -> old.first == new.first }.test { + upstream.value = 1 to "" + assertEquals(1 to "", expectData()) + + upstream.value = 1 to "a" + expectNoData() + + upstream.value = 2 to "b" + assertEquals(2 to "b", expectData()) + + upstream.value = 3 to "b" + assertEquals(3 to "b", expectData()) + } + } +}