Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RKOTLIN-1023] Add support for changing App Services base URL #1733

Merged
merged 8 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/include-deploy-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:

jobs:
deploy:
runs-on: macos-latest
runs-on: macos-12
name: Deploy release

steps:
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/include-integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:

# TODO: The Monkey seems to crash the app all the time, but with failures that are not coming from the app. Figure out why.
# android-sample-app:
# runs-on: macos-latest
# runs-on: macos-12
# steps:
# - name: Checkout code
# uses: actions/checkout@v3
Expand Down Expand Up @@ -87,7 +87,7 @@ jobs:
./gradlew assembleDebug jvmJar
realm-java-compatibiliy:
runs-on: macos-latest
runs-on: macos-12
steps:
- name: Checkout code
uses: actions/checkout@v3
Expand Down Expand Up @@ -217,7 +217,7 @@ jobs:
- type: gradle75
path: integration-tests/gradle/gradle75-test
arguments: integrationTest
runs-on: macos-latest
runs-on: macos-12
steps:
- uses: actions/checkout@v3

Expand Down Expand Up @@ -295,7 +295,7 @@ jobs:
- type: gradle8
path: integration-tests/gradle/gradle8-test
arguments: integrationTest
runs-on: macos-latest
runs-on: macos-12
steps:
- uses: actions/checkout@v3

Expand Down
32 changes: 16 additions & 16 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ jobs:
retention-days: 1

build-jvm-macos-native-lib:
runs-on: macos-latest
runs-on: macos-12
needs: [check-cache, build-jni-swig-stub]
if: |
always() &&
Expand Down Expand Up @@ -395,7 +395,7 @@ jobs:
# This task is also responsible for creating the Gradle and Compiler Plugin as well as
# all Kotlin Multiplatform Metadata
build-jvm-packages:
runs-on: macos-latest
runs-on: macos-12
needs: [check-cache, build-jvm-linux-native-lib, build-jvm-windows-native-lib, build-jvm-macos-native-lib]
if: |
always() &&
Expand Down Expand Up @@ -638,7 +638,7 @@ jobs:
# TODO: ccache is not being used by this build for some reason
build-macos-x64-packages:
runs-on: macos-latest
runs-on: macos-12
needs: check-cache
if: always() && !cancelled() && needs.check-cache.outputs.packages-macos-x64-cache-hit != 'true'

Expand Down Expand Up @@ -706,7 +706,7 @@ jobs:
retention-days: 1

build-macos-arm64-packages:
runs-on: macos-latest
runs-on: macos-12
needs: check-cache
# needs: static-analysis
if: always() && !cancelled() && needs.check-cache.outputs.packages-macos-arm64-cache-hit != 'true'
Expand Down Expand Up @@ -774,7 +774,7 @@ jobs:
retention-days: 1

build-ios-x64-packages:
runs-on: macos-latest
runs-on: macos-12
needs: check-cache
# needs: static-analysis
if: always() && !cancelled() && needs.check-cache.outputs.packages-ios-x64-cache-hit != 'true'
Expand Down Expand Up @@ -843,7 +843,7 @@ jobs:
retention-days: 1

build-ios-arm64-packages:
runs-on: macos-latest
runs-on: macos-12
needs: check-cache
# needs: static-analysis
if: always() && !cancelled() && needs.check-cache.outputs.packages-ios-arm64-cache-hit != 'true'
Expand Down Expand Up @@ -931,7 +931,7 @@ jobs:
- type: sync
test-title: Unit Test Results - Android Sync (Emulator)

runs-on: macos-latest
runs-on: macos-12
needs: [check-cache, build-android-packages, build-jvm-packages, build-kotlin-metadata-package]
if: |
always() &&
Expand Down Expand Up @@ -1175,15 +1175,15 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [macos-latest] # , macos-arm]
os: [macos-12] # , macos-arm]
type: [base, sync]
include:
- os: macos-latest
- os: macos-12
type: base
os-id: macos
package-prefix: macos-x64
test-title: Unit Test Results - MacOS x64 Base
- os: macos-latest
- os: macos-12
type: sync
os-id: macos
package-prefix: macos-x64
Expand Down Expand Up @@ -1297,15 +1297,15 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [macos-latest] # , macos-arm]
os: [macos-12] # , macos-arm]
type: [base, sync]
include:
- os: macos-latest
- os: macos-12
type: base
package-prefix: x64
test-title: Unit Test Results - iOS x64 Base
test-task: iosTest
- os: macos-latest
- os: macos-12
type: sync
package-prefix: x64
test-title: Unit Test Results - iOS x64 Sync
Expand Down Expand Up @@ -1419,10 +1419,10 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [macos-latest, ubuntu-latest, windows-latest] # TODO Should we also test om MacOS arm64?
os: [macos-12, ubuntu-latest, windows-latest] # TODO Should we also test om MacOS arm64?
type: [base, sync]
include:
- os: macos-latest
- os: macos-12
os-id: mac
type: base
test-title: Unit Test Results - Base JVM MacOS x64
Expand All @@ -1434,7 +1434,7 @@ jobs:
os-id: win
type: base
test-title: Unit Test Results - Base JVM Windows
- os: macos-latest
- os: macos-12
os-id: mac
type: sync
test-title: Unit Test Results - Sync JVM MacOS x64
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This release will bump the Realm file format from version 23 to 24. Opening a fi
* None.

### Enhancements
* None.
* Add support for changing the App Services base URL. It allows to roam between Atlas and Edge Server. Changing the url would trigger a client reset. ([JIRA]https://jira.mongodb.org/browse/RKOTLIN-1023)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably continue linking to the github issues.


### Fixed
* None.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,16 @@ expect object RealmInterop {
callback: AppCallback<Array<ApiKeyWrapper>>,
)

fun realm_app_get_base_url(
app: RealmAppPointer,
): String

fun realm_app_update_base_url(
app: RealmAppPointer,
baseUrl: String,
callback: AppCallback<Unit>,
)

// User
fun realm_user_get_all_identities(user: RealmUserPointer): List<SyncUserIdentity>
fun realm_user_get_identity(user: RealmUserPointer): String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,18 @@ actual object RealmInterop {
return result
}

actual fun realm_app_get_base_url(
app: RealmAppPointer,
): String = realmc.realm_app_get_base_url(app.cptr())

actual fun realm_app_update_base_url(
app: RealmAppPointer,
baseUrl: String,
callback: AppCallback<Unit>,
) {
realmc.realm_app_update_base_url(app.cptr(), baseUrl, callback)
}

actual fun realm_user_get_all_identities(user: RealmUserPointer): List<SyncUserIdentity> {
val count = AuthProvider.values().size.toLong() // Optimistically allocate the max size of the array
val keys = realmc.new_identityArray(count.toInt())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2309,6 +2309,33 @@ actual object RealmInterop {
.also { realm_wrapper.realm_free(cPath) }
}

actual fun realm_app_get_base_url(
app: RealmAppPointer,
): String = realm_wrapper.realm_app_get_base_url(app.cptr())?.toKString()!!

actual fun realm_app_update_base_url(
app: RealmAppPointer,
baseUrl: String,
callback: AppCallback<Unit>,
) {
checkedBooleanResult(
realm_wrapper.realm_app_update_base_url(
app.cptr(),
baseUrl,
callback = staticCFunction { userData, error ->
handleAppCallback(
userData,
error
) { /* No-op, returns Unit */ }
},
StableRef.create(callback).asCPointer(),
staticCFunction { userdata ->
disposeUserData<AppCallback<Unit>>(userdata)
}
)
)
}

actual fun realm_user_get_all_identities(user: RealmUserPointer): List<SyncUserIdentity> {
memScoped {
val count = AuthProvider.values().size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ public interface App {
*/
public val sync: Sync

/**
* Current base URL to communicate with App Services.
*/
public val baseUrl: String

/**
* Sets the App Services base url. Changing the URL would trigger a client reset.
*
* @param baseUrl The new App Services base url. Use [AppConfiguration.DEFAULT_BASE_URL] to set it
* to the default value.
*/
public suspend fun updateBaseUrl(baseUrl: String)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing an empty string to updateBaseUrl would result in the base URL being set to a default value set in core.

Because the default URL is already set in core, we could replace the actual URL value in the SDK with an empty string. This way we could avoid maintaining the URL on the SDK.

AppConfiguration.DEFAULT_BASE_URL = ""

We would address this on a different PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also make baseUrl optional and allow users to supply null here to the same effect of supplying empty string. I feel this is a bit more obvious and the C API already works that way.


/**
* Returns all known users that are either [User.State.LOGGED_IN] or [User.State.LOGGED_OUT].
* Only users that at some point logged into this device will be returned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@ public class AppImpl(
@Suppress("MagicNumber")
private val reconnectThreshold = 5.seconds

override val baseUrl: String
get() = RealmInterop.realm_app_get_base_url(nativePointer)

override suspend fun updateBaseUrl(baseUrl: String) {
Channel<Result<Unit>>(1).use { channel ->
RealmInterop.realm_app_update_base_url(
app = nativePointer,
baseUrl = baseUrl.trimEnd('/'), // trailing slashes are not handled properly in core
callback = channelResultCallback<Unit, Unit>(channel) {
// No-op
}
)
channel.receive().getOrThrow()
}
}

@Suppress("invisible_member", "invisible_reference", "MagicNumber")
private val connectionListener = NetworkStateObserver.ConnectionListener { connectionAvailable ->
// In an ideal world, we would be able to reliably detect the network coming and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import io.realm.kotlin.mongodb.LoggedOut
import io.realm.kotlin.mongodb.Removed
import io.realm.kotlin.mongodb.User
import io.realm.kotlin.mongodb.exceptions.InvalidCredentialsException
import io.realm.kotlin.mongodb.exceptions.ServiceException
import io.realm.kotlin.mongodb.sync.SyncConfiguration
import io.realm.kotlin.test.mongodb.TEST_APP_FLEX
import io.realm.kotlin.test.mongodb.TestApp
Expand Down Expand Up @@ -478,4 +479,70 @@ class AppTests {
}
}
}

/**
* The app id must exist on the new base url, it is validated and an exception would be thrown.
*
* This test case circumvents this issue by initializing an app to a url that does not contain the
* app, as it is not validated on initialization. And then updating the base url the the test server.
*/
@Test
fun changeBaseUrl() {
TestApp(
testId = "changeBaseUrl",
builder = { builder ->
// We create a test app that points to the default base url
// this app is not going to be validated yet.
builder.baseUrl(AppConfiguration.DEFAULT_BASE_URL)
}
).use { testApp ->
assertEquals(AppConfiguration.DEFAULT_BASE_URL, testApp.baseUrl)

runBlocking {
// Update the base url, this method will validate the app
// if the app id is not available it would fail.
testApp.updateBaseUrl(app.configuration.baseUrl)
}
assertEquals(app.configuration.baseUrl, testApp.baseUrl)
}
}

@Test
@Ignore // See https://github.com/realm/realm-kotlin/issues/1734
fun changeBaseUrl_trailing_slashes_trimmed() {
assertFailsWithMessage<ServiceException>("cannot find app using Client App ID") {
runBlocking {
app.updateBaseUrl(AppConfiguration.DEFAULT_BASE_URL + "///")
}
}
}

@Test
@Ignore // see https://github.com/realm/realm-kotlin/issues/1734
fun changeBaseUrl_empty() {
assertFailsWithMessage<ServiceException>("cannot find app using Client App ID") {
runBlocking {
app.updateBaseUrl("")
}
}
}

@Test
fun changeBaseUrl_invalidUrl() {
assertFailsWithMessage<IllegalArgumentException>("URL missing scheme separator") {
runBlocking {
app.updateBaseUrl("hello world")
}
}
}

@Test
@Ignore // see https://github.com/realm/realm-kotlin/issues/1734
fun changeBaseUrl_nonAppServicesUrl() {
assertFailsWithMessage<ServiceException>("http error code considered fatal") {
runBlocking {
app.updateBaseUrl("https://www.google.com/")
}
}
}
}
Loading