Refactor: Move fetchUrl up from FetchResult into Fetching

The fetchUrl is independent of the result, it better belongs into Fetching.
This commit is contained in:
Thore Goebel 2024-04-14 15:05:25 +02:00
parent 429eae6f9e
commit 688dcec7f4
3 changed files with 36 additions and 28 deletions

View File

@ -197,7 +197,8 @@ fun LazyItemScope.RepoPreviewApp(
@Preview
@Composable
fun RepoPreviewScreenFetchingPreview() {
val repo = FDroidApp.createSwapRepo("https://example.org", "foo bar")
val address = "https://example.org"
val repo = FDroidApp.createSwapRepo(address, "foo bar")
val app1 = object : MinimalApp {
override val repoId = 0L
override val packageName = "org.example"
@ -225,7 +226,7 @@ fun RepoPreviewScreenFetchingPreview() {
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching(repo, listOf(app1, app2, app3), IsNewRepository("foo"))
Fetching(address, repo, listOf(app1, app2, app3), IsNewRepository)
) {}
}
}
@ -233,11 +234,12 @@ fun RepoPreviewScreenFetchingPreview() {
@Composable
@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES, widthDp = 720, heightDp = 360)
fun RepoPreviewScreenNewMirrorPreview() {
val repo = FDroidApp.createSwapRepo("https://example.org", "foo bar")
val address = "https://example.org"
val repo = FDroidApp.createSwapRepo(address, "foo bar")
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching(repo, emptyList(), IsNewMirror(0L, "foo"))
Fetching(address, repo, emptyList(), IsNewMirror(0L))
) {}
}
}
@ -245,8 +247,12 @@ fun RepoPreviewScreenNewMirrorPreview() {
@Preview
@Composable
fun RepoPreviewScreenExistingRepoPreview() {
val repo = FDroidApp.createSwapRepo("https://example.org", "foo bar")
val address = "https://example.org"
val repo = FDroidApp.createSwapRepo(address, "foo bar")
FDroidContent {
RepoPreviewScreen(PaddingValues(0.dp), Fetching(repo, emptyList(), IsExistingRepository)) {}
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching(address, repo, emptyList(), IsExistingRepository)
) {}
}
}

View File

@ -50,6 +50,7 @@ public sealed class AddRepoState
public object None : AddRepoState()
public class Fetching(
public val fetchUrl: String,
public val repo: Repository?,
public val apps: List<MinimalApp>,
public val fetchResult: FetchResult?,
@ -65,8 +66,8 @@ public class Fetching(
(fetchResult != null && fetchResult !is FetchResult.IsExistingRepository)
override fun toString(): String {
return "Fetching(repo=${repo?.address}, apps=${apps.size}, fetchResult=$fetchResult, " +
"done=$done, canAdd=$canAdd)"
return "Fetching(fetchUrl=$fetchUrl, repo=${repo?.address}, apps=${apps.size}, " +
"fetchResult=$fetchResult, done=$done, canAdd=$canAdd)"
}
}
@ -90,13 +91,10 @@ public data class AddRepoError(
}
public sealed class FetchResult {
public data class IsNewRepository(internal val addUrl: String) : FetchResult()
public data class IsNewMirror(
internal val existingRepoId: Long,
internal val newMirrorUrl: String,
) : FetchResult()
public data object IsNewRepository : FetchResult()
public data class IsNewMirror(internal val existingRepoId: Long) : FetchResult()
public object IsExistingRepository : FetchResult()
public data object IsExistingRepository : FetchResult()
}
@OptIn(DelicateCoroutinesApi::class)
@ -147,11 +145,13 @@ internal class RepoAdder(
addRepoState.value = AddRepoError(IS_ARCHIVE_REPO)
return
}
val fetchUrl = nUri.uri.toString()
// some plumping to receive the repo preview
var receivedRepo: Repository? = null
val apps = ArrayList<AppOverviewItem>()
var fetchResult: FetchResult? = null
val receiver = object : RepoPreviewReceiver {
override fun onRepoReceived(repo: Repository) {
receivedRepo = repo
@ -161,17 +161,17 @@ internal class RepoAdder(
"Known fingerprint different from given one: ${repo.fingerprint}"
)
}
fetchResult = getFetchResult(nUri.uri.toString(), repo)
addRepoState.value = Fetching(receivedRepo, apps.toList(), fetchResult)
fetchResult = getFetchResult(fetchUrl, repo)
addRepoState.value = Fetching(fetchUrl, receivedRepo, apps.toList(), fetchResult)
}
override fun onAppReceived(app: AppOverviewItem) {
apps.add(app)
addRepoState.value = Fetching(receivedRepo, apps.toList(), fetchResult)
addRepoState.value = Fetching(fetchUrl, receivedRepo, apps.toList(), fetchResult)
}
}
// set a state early, so the ui can show progress animation
addRepoState.value = Fetching(receivedRepo, apps, fetchResult)
addRepoState.value = Fetching(fetchUrl, receivedRepo, apps, fetchResult)
// try fetching repo with v2 format first and fallback to v1
try {
@ -198,7 +198,7 @@ internal class RepoAdder(
if (finalRepo == null) {
addRepoState.value = AddRepoError(INVALID_INDEX)
} else {
addRepoState.value = Fetching(finalRepo, apps, fetchResult, done = true)
addRepoState.value = Fetching(fetchUrl, finalRepo, apps, fetchResult, done = true)
}
}
@ -230,8 +230,9 @@ internal class RepoAdder(
private fun getFetchResult(url: String, repo: Repository): FetchResult {
val cert = repo.certificate ?: error("Certificate was null")
val existingRepo = repositoryDao.getRepository(cert)
return if (existingRepo == null) {
FetchResult.IsNewRepository(url)
FetchResult.IsNewRepository
} else {
val existingMirror = if (existingRepo.address.trimEnd('/') == url) {
url
@ -240,7 +241,7 @@ internal class RepoAdder(
?: existingRepo.userMirrors.find { it.trimEnd('/') == url }
}
if (existingMirror == null) {
FetchResult.IsNewMirror(existingRepo.repoId, url)
FetchResult.IsNewMirror(existingRepo.repoId)
} else {
FetchResult.IsExistingRepository
}
@ -281,10 +282,10 @@ internal class RepoAdder(
db.runInTransaction<Repository> {
val repoId = repositoryDao.insert(newRepo)
// add user mirror, if URL is not the repo address and not a known mirror
if (fetchResult.addUrl != repo.address.trimEnd('/') &&
repo.mirrors.find { fetchResult.addUrl == it.url.trimEnd('/') } == null
if (state.fetchUrl != repo.address.trimEnd('/') &&
repo.mirrors.find {state.fetchUrl == it.url.trimEnd('/') } == null
) {
val userMirrors = listOf(fetchResult.addUrl)
val userMirrors = listOf(state.fetchUrl)
repositoryDao.updateUserMirrors(repoId, userMirrors)
}
repositoryDao.getRepository(repoId) ?: error("New repository not found in DB")
@ -297,7 +298,7 @@ internal class RepoAdder(
val existingRepo = repositoryDao.getRepository(repoId)
?: error("No repo with $repoId")
val userMirrors = existingRepo.userMirrors.toMutableList().apply {
add(fetchResult.newMirrorUrl)
add(state.fetchUrl)
}
repositoryDao.updateUserMirrors(repoId, userMirrors)
existingRepo
@ -319,6 +320,7 @@ internal class RepoAdder(
if (repo.isArchiveRepo) error { "Repo ${repo.address} is already an archive repo." }
val address = repo.address.replace(Regex("repo/?$"), "archive")
@Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")
val receiver = object : RepoPreviewReceiver {
override fun onRepoReceived(archiveRepo: Repository) {

View File

@ -144,7 +144,7 @@ internal class RepoAdderTest {
// repo not in DB
every { repoDao.getRepository(any<String>()) } returns null
expectMinRepoPreview(repoName, FetchResult.IsNewRepository(urlTrimmed)) {
expectMinRepoPreview(repoName, FetchResult.IsNewRepository) {
repoAdder.fetchRepository(url = url, proxy = null)
}
@ -206,7 +206,7 @@ internal class RepoAdderTest {
)
every { repoDao.getRepository(any<String>()) } returns existingRepo
expectMinRepoPreview(repoName, FetchResult.IsNewMirror(42L, url.trimEnd('/'))) {
expectMinRepoPreview(repoName, FetchResult.IsNewMirror(42L)) {
repoAdder.fetchRepository(url = url, proxy = null)
}
@ -783,7 +783,7 @@ internal class RepoAdderTest {
// repo not in DB
every { repoDao.getRepository(any<String>()) } returns null
expectMinRepoPreview(repoName, FetchResult.IsNewRepository(urlTrimmed)) {
expectMinRepoPreview(repoName, FetchResult.IsNewRepository) {
repoAdder.fetchRepository(url = url, proxy = null)
}