diff --git a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorScreen.kt b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorScreen.kt index 95d2cfe..c99a702 100644 --- a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorScreen.kt +++ b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorScreen.kt @@ -58,7 +58,6 @@ import androidx.compose.runtime.mutableDoubleStateOf import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -895,19 +894,21 @@ private fun AddActionDialog( var notifyTitle by remember { mutableStateOf("情景完成") } var notifyMsg by remember { mutableStateOf("自动化任务执行完成") } - val coroutineScope = rememberCoroutineScope() - LaunchedEffect(availableRemotes, selectedRemoteIndex) { if (availableRemotes.isNotEmpty() && selectedRemoteIndex in availableRemotes.indices) { val remote = availableRemotes[selectedRemoteIndex] - coroutineScope.launch { - val bindings = viewModel.getKeyBindings(remote.id) - keyBindings = bindings - if (bindings.isNotEmpty()) { - selectedButtonId = bindings[0].buttonId - selectedButtonName = bindings[0].displayName.ifBlank { bindings[0].buttonId } - } + keyBindings = emptyList() + val bindings = viewModel.getKeyBindings(remote.id) + keyBindings = bindings + if (bindings.isNotEmpty()) { + selectedButtonId = bindings[0].buttonId + selectedButtonName = bindings[0].displayName.ifBlank { bindings[0].buttonId } + } else { + selectedButtonId = "power" + selectedButtonName = "电源" } + } else { + keyBindings = emptyList() } } diff --git a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModel.kt b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModel.kt index 66127a0..17d5dcf 100644 --- a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModel.kt +++ b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModel.kt @@ -54,7 +54,7 @@ data class SceneEditorState( val isSaved: Boolean = false, val errorMessage: String? = null, ) { - val isValid: Boolean get() = name.isNotBlank() && actions.isNotEmpty() + val isValid: Boolean get() = name.isNotBlank() && actions.isNotEmpty() && (type != AutomationType.AUTOMATIC || triggers.isNotEmpty()) val isEditing: Boolean get() = sceneId != null } @@ -68,6 +68,9 @@ class SceneEditorViewModel @Inject constructor( private val _state = MutableStateFlow(SceneEditorState()) val state: StateFlow = _state.asStateFlow() + private var initializedSceneId: SceneId? = null + private var isInitialized: Boolean = false + init { observeAvailableRemotes() } @@ -87,6 +90,10 @@ class SceneEditorViewModel @Inject constructor( } fun initialize(sceneId: SceneId?, initialType: AutomationType? = null) { + if (isInitialized && initializedSceneId == sceneId) return + isInitialized = true + initializedSceneId = sceneId + if (sceneId == null) { _state.update { SceneEditorState( diff --git a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModel.kt b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModel.kt index f2acfac..8bc1c00 100644 --- a/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModel.kt +++ b/android/app/src/main/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModel.kt @@ -34,7 +34,13 @@ class SceneExecutionViewModel @Inject constructor( private val _uiState = MutableStateFlow(SceneExecutionUiState.Loading) val uiState: StateFlow = _uiState.asStateFlow() + private var initializedSceneId: SceneId? = null + fun initialize(sceneId: SceneId) { + if (initializedSceneId == sceneId && _uiState.value !is SceneExecutionUiState.Loading) { + return + } + initializedSceneId = sceneId _uiState.value = SceneExecutionUiState.Loading viewModelScope.launch { val profile = profileStore.activeProfile() @@ -53,11 +59,18 @@ class SceneExecutionViewModel @Inject constructor( fun execute() { val current = _uiState.value - val scene = (current as? SceneExecutionUiState.Ready)?.scene - ?: (current as? SceneExecutionUiState.Running)?.scene - ?: return + val scene = when (current) { + is SceneExecutionUiState.Ready -> current.scene + is SceneExecutionUiState.Completed -> current.result.scene + else -> return + } _uiState.value = SceneExecutionUiState.Running(scene) viewModelScope.launch { + val profile = profileStore.activeProfile() + if (profile == null || profile.id != scene.profileId) { + _uiState.value = SceneExecutionUiState.Failed("Active profile mismatch") + return@launch + } when (val result = sceneExecutor.execute(scene)) { is DomainResult.Success -> _uiState.value = SceneExecutionUiState.Completed(result.value) is DomainResult.Failure -> _uiState.value = SceneExecutionUiState.Failed( diff --git a/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModelTest.kt b/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModelTest.kt index cd02189..863c6fa 100644 --- a/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModelTest.kt +++ b/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneEditorViewModelTest.kt @@ -158,5 +158,39 @@ class SceneEditorViewModelTest { assertTrue(viewModel.state.value.isSaved) assertFalse(viewModel.state.value.isSaving) } + + @Test + fun automaticSceneWithoutTriggersIsInvalid() = runTest { + setupProfile() + val viewModel = SceneEditorViewModel(sceneRepository, virtualRemoteRepository, profileStore) + viewModel.initialize(null, AutomationType.AUTOMATIC) + viewModel.updateName("Auto Without Trigger") + viewModel.addSocketAction("socket-1", SocketPowerState.ON) + advanceUntilIdle() + + // Missing triggers -> invalid + assertFalse(viewModel.state.value.isValid) + + // Adding trigger makes it valid + viewModel.addTemperatureTrigger("sensor-1", ComparisonOperator.GREATER_THAN, 28.0) + advanceUntilIdle() + assertTrue(viewModel.state.value.isValid) + } + + @Test + fun initializeIsIdempotentAndPreservesDraftState() = runTest { + setupProfile() + val viewModel = SceneEditorViewModel(sceneRepository, virtualRemoteRepository, profileStore) + viewModel.initialize(existingScene.id) + advanceUntilIdle() + + viewModel.updateName("Modified Unsaved Name") + assertEquals("Modified Unsaved Name", viewModel.state.value.name) + + // Re-invoking initialize with same sceneId should NOT overwrite draft + viewModel.initialize(existingScene.id) + advanceUntilIdle() + assertEquals("Modified Unsaved Name", viewModel.state.value.name) + } } diff --git a/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModelTest.kt b/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModelTest.kt index 02eb348..0a4e444 100644 --- a/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModelTest.kt +++ b/android/app/src/test/kotlin/com/flagship/abox/manager/ui/scene/SceneExecutionViewModelTest.kt @@ -87,4 +87,39 @@ class SceneExecutionViewModelTest { assertEquals(testScene.id, completed.result.scene.id) assertEquals(1, completed.result.results.size) } + + @Test + fun initializeIsIdempotentAndPreservesCompletedState() = runTest { + setupProfile() + val viewModel = SceneExecutionViewModel(sceneRepository, sceneExecutor, profileStore) + viewModel.initialize(testScene.id) + advanceUntilIdle() + + viewModel.execute() + advanceUntilIdle() + assertTrue(viewModel.uiState.value is SceneExecutionUiState.Completed) + + // Re-initializing with same sceneId should preserve Completed state + viewModel.initialize(testScene.id) + advanceUntilIdle() + assertTrue(viewModel.uiState.value is SceneExecutionUiState.Completed) + } + + @Test + fun executeFailsWhenActiveProfileMismatches() = runTest { + setupProfile() + val viewModel = SceneExecutionViewModel(sceneRepository, sceneExecutor, profileStore) + viewModel.initialize(testScene.id) + advanceUntilIdle() + + // Switch to a different profile + profileStore.setActiveProfile(ProviderProfileId("different-profile")) + + viewModel.execute() + advanceUntilIdle() + + assertTrue(viewModel.uiState.value is SceneExecutionUiState.Failed) + val failed = viewModel.uiState.value as SceneExecutionUiState.Failed + assertEquals("Active profile mismatch", failed.message) + } } diff --git a/domain/src/main/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutor.kt b/domain/src/main/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutor.kt index 162f2eb..127a5bd 100644 --- a/domain/src/main/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutor.kt +++ b/domain/src/main/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutor.kt @@ -2,8 +2,12 @@ package com.flagship.abox.manager.domain import com.flagship.abox.manager.domain.infrared.SendVirtualRemoteAction import com.flagship.abox.manager.domain.infrared.VirtualRemoteId -import kotlinx.coroutines.delay import java.util.concurrent.ConcurrentHashMap +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.delay /** * Production implementation of [SceneExecutor]. @@ -29,32 +33,47 @@ class DefaultSceneExecutor( ) } val startTime = System.currentTimeMillis() - val results = mutableListOf() - var aborted = false return try { - for (action in scene.actions) { - if (aborted) { - results.add( - SceneActionExecutionResult( - action = action, - outcome = SceneActionOutcome.SKIPPED, - error = null, - ), - ) - continue + val results: List = if (scene.concurrencyMode == ConcurrencyMode.PARALLEL) { + coroutineScope { + scene.actions.map { action -> + async { + val actionStart = System.currentTimeMillis() + val execResult = executeAction(scene.profileId, action) + val duration = System.currentTimeMillis() - actionStart + execResult.copy(executionDurationMillis = duration) + } + }.awaitAll() } + } else { + val sequentialResults = mutableListOf() + var aborted = false + for (action in scene.actions) { + if (aborted) { + sequentialResults.add( + SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.SKIPPED, + error = null, + ), + ) + continue + } - val actionStart = System.currentTimeMillis() - val execResult = executeAction(scene.profileId, action) - val duration = System.currentTimeMillis() - actionStart - val enrichedResult = execResult.copy(executionDurationMillis = duration) - results.add(enrichedResult) + val actionStart = System.currentTimeMillis() + val execResult = executeAction(scene.profileId, action) + val duration = System.currentTimeMillis() - actionStart + val enrichedResult = execResult.copy(executionDurationMillis = duration) + sequentialResults.add(enrichedResult) - if (enrichedResult.outcome == SceneActionOutcome.FAILURE && scene.strategy == ExecutionStrategy.ABORT_ON_ERROR) { - aborted = true + if (enrichedResult.outcome == SceneActionOutcome.FAILURE && scene.strategy == ExecutionStrategy.ABORT_ON_ERROR) { + aborted = true + } } + sequentialResults } + val totalDuration = System.currentTimeMillis() - startTime DomainResult.Success( SceneExecutionResult( @@ -63,6 +82,8 @@ class DefaultSceneExecutor( totalDurationMillis = totalDuration, ), ) + } catch (e: CancellationException) { + throw e } finally { runningScenes.remove(scene.id) } @@ -72,66 +93,70 @@ class DefaultSceneExecutor( profileId: ProviderProfileId, action: SceneAction, ): SceneActionExecutionResult { - return when (action) { - is SceneAction.SocketPower -> { - val res = socketGateway.setPower( - profileId = profileId, - deviceName = action.deviceName, - powerState = action.powerState, - ) - toExecutionResult(action, res) - } - - is SceneAction.InfraredKey -> { - val res = infraredGateway.sendKey( - profileId = profileId, - deviceName = action.deviceName, - key = action.key, - ) - toExecutionResult(action, res) - } - - is SceneAction.VirtualRemoteButton -> { - if (sendVirtualRemoteAction != null) { - val res = sendVirtualRemoteAction.sendButton( + return try { + when (action) { + is SceneAction.SocketPower -> { + val res = socketGateway.setPower( profileId = profileId, - remoteId = VirtualRemoteId(action.remoteId), - buttonId = action.buttonId, + deviceName = action.deviceName, + powerState = action.powerState, ) toExecutionResult(action, res) - } else { - SceneActionExecutionResult( - action = action, - outcome = SceneActionOutcome.SUCCESS, - error = null, - ) } - } - is SceneAction.Delay -> { - try { + is SceneAction.InfraredKey -> { + val res = infraredGateway.sendKey( + profileId = profileId, + deviceName = action.deviceName, + key = action.key, + ) + toExecutionResult(action, res) + } + + is SceneAction.VirtualRemoteButton -> { + if (sendVirtualRemoteAction != null) { + val res = sendVirtualRemoteAction.sendButton( + profileId = profileId, + remoteId = VirtualRemoteId(action.remoteId), + buttonId = action.buttonId, + ) + toExecutionResult(action, res) + } else { + SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.FAILURE, + error = DomainError.OperationFailed( + detail = "Virtual remote action executor is not available", + ), + ) + } + } + + is SceneAction.Delay -> { delayDurationProvider(action.durationSeconds * 1000L) SceneActionExecutionResult( action = action, outcome = SceneActionOutcome.SUCCESS, error = null, ) - } catch (e: Exception) { + } + + is SceneAction.Notification -> { SceneActionExecutionResult( action = action, - outcome = SceneActionOutcome.FAILURE, - error = DomainError.OperationFailed(detail = e.message), + outcome = SceneActionOutcome.SUCCESS, + error = null, ) } } - - is SceneAction.Notification -> { - SceneActionExecutionResult( - action = action, - outcome = SceneActionOutcome.SUCCESS, - error = null, - ) - } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.FAILURE, + error = DomainError.OperationFailed(detail = e.message), + ) } } diff --git a/domain/src/main/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepository.kt b/domain/src/main/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepository.kt index 4112ef7..c642da1 100644 --- a/domain/src/main/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepository.kt +++ b/domain/src/main/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepository.kt @@ -3,6 +3,8 @@ package com.flagship.abox.manager.domain import java.util.concurrent.ConcurrentHashMap import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock /** * Provider-isolated in-memory [SceneRepository] implementation that supports @@ -10,6 +12,10 @@ import kotlinx.coroutines.flow.MutableStateFlow */ class InMemorySceneRepository : SceneRepository { private val flows = ConcurrentHashMap>>>() + private val mutexes = ConcurrentHashMap() + + private fun mutexFor(profileId: ProviderProfileId): Mutex = + mutexes.getOrPut(profileId) { Mutex() } override fun observeScenes(profileId: ProviderProfileId): Flow>> = flowFor(profileId) @@ -25,10 +31,13 @@ class InMemorySceneRepository : SceneRepository { override suspend fun createScene( profileId: ProviderProfileId, scene: Scene, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { require(scene.profileId == profileId) { "Scene profileId must match the supplied profileId" } + require(scene.actions.all { it.profileId == profileId }) { + "All scene actions must belong to the supplied profileId" + } val scenes = currentScenes(profileId).orEmpty() if (scenes.any { it.id == scene.id }) { return DomainResult.Failure( @@ -39,39 +48,45 @@ class InMemorySceneRepository : SceneRepository { } val updated = scenes + scene publish(profileId, updated) - return DomainResult.Success(scene) + DomainResult.Success(scene) } override suspend fun updateScene( profileId: ProviderProfileId, scene: Scene, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { require(scene.profileId == profileId) { "Scene profileId must match the supplied profileId" } + require(scene.actions.all { it.profileId == profileId }) { + "All scene actions must belong to the supplied profileId" + } val scenes = currentScenes(profileId).orEmpty() val index = scenes.indexOfFirst { it.id == scene.id } if (index < 0) return notFound(scene.id) val updated = scenes.toMutableList().apply { set(index, scene) } publish(profileId, updated) - return DomainResult.Success(scene) + DomainResult.Success(scene) } override suspend fun deleteScene( profileId: ProviderProfileId, sceneId: SceneId, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { val scenes = currentScenes(profileId).orEmpty() val updated = scenes.filterNot { it.id == sceneId } if (updated.size == scenes.size) return notFound(sceneId) publish(profileId, updated) - return DomainResult.Success(Unit) + DomainResult.Success(Unit) } fun seedDefaults(profileId: ProviderProfileId, scenes: List) { require(scenes.all { it.profileId == profileId }) { "All scenes must belong to the supplied profileId" } + require(scenes.all { s -> s.actions.all { it.profileId == profileId } }) { + "All scene actions must belong to the supplied profileId" + } publish(profileId, scenes) } diff --git a/domain/src/test/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutorTest.kt b/domain/src/test/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutorTest.kt index 90173e0..ae7a092 100644 --- a/domain/src/test/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutorTest.kt +++ b/domain/src/test/kotlin/com/flagship/abox/manager/domain/DefaultSceneExecutorTest.kt @@ -221,5 +221,92 @@ class DefaultSceneExecutorTest { assertEquals(SceneActionOutcome.SUCCESS, executionResult.results[0].outcome) assertEquals(5000L, delayCalled) } + + @Test + fun executesActionsConcurrentlyWhenParallelMode() = runTest { + val socketGateway = TestSocketGateway() + val infraredGateway = TestInfraredGateway() + val executor = DefaultSceneExecutor(socketGateway, infraredGateway) + + val action1 = SceneAction.SocketPower( + id = SceneActionId("a1"), + profileId = profileId, + deviceName = DeviceName("socket-1"), + powerState = SocketPowerState.ON, + ) + val action2 = SceneAction.InfraredKey( + id = SceneActionId("a2"), + profileId = profileId, + deviceName = DeviceName("ir-1"), + key = "power", + ) + val scene = Scene( + id = SceneId("s1"), + profileId = profileId, + name = "Parallel Scene", + concurrencyMode = ConcurrencyMode.PARALLEL, + actions = listOf(action1, action2), + ) + + val result = executor.execute(scene) + assertTrue(result is DomainResult.Success) + val executionResult = (result as DomainResult.Success).value + assertEquals(2, executionResult.results.size) + assertEquals(SceneActionOutcome.SUCCESS, executionResult.results[0].outcome) + assertEquals(SceneActionOutcome.SUCCESS, executionResult.results[1].outcome) + assertEquals(1, socketGateway.calls.size) + assertEquals(1, infraredGateway.calls.size) + } + + @Test + fun failsVirtualRemoteButtonActionWhenSenderNotConfigured() = runTest { + val executor = DefaultSceneExecutor( + socketGateway = TestSocketGateway(), + infraredGateway = TestInfraredGateway(), + sendVirtualRemoteAction = null, + ) + val action = SceneAction.VirtualRemoteButton( + id = SceneActionId("vr1"), + profileId = profileId, + remoteId = "tv-remote", + remoteDisplayName = "TV", + buttonId = "power", + buttonDisplayName = "Power", + ) + val scene = Scene( + id = SceneId("s-vr"), + profileId = profileId, + name = "VR Scene", + actions = listOf(action), + ) + + val result = executor.execute(scene) + assertTrue(result is DomainResult.Success) + val executionResult = (result as DomainResult.Success).value + assertEquals(1, executionResult.results.size) + assertEquals(SceneActionOutcome.FAILURE, executionResult.results[0].outcome) + assertTrue(executionResult.results[0].error is DomainError.OperationFailed) + } + + @Test(expected = kotlinx.coroutines.CancellationException::class) + fun rethrowsCancellationExceptionWhenDelayCancelled() = runTest { + val executor = DefaultSceneExecutor( + socketGateway = TestSocketGateway(), + infraredGateway = TestInfraredGateway(), + delayDurationProvider = { throw kotlinx.coroutines.CancellationException("Job cancelled") }, + ) + val action = SceneAction.Delay( + id = SceneActionId("d1"), + profileId = profileId, + durationSeconds = 10, + ) + val scene = Scene( + id = SceneId("s-cancelled"), + profileId = profileId, + name = "Cancelled Scene", + actions = listOf(action), + ) + executor.execute(scene) + } } diff --git a/domain/src/test/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepositoryTest.kt b/domain/src/test/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepositoryTest.kt index 9ffda08..448f6c9 100644 --- a/domain/src/test/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepositoryTest.kt +++ b/domain/src/test/kotlin/com/flagship/abox/manager/domain/InMemorySceneRepositoryTest.kt @@ -1,6 +1,9 @@ package com.flagship.abox.manager.domain import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -83,4 +86,43 @@ class InMemorySceneRepositoryTest { val getAfterDelete = repository.getScene(aboxProfile, sceneId) assertNull((getAfterDelete as DomainResult.Success).value) } + + @Test(expected = IllegalArgumentException::class) + fun rejectsSceneWithMismatchedActionProfileId() = runTest { + val repository = InMemorySceneRepository() + val crossAction = SceneAction.SocketPower( + id = SceneActionId("a-cross"), + profileId = ProviderProfileId("other-profile"), + deviceName = DeviceName("socket-1"), + powerState = SocketPowerState.ON, + ) + val scene = Scene( + id = SceneId("abox:admin:scene-invalid"), + profileId = aboxProfile, + name = "Invalid Scene", + actions = listOf(crossAction), + ) + repository.createScene(aboxProfile, scene) + } + + @Test + fun concurrentCreationsAreThreadSafe() = runTest { + val repository = InMemorySceneRepository() + val count = 20 + coroutineScope { + (1..count).map { idx -> + async { + val scene = Scene( + id = SceneId("abox:admin:scene-$idx"), + profileId = aboxProfile, + name = "Scene $idx", + actions = emptyList(), + ) + repository.createScene(aboxProfile, scene) + } + }.awaitAll() + } + val scenes = (repository.observeScenes(aboxProfile).first() as DomainResult.Success).value + assertEquals(count, scenes.size) + } } diff --git a/testing/fakes/src/main/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepository.kt b/testing/fakes/src/main/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepository.kt index 28e620b..bce1d66 100644 --- a/testing/fakes/src/main/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepository.kt +++ b/testing/fakes/src/main/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepository.kt @@ -1,8 +1,10 @@ package com.flagship.abox.manager.testing.fakes +import com.flagship.abox.manager.domain.ConcurrencyMode import com.flagship.abox.manager.domain.DeviceName import com.flagship.abox.manager.domain.DomainError import com.flagship.abox.manager.domain.DomainResult +import com.flagship.abox.manager.domain.ExecutionStrategy import com.flagship.abox.manager.domain.InfraredGateway import com.flagship.abox.manager.domain.ProviderProfileId import com.flagship.abox.manager.domain.Scene @@ -18,9 +20,15 @@ import com.flagship.abox.manager.domain.SceneId import com.flagship.abox.manager.domain.SceneRepository import com.flagship.abox.manager.domain.SocketGateway import com.flagship.abox.manager.domain.SocketPowerState +import java.util.concurrent.ConcurrentHashMap +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow -import java.util.concurrent.ConcurrentHashMap +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock /** * In-memory [SceneRepository] used by the scene UI and ViewModel tests. Scenes are isolated by @@ -35,6 +43,10 @@ class FakeSceneRepository : SceneRepository { ProviderProfileId, MutableStateFlow>>, >() + private val mutexes = ConcurrentHashMap() + + private fun mutexFor(profileId: ProviderProfileId): Mutex = + mutexes.getOrPut(profileId) { Mutex() } override fun observeScenes( profileId: ProviderProfileId, @@ -51,10 +63,13 @@ class FakeSceneRepository : SceneRepository { override suspend fun createScene( profileId: ProviderProfileId, scene: Scene, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { require(scene.profileId == profileId) { "Scene profileId must match the supplied profileId" } + require(scene.actions.all { it.profileId == profileId }) { + "All scene actions must belong to the supplied provider profile" + } val scenes = currentScenes(profileId).orEmpty() if (scenes.any { it.id == scene.id }) { return DomainResult.Failure( @@ -65,33 +80,36 @@ class FakeSceneRepository : SceneRepository { } val updated = scenes + scene publish(profileId, updated) - return DomainResult.Success(scene) + DomainResult.Success(scene) } override suspend fun updateScene( profileId: ProviderProfileId, scene: Scene, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { require(scene.profileId == profileId) { "Scene profileId must match the supplied profileId" } + require(scene.actions.all { it.profileId == profileId }) { + "All scene actions must belong to the supplied provider profile" + } val scenes = currentScenes(profileId).orEmpty() val index = scenes.indexOfFirst { it.id == scene.id } if (index < 0) return notFound(scene.id) val updated = scenes.toMutableList().apply { set(index, scene) } publish(profileId, updated) - return DomainResult.Success(scene) + DomainResult.Success(scene) } override suspend fun deleteScene( profileId: ProviderProfileId, sceneId: SceneId, - ): DomainResult { + ): DomainResult = mutexFor(profileId).withLock { val scenes = currentScenes(profileId).orEmpty() val updated = scenes.filterNot { it.id == sceneId } if (updated.size == scenes.size) return notFound(sceneId) publish(profileId, updated) - return DomainResult.Success(Unit) + DomainResult.Success(Unit) } /** @@ -102,6 +120,9 @@ class FakeSceneRepository : SceneRepository { require(scenes.all { it.profileId == profileId }) { "All scenes must belong to the supplied provider profile" } + require(scenes.all { s -> s.actions.all { it.profileId == profileId } }) { + "All scene actions must belong to the supplied provider profile" + } publish(profileId, scenes) return this } @@ -198,13 +219,43 @@ class FakeSceneExecutor( ) } val startTime = System.currentTimeMillis() - val results = mutableListOf() + return try { - for (action in scene.actions) { - val actionStart = System.currentTimeMillis() - val execResult = executeAction(scene.profileId, action) - val duration = System.currentTimeMillis() - actionStart - results.add(execResult.copy(executionDurationMillis = duration)) + val results: List = if (scene.concurrencyMode == ConcurrencyMode.PARALLEL) { + coroutineScope { + scene.actions.map { action -> + async { + val actionStart = System.currentTimeMillis() + val execResult = executeAction(scene.profileId, action) + val duration = System.currentTimeMillis() - actionStart + execResult.copy(executionDurationMillis = duration) + } + }.awaitAll() + } + } else { + val sequentialResults = mutableListOf() + var aborted = false + for (action in scene.actions) { + if (aborted) { + sequentialResults.add( + SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.SKIPPED, + error = null, + ), + ) + continue + } + val actionStart = System.currentTimeMillis() + val execResult = executeAction(scene.profileId, action) + val duration = System.currentTimeMillis() - actionStart + val enrichedResult = execResult.copy(executionDurationMillis = duration) + sequentialResults.add(enrichedResult) + if (enrichedResult.outcome == SceneActionOutcome.FAILURE && scene.strategy == ExecutionStrategy.ABORT_ON_ERROR) { + aborted = true + } + } + sequentialResults } val totalDuration = System.currentTimeMillis() - startTime DomainResult.Success( @@ -214,6 +265,8 @@ class FakeSceneExecutor( totalDurationMillis = totalDuration, ), ) + } catch (e: CancellationException) { + throw e } finally { running.remove(scene.id) } @@ -223,34 +276,44 @@ class FakeSceneExecutor( profileId: ProviderProfileId, action: SceneAction, ): SceneActionExecutionResult { - val result = when (action) { - is SceneAction.SocketPower -> socketGateway.setPower( - profileId = profileId, - deviceName = action.deviceName, - powerState = action.powerState, - ) + return try { + val result = when (action) { + is SceneAction.SocketPower -> socketGateway.setPower( + profileId = profileId, + deviceName = action.deviceName, + powerState = action.powerState, + ) - is SceneAction.InfraredKey -> infraredGateway.sendKey( - profileId = profileId, - deviceName = action.deviceName, - key = action.key, - ) + is SceneAction.InfraredKey -> infraredGateway.sendKey( + profileId = profileId, + deviceName = action.deviceName, + key = action.key, + ) - is SceneAction.VirtualRemoteButton, - is SceneAction.Delay, - is SceneAction.Notification -> DomainResult.Success(Unit) - } - return when (result) { - is DomainResult.Success -> SceneActionExecutionResult( - action = action, - outcome = SceneActionOutcome.SUCCESS, - error = null, - ) + is SceneAction.VirtualRemoteButton, + is SceneAction.Delay, + is SceneAction.Notification -> DomainResult.Success(Unit) + } + when (result) { + is DomainResult.Success -> SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.SUCCESS, + error = null, + ) - is DomainResult.Failure -> SceneActionExecutionResult( + is DomainResult.Failure -> SceneActionExecutionResult( + action = action, + outcome = SceneActionOutcome.FAILURE, + error = result.error, + ) + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + SceneActionExecutionResult( action = action, outcome = SceneActionOutcome.FAILURE, - error = result.error, + error = DomainError.OperationFailed(detail = e.message), ) } } diff --git a/testing/fakes/src/test/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepositoryTest.kt b/testing/fakes/src/test/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepositoryTest.kt index e67f916..ee00b6d 100644 --- a/testing/fakes/src/test/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepositoryTest.kt +++ b/testing/fakes/src/test/kotlin/com/flagship/abox/manager/testing/fakes/FakeSceneRepositoryTest.kt @@ -11,6 +11,9 @@ import com.flagship.abox.manager.domain.SceneColor import com.flagship.abox.manager.domain.SceneIcon import com.flagship.abox.manager.domain.SceneId import com.flagship.abox.manager.domain.SocketPowerState +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -122,6 +125,45 @@ class FakeSceneRepositoryTest { assertNotNull((value as DomainResult.Failure).error) } + @Test(expected = IllegalArgumentException::class) + fun createSceneRejectsMismatchedActionProfileId() = runTest { + val repo = FakeSceneRepository() + val crossAction = SceneAction.SocketPower( + id = SceneActionId("a-cross"), + profileId = ProviderProfileId("other"), + deviceName = DeviceName("socket-1"), + powerState = SocketPowerState.ON, + ) + val scene = Scene( + id = SceneId("${localProfile.value}:scene-cross"), + profileId = localProfile, + name = "Cross Action", + actions = listOf(crossAction), + ) + repo.createScene(localProfile, scene) + } + + @Test + fun concurrentCreationsAreThreadSafe() = runTest { + val repo = FakeSceneRepository() + val count = 20 + coroutineScope { + (1..count).map { idx -> + async { + val scene = Scene( + id = SceneId("${aboxProfile.value}:scene-$idx"), + profileId = aboxProfile, + name = "Scene $idx", + actions = emptyList(), + ) + repo.createScene(aboxProfile, scene) + } + }.awaitAll() + } + val scenes = scenesFor(repo, aboxProfile) + assertEquals(count, scenes.size) + } + private suspend fun scenesFor( repo: FakeSceneRepository, profileId: ProviderProfileId,