Skip to content

Commit cfcdcbf

Browse files
authored
Make sure that widgets don't crash the app when server is removed (#6295)
1 parent c6fc53a commit cfcdcbf

File tree

8 files changed

+103
-22
lines changed

8 files changed

+103
-22
lines changed

app/src/main/kotlin/io/homeassistant/companion/android/widgets/camera/CameraWidget.kt

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import io.homeassistant.companion.android.widgets.BaseWidgetProvider.Companion.w
3535
import io.homeassistant.companion.android.widgets.EXTRA_WIDGET_ENTITY
3636
import io.homeassistant.companion.android.widgets.common.RemoteViewsTarget
3737
import javax.inject.Inject
38+
import kotlinx.coroutines.CancellationException
3839
import kotlinx.coroutines.CoroutineScope
3940
import kotlinx.coroutines.Dispatchers
4041
import kotlinx.coroutines.Job
@@ -127,21 +128,28 @@ class CameraWidget : AppWidgetProvider() {
127128
var widgetCameraError = false
128129
var url: String? = null
129130
if (widget != null) {
130-
try {
131-
val entityPictureUrl = retrieveCameraImageUrl(widget.serverId, widget.entityId)
132-
133-
val urlState = serverManager.connectionStateProvider(
134-
widget.serverId,
135-
).urlFlow().first()
136-
if (urlState is UrlState.HasUrl) {
137-
val baseUrl = urlState.url?.toString()?.removeSuffix("/") ?: ""
138-
url = "$baseUrl$entityPictureUrl"
139-
} else {
140-
throw IllegalStateException("No URL available to retrieve picture")
141-
}
142-
} catch (e: Exception) {
143-
Timber.e(e, "Failed to fetch entity or entity does not exist")
131+
if (serverManager.getServer(widget.serverId) == null) {
132+
Timber.w("Widget is attached to a server that has been removed")
144133
widgetCameraError = true
134+
} else {
135+
try {
136+
val entityPictureUrl = retrieveCameraImageUrl(widget.serverId, widget.entityId)
137+
138+
val urlState = serverManager.connectionStateProvider(
139+
widget.serverId,
140+
).urlFlow().first()
141+
if (urlState is UrlState.HasUrl) {
142+
val baseUrl = urlState.url?.toString()?.removeSuffix("/") ?: ""
143+
url = "$baseUrl$entityPictureUrl"
144+
} else {
145+
throw IllegalStateException("No URL available to retrieve picture")
146+
}
147+
} catch (e: CancellationException) {
148+
throw e
149+
} catch (e: Exception) {
150+
Timber.e(e, "Failed to fetch entity or entity does not exist")
151+
widgetCameraError = true
152+
}
145153
}
146154
}
147155

app/src/main/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidget.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ class EntityWidget : BaseWidgetProvider<StaticWidgetEntity, StaticWidgetDao>() {
161161
} else {
162162
entityId?.let { serverManager.integrationRepository(serverId).getEntity(it) }
163163
}
164+
} catch (e: CancellationException) {
165+
throw e
164166
} catch (e: Exception) {
165167
Timber.e(e, "Unable to fetch entity")
166168
entityCaughtException = true

app/src/main/kotlin/io/homeassistant/companion/android/widgets/mediaplayer/MediaPlayerControlsWidget.kt

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,16 @@ class MediaPlayerControlsWidget : BaseWidgetProvider<MediaPlayerControlsWidgetEn
244244
)
245245

246246
val entityPictureUrl = entity?.attributes?.get("entity_picture")?.toString()
247-
val baseUrl = serverManager.connectionStateProvider(
248-
widget.serverId,
249-
).urlFlow().firstUrlOrNull()?.toString()?.removeSuffix("/") ?: ""
247+
val baseUrl = try {
248+
serverManager.connectionStateProvider(
249+
widget.serverId,
250+
)
251+
} catch (e: CancellationException) {
252+
throw e
253+
} catch (e: IllegalStateException) {
254+
Timber.e(e, "The server does not exist anymore, could have been removed")
255+
null
256+
}?.urlFlow()?.firstUrlOrNull()?.toString()?.removeSuffix("/") ?: ""
250257
val url = if (entityPictureUrl?.startsWith("http") ==
251258
true
252259
) {
@@ -427,8 +434,10 @@ class MediaPlayerControlsWidget : BaseWidgetProvider<MediaPlayerControlsWidgetEn
427434
}
428435
return entities[0]
429436
}
437+
} catch (e: CancellationException) {
438+
throw e
430439
} catch (e: Exception) {
431-
Timber.d("Failed to fetch entity or entity does not exist")
440+
Timber.d(e, "Failed to fetch entity or entity does not exist")
432441
if (lastIntent == UPDATE_MEDIA_IMAGE) {
433442
Toast.makeText(context, commonR.string.widget_entity_fetch_error, Toast.LENGTH_LONG).show()
434443
}

app/src/main/kotlin/io/homeassistant/companion/android/widgets/todo/TodoWidgetActions.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ class ToggleTodoAction : ActionCallback {
118118
return
119119
}
120120

121+
if (serverManager.getServer(widgetEntity.serverId) == null) {
122+
Timber.w("Aborting the server has been removed, widget needs to be configured again")
123+
return
124+
}
125+
121126
val result = serverManager.webSocketRepository(widgetEntity.serverId).updateTodo(
122127
entityId = widgetEntity.entityId,
123128
todoItem = todoItem.uid,

app/src/main/kotlin/io/homeassistant/companion/android/widgets/todo/TodoWidgetStateUpdater.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ internal class TodoWidgetStateUpdater @Inject constructor(
4949
}
5050

5151
private suspend fun getAndSubscribeEntityUpdates(serverId: Int, listEntityId: String): Flow<Entity?>? {
52+
if (serverManager.getServer(serverId) == null) {
53+
Timber.w("Server has been removed and the widget needs to be reconfigured")
54+
return null
55+
}
56+
5257
// Since we might be re-subscribing we might not have get the entity update when subscribing so we query it first
5358
val currentEntity = serverManager.integrationRepository(serverId).getEntity(listEntityId)
5459

app/src/test/kotlin/io/homeassistant/companion/android/widgets/todo/TodoWidgetStateUpdaterTest.kt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ Initial state emission
6464
awaitClose()
6565
}
6666
coEvery { dao.get(widgetId) } returns todoWidgetEntity
67+
coEvery { serverManager.getServer(any<Int>()) } returns mockk()
6768
coJustAwait { integrationRepository.getEntity(entityId) }
6869

6970
updater.stateFlow(42).test {
@@ -73,6 +74,27 @@ Initial state emission
7374
}
7475
}
7576

77+
@Test
78+
fun `Given widgetId in DAO with a removed server when subscribing to stateFlow then emits DAO Entry current state out of sync`() = runTest {
79+
val widgetId = 42
80+
val entityId = "test"
81+
val todoWidgetEntity = TodoWidgetEntity(widgetId, 1, entityId)
82+
83+
coEvery { dao.getFlow(widgetId) } returns channelFlow {
84+
send(todoWidgetEntity)
85+
awaitClose()
86+
}
87+
coEvery { dao.get(widgetId) } returns todoWidgetEntity
88+
coEvery { serverManager.getServer(any<Int>()) } returns null
89+
coJustAwait { integrationRepository.getEntity(entityId) }
90+
91+
updater.stateFlow(42).test {
92+
assertEquals(TodoStateWithData.from(todoWidgetEntity), awaitItem())
93+
assertEquals(TodoStateWithData.from(todoWidgetEntity), awaitItem())
94+
expectNoEvents()
95+
}
96+
}
97+
7698
@Test
7799
fun `Given widgetID when subscribing to stateFlow with error then it catches and complete the flow`() = runTest {
78100
val widgetId = 42
@@ -106,6 +128,7 @@ Watch for update
106128
awaitClose()
107129
}
108130
coJustRun { dao.updateWidgetLastUpdate(any(), any()) }
131+
coEvery { serverManager.getServer(any<Int>()) } returns mockk()
109132

110133
coEvery { integrationRepository.getEntity(entityId) } returns serverEntity
111134
coEvery { integrationRepository.getEntityUpdates(any()) } returns channelFlow {
@@ -156,6 +179,7 @@ Watch for update
156179
awaitClose()
157180
}
158181
coJustRun { dao.updateWidgetLastUpdate(any(), any()) }
182+
coEvery { serverManager.getServer(any<Int>()) } returns mockk()
159183

160184
coEvery { integrationRepository.getEntity(entityId) } returns serverEntity
161185
coEvery { integrationRepository.getEntityUpdates(any()) } returns channelFlow {
@@ -212,6 +236,7 @@ Watch for update
212236
awaitClose()
213237
}
214238
coJustRun { dao.updateWidgetLastUpdate(any(), any()) }
239+
coEvery { serverManager.getServer(any<Int>()) } returns mockk()
215240

216241
coEvery { integrationRepository.getEntity(entityId) } returns serverEntity
217242
coEvery { integrationRepository.getEntityUpdates(any()) } returns null

app/src/test/kotlin/io/homeassistant/companion/android/widgets/todo/TodoWidgetToggleActionsTest.kt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ class TodoWidgetToggleActionsTest {
9898
coEvery {
9999
entryPoints.webSocketRepository.updateTodo(any(), any(), any(), any())
100100
} returns true
101+
coEvery { entryPoints.serverManager.getServer(42) } returns mockk()
101102

102103
coEvery { entryPoints.dao().get(widgetId) } returns todoItem
103104

@@ -116,6 +117,32 @@ class TodoWidgetToggleActionsTest {
116117
}
117118
}
118119

120+
@Test
121+
fun `Given a widgetID and todo item state with uid but server removed when present in DAO and invoking onAction then do nothing`() = runTest {
122+
val action = spyk<ToggleTodoAction>()
123+
val glanceManager = mockk<GlanceAppWidgetManager>()
124+
val widgetId = 1
125+
val todoItem = TodoWidgetEntity(1, 42, "HA")
126+
val parameters = actionParametersOf(TOGGLE_KEY to TodoItemState("42", "", false))
127+
128+
every { action.getEntryPoints(any()) } returns entryPoints
129+
every { action.getGlanceManager(any()) } returns glanceManager
130+
every { glanceManager.getAppWidgetId(any()) } returns widgetId
131+
coEvery { entryPoints.serverManager.getServer(42) } returns null
132+
133+
coEvery { entryPoints.dao().get(widgetId) } returns todoItem
134+
135+
action.onAction(mockk(), FakeGlanceId(widgetId), parameters)
136+
137+
coVerify(exactly = 1) {
138+
entryPoints.dao().get(widgetId)
139+
entryPoints.serverManager().getServer(42)
140+
}
141+
coVerify(exactly = 0) {
142+
entryPoints.serverManager().webSocketRepository(any())
143+
}
144+
}
145+
119146
@Test
120147
fun `Given boolean value when invoking toggleStatus then mapping is valid from the API point of view`() {
121148
val action = ToggleTodoAction()

common/src/main/kotlin/io/homeassistant/companion/android/common/data/servers/ServerManager.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ interface ServerManager {
8181
*
8282
* @param serverId the server ID, or [SERVER_ID_ACTIVE] for the currently active server
8383
* @return [AuthenticationRepository] for the server
84-
* @throws IllegalArgumentException if there is no server with the provided ID
84+
* @throws IllegalStateException if there is no server with the provided ID
8585
*/
8686
suspend fun authenticationRepository(serverId: Int = SERVER_ID_ACTIVE): AuthenticationRepository
8787

@@ -90,7 +90,7 @@ interface ServerManager {
9090
*
9191
* @param serverId the server ID, or [SERVER_ID_ACTIVE] for the currently active server
9292
* @return [IntegrationRepository] for the server
93-
* @throws IllegalArgumentException if there is no server with the provided ID
93+
* @throws IllegalStateException if there is no server with the provided ID
9494
*/
9595
suspend fun integrationRepository(serverId: Int = SERVER_ID_ACTIVE): IntegrationRepository
9696

@@ -99,7 +99,7 @@ interface ServerManager {
9999
*
100100
* @param serverId the server ID, or [SERVER_ID_ACTIVE] for the currently active server
101101
* @return [WebSocketRepository] for the server
102-
* @throws IllegalArgumentException if there is no server with the provided ID
102+
* @throws IllegalStateException if there is no server with the provided ID
103103
*/
104104
suspend fun webSocketRepository(serverId: Int = SERVER_ID_ACTIVE): WebSocketRepository
105105

@@ -108,7 +108,7 @@ interface ServerManager {
108108
*
109109
* @param serverId the server ID, or [SERVER_ID_ACTIVE] for the currently active server
110110
* @return [ServerConnectionStateProvider] for the server
111-
* @throws IllegalArgumentException if there is no server with the provided ID
111+
* @throws IllegalStateException if there is no server with the provided ID
112112
*/
113113
suspend fun connectionStateProvider(serverId: Int = SERVER_ID_ACTIVE): ServerConnectionStateProvider
114114
}

0 commit comments

Comments
 (0)