Skip to content

Commit b0afac6

Browse files
TimoPtrjpelgrom
TimoPtr
and
jpelgrom
authored
Refactor connection state management (#6131)
* Introduce HttpUtl hasSameOrigin * Add ability to watch for specific server change * Move allowInsecureConnection to ServerConnectionInfo * Remove logic from ServerConnectionInfo and cache URL parsing to HTTPUrl * Introduce ServerConnectionStateProvider * Replace GetUrl with proper calls from connection state provider * Adjust webview to the new way of getting the URL * Update ViewModels that set allowInsecureConnection * Handle discard of ConnectionSecurityFragment * Avoid blinking in BlockInsecureScreen * Use viewModel in BlockInsecureFragment to properly retry --------- Co-authored-by: Joris Pelgröm <[email protected]>
1 parent aa68eec commit b0afac6

File tree

57 files changed

+4657
-952
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+4657
-952
lines changed

‎app/lint-baseline.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
8686
<location
8787
file="src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt"
88-
line="1053"
88+
line="1055"
8989
column="17"/>
9090
</issue>
9191

@@ -129,7 +129,7 @@
129129
errorLine2=" ~~~~~~~~~~~~~">
130130
<location
131131
file="src/main/kotlin/io/homeassistant/companion/android/controls/HaControlsProviderService.kt"
132-
line="46"
132+
line="47"
133133
column="30"/>
134134
</issue>
135135

‎app/src/full/kotlin/io/homeassistant/companion/android/onboarding/WearOnboardingListener.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,13 @@ class WearOnboardingListener : WearableListenerService() {
3131
private fun sendHomeAssistantInstance(nodeId: String) = runBlocking {
3232
Timber.d("sendHomeAssistantInstance: $nodeId")
3333
// Retrieve current instance
34-
val url = serverManager.getServer()?.connection?.getUrl(false)
34+
val server = serverManager.getServer()
35+
val url = server?.let { serverManager.connectionStateProvider(it.id).getExternalUrl() }
3536

3637
if (url != null) {
3738
// Put as DataMap in data layer
3839
val putDataReq: PutDataRequest = PutDataMapRequest.create("/home_assistant_instance").run {
39-
dataMap.putString("name", url.host.toString())
40+
dataMap.putString("name", url.host.orEmpty())
4041
dataMap.putString("url", url.toString())
4142
setUrgent()
4243
asPutDataRequest()

‎app/src/main/kotlin/io/homeassistant/companion/android/controls/HaControlsProviderService.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import io.homeassistant.companion.android.common.data.integration.applyCompresse
1414
import io.homeassistant.companion.android.common.data.integration.domain
1515
import io.homeassistant.companion.android.common.data.prefs.PrefsRepository
1616
import io.homeassistant.companion.android.common.data.servers.ServerManager
17+
import io.homeassistant.companion.android.common.data.servers.firstUrlOrNull
1718
import io.homeassistant.companion.android.common.data.websocket.impl.entities.AreaRegistryResponse
1819
import io.homeassistant.companion.android.common.data.websocket.impl.entities.DeviceRegistryResponse
1920
import io.homeassistant.companion.android.common.data.websocket.impl.entities.EntityRegistryResponse
@@ -301,7 +302,9 @@ class HaControlsProviderService : ControlsProviderService() {
301302
}
302303
}
303304
val entities = mutableMapOf<String, Entity>()
304-
val baseUrl = serverManager.getServer(serverId)?.connection?.getUrl()?.toString()?.removeSuffix("/") ?: ""
305+
val baseUrl =
306+
serverManager.connectionStateProvider(serverId).urlFlow().firstUrlOrNull()?.toString()?.removeSuffix("/")
307+
?: ""
305308

306309
areaRegistry[serverId] = getAreaRegistry.await()
307310
deviceRegistry[serverId] = getDeviceRegistry.await()

‎app/src/main/kotlin/io/homeassistant/companion/android/launcher/LauncherViewModel.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ internal class LauncherViewModel @AssistedInject constructor(
9696
getServerConnectedAndRegistered(serverId)?.let { server ->
9797
Timber.d("Server (id=${server.id}) is connected and registered checking network status")
9898

99-
networkStatusMonitor.observeNetworkStatus(server.connection)
99+
networkStatusMonitor.observeNetworkStatus(serverManager.connectionStateProvider(server.id))
100100
.takeWhile { state ->
101101
// Until the network is ready we continue to observe network status changes
102102
!handleNetworkState(state, LauncherNavigationEvent.Frontend(path, serverId))

‎app/src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt‎

Lines changed: 65 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import io.homeassistant.companion.android.authenticator.Authenticator
4949
import io.homeassistant.companion.android.common.R as commonR
5050
import io.homeassistant.companion.android.common.data.prefs.PrefsRepository
5151
import io.homeassistant.companion.android.common.data.servers.ServerManager
52+
import io.homeassistant.companion.android.common.data.servers.UrlState
5253
import io.homeassistant.companion.android.common.notifications.DeviceCommandData
5354
import io.homeassistant.companion.android.common.notifications.NotificationData
5455
import io.homeassistant.companion.android.common.notifications.clearNotification
@@ -98,6 +99,7 @@ import kotlinx.coroutines.Dispatchers
9899
import kotlinx.coroutines.Job
99100
import kotlinx.coroutines.async
100101
import kotlinx.coroutines.awaitAll
102+
import kotlinx.coroutines.flow.first
101103
import kotlinx.coroutines.launch
102104
import kotlinx.coroutines.runBlocking
103105
import kotlinx.coroutines.withContext
@@ -1257,10 +1259,15 @@ class MessagingManager @Inject constructor(
12571259
if (!iconUrl.isNullOrBlank()) {
12581260
val dataIcon = iconUrl.trim().replace(" ", "%20")
12591261
val serverId = data[THIS_SERVER_ID]!!.toInt()
1260-
val url = UrlUtil.handle(serverManager.getServer(serverId)?.connection?.getUrl(), dataIcon)
1261-
val bitmap = getImageBitmap(serverId, url, !UrlUtil.isAbsoluteUrl(dataIcon))
1262-
if (bitmap != null) {
1263-
builder.setLargeIcon(bitmap)
1262+
val urlState = serverManager.connectionStateProvider(serverId).urlFlow().first()
1263+
if (urlState is UrlState.HasUrl) {
1264+
val url = UrlUtil.handle(urlState.url, dataIcon)
1265+
val bitmap = getImageBitmap(serverId, url, !UrlUtil.isAbsoluteUrl(dataIcon))
1266+
if (bitmap != null) {
1267+
builder.setLargeIcon(bitmap)
1268+
}
1269+
} else {
1270+
Timber.w("Not fetching icon since URL is unavailable")
12641271
}
12651272
}
12661273
}
@@ -1270,27 +1277,32 @@ class MessagingManager @Inject constructor(
12701277
if (!imageUrl.isNullOrBlank()) {
12711278
val dataImage = imageUrl.trim().replace(" ", "%20")
12721279
val serverId = data[THIS_SERVER_ID]!!.toInt()
1273-
val url = UrlUtil.handle(serverManager.getServer(serverId)?.connection?.getUrl(), dataImage)
1274-
val bitmap = getImageBitmap(serverId, url, !UrlUtil.isAbsoluteUrl(dataImage))
1275-
if (bitmap != null) {
1276-
builder
1277-
.setLargeIcon(bitmap)
1278-
.setStyle(
1279-
NotificationCompat.BigPictureStyle().also { style ->
1280-
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
1281-
saveTempAnimatedImage(
1282-
serverId,
1283-
url,
1284-
!UrlUtil.isAbsoluteUrl(dataImage),
1285-
)?.let { filePath ->
1286-
style.bigPicture(Icon.createWithContentUri(filePath))
1287-
} ?: run { style.bigPicture(bitmap) }
1288-
} else {
1289-
style.bigPicture(bitmap)
1280+
val urlState = serverManager.connectionStateProvider(serverId).urlFlow().first()
1281+
if (urlState is UrlState.HasUrl) {
1282+
val url = UrlUtil.handle(urlState.url, dataImage)
1283+
val bitmap = getImageBitmap(serverId, url, !UrlUtil.isAbsoluteUrl(dataImage))
1284+
if (bitmap != null) {
1285+
builder
1286+
.setLargeIcon(bitmap)
1287+
.setStyle(
1288+
NotificationCompat.BigPictureStyle().also { style ->
1289+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
1290+
saveTempAnimatedImage(
1291+
serverId,
1292+
url,
1293+
!UrlUtil.isAbsoluteUrl(dataImage),
1294+
)?.let { filePath ->
1295+
style.bigPicture(Icon.createWithContentUri(filePath))
1296+
} ?: run { style.bigPicture(bitmap) }
1297+
} else {
1298+
style.bigPicture(bitmap)
1299+
}
12901300
}
1291-
}
1292-
.bigLargeIcon(null as Bitmap?),
1293-
)
1301+
.bigLargeIcon(null as Bitmap?),
1302+
)
1303+
}
1304+
} else {
1305+
Timber.w("Not fetching image since URL is unavailable")
12941306
}
12951307
}
12961308
}
@@ -1363,35 +1375,40 @@ class MessagingManager @Inject constructor(
13631375
if (!videoUrl.isNullOrBlank()) {
13641376
val dataVideo = videoUrl.trim().replace(" ", "%20")
13651377
val serverId = data[THIS_SERVER_ID]!!.toInt()
1366-
val url = UrlUtil.handle(serverManager.getServer(serverId)?.connection?.getUrl(), dataVideo)
1367-
getVideoFrames(serverId, url, !UrlUtil.isAbsoluteUrl(dataVideo))?.let { frames ->
1368-
Timber.d("Found ${frames.size} frames for video notification")
1369-
RemoteViews(context.packageName, R.layout.view_image_flipper).let { remoteViewFlipper ->
1370-
if (frames.isNotEmpty()) {
1371-
frames.forEach { frame ->
1372-
remoteViewFlipper.addView(
1373-
R.id.frame_flipper,
1374-
RemoteViews(context.packageName, R.layout.view_single_frame).apply {
1375-
setImageViewBitmap(
1376-
R.id.frame,
1377-
frame,
1378-
)
1379-
},
1380-
)
1381-
}
1378+
val urlState = serverManager.connectionStateProvider(serverId).urlFlow().first()
1379+
if (urlState is UrlState.HasUrl) {
1380+
val url = UrlUtil.handle(urlState.url, dataVideo)
1381+
getVideoFrames(serverId, url, !UrlUtil.isAbsoluteUrl(dataVideo))?.let { frames ->
1382+
Timber.d("Found ${frames.size} frames for video notification")
1383+
RemoteViews(context.packageName, R.layout.view_image_flipper).let { remoteViewFlipper ->
1384+
if (frames.isNotEmpty()) {
1385+
frames.forEach { frame ->
1386+
remoteViewFlipper.addView(
1387+
R.id.frame_flipper,
1388+
RemoteViews(context.packageName, R.layout.view_single_frame).apply {
1389+
setImageViewBitmap(
1390+
R.id.frame,
1391+
frame,
1392+
)
1393+
},
1394+
)
1395+
}
13821396

1383-
data[NotificationData.TITLE]?.let { rawTitle ->
1384-
remoteViewFlipper.setTextViewText(R.id.title, rawTitle)
1385-
}
1397+
data[NotificationData.TITLE]?.let { rawTitle ->
1398+
remoteViewFlipper.setTextViewText(R.id.title, rawTitle)
1399+
}
13861400

1387-
data[NotificationData.MESSAGE]?.let { rawMessage ->
1388-
remoteViewFlipper.setTextViewText(R.id.info, rawMessage)
1389-
}
1401+
data[NotificationData.MESSAGE]?.let { rawMessage ->
1402+
remoteViewFlipper.setTextViewText(R.id.info, rawMessage)
1403+
}
13901404

1391-
builder.setCustomBigContentView(remoteViewFlipper)
1392-
builder.setStyle(NotificationCompat.DecoratedCustomViewStyle())
1405+
builder.setCustomBigContentView(remoteViewFlipper)
1406+
builder.setStyle(NotificationCompat.DecoratedCustomViewStyle())
1407+
}
13931408
}
13941409
}
1410+
} else {
1411+
Timber.w("Not fetching video since URL is unavailable")
13951412
}
13961413
}
13971414
}

‎app/src/main/kotlin/io/homeassistant/companion/android/onboarding/locationforsecureconnection/LocationForSecureConnectionViewModel.kt‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class LocationForSecureConnectionViewModel @VisibleForTesting constructor(
2727

2828
val allowInsecureConnection: Flow<Boolean?> = flow {
2929
try {
30-
val value = serverManager.integrationRepository(serverId).getAllowInsecureConnection()
30+
val value = serverManager.getServer(serverId)?.connection?.allowInsecureConnection
3131
emit(value)
3232
} catch (e: Exception) {
3333
Timber.e(e, "Failed to get initial AllowInsecureConnection for server $serverId")
@@ -38,7 +38,15 @@ class LocationForSecureConnectionViewModel @VisibleForTesting constructor(
3838
fun allowInsecureConnection(allowInsecureConnection: Boolean) {
3939
viewModelScope.launch {
4040
try {
41-
serverManager.integrationRepository(serverId).setAllowInsecureConnection(allowInsecureConnection)
41+
serverManager.getServer(serverId)?.let { server ->
42+
serverManager.updateServer(
43+
server.copy(
44+
connection = server.connection.copy(
45+
allowInsecureConnection = allowInsecureConnection,
46+
),
47+
),
48+
)
49+
}
4250
} catch (e: Exception) {
4351
Timber.e(
4452
e,

‎app/src/main/kotlin/io/homeassistant/companion/android/onboarding/nameyourdevice/NameYourDeviceViewModel.kt‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,9 @@ internal class NameYourDeviceViewModel @VisibleForTesting constructor(
9494
viewModelScope.launch {
9595
try {
9696
_isSavingFlow.emit(true)
97-
val serverId = addServer()
9897
val url = route.url
9998
val hasPlainTextAccess = url.startsWith("http://")
100-
101-
enforceSecureConnectionIfNeeded(serverId = serverId, hasPlainTextAccess = hasPlainTextAccess)
99+
val serverId = addServer(hasPlainTextAccess = hasPlainTextAccess)
102100

103101
_navigationEventsFlow.emit(
104102
NameYourDeviceNavigationEvent.DeviceNameSaved(
@@ -109,10 +107,10 @@ internal class NameYourDeviceViewModel @VisibleForTesting constructor(
109107
)
110108
} catch (e: Exception) {
111109
Timber.e(e, "Error while adding server")
112-
val messageRes = when {
113-
e is HttpException && e.code() == 404 -> commonR.string.error_with_registration
114-
e is SSLHandshakeException -> commonR.string.webview_error_FAILED_SSL_HANDSHAKE
115-
e is SSLException -> commonR.string.webview_error_SSL_INVALID
110+
val messageRes = when (e) {
111+
is HttpException if e.code() == 404 -> commonR.string.error_with_registration
112+
is SSLHandshakeException -> commonR.string.webview_error_FAILED_SSL_HANDSHAKE
113+
is SSLException -> commonR.string.webview_error_SSL_INVALID
116114
else -> commonR.string.webview_error
117115
}
118116
_navigationEventsFlow.emit(NameYourDeviceNavigationEvent.Error(messageRes))
@@ -130,33 +128,13 @@ internal class NameYourDeviceViewModel @VisibleForTesting constructor(
130128
return name.isNotEmpty()
131129
}
132130

133-
/**
134-
* Enforces secure connections for servers that were onboarded using HTTPS.
135-
*
136-
* If the server was accessed via HTTPS during onboarding, this function ensures that future
137-
* connections will only be allowed over secure channels to prevent future usage of HTTP.
138-
*
139-
* @param serverId The ID of the server to configure
140-
* @param hasPlainTextAccess Whether the server was accessed via HTTP during onboarding
141-
*/
142-
private suspend fun enforceSecureConnectionIfNeeded(serverId: Int, hasPlainTextAccess: Boolean) {
143-
if (!hasPlainTextAccess) {
144-
// Until https://github.com/home-assistant/android/issues/6005 is not addressed this is going to fail.
145-
// It means that the flag allow insecure won't be set for HTTPS
146-
runCatching {
147-
serverManager.integrationRepository(serverId).setAllowInsecureConnection(false)
148-
}.onFailure { exception ->
149-
Timber.e(exception, "Failed to enforce secure connection for server $serverId")
150-
}
151-
}
152-
}
153-
154-
private suspend fun addServer(): Int {
131+
private suspend fun addServer(hasPlainTextAccess: Boolean): Int {
155132
val server = Server(
156133
_name = "",
157134
type = ServerType.TEMPORARY,
158135
connection = ServerConnectionInfo(
159136
externalUrl = route.url,
137+
allowInsecureConnection = if (!hasPlainTextAccess) false else null,
160138
),
161139
session = ServerSessionInfo(),
162140
user = ServerUserInfo(),

‎app/src/main/kotlin/io/homeassistant/companion/android/onboarding/serverdiscovery/ServerDiscoveryViewModel.kt‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ data class ServersDiscovered(val servers: List<ServerDiscovered>) : DiscoverySta
6767
@OptIn(FlowPreview::class)
6868
@HiltViewModel
6969
internal class ServerDiscoveryViewModel @VisibleForTesting constructor(
70-
discoveryMode: ServerDiscoveryMode,
70+
private val discoveryMode: ServerDiscoveryMode,
7171
private val searcher: HomeAssistantSearcher,
72-
serverManager: ServerManager,
72+
private val serverManager: ServerManager,
7373
) : ViewModel() {
7474

7575
@Inject
@@ -96,14 +96,7 @@ internal class ServerDiscoveryViewModel @VisibleForTesting constructor(
9696
emptyList()
9797
}
9898

99-
private val _discoveryFlow =
100-
MutableStateFlow(
101-
if (discoveryMode == ServerDiscoveryMode.ADD_EXISTING) {
102-
getInstances(serverManager)
103-
} else {
104-
Started
105-
},
106-
)
99+
private val _discoveryFlow = MutableStateFlow<DiscoveryState>(Started)
107100

108101
/**
109102
* A flow that emits the current [DiscoveryState] of the server discovery process.
@@ -124,6 +117,9 @@ internal class ServerDiscoveryViewModel @VisibleForTesting constructor(
124117

125118
private fun discoverInstances() {
126119
viewModelScope.launch {
120+
if (discoveryMode == ServerDiscoveryMode.ADD_EXISTING) {
121+
_discoveryFlow.value = getExistingInstances(serverManager)
122+
}
127123
try {
128124
searcher.discoveredInstanceFlow()
129125
.filter { instanceFound ->
@@ -196,10 +192,11 @@ internal class ServerDiscoveryViewModel @VisibleForTesting constructor(
196192
}
197193
}
198194

199-
private fun getInstances(serverManager: ServerManager): DiscoveryState {
195+
private suspend fun getExistingInstances(serverManager: ServerManager): DiscoveryState {
200196
return serverManager.defaultServers
201197
.mapNotNull { server ->
202-
val url = server.connection.getUrl(isInternal = false) ?: return@mapNotNull null
198+
val url =
199+
serverManager.connectionStateProvider(server.id).getExternalUrl() ?: return@mapNotNull null
203200
val version = server.version ?: return@mapNotNull null
204201
ServerDiscovered(server.friendlyName, url, version)
205202
}

0 commit comments

Comments
 (0)