Merge branch 'add-repo-existing-mirror' into 'master'

Handle mirrors in "Add repo" preview better

See merge request fdroid/fdroidclient!1383
This commit is contained in:
Thore Göbel 2024-04-14 17:46:29 +00:00
commit c85e2f740a
4 changed files with 145 additions and 62 deletions

View File

@ -45,14 +45,15 @@ import org.fdroid.fdroid.Utils.getDownloadRequest
import org.fdroid.fdroid.compose.ComposeUtils.FDroidButton
import org.fdroid.fdroid.compose.ComposeUtils.FDroidContent
import org.fdroid.index.v2.FileV2
import org.fdroid.repo.FetchResult.IsExistingMirror
import org.fdroid.repo.FetchResult.IsExistingRepository
import org.fdroid.repo.FetchResult.IsNewMirror
import org.fdroid.repo.FetchResult.IsNewRepoAndNewMirror
import org.fdroid.repo.FetchResult.IsNewRepository
import org.fdroid.repo.Fetching
@Composable
fun RepoPreviewScreen(paddingValues: PaddingValues, state: Fetching, onAddRepo: () -> Unit) {
val isPreview = LocalInspectionMode.current
val localeList = LocaleListCompat.getDefault()
LazyColumn(
contentPadding = PaddingValues(16.dp),
@ -62,16 +63,19 @@ fun RepoPreviewScreen(paddingValues: PaddingValues, state: Fetching, onAddRepo:
.fillMaxWidth(),
) {
item {
RepoPreviewHeader(state, onAddRepo, localeList, isPreview)
RepoPreviewHeader(state, onAddRepo, localeList)
}
if (state.fetchResult == null || state.fetchResult is IsNewRepository) {
if (state.fetchResult == null
|| state.fetchResult is IsNewRepository
|| state.fetchResult is IsNewRepoAndNewMirror
) {
item {
Row(
verticalAlignment = CenterVertically,
horizontalArrangement = spacedBy(8.dp)
) {
Text(
text = "Included apps:",
text = stringResource(R.string.repo_preview_included_apps),
style = MaterialTheme.typography.body1,
)
Text(
@ -82,7 +86,7 @@ fun RepoPreviewScreen(paddingValues: PaddingValues, state: Fetching, onAddRepo:
}
}
items(items = state.apps, key = { it.packageName }) { app ->
RepoPreviewApp(state.repo ?: error("no repo"), app, localeList, isPreview)
RepoPreviewApp(state.repo ?: error("no repo"), app, localeList)
}
}
}
@ -93,15 +97,15 @@ fun RepoPreviewHeader(
state: Fetching,
onAddRepo: () -> Unit,
localeList: LocaleListCompat,
isPreview: Boolean,
) {
val isDevPreview = LocalInspectionMode.current
Column(
verticalArrangement = spacedBy(8.dp),
modifier = Modifier.fillMaxWidth(),
) {
val repo = state.repo ?: error("repo was null")
Row(
horizontalArrangement = spacedBy(8.dp),
horizontalArrangement = spacedBy(16.dp),
verticalAlignment = CenterVertically,
) {
RepoIcon(repo, Modifier.size(48.dp))
@ -117,28 +121,47 @@ fun RepoPreviewHeader(
style = MaterialTheme.typography.body2,
modifier = Modifier.alpha(ContentAlpha.medium),
)
if (state.isMirror) Text(
text = state.fetchUrl.replace("https://", ""),
style = MaterialTheme.typography.body2,
modifier = Modifier.alpha(ContentAlpha.medium),
)
Text(
text = Utils.formatLastUpdated(LocalContext.current.resources, repo.timestamp),
style = MaterialTheme.typography.body2,
)
}
}
if (state.canAdd) FDroidButton(
text = when (state.fetchResult) {
is IsNewRepository -> stringResource(R.string.repo_add_new_title)
is IsNewMirror -> stringResource(R.string.repo_add_new_mirror)
is IsNewRepoAndNewMirror -> stringResource(R.string.repo_add_repo_and_mirror)
is IsNewMirror -> stringResource(R.string.repo_add_mirror)
else -> error("Unexpected fetch state: ${state.fetchResult}")
},
onClick = onAddRepo,
modifier = Modifier.align(End)
) else if (state.fetchResult is IsExistingRepository) {
Text(
text = stringResource(R.string.repo_exists),
style = MaterialTheme.typography.body1,
color = MaterialTheme.colors.error,
)
}
val description = if (isPreview) {
modifier = Modifier.align(End),
) else Text(
text = when (state.fetchResult) {
is IsExistingRepository -> stringResource(R.string.repo_exists)
is IsExistingMirror -> stringResource(R.string.repo_mirror_exists)
else -> error("Unexpected fetch state: ${state.fetchResult}")
},
style = MaterialTheme.typography.body1,
color = MaterialTheme.colors.error,
)
if (state.isMirror) Text(
text = when (state.fetchResult) {
is IsNewRepoAndNewMirror -> stringResource(R.string.repo_and_mirror_add_both_info)
is IsNewMirror, IsExistingMirror -> stringResource(R.string.repo_mirror_add_info)
else -> error("Unexpected fetch state: ${state.fetchResult}")
},
style = MaterialTheme.typography.body2,
)
val description = if (isDevPreview) {
LoremIpsum(42).values.joinToString(" ")
} else {
repo.getDescription(localeList)
@ -156,8 +179,8 @@ fun LazyItemScope.RepoPreviewApp(
repo: Repository,
app: MinimalApp,
localeList: LocaleListCompat,
isPreview: Boolean,
) {
val isDevPreview = LocalInspectionMode.current
Card(
modifier = Modifier
.animateItemPlacement()
@ -167,7 +190,7 @@ fun LazyItemScope.RepoPreviewApp(
horizontalArrangement = spacedBy(8.dp),
modifier = Modifier.padding(8.dp),
) {
if (isPreview) Image(
if (isDevPreview) Image(
painter = rememberDrawablePainter(
getDrawable(LocalContext.current.resources, R.drawable.ic_launcher, null)
),
@ -197,7 +220,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 +249,7 @@ fun RepoPreviewScreenFetchingPreview() {
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching(repo, listOf(app1, app2, app3), IsNewRepository("foo"))
Fetching(address, repo, listOf(app1, app2, app3), IsNewRepository)
) {}
}
}
@ -237,7 +261,19 @@ fun RepoPreviewScreenNewMirrorPreview() {
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching(repo, emptyList(), IsNewMirror(0L, "foo"))
Fetching("https://mirror.example.org", repo, emptyList(), IsNewMirror(0L))
) {}
}
}
@Composable
@Preview
fun RepoPreviewScreenNewRepoAndNewMirrorPreview() {
val repo = FDroidApp.createSwapRepo("https://example.org", "foo bar")
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching("https://mirror.example.org", repo, emptyList(), IsNewRepoAndNewMirror)
) {}
}
}
@ -245,8 +281,24 @@ 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)
) {}
}
}
@Preview
@Composable
fun RepoPreviewScreenExistingMirrorPreview() {
val repo = FDroidApp.createSwapRepo("https://example.org", "foo bar")
FDroidContent {
RepoPreviewScreen(
PaddingValues(0.dp),
Fetching("https://mirror.example.org", repo, emptyList(), IsExistingMirror)
) {}
}
}

View File

@ -188,7 +188,7 @@ This often occurs with apps installed via Google Play or other sources, if they
<string name="repo_add_new_title">Add repository</string>
<string name="repo_add_add">Add</string>
<string name="repo_add_mirror">Add mirror</string>
<string name="repo_add_new_mirror">Add as new mirror</string>
<string name="repo_add_repo_and_mirror">Add repository and mirror</string>
<string name="links">Links</string>
<string name="versions">Versions</string>
<string name="more">More</string>
@ -212,6 +212,7 @@ This often occurs with apps installed via Google Play or other sources, if they
<string name="repo_enter_url">Enter repository URL manually</string>
<string name="repo_add_url">Repository address</string>
<string name="repo_add_fingerprint">Fingerprint (optional)</string>
<string name="repo_preview_included_apps">Included apps:</string>
<string name="repo_exists">This repo was already added.</string>
<string name="repo_exists_add_fingerprint">%1$s is already setup, this will add new key information.</string>
<string name="repo_exists_enable">%1$s is already setup, confirm that you want to re-enable it.</string>
@ -222,6 +223,9 @@ This often occurs with apps installed via Google Play or other sources, if they
<string name="repo_io_error">Error connecting to the repository.</string>
<string name="repo_error_adding_archive">Archive repositories can not be added directly. Tap the repository in the list and enable the archive there.</string>
<string name="repo_share_not_found">Could not find repo address in shared text.</string>
<string name="repo_mirror_exists">This mirror was already added.</string>
<string name="repo_mirror_add_info">The URL you are trying to add is a mirror of an existing repository.</string>
<string name="repo_and_mirror_add_both_info">The URL you are trying to add is a mirror of a new repository. Both the original repository and the mirror will be added.</string>
<string name="bad_fingerprint">Bad fingerprint</string>
<string name="invalid_url">This is not a valid URL.</string>
<string name="malformed_repo_uri">Ignoring malformed repo URI: %s</string>

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?,
@ -62,11 +63,20 @@ public class Fetching(
* true if the repository can be added (be it as new [Repository] or new mirror).
*/
public val canAdd: Boolean = repo != null &&
(fetchResult != null && fetchResult !is FetchResult.IsExistingRepository)
fetchResult != null &&
fetchResult !is FetchResult.IsExistingRepository &&
fetchResult !is FetchResult.IsExistingMirror
public val isMirror: Boolean = repo != null &&
fetchResult != null &&
(fetchResult is FetchResult.IsNewMirror ||
fetchResult is FetchResult.IsExistingMirror ||
fetchResult is FetchResult.IsNewRepoAndNewMirror
)
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 +100,12 @@ 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 object IsNewRepoAndNewMirror : FetchResult()
public data class IsNewMirror(internal val existingRepoId: Long) : FetchResult()
public object IsExistingRepository : FetchResult()
public data object IsExistingRepository : FetchResult()
public data object IsExistingMirror : FetchResult()
}
@OptIn(DelicateCoroutinesApi::class)
@ -147,11 +156,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 +172,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 +209,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,19 +241,24 @@ 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)
} else {
val existingMirror = if (existingRepo.address.trimEnd('/') == url) {
url
val isUserMirror = url.trimEnd('/') != repo.address.trimEnd('/') &&
repo.mirrors.find { url.trimEnd('/') == it.url.trimEnd('/') } == null
if (isUserMirror) {
FetchResult.IsNewRepoAndNewMirror
} else {
existingRepo.mirrors.find { it.url.trimEnd('/') == url }
?: existingRepo.userMirrors.find { it.trimEnd('/') == url }
FetchResult.IsNewRepository
}
} else if (existingRepo.address.trimEnd('/') == url) {
FetchResult.IsExistingRepository
} else {
val existingMirror = existingRepo.mirrors.find { it.url.trimEnd('/') == url }
?: existingRepo.userMirrors.find { it.trimEnd('/') == url }
if (existingMirror == null) {
FetchResult.IsNewMirror(existingRepo.repoId, url)
FetchResult.IsNewMirror(existingRepo.repoId)
} else {
FetchResult.IsExistingRepository
FetchResult.IsExistingMirror
}
}
}
@ -265,8 +281,9 @@ internal class RepoAdder(
?: throw IllegalStateException("No fetchResult: ${addRepoState.value}")
val modifiedRepo: Repository = when (fetchResult) {
is FetchResult.IsExistingRepository -> error("Unexpected result: $fetchResult")
is FetchResult.IsNewRepository -> {
is FetchResult.IsExistingRepository -> error("Repo exists: $fetchResult")
is FetchResult.IsExistingMirror -> error("Mirror exists: $fetchResult")
is FetchResult.IsNewRepository, is FetchResult.IsNewRepoAndNewMirror -> {
// reset the timestamp of the actual repo,
// so a following repo update will pick this up
val newRepo = NewRepository(
@ -279,12 +296,13 @@ internal class RepoAdder(
password = repo.password,
)
db.runInTransaction<Repository> {
// add the repo
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
) {
val userMirrors = listOf(fetchResult.addUrl)
// add user mirror
// this can happen if the user was adding a mirror URL, and they originally had neither the repo nor the mirror added
if (fetchResult is FetchResult.IsNewRepoAndNewMirror) {
val userMirrors = listOf(state.fetchUrl)
repositoryDao.updateUserMirrors(repoId, userMirrors)
}
repositoryDao.getRepository(repoId) ?: error("New repository not found in DB")
@ -297,7 +315,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 +337,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.IsNewRepoAndNewMirror) {
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)
}
@ -358,8 +358,12 @@ internal class RepoAdderTest {
// repo is already in the DB
every { repoDao.getRepository(any<String>()) } returns existingRepo
expectMinRepoPreview(repoName, FetchResult.IsExistingRepository, canAdd = false) {
repoAdder.fetchRepository(url = url, proxy = null)
val expectedFetchResult =
if (existingRepo.address == url) FetchResult.IsExistingRepository
else FetchResult.IsExistingMirror
expectMinRepoPreview(repoName, expectedFetchResult, canAdd = false) {
repoAdder.fetchRepository(url = downloadUrl, proxy = null)
}
assertFailsWith<IllegalStateException> {
repoAdder.addFetchedRepository()
@ -783,7 +787,7 @@ internal class RepoAdderTest {
// repo not in DB
every { repoDao.getRepository(any<String>()) } returns null
expectMinRepoPreview(repoName, FetchResult.IsNewRepository(urlTrimmed)) {
expectMinRepoPreview(repoName, FetchResult.IsNewRepoAndNewMirror) {
repoAdder.fetchRepository(url = url, proxy = null)
}
@ -857,7 +861,7 @@ internal class RepoAdderTest {
private suspend fun expectMinRepoPreview(
repoName: String?,
fetchResult: FetchResult,
expectedFetchResult: FetchResult,
canAdd: Boolean = true,
block: suspend () -> Unit = {},
) {
@ -871,6 +875,7 @@ internal class RepoAdderTest {
block()
}
// early empty state
val state1 = awaitItem()
assertIs<Fetching>(state1)
assertNull(state1.repo)
@ -878,17 +883,19 @@ internal class RepoAdderTest {
assertFalse(state1.canAdd)
assertFalse(state1.done)
// onRepoReceived
val state2 = awaitItem()
assertIs<Fetching>(state2)
val repo = state2.repo ?: fail()
assertEquals(TestDataMinV2.repo.address, repo.address)
assertEquals(repoName, repo.getName(localeList))
val result = state2.fetchResult ?: fail()
assertEquals(fetchResult, result)
assertEquals(expectedFetchResult, result)
assertTrue(state2.apps.isEmpty())
assertEquals(canAdd, state2.canAdd)
assertFalse(state2.done)
// onAppReceived
val state3 = awaitItem()
assertIs<Fetching>(state3)
assertEquals(TestDataMinV2.packages.size, state3.apps.size)
@ -896,6 +903,7 @@ internal class RepoAdderTest {
assertEquals(canAdd, state3.canAdd)
assertFalse(state3.done)
// final result
val state4 = awaitItem()
assertIs<Fetching>(state4)
assertEquals(canAdd, state4.canAdd)