Skip to content

Commit 8daf085

Browse files
TimoPtrTimoPtr
authored
Fix server manager race condition read on DB only and don't store temporary server (#6212)
* Move from defaultServers to servers() * serversFlow now return a flow from the DB directly * Remove temporary server from ServerManager * Add tests for Server and ServerManager and remove ServerType
1 parent f368b62 commit 8daf085

103 files changed

Lines changed: 2538 additions & 940 deletions

File tree

Some content is hidden

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

‎app/src/full/kotlin/io/homeassistant/companion/android/matter/MatterCommissioningActivity.kt‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,18 @@ import androidx.lifecycle.lifecycleScope
1414
import com.google.android.gms.home.matter.Matter
1515
import com.google.android.gms.home.matter.commissioning.SharedDeviceData
1616
import dagger.hilt.android.AndroidEntryPoint
17-
import io.homeassistant.companion.android.common.data.servers.ServerManager
18-
import io.homeassistant.companion.android.database.server.Server
1917
import io.homeassistant.companion.android.matter.views.MatterCommissioningView
2018
import io.homeassistant.companion.android.util.compose.HomeAssistantAppTheme
2119
import io.homeassistant.companion.android.util.enableEdgeToEdgeCompat
2220
import io.homeassistant.companion.android.webview.WebViewActivity
23-
import javax.inject.Inject
2421
import kotlinx.coroutines.launch
2522
import timber.log.Timber
2623

2724
@AndroidEntryPoint
2825
class MatterCommissioningActivity : AppCompatActivity() {
29-
30-
@Inject
31-
lateinit var serverManager: ServerManager
32-
3326
private val viewModel: MatterCommissioningViewModel by viewModels()
3427
private var deviceCode: String? = null
3528
private var deviceName by mutableStateOf<String?>(null)
36-
private var servers by mutableStateOf<List<Server>>(emptyList())
3729
private var newMatterDevice = false
3830

3931
private val threadPermissionLauncher =
@@ -50,15 +42,14 @@ class MatterCommissioningActivity : AppCompatActivity() {
5042
MatterCommissioningView(
5143
step = viewModel.step,
5244
deviceName = deviceName,
53-
servers = servers,
45+
servers = viewModel.servers,
5446
onSelectServer = viewModel::checkSupport,
5547
onConfirmCommissioning = { startCommissioning() },
5648
onClose = { finish() },
5749
onContinue = { continueToApp(false) },
5850
)
5951
}
6052
}
61-
servers = serverManager.defaultServers
6253
}
6354

6455
override fun onResume() {

‎app/src/full/kotlin/io/homeassistant/companion/android/matter/MatterCommissioningViewModel.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import androidx.lifecycle.AndroidViewModel
1111
import androidx.lifecycle.viewModelScope
1212
import dagger.hilt.android.lifecycle.HiltViewModel
1313
import io.homeassistant.companion.android.common.data.servers.ServerManager
14+
import io.homeassistant.companion.android.database.server.Server
1415
import io.homeassistant.companion.android.thread.ThreadManager
1516
import javax.inject.Inject
1617
import kotlinx.coroutines.launch
@@ -41,6 +42,9 @@ class MatterCommissioningViewModel @Inject constructor(
4142
var serverId by mutableIntStateOf(0)
4243
private set
4344

45+
var servers by mutableStateOf<List<Server>>(emptyList())
46+
private set
47+
4448
fun checkSetup(isNewDevice: Boolean) {
4549
viewModelScope.launch {
4650
if (!isNewDevice && step != CommissioningFlowStep.NotStarted) {
@@ -53,8 +57,8 @@ class MatterCommissioningViewModel @Inject constructor(
5357
step = CommissioningFlowStep.NotRegistered
5458
return@launch
5559
}
56-
57-
if (serverManager.defaultServers.size > 1) {
60+
servers = serverManager.servers()
61+
if (servers.size > 1) {
5862
step = CommissioningFlowStep.SelectServer
5963
} else {
6064
serverManager.getServer()?.id?.let {

‎app/src/full/kotlin/io/homeassistant/companion/android/notifications/FirebaseCloudMessagingService.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class FirebaseCloudMessagingService : FirebaseMessagingService() {
4545
Timber.d("Not trying to update registration since we aren't authenticated.")
4646
return@launch
4747
}
48-
serverManager.defaultServers.forEach {
48+
serverManager.servers().forEach {
4949
launch {
5050
try {
5151
serverManager.integrationRepository(it.id).updateRegistration(

‎app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ class LocationSensorManager :
415415
lastHighAccuracyMode = highAccuracyModeEnabled
416416
lastHighAccuracyUpdateInterval = updateIntervalHighAccuracySeconds
417417

418-
serverManager(latestContext).defaultServers.forEach {
418+
serverManager(latestContext).servers().forEach {
419419
getSendLocationAsSetting(it.id) // Sets up the setting, value isn't used right now
420420
}
421421
}

‎app/src/full/kotlin/io/homeassistant/companion/android/settings/wear/SettingsWearRepository.kt‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package io.homeassistant.companion.android.settings.wear
2+
3+
import io.homeassistant.companion.android.common.data.authentication.AuthorizationException
4+
import io.homeassistant.companion.android.common.data.authentication.impl.AuthenticationService
5+
import io.homeassistant.companion.android.common.data.authentication.impl.AuthenticationService.Companion.SEGMENT_AUTH_TOKEN
6+
import io.homeassistant.companion.android.common.data.integration.Entity
7+
import io.homeassistant.companion.android.common.data.integration.IntegrationException
8+
import io.homeassistant.companion.android.common.data.integration.impl.IntegrationService
9+
import io.homeassistant.companion.android.common.data.integration.impl.entities.RenderTemplateIntegrationRequest
10+
import io.homeassistant.companion.android.common.data.integration.impl.entities.Template
11+
import io.homeassistant.companion.android.common.data.servers.tryOnUrls
12+
import io.homeassistant.companion.android.common.util.FailFast
13+
import javax.inject.Inject
14+
import kotlinx.serialization.json.JsonPrimitive
15+
import kotlinx.serialization.json.contentOrNull
16+
import okhttp3.HttpUrl
17+
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
18+
import timber.log.Timber
19+
20+
/**
21+
* A lightweight server representation used exclusively for Wear OS onboarding operations.
22+
*
23+
* This class exists to avoid persisting a server in the app's database when onboarding a Wear
24+
* device to a Home Assistant instance that may not be registered on the phone. By keeping this
25+
* as a transient data structure, the server management logic remains simpler and doesn't need
26+
* to handle temporary or Wear-only servers.
27+
*
28+
* The tradeoff is some code duplication that already exists in
29+
* the integration layer, but this is acceptable to maintain clear separation of concerns.
30+
*/
31+
data class WearServer(
32+
val serverId: Int,
33+
val externalUrl: String,
34+
val cloudUrl: String?,
35+
val webhookId: String,
36+
val cloudhookUrl: String?,
37+
val accessToken: String?,
38+
) {
39+
/**
40+
* Returns available base URLs for API calls, prioritizing cloud URL over external URL.
41+
*/
42+
fun getBaseUrls(): List<HttpUrl> = buildList {
43+
cloudUrl?.toHttpUrlOrNull()?.let(::add)
44+
externalUrl.toHttpUrlOrNull()?.let(::add)
45+
}
46+
47+
/**
48+
* Returns available webhook URLs, prioritizing cloudhook URL over the external webhook path.
49+
*/
50+
fun getWebhookUrls(): List<HttpUrl> = buildList {
51+
cloudhookUrl?.toHttpUrlOrNull()?.let(::add)
52+
externalUrl.toHttpUrlOrNull()?.newBuilder()
53+
?.addPathSegments("api/webhook/$webhookId")?.build()?.let(::add)
54+
}
55+
}
56+
57+
/**
58+
* Handles Home Assistant server operations specifically for Wear OS device onboarding.
59+
*
60+
* This repository is communicating directly with the Home Assistant server using [WearServer]
61+
* credentials without requiring a persisted server in the app's database.
62+
*/
63+
class SettingsWearRepository @Inject constructor(
64+
private val authenticationService: AuthenticationService,
65+
private val integrationService: IntegrationService,
66+
) {
67+
68+
/**
69+
* Exchanges a refresh token for a new access token and returns an updated [WearServer].
70+
*
71+
* @param server The server configuration containing the URLs to try.
72+
* @param refreshToken The OAuth refresh token to exchange.
73+
* @return A copy of [server] with the new access token populated.
74+
* @throws AuthorizationException If the token response is empty.
75+
* @throws IntegrationException If the refresh request fails.
76+
*/
77+
suspend fun registerRefreshToken(server: WearServer, refreshToken: String): WearServer {
78+
return tryOnUrls(server.getBaseUrls(), "refresh_token") {
79+
val response = authenticationService.refreshToken(
80+
it.newBuilder().addPathSegments(SEGMENT_AUTH_TOKEN).build(),
81+
AuthenticationService.GRANT_TYPE_REFRESH,
82+
refreshToken,
83+
AuthenticationService.CLIENT_ID,
84+
)
85+
if (response.isSuccessful) {
86+
val refreshedToken = response.body() ?: throw AuthorizationException()
87+
server.copy(accessToken = refreshedToken.accessToken)
88+
} else {
89+
throw IntegrationException(
90+
"Error calling refresh token",
91+
response.code(),
92+
response.errorBody(),
93+
)
94+
}
95+
}
96+
}
97+
98+
/**
99+
* Renders a Home Assistant template string on the server.
100+
*
101+
* @param wearServer The server configuration containing the webhook URLs.
102+
* @param template The Jinja2 template string to render.
103+
* @return The rendered template result as a string, or `null` if the result is null.
104+
*/
105+
suspend fun renderTemplate(wearServer: WearServer, template: String): String? {
106+
val templateResult = tryOnUrls(
107+
wearServer.getWebhookUrls(),
108+
"render_template",
109+
) { url ->
110+
integrationService.getTemplate(
111+
url,
112+
RenderTemplateIntegrationRequest(
113+
mapOf("template" to Template(template, emptyMap())),
114+
),
115+
)["template"]
116+
}
117+
// We check if the result is a JsonPrimitive instead of a simple global toString to avoid rendering " around the string
118+
return if (templateResult is JsonPrimitive) templateResult.contentOrNull else templateResult.toString()
119+
}
120+
121+
/**
122+
* Fetches all entities from the Home Assistant server.
123+
*
124+
* Requires [WearServer.accessToken] to be set. Call [registerRefreshToken] first
125+
* to obtain an access token.
126+
*
127+
* @param wearServer The server configuration with a valid access token.
128+
* @return A list of all entities, or an empty list if the request fails or no access token is set.
129+
*/
130+
suspend fun getEntities(wearServer: WearServer): List<Entity> {
131+
if (wearServer.accessToken == null) {
132+
FailFast.fail { "Missing access token, you should invoke registerRefreshToken first" }
133+
return emptyList()
134+
}
135+
136+
return try {
137+
tryOnUrls(wearServer.getBaseUrls(), "get_entities") { url ->
138+
integrationService.getStates(
139+
url.newBuilder().addPathSegments("api/states").build(),
140+
"Bearer ${wearServer.accessToken}",
141+
)
142+
}.map {
143+
Entity(
144+
it.entityId,
145+
it.state,
146+
it.attributes,
147+
it.lastChanged,
148+
it.lastUpdated,
149+
)
150+
}
151+
} catch (e: IntegrationException) {
152+
Timber.e(e, "Fail to get entities")
153+
emptyList()
154+
}
155+
}
156+
}

0 commit comments

Comments
 (0)