Add frontend url manager#6382
Conversation
TimoPtr
added
the
WebViewActivity replacement
| var pathConsumed = false | ||
| serverManager.connectionStateProvider(actualServerId).urlFlow().collect { urlState -> | ||
| val currentPath = if (pathConsumed) null else path | ||
| pathConsumed = true |
There was a problem hiding this comment.
Would it make sense to not consume the path until the state actually returns an URL?
Example use case: I have sent a notification which deeplinks to something, but when I tap the notification I'm not yet connected to my home network so loading is blocked, and when I connect to Wi-Fi it reloads but ignores the path as it was already consumed by the blocked screen.
There was a problem hiding this comment.
I'm not sure if we want to change the behavior on this PR, but yes it would make sense. I saw also this PR #6403 that is somehow related.
| * | ||
| * @param serverId The server ID that had security level configured | ||
| */ | ||
| fun onSecurityLevelConfigured(serverId: Int) { |
There was a problem hiding this comment.
The user can ignore the screen so it is not known whether or not they actually configured it.
| fun onSecurityLevelConfigured(serverId: Int) { | |
| fun onSecurityLevelShown(serverId: Int) { |
| } | ||
|
|
||
| @Test | ||
| fun `Given insecure state when serverUrlFlow then returns InsecureBlocked`() = runTest { |
There was a problem hiding this comment.
This test is for nothing setup. The next test is for home setup but not home + location disabled. I don't see tests for the other possible combinations. Why pick these two?
| @Test | ||
| fun `Given SERVER_ID_ACTIVE when serverUrlFlow then resolves to actual server ID`() = runTest { | ||
| val server = createTestServer(id = 42, externalUrl = "https://home.example.com") | ||
| coEvery { serverManager.getServer(ServerManager.SERVER_ID_ACTIVE) } returns server | ||
| coEvery { serverManager.getServer(42) } returns server | ||
| coEvery { sessionManager.isSessionConnected(42) } returns SessionCheckResult.Connected | ||
| coEvery { serverManager.activateServer(42) } just runs | ||
| coEvery { serverManager.connectionStateProvider(42) } returns connectionStateProvider | ||
| every { connectionStateProvider.urlFlow(any()) } returns flowOf( | ||
| UrlState.HasUrl(URL("https://home.example.com")), | ||
| ) | ||
|
|
||
| urlManager.serverUrlFlow(serverId = ServerManager.SERVER_ID_ACTIVE, path = null).test { | ||
| val result = awaitItem() | ||
| assertTrue(result is UrlLoadResult.Success) | ||
| assertEquals(42, (result as UrlLoadResult.Success).serverId) | ||
| awaitComplete() | ||
| } | ||
|
|
||
| coVerify { serverManager.activateServer(42) } | ||
| coVerify { serverManager.connectionStateProvider(42) } | ||
| } |
There was a problem hiding this comment.
Should be moved up or down, right now it is in the middle of the connection security level related tests
| coEvery { serverManager.activateServer(1) } just runs | ||
| coEvery { serverManager.connectionStateProvider(1) } returns connectionStateProvider | ||
| every { connectionStateProvider.urlFlow(any()) } returns flowOf( | ||
| UrlState.HasUrl(URL("http://home.example.com")), |
There was a problem hiding this comment.
Why does it do this? HTTP + disallow insecure shouldn't return anything.
2 participants
Summary
Introduces FrontendUrlManager, a dedicated class for handling URL resolution and security checks for the frontend WebView. This extracts URL loading logic from WebViewPresenterImpl into a focused, testable component.
Checklist
Any other notes
This code is not yet used but it is going to be in a next PR this is based on #6380