优先选择偏好类型数据源，不需要等其他源查询完毕 #1002

NieR4ever · 2024-09-24T06:31:19Z

resolve #816

Him188

需要增加测试, 当一个源完成后就立即选择

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

app/shared/app-data/src/commonMain/kotlin/data/source/media/selector/MediaSelector.kt

NieR4ever · 2024-09-25T16:31:04Z

不清楚怎么测试，感觉比较难观察 hasCompleted 的变化

Him188

Looks good, 但是需要有 test 确保以后不会 regress

需要测试:

prefer BT 时, WEB 完成了, 应当继续等待
prefer BT 时, BT 完成了, 应当自动选择
prefer BT 时, 没有启用 BT 源, 会怎么样?
prefer BT 时, 但 BT 没有搜到东西, 是什么行为?
没有 preferefence, BT 完成了但 web 没有完成, 应当继续等待
没有 preferefence, 都完成了, 自动选择 (应该是现有 case?)

Him188 · 2024-09-25T17:28:43Z

test 中协程是单线程的, 你可以控制的

在数据源实现中, 用 CompletableDeferred<Unit> 来等待. 然后你在后面可以 deferred.complete(Unit), 然后 runCurrent() 让 fetch 的协程跑.

Him188 · 2024-09-27T20:18:42Z

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

+@JvmInline
+value class CompletedConditions(


Suggested change

@JvmInline

value class CompletedConditions(

class CompletedConditions(

这个东西看起来会用在被 box 的地方, 那其实不需要 value

Him188 · 2024-09-27T20:21:17Z

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

+ states.all { it is MediaSourceFetchState.Disabled } -> null
+ states.all { it is MediaSourceFetchState.Completed || it is MediaSourceFetchState.Disabled } -> true
+ else -> false


为什么要有 null? 不能用 false 吗

Him188 · 2024-09-27T20:23:01Z

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

+ }
+ }
+
+ combine(map) { pairs ->


目前两个 combine 的算法会导致启动数倍多的协程

用一个 combine 就足够解决问题了, 让 combine 的 lambda 返回 ImmutableEnumMap<MSK, Boolean>

Him188 · 2024-09-27T20:27:20Z

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

 override val hasCompleted = if (mediaSourceResults.isEmpty()) {
- flowOf(true)
+ flowOf(CompletedConditions.AllCompleted)
 } else {
- combine(mediaSourceResults.map { it.state }) { states ->
- states.all { it is MediaSourceFetchState.Completed || it is MediaSourceFetchState.Disabled }
+ val map = MediaSourceKind.entries.map { kind ->


目前这样改会为那些原本总是期待所有数据源加载完成的代码增加计算量 (计算是否完成并构造 EnumMap 等), 导致性能变差

这是可以避免的, 我觉得可能要做一个另外的属性来支持中间状态

Him188 · 2024-09-27T20:27:44Z

app/shared/app-data/src/commonMain/kotlin/data/source/media/fetch/MediaFetcher.kt

+ null
+ }
+
+ fun copy(


未使用, 没必要有

Him188 · 2024-09-27T20:32:49Z

app/shared/src/desktopTest/kotlin/data/source/media/selector/MediaSelectorAutoSelectTest.kt

+ val mediaList = mutableListOf<DefaultMedia>()
+ if (webEnabled) mediaList.addAll(TestMediaList.map { it.copy(kind = MediaSourceKind.WEB) })
+ if (btEnabled) mediaList.addAll(TestMediaList.map { it.copy(kind = MediaSourceKind.BitTorrent) })
+ this.mediaList.value = mediaList


这看起来可能会意外地影响 test 的结果
我感觉应当至少保持一个其他类型的

Him188 · 2024-09-27T20:34:09Z

app/shared/src/desktopTest/kotlin/data/source/media/selector/MediaSelectorAutoSelectTest.kt

+ @Test
+ fun `awaitCompletedAndSelectDefault selects one when prefer bt and bt done`() = runTest {


我觉得要新加一个 section 表示这些是来测试 "
优先选择偏好类型数据源，不需要等其他源查询完毕" 的, 否则未来的人可能不会知道这些 case 的用途

Him188 · 2024-09-27T20:34:39Z

app/shared/src/desktopTest/kotlin/data/source/media/selector/MediaSelectorAutoSelectTest.kt

+ ),
+ ),
+ ),
+ flowContext = EmptyCoroutineContext,


memory leak

Him188 · 2024-09-27T20:36:06Z

app/shared/src/desktopTest/kotlin/data/source/media/selector/MediaSelectorAutoSelectTest.kt

+ val mediaFetcher = MediaSourceMediaFetcher(
+ configProvider = { MediaFetcherConfig.Default },


当用处超过两个就最好弄一个 createMediaSourceFetcher. 否则未来给 MediaSourceMediaFetcher 增加参数时要修改很多地方. 如果有 createMediaSourceFetcher 就只需要改一个地方

Him188 · 2024-09-27T20:39:19Z

app/shared/src/desktopTest/kotlin/data/source/media/selector/MediaSelectorAutoSelectTest.kt

+ btEnabled: Boolean = true,
+ webEnabled: Boolean = true,


Suggested change

btEnabled: Boolean = true,

webEnabled: Boolean = true,

addBtSources: Boolean = true,

addWebSources: Boolean = true,

说明它的直接意图

auto select

8744284

Him188 self-requested a review September 24, 2024 09:26

Him188 added the s: media 子系统: 数据源基础 label Sep 24, 2024

Him188 requested changes Sep 24, 2024

View reviewed changes

NieR4ever added 4 commits September 25, 2024 15:08

optimize something

14ba6db

add argument preferKind

91c3cfb

use value class

bb31dcd

fix bug

98f3411

NieR4ever requested a review from Him188 September 25, 2024 16:33

Him188 reviewed Sep 25, 2024

View reviewed changes

fix bug and add test

358eec8

NieR4ever requested a review from Him188 September 27, 2024 12:20

Him188 reviewed Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

优先选择偏好类型数据源，不需要等其他源查询完毕 #1002

优先选择偏好类型数据源，不需要等其他源查询完毕 #1002

NieR4ever commented Sep 24, 2024

Him188 left a comment

NieR4ever commented Sep 25, 2024

Him188 left a comment

Him188 commented Sep 25, 2024 •

edited

Loading

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

Him188 Sep 27, 2024

	@JvmInline
	value class CompletedConditions(
	class CompletedConditions(

		@Test
		fun `awaitCompletedAndSelectDefault selects one when prefer bt and bt done`() = runTest {

		val mediaFetcher = MediaSourceMediaFetcher(
		configProvider = { MediaFetcherConfig.Default },

优先选择偏好类型数据源，不需要等其他源查询完毕 #1002

Are you sure you want to change the base?

优先选择偏好类型数据源，不需要等其他源查询完毕 #1002

Conversation

NieR4ever commented Sep 24, 2024

Him188 left a comment

Choose a reason for hiding this comment

NieR4ever commented Sep 25, 2024

Him188 left a comment

Choose a reason for hiding this comment

Him188 commented Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Him188 commented Sep 25, 2024 •

edited

Loading