Feat kmp sdk #1040
Feat kmp sdk #1040
Conversation
…to feat-kmp-sdk
…to feat-kmp-sdk
…to feat-kmp-sdk
Certain data classes weren't serializing properly, moved actualSerializer to be above apiParams, added json encoding to data.
The socket was being created unnecessarily whenever a subscription was removed. This change ensures that the socket is only created when there are active channels.
templates/kmp/settings.gradle.kts
Outdated
| } | ||
| } | ||
|
|
||
| rootProject.name = "Appwrite_KMP_SDK" |
| } else { | ||
| engine { | ||
| config { | ||
| val trustManagerFactory = | ||
| TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()) | ||
| trustManagerFactory.init(null as KeyStore?) | ||
| val sslContext = SSLContext.getInstance("TLS").apply { | ||
| init(null, trustManagerFactory.trustManagers, SecureRandom()) | ||
| } | ||
| sslSocketFactory( | ||
| sslContext.socketFactory, | ||
| trustManagerFactory.trustManagers[0] as X509TrustManager | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Can this else block be skipped? Is it the same as a the default behaviour?
There was a problem hiding this comment.
I'm not super familiar with certificates, but from my understanding this uses a PKIX algorithm instead of Trust All. I'm not sure if that's the correct approach, but I figured if there was an option to not run setSelfSigned then it should use a standard algorithm?
There was a problem hiding this comment.
We can remove the else block, since the behavior when not setting self-signed will be the same with or without it 👌
| $ret = $this->toPascalCase($method['responseModel']); | ||
|
|
||
| if ($this->hasGenericType($method['responseModel'], $spec)) { | ||
| if ($this->hasGenericType($method['responseModel'], $spec) && $withGeneric) { |
There was a problem hiding this comment.
What's the use case for skipping the generic?
There was a problem hiding this comment.
It's for situations like this:
serializer = io.appwrite.models.Document.serializer(actualSerializer)
without the skip I would get serializer = io.appwrite.models.Document<T>.serializer(actualSerializer)
I just found how it's been done for the same situation:
{{sdk.namespace | caseDot}}.models.{{ method.responseModel | caseUcfirst }}
I can change it to match the original method and get rid of this change if you like that better.
There was a problem hiding this comment.
Yep let's do it so we can handle both generics and non-generics in one shot
templates/android/library/src/main/java/io/package/WebAuthComponent.kt.twig
Show resolved
Hide resolved
| private val appVersion by lazy { | ||
| try { | ||
| val properties = Properties() | ||
| properties.load(this.javaClass.getResourceAsStream("/version.properties")) |
There was a problem hiding this comment.
Is this safe? Can we guarantee the file will always exist?
There was a problem hiding this comment.
I'm not in a position to get the JVM stuff working properly right now so I was thinking of removing it entirely. I added it for a quick test that I was doing so I just did a port from the androidMain to jvmMain with chatGPT. I wasn't planning on this PR being a v1.0 or anything so I thought I'd add my JVM stuff to get the ball rolling. Let me know if you think JVM should just be removed in this case. It shouldn't be too difficult to add back since it's so similar to the android side.
There was a problem hiding this comment.
Makes sense, we can remove for now and add other platform support in phases 👌
| "origin" to "appwrite-jvm://app", | ||
| "content-type" to "application/json", | ||
| "origin" to "{{ spec.title | caseLower }}-jvm://app", | ||
| "user-agent" to "JVM/$appVersion, ${System.getProperty("java.version")}", |
There was a problem hiding this comment.
What's changed in these php tests?
There was a problem hiding this comment.
Nothing, I think they got caught in me updating the line endings. I'll revert them
tests/KMPAndroid14Java17Test.php
Outdated
| 'chmod +x tests/sdks/kmp/gradlew', | ||
| ]; | ||
| protected string $command = | ||
| 'docker run --rm --platform linux/amd64 --network="mockapi" -v $(pwd):/app -w /app/tests/sdks/kmp alvrme/alpine-android:android-34-jdk17 sh -c "./gradlew :shared:testDebugUnitTest --stacktrace -q && cat shared/result.txt"'; |
There was a problem hiding this comment.
Do we need to specify platform here? I think this image is multi-arch right?
There was a problem hiding this comment.
Please add tests for iOS and JVM too. We can also add them in .github/workflows/tests.yml so they run for each PR
…to feat-kmp-sdk
…ded, Fix http/https check in Client
This commit refactors the OAuth2 authentication flow on iOS to use `ASWebAuthenticationSession` instead of `UIApplication.openURL`. This provides a more secure and user-friendly authentication experience.
Key changes:
- `WebAuthComponent.ios.kt`:
- Replaced `UIApplication.openURL` with `ASWebAuthenticationSession`.
- Added `PresentationContextProviderImpl` to provide a presentation anchor for the authentication session.
- Updated `PendingAuth` to store the `ASWebAuthenticationSession`.
- Improved error handling and cancellation.
- Added `setCookieStorage` to allow passing the cookie storage instance.
- Modified `handleIncomingCookie` to parse cookie details from the callback URL and store them using the provided `IosCookieStorage` and `NSUserDefaults`.
- Added a `cleanup` method to cancel pending authentication sessions.
- `OAuth2Extensions.kt`:
- Updated `createOAuth2Session` and `createOAuth2Token` to use the new `WebAuthComponent` and pass the `iosCookieStorage`.
- Ensured cookies are correctly constructed and stored.
- Improved error handling for missing authentication cookies.
templates/kmp/README.md.twig
Outdated
| sourceSets { | ||
| commonMain { | ||
| dependencies { | ||
| implementation("io.github.camka14.appwrite:sdk-for-kmp:0.1.0") |
There was a problem hiding this comment.
Let's template the group name and version here
| @@ -0,0 +1,3 @@ | |||
| <resources> | |||
| <string name="app_name">Appwrite KMP SDK</string> | |||
| @@ -0,0 +1,16 @@ | |||
| <resources xmlns:tools="http://schemas.android.com/tools"> | |||
| <!-- Base application theme. --> | |||
| <style name="Theme.AppwriteKMPSDK" parent="Theme.MaterialComponents.DayNight.DarkActionBar"> | |||
| import okio.Path.Companion.toPath | ||
|
|
||
| actual class Client constructor( | ||
| private val context: Context, |
There was a problem hiding this comment.
Any thoughts here? If users don't pass application context specifically this will be a memory leak if the client outlives the context
templates/kmp/shared/src/androidMain/kotlin/io/package/HttpClientConfig.kt.twig
Show resolved
Hide resolved
| storage = dataStoreCookieStorage | ||
| } | ||
| install(WebSockets) { | ||
| pingInterval = 30.seconds |
There was a problem hiding this comment.
Does this integrate with our custom ping 🤔
| val properties = mutableMapOf<Any?, Any?>( | ||
| NSHTTPCookieName to storedCookie.name, | ||
| NSHTTPCookieValue to storedCookie.value, | ||
| NSHTTPCookiePath to storedCookie.path, | ||
| NSHTTPCookieDomain to storedCookie.domain | ||
| ) |
There was a problem hiding this comment.
Lets also handle expires, max-age, samesite, httponly, and secure
There was a problem hiding this comment.
OAuth methods should be handled in the service template so they appear along other Account methods
| } | ||
|
|
||
| result.getOrNull()?.let { callbackUrl -> | ||
| runBlocking { |
| private val appVersion by lazy { | ||
| try { | ||
| val properties = Properties() | ||
| properties.load(this.javaClass.getResourceAsStream("/version.properties")) |
There was a problem hiding this comment.
Makes sense, we can remove for now and add other platform support in phases 👌
… into feat-kmp-sdk
…to feat-kmp-sdk
|
Any news when the sdk will be available? |
I missed Jake's previous comments, for some reason I haven't been getting notifications from Github like I used to. I'll implement his suggestions and we'll see if anything else is needed. |
- Updated OAuth2Extensions.kt.twig to use CoroutineScope for asynchronous operations during OAuth authentication. - Removed AllCertsTrustManager.kt.twig, Client.jvm.kt.twig, HttpClient.kt.twig, WebAuthComponent.jvm.kt.twig, and related files as they are no longer needed. - Cleaned up OAuth2Extensions.kt.twig by removing redundant code and improving error handling. - Removed InputFile.jvm.kt.twig, JvmParsedUrl.kt.twig, and UrlParser.jvm.kt.twig as part of the cleanup. - Updated test files for Android and KMP to reflect changes in the SDK structure and ensure compatibility with the latest changes.
In the mean time you can use my sdk here: https://github.com/camka14/sdk-for-kmp |
WalkthroughAdds Kotlin Multiplatform (KMP) support to the SDK generator by introducing a new language class Appwrite\SDK\Language\KMP and a large set of KMP templates (project scaffolding, Gradle, Android/iOS apps, shared common code, models, services, serializers, utilities, cookie stores, WebAuth components, and tests). Updates Kotlin and Android language helpers to accept an optional withGeneric flag for return-type generation. Adds example.php routing for KMP SDK generation and multiple CI/workflow, resource, and test files for KMP projects. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus review on:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 44
♻️ Duplicate comments (1)
templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig (1)
12-45: Do not retain the passed-in Context; promote applicationContext.We’re still caching whatever Context the caller supplies. If they hand us an Activity (very likely in sample apps), we leak it for the lifetime of the client—exactly the concern from the previous review. Capture
context.applicationContext ?: contextonce and use that everywhere so we only retain the Application.-actual class Client constructor( - private val context: Context, +actual class Client constructor( + context: Context, endpoint: String = "{{spec.endpoint}}", endpointRealtime: String? = null, selfSigned: Boolean = false, ) : BaseClient<Client>(endpoint, endpointRealtime) { + private val appContext = context.applicationContext ?: context actual override val coroutineContext = Job() + Dispatchers.Default private val dataStoreManager = DataStoreManager( PreferenceDataStoreFactory.createWithPath ( - produceFile = { context.filesDir.resolve("appwriteCookies.preferences_pb").absolutePath.toPath() } + produceFile = { appContext.filesDir.resolve("appwriteCookies.preferences_pb").absolutePath.toPath() } )) val dataStoreCookieStorage = DataStoreCookieStorage(dataStoreManager) private val appVersion by lazy { try { - val pInfo = context.packageManager.getPackageInfo(context.packageName, 0) + val pInfo = appContext.packageManager.getPackageInfo(appContext.packageName, 0) return@lazy pInfo.versionName } catch (e: PackageManager.NameNotFoundException) { e.printStackTrace() return@lazy "" } @@ httpClient = createHttpClient(selfSigned, dataStoreCookieStorage) headers = mutableMapOf( "content-type" to "application/json", - "origin" to "{{ spec.title | caseLower }}-android://${context.packageName}", - "user-agent" to "${context.packageName}/${appVersion}, ${System.getProperty("http.agent")}", + "origin" to "{{ spec.title | caseLower }}-android://${appContext.packageName}", + "user-agent" to "${appContext.packageName}/${appVersion}, ${System.getProperty("http.agent")}", "x-sdk-name" to "{{ sdk.name }}", "x-sdk-platform" to "{{ sdk.platform }}", "x-sdk-language" to "{{ language.name | caseLower }}",
🧹 Nitpick comments (17)
templates/kmp/shared/src/commonMain/kotlin/io/package/coroutines/Callback.kt.twig (2)
8-10: Add KDoc documentation to the public Callback interface.This is a public API in the SDK and lacks documentation describing the callback contract. Consider adding KDoc to clarify behavior.
/** * Callback interface for handling asynchronous operation results. * * @param T The type of the result value on success. */ interface Callback<T> { /** * Called when the operation completes. * * Exactly one of [result] or [error] will be non-null: * - If successful, [result] contains the value and [error] is null. * - If failed, [error] contains the exception and [result] is null. * * @param result The result value, or null if the operation failed. * @param error The exception if the operation failed, or null if successful. */ fun onComplete(result: T?, error: Throwable?) }
12-19: CoroutineCallback bridge is correctly implemented.The class properly implements
Continuation<T>and correctly unwrapsResult<T>usinggetOrNull()andexceptionOrNull(), ensuring the callback receives exactly one of value or error as non-null. The use of@JvmOverloadsandDispatchers.Defaultas the default context is idiomatic and appropriate for Kotlin/Java interop.Consider adding KDoc for consistency and to document the bridge pattern.
/** * Bridges Kotlin coroutine [Continuation] to a [Callback]-based API. * * This adapter allows coroutine-based code to drive callback-based APIs * by converting [Result<T>] completions into [Callback] invocations. * * @param T The type of the result value. * @param callback The callback to invoke upon completion. * @param context The coroutine context for this continuation. Defaults to [Dispatchers.Default]. */ class CoroutineCallback<T> @JvmOverloads constructor( private val callback: Callback<T>, override val context: CoroutineContext = Dispatchers.Default ) : Continuation<T> { override fun resumeWith(result: Result<T>) { callback.onComplete(result.getOrNull(), result.exceptionOrNull()) } }templates/kmp/androidApp/src/main/res/values/themes.xml.twig (1)
5-11: Consider making the color scheme more generic for SDK scaffolding.The hardcoded purple and teal color scheme is typical for demo applications but may not be suitable for production SDK scaffolding. SDK consumers typically want to define their own branding colors.
Consider either:
- Using more neutral/generic colors (e.g., grays)
- Making the color scheme configurable via template variables
- Adding comments in the generated code encouraging developers to customize the colors
templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/TypeExtensions.kt.twig (1)
6-9: Replace force-unwrap operator for better error handling.Line 8 uses the
!!operator, which will throw an NPE with no context ifclassifieris null. While this should not happen for reified types in practice, using a safer alternative provides better error messages during debugging.Apply this diff to use
requireNotNullinstead:inline fun <reified T : Any> classOf(): KClass<T> { @Suppress("UNCHECKED_CAST") - return (typeOf<T>().classifier!! as KClass<T>) + return requireNotNull(typeOf<T>().classifier as? KClass<T>) { + "Unable to obtain KClass for type ${typeOf<T>()}" + } }templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/IOExtensions.kt.twig (1)
9-10: Replace force-unwrap for safer file size access.Line 10 uses the
!!operator onsize, which will throw an NPE if the size is null (e.g., for certain file types or in error conditions). While Okio typically provides size for regular files, using a safer approach prevents runtime crashes.Apply this diff:
fun readFileSize(filePath: String): Long = - FileSystem.SYSTEM.metadata(filePath.toPath()).size!! + FileSystem.SYSTEM.metadata(filePath.toPath()).size + ?: error("Unable to determine file size for: $filePath")Or if you expect size to always be present for valid files:
fun readFileSize(filePath: String): Long = - FileSystem.SYSTEM.metadata(filePath.toPath()).size!! + requireNotNull(FileSystem.SYSTEM.metadata(filePath.toPath()).size) { + "File size is unavailable for: $filePath" + }templates/kmp/shared/src/commonMain/kotlin/io/package/webInterface/ParsedUrl.kt.twig (1)
3-5: Consider removingoverridekeyword for convention.While technically valid since interfaces implicitly inherit from
Any, using theoverridekeyword ontoString()in an interface declaration is unconventional in Kotlin. The standard practice is to declarefun toString(): Stringwithout theoverridemodifier.Apply this diff for more idiomatic Kotlin:
interface ParsedUrl { - override fun toString(): String + fun toString(): String }templates/kmp/androidApp/build.gradle.kts.twig (1)
31-37: Consider upgrading to Java 11 or 17.While Java 1.8 is still widely supported, modern Android development typically uses Java 11 or Java 17 for better language features and performance. Since this is a template for new SDK projects, consider defaulting to a more recent Java version.
Apply this diff to upgrade to Java 17:
compileOptions { - sourceCompatibility = JavaVersion.VERSION_1_8 - targetCompatibility = JavaVersion.VERSION_1_8 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 } kotlinOptions { - jvmTarget = "1.8" + jvmTarget = "17" }templates/kmp/androidApp/src/main/java/io/package/android/services/MessagingService.kt.twig (2)
27-35: Consider using a coroutine scope instead ofrunBlocking.The
runBlockingcall blocks the Firebase messaging thread, which can impact performance. Consider usingGlobalScope.launchor a properCoroutineScopefor better concurrency handling.Apply this diff:
- runBlocking { + GlobalScope.launch { if (targetId == null) { val target = account!!.createPushTarget(ID.unique(), token) prefs.edit().putString("targetId", target.id).apply() } else { account!!.updatePushTarget(targetId, token) } }Additionally, add the import:
import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch
29-33: Prefer safe call orletover force unwrap (!!).After the null check at Line 21,
accountis guaranteed non-null, but the!!operator is considered a code smell in Kotlin.Apply this diff:
runBlocking { + val acc = account ?: return@runBlocking if (targetId == null) { - val target = account!!.createPushTarget(ID.unique(), token) + val target = acc.createPushTarget(ID.unique(), token) prefs.edit().putString("targetId", target.id).apply() } else { - account!!.updatePushTarget(targetId, token) + acc.updatePushTarget(targetId, token) } }templates/kmp/androidApp/src/main/java/io/package/android/ui/accounts/AccountsFragment.kt.twig (1)
35-43: Consider extracting SharedPreferences helper.The SharedPreferences access pattern is repeated multiple times. For improved maintainability, consider extracting a helper function or property.
Example refactor:
private val examplePrefs get() = context?.getSharedPreferences("example", Context.MODE_PRIVATE) // Then use: binding.login.setOnClickListener{ viewModel.onLogin( binding.email.text.toString(), binding.password.text.toString(), examplePrefs?.getString("fcmToken", null) ?: "" ) }Also applies to: 73-81
templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/stores/DataStoreManager.kt.twig (1)
14-23: Consider adding error handling for DataStore operations.Both
read()andwrite()operations can fail due to I/O errors, but no exception handling is present. Consider wrapping these operations in try-catch blocks or documenting that callers must handle potential exceptions.Example with error handling:
suspend fun read(): String? { - val preferences = dataStore.data.first() - return preferences[key] + return try { + val preferences = dataStore.data.first() + preferences[key] + } catch (e: Exception) { + null // or log and rethrow + } } suspend fun write(value: String) { - dataStore.edit { preferences -> - preferences[key] = value + try { + dataStore.edit { preferences -> + preferences[key] = value + } + } catch (e: Exception) { + // log error + throw e } }templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/DynamicLookupSerializer.kt.twig (2)
67-82: Recursive deserialization lacks depth protection.The recursive calls in
deserializeJsonElement(lines 79) and the recursive nature of the serializer (line 50) have no depth limit, which could lead to stack overflow with deeply nested structures.Consider adding a depth counter:
private fun deserializeJsonElement(element: JsonElement, depth: Int = 0): Any? { require(depth < 1000) { "Maximum nesting depth exceeded" } return when (element) { // ... existing cases ... is JsonArray -> element.map { deserializeJsonElement(it, depth + 1) } // ... etc } }
58-58: Fallback totoString()may lose type information.For types not explicitly handled, serialization falls back to encoding the
toString()representation. This means custom objects will be serialized as strings and cannot be deserialized back to their original type, potentially causing data loss.Consider either:
- Documenting this limitation clearly
- Throwing an exception for unsupported types to fail fast
- Supporting additional common types (ByteArray, Enum, etc.)
templates/kmp/shared/src/commonMain/kotlin/io/package/models/InputFile.kt.twig (1)
9-15: Lateinit properties risk uninitialized access.All properties use
lateinit, which will throwUninitializedPropertyAccessExceptionif accessed before initialization. Since the constructor is private and only factory methods should be used, consider making these properties non-nullable with proper initialization, or add validation that all required properties are set.Alternative approach with constructor parameters:
class InputFile private constructor( val path: String = "", val filename: String, val sourceType: String, val mimeType: String, @Contextual val data: Buffer = Buffer() )src/SDK/Language/KMP.php (1)
44-44: Remove unused$specparameter.The
$specparameter is not used in the method body and should be removed to clean up the signature.Apply this diff:
- protected function getJsonPropertyAssignment(array $property, array $spec): string + protected function getJsonPropertyAssignment(array $property): stringAlso update the filter registration on line 35:
- $filters[] = new TwigFilter('propertyAssignmentKmp', function (array $property, array $spec) { - return $this->getJsonPropertyAssignment($property, $spec); + $filters[] = new TwigFilter('propertyAssignmentKmp', function (array $property) { + return $this->getJsonPropertyAssignment($property);Based on learnings
templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig (1)
380-407: Duplicate empty body check is redundant.Line 382 checks if the body is empty, but lines 386-393 contain another nested empty body check. The outer check makes the inner one at line 386 unreachable.
Apply this diff to remove the duplicate check:
else -> { val body = response.bodyAsText() - if (body.isEmpty()) { - true as T - } else { - when { - body.isEmpty() -> when (responseType) { - String::class -> "" as T - Boolean::class -> true as T - else -> throw {{ spec.title | caseUcfirst }}Exception( - "Empty response body", - response.status.value - ) - } - - responseType == String::class -> body as T - else -> try { - serializer?.let { json.decodeFromString(it, body) } - ?: json.decodeFromString(responseType.serializer(), body) - } catch (e: Exception) { - throw {{ spec.title | caseUcfirst }}Exception( - "Failed to parse JSON response: ${e.message}", - response.status.value, - response = body - ) - } + when { + body.isEmpty() -> when (responseType) { + String::class -> "" as T + Boolean::class -> true as T + else -> throw {{ spec.title | caseUcfirst }}Exception( + "Empty response body", + response.status.value + ) } + + responseType == String::class -> body as T + else -> try { + serializer?.let { json.decodeFromString(it, body) } + ?: json.decodeFromString(responseType.serializer(), body) + } catch (e: Exception) { + throw {{ spec.title | caseUcfirst }}Exception( + "Failed to parse JSON response: ${e.message}", + response.status.value, + response = body + ) + } } }templates/kmp/shared/src/commonMain/kotlin/io/package/services/Service.kt.twig (1)
35-35: Inconsistent newline handling in method descriptions.Line 35 removes newlines entirely from method descriptions (
replace({"\n": "", "\r": ""})), while line 27 replaces them with spaces. This creates inconsistent formatting where method descriptions become hard to read as run-on text.Apply this diff to use spaces consistently:
- * {{ method.description | raw | replace({"\n": "", "\r": ""}) }} + * {{ method.description | raw | replace({"\n": " ", "\r": " "}) }}Also apply to line 160 which has the same pattern.
Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.locktemplates/kmp/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (84)
example.php(2 hunks)src/SDK/Language/Android.php(1 hunks)src/SDK/Language/KMP.php(1 hunks)src/SDK/Language/Kotlin.php(3 hunks)templates/kmp/.github/workflows/autoclose.yml(1 hunks)templates/kmp/.github/workflows/publish.yml(1 hunks)templates/kmp/.gitignore(1 hunks)templates/kmp/CHANGELOG.md.twig(1 hunks)templates/kmp/LICENSE.md.twig(1 hunks)templates/kmp/README.md.twig(1 hunks)templates/kmp/androidApp/build.gradle.kts.twig(1 hunks)templates/kmp/androidApp/src/main/AndroidManifest.xml.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/MainActivity.kt.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/services/MessagingService.kt.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/ui/accounts/AccountsFragment.kt.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/ui/accounts/AccountsViewModel.kt.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/utils/Client.kt.twig(1 hunks)templates/kmp/androidApp/src/main/java/io/package/android/utils/Event.kt.twig(1 hunks)templates/kmp/androidApp/src/main/res/drawable/ic_launcher_background.xml(1 hunks)templates/kmp/androidApp/src/main/res/drawable/ic_launcher_foreground.xml(1 hunks)templates/kmp/androidApp/src/main/res/layout/activity_main.xml(1 hunks)templates/kmp/androidApp/src/main/res/layout/fragment_account.xml(1 hunks)templates/kmp/androidApp/src/main/res/mipmap-anydpi-v26/ic_launcher.xml(1 hunks)templates/kmp/androidApp/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml(1 hunks)templates/kmp/androidApp/src/main/res/values/colors.xml(1 hunks)templates/kmp/androidApp/src/main/res/values/strings.xml.twig(1 hunks)templates/kmp/androidApp/src/main/res/values/themes.xml.twig(1 hunks)templates/kmp/build.gradle.kts.twig(1 hunks)templates/kmp/gradle.properties(1 hunks)templates/kmp/gradle/libs.versions.toml(1 hunks)templates/kmp/gradle/wrapper/gradle-wrapper.properties(1 hunks)templates/kmp/gradlew(1 hunks)templates/kmp/gradlew.bat(1 hunks)templates/kmp/settings.gradle.kts(1 hunks)templates/kmp/shared/build.gradle.kts.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/HttpClientConfig.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/KeepAliveService.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/WebAuthComponent.android.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/SerializableCookie.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/stores/DataStoreCookieStorage.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/stores/DataStoreManager.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/models/InputFile.android.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/views/CallbackActivity.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/webInterface/AndroidParsedUrl.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/webInterface/UrlParser.android.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/Client.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/ID.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/Permission.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/Role.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/Service.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/WebAuthComponent.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/coroutines/Callback.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/enums/Enum.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/exceptions/Exception.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/CollectionExtensions.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/IOExtensions.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/JsonExtensions.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/TypeExtensions.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/models/InputFile.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/models/Model.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/models/RealtimeModels.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/models/UploadProgress.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/DynamicLookupSerializer.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/StringCollectionSeriailizer.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/services/Realtime.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/services/Service.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/webInterface/ParsedUrl.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/webInterface/UrlParser.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/Client.ios.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/HttpClientConfig.ios.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/cookies/IosCookieStorage.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/models/InputFile.ios.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/IosParsedUrl.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig(1 hunks)tests/Android14Java11Test.php(1 hunks)tests/Android14Java8Test.php(1 hunks)tests/KMPAndroid14Java17Test.php(1 hunks)tests/languages/kmp/Tests.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
example.php (2)
src/SDK/SDK.php (1)
SDK(21-941)src/SDK/Language/KMP.php (1)
KMP(7-636)
tests/KMPAndroid14Java17Test.php (1)
tests/Base.php (1)
Base(17-326)
src/SDK/Language/Kotlin.php (3)
src/SDK/Language/KMP.php (1)
getReturnType(101-107)src/SDK/Language/Apple.php (1)
getReturnType(494-500)src/SDK/Language/Android.php (1)
getReturnType(347-353)
src/SDK/Language/KMP.php (3)
src/SDK/SDK.php (2)
SDK(21-941)hasWebAuth(868-877)src/SDK/Language.php (2)
Language(5-279)toPascalCase(115-118)src/SDK/Language/Kotlin.php (6)
Kotlin(8-651)getName(13-16)getFilters(475-500)getPropertyType(536-560)getReturnType(502-526)getFiles(302-473)
src/SDK/Language/Android.php (4)
src/SDK/Language/KMP.php (1)
getReturnType(101-107)src/SDK/Language/Kotlin.php (1)
getReturnType(502-526)src/SDK/Language/Swift.php (1)
getReturnType(555-579)src/SDK/Language/Apple.php (1)
getReturnType(494-500)
tests/Android14Java8Test.php (1)
tests/Base.php (1)
Base(17-326)
🪛 detekt (1.23.8)
tests/languages/kmp/Tests.kt
[warning] 326-326: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🪛 PHPMD (2.15.0)
src/SDK/Language/KMP.php
44-44: Avoid unused parameters such as '$spec'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (36)
templates/kmp/androidApp/src/main/res/values/strings.xml.twig (1)
1-3: Well-formed Android string resource template.The template is properly structured, follows Android naming conventions for string resources, and appropriately uses the Twig variable for dynamic app name generation. The identifier
app_nameand resource format align with Android best practices.templates/kmp/shared/src/commonMain/kotlin/io/package/coroutines/Callback.kt.twig (1)
1-6: Package and imports look good.Template variable in package name is appropriate for the code generator context, and all imports are necessary and correctly ordered.
templates/kmp/androidApp/src/main/res/drawable/ic_launcher_background.xml (1)
1-170: Launcher background template resource looks good.The vector drawable is syntactically correct and uses standard Android attributes. The grid-based design is an unconventional but valid template launcher background for the Android app module.
templates/kmp/androidApp/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml (1)
1-5: Adaptive launcher icon template is correct.The resource follows standard Android adaptive icon structure for API 26+ and correctly references the background and foreground drawables.
templates/kmp/androidApp/src/main/res/mipmap-anydpi-v26/ic_launcher.xml (1)
1-5: Standard adaptive launcher icon template.The structure is correct and follows Android best practices. The identical definition shared with
ic_launcher_round.xmlis intentional—both can reference the same drawables when the design intent is consistent across icon variants.templates/kmp/androidApp/src/main/res/drawable/ic_launcher_foreground.xml (1)
1-30: Launcher foreground template resource is properly structured.The vector drawable correctly uses AAPT for gradient fills and includes valid SVG path data. The design appears to be a stylized mascot suitable for a template launcher icon.
templates/kmp/shared/src/commonMain/kotlin/io/package/enums/Enum.kt.twig (3)
1-4: LGTM! Package and imports are correct.The package declaration and serialization imports are properly structured for KMP enum generation.
6-7: LGTM! Enum declaration follows Kotlin best practices.The enum class is properly annotated with
@Serializableand uses avalueparameter to store the serialized string value, which is the standard pattern for string-based enums.
15-15: LGTM! toString() override is appropriate.Overriding
toString()to return thevalueproperty is the standard pattern for string-based enums, ensuring consistent serialization and string representation.tests/Android14Java11Test.php (1)
1-38: LGTM! Formatting normalization.The changes are purely whitespace/formatting adjustments with no functional modifications.
templates/kmp/LICENSE.md.twig (1)
1-1: LGTM! Simple and correct license template.The template appropriately uses the
rawfilter to output license content without HTML escaping.templates/kmp/.gitignore (1)
1-10: LGTM! Comprehensive ignore rules for KMP projects.The .gitignore appropriately covers IDE files, build artifacts, platform-specific files, and Gradle cache directories.
tests/Android14Java8Test.php (1)
1-38: LGTM! Formatting normalization.The changes are purely whitespace/formatting adjustments with no functional modifications.
templates/kmp/.github/workflows/autoclose.yml (1)
1-11: LGTM! Safe use of pull_request_target.The workflow appropriately delegates to a reusable workflow from the official Appwrite organization without executing untrusted code from external PRs.
templates/kmp/gradle.properties (1)
1-12: LGTM! Well-configured Gradle properties for KMP.The configuration appropriately sets memory limits, enables caching for performance, and includes necessary KMP-specific settings for iOS interoperability.
templates/kmp/.github/workflows/publish.yml (1)
11-34: LGTM! Environment setup and build steps are well-structured.The workflow properly sets up JDK 17, decodes and stores the GPG signing key securely, and executes the build and documentation generation steps.
templates/kmp/shared/src/androidMain/kotlin/io/package/KeepAliveService.kt.twig (1)
1-13: LGTM! Standard bound service implementation.The KeepAliveService follows the standard Android bound service pattern with a singleton binder, which is appropriate for a keep-alive service that doesn't need client-specific state.
templates/kmp/README.md.twig (1)
55-66: LGTM!The Maven installation instructions are properly templated and follow standard conventions.
templates/kmp/androidApp/src/main/java/io/package/android/utils/Event.kt.twig (1)
1-27: LGTM!This is a standard and correct implementation of the Event wrapper pattern for single-use LiveData events. The logic properly prevents re-consumption via
getContentIfNotHandled()while still allowing inspection viapeekContent().templates/kmp/gradlew.bat (1)
1-89: LGTM!This is the standard Gradle wrapper batch script for Windows. The implementation follows Gradle conventions and requires no changes.
templates/kmp/shared/src/iosMain/kotlin/io/package/models/InputFile.ios.kt.twig (1)
16-44: LGTM!The iOS MIME type detection correctly uses CoreServices UTI APIs with proper CF bridging and memory management via
CFBridgingRelease. The fallback to "application/octet-stream" is appropriate for missing extensions or MIME types.templates/kmp/gradlew (1)
1-185: LGTM!This is the standard Gradle wrapper shell script for Unix-based systems. The implementation follows Gradle conventions and handles various platform-specific paths and configurations correctly.
templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/IOExtensions.kt.twig (2)
1-1: Verify the intended package name.The package is declared as
{{ sdk.namespace | caseDot }}.fileOperations, but the AI summary indicates these extensions should be in the.extensionspackage. Please confirm which package is correct and ensure consistency across related files.
17-22: LGTM!The chunk reading implementation correctly uses Okio's buffered source with skip and read operations. The
useblock ensures proper resource cleanup.templates/kmp/shared/src/commonMain/kotlin/io/package/Service.kt.twig (1)
1-8: LGTM!Clean base class design with immutable client reference and clear documentation.
templates/kmp/build.gradle.kts.twig (1)
1-9: LGTM!Correct use of the convention plugins pattern with
apply(false)to centralize plugin version management while allowing subprojects to selectively apply plugins.templates/kmp/shared/src/commonMain/kotlin/io/package/models/UploadProgress.kt.twig (1)
3-9: LGTM!Well-designed immutable data class with appropriate types for upload progress tracking.
templates/kmp/shared/src/androidMain/kotlin/io/package/webInterface/AndroidParsedUrl.kt.twig (1)
5-7: LGTM!Clean platform-specific implementation with proper encapsulation and straightforward delegation to Android's
Uri.templates/kmp/androidApp/build.gradle.kts.twig (1)
1-47: Configuration looks solid overall.The Android app module configuration follows modern best practices with Compose support, appropriate SDK versions, and clean dependency management.
templates/kmp/shared/src/androidMain/kotlin/io/package/webInterface/UrlParser.android.kt.twig (1)
5-11: LGTM! Implementation is correct.The Android UrlParser implementation correctly delegates to
android.net.Uri. Each method independently parses the URL string, which is straightforward and correct for the template-generated code.src/SDK/Language/Android.php (1)
347-353: LGTM! Signature change aligns with parent class.The addition of the
$withGenericparameter with a default value oftruemaintains backward compatibility while enabling generic type suppression when needed. The parameter is correctly forwarded to the parent method.templates/kmp/shared/src/commonMain/kotlin/io/package/WebAuthComponent.kt.twig (1)
3-8: LGTM! Correct KMP expect declaration.The
expect classpattern is correctly used to define a multiplatform interface with platform-specific implementations. The companion object provides static-like functions for callback and cookie handling.templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/StringCollectionSeriailizer.kt.twig (1)
12-25: LGTM! Serializer implementation is correct.The custom serializer properly handles
Collection<String>by encoding/decoding asJsonArraywith string primitives. The descriptor delegation toListSerializeris appropriate.templates/kmp/shared/src/iosMain/kotlin/io/package/HttpClientConfig.ios.kt.twig (1)
36-45: LGTM! Self-signed certificate handling is correctly implemented.The conditional self-signed certificate acceptance is appropriate for development and testing. The implementation correctly uses
NSURLSessionAuthChallengeUseCredentialand creates credentials from the server trust.Note: Ensure users understand that enabling
selfSignedmode bypasses certificate validation and should only be used in development environments.templates/kmp/shared/src/iosMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig (1)
48-49: Fix malformed callback scheme string literal.The nested
client.config["project"]breaks Kotlin string interpolation and the generated code will not compile. Please build the string without embedding double quotes inside the template expression.- val callbackUrlScheme = "{{ spec.title | caseLower }}-callback-${client.config["project"]}" + val callbackUrlScheme = "{{ spec.title | caseLower }}-callback-" + (client.config["project"]?.toString() ?: "")Likely an incorrect or invalid review comment.
src/SDK/Language/KMP.php (1)
183-192: Address commented-out documentation generation code.The commented-out code for Kotlin and Java example documentation has been disabled. Per the past review discussion, this should either be uncommented and tested to ensure docs generate correctly, or removed entirely if not needed.
Please clarify whether documentation generation is:
- Temporarily disabled and should be re-enabled after testing, or
- Not needed for KMP and should be removed
Based on learnings
| - name: Publish to Maven Central | ||
| run: | | ||
| if ${{ contains(github.event.release.tag_name, '-rc') }}; then | ||
| echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}" | ||
| ./gradlew publishAllPublicationsToSonatypeRepository | ||
| else | ||
| echo "Publishing Release Version ${{ github.event.release.tag_name}}" | ||
| ./gradlew publishAllPublicationsToSonatypeRepository closeAndReleaseSonatypeStagingRepository | ||
| fi |
There was a problem hiding this comment.
Fix the conditional syntax for tag-based publishing.
The shell conditional syntax is incorrect. The expression if ${{ contains(...) }} will evaluate to literal strings if true or if false in bash, which won't work as intended.
Apply this diff to fix the conditional:
- - name: Publish to Maven Central
- run: |
- if ${{ contains(github.event.release.tag_name, '-rc') }}; then
- echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}"
- ./gradlew publishAllPublicationsToSonatypeRepository
- else
- echo "Publishing Release Version ${{ github.event.release.tag_name}}"
- ./gradlew publishAllPublicationsToSonatypeRepository closeAndReleaseSonatypeStagingRepository
- fi
+ - name: Publish Snapshot to Maven Central
+ if: contains(github.event.release.tag_name, '-rc')
+ run: |
+ echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}"
+ ./gradlew publishAllPublicationsToSonatypeRepository
+ env:
+ OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }}
+ OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }}
+ SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }}
+ SIGNING_PASSWORD: ${{ secrets.SIGNING_PASSWORD }}
+ SIGNING_SECRET_KEY_RING_FILE: ${{ secrets.SIGNING_SECRET_KEY_RING_FILE }}
+ SONATYPE_STAGING_PROFILE_ID: ${{ secrets.SONATYPE_STAGING_PROFILE_ID }}
+ SDK_VERSION: ${{ github.event.release.tag_name }}
+
+ - name: Publish Release to Maven Central
+ if: "!contains(github.event.release.tag_name, '-rc')"
+ run: |
+ echo "Publishing Release Version ${{ github.event.release.tag_name}}"
+ ./gradlew publishAllPublicationsToSonatypeRepository closeAndReleaseSonatypeStagingRepository
+ env:
+ OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }}
+ OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }}
+ SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }}
+ SIGNING_PASSWORD: ${{ secrets.SIGNING_PASSWORD }}
+ SIGNING_SECRET_KEY_RING_FILE: ${{ secrets.SIGNING_SECRET_KEY_RING_FILE }}
+ SONATYPE_STAGING_PROFILE_ID: ${{ secrets.SONATYPE_STAGING_PROFILE_ID }}
+ SDK_VERSION: ${{ github.event.release.tag_name }}Alternatively, if you prefer to keep a single step with bash conditionals:
- name: Publish to Maven Central
run: |
- if ${{ contains(github.event.release.tag_name, '-rc') }}; then
+ if [[ "${{ contains(github.event.release.tag_name, '-rc') }}" == "true" ]]; then
echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}"
./gradlew publishAllPublicationsToSonatypeRepository
else📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Publish to Maven Central | |
| run: | | |
| if ${{ contains(github.event.release.tag_name, '-rc') }}; then | |
| echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}" | |
| ./gradlew publishAllPublicationsToSonatypeRepository | |
| else | |
| echo "Publishing Release Version ${{ github.event.release.tag_name}}" | |
| ./gradlew publishAllPublicationsToSonatypeRepository closeAndReleaseSonatypeStagingRepository | |
| fi | |
| - name: Publish Snapshot to Maven Central | |
| if: contains(github.event.release.tag_name, '-rc') | |
| run: | | |
| echo "Publishing Snapshot Version ${{ github.event.release.tag_name}}" | |
| ./gradlew publishAllPublicationsToSonatypeRepository | |
| env: | |
| OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }} | |
| OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }} | |
| SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }} | |
| SIGNING_PASSWORD: ${{ secrets.SIGNING_PASSWORD }} | |
| SIGNING_SECRET_KEY_RING_FILE: ${{ secrets.SIGNING_SECRET_KEY_RING_FILE }} | |
| SONATYPE_STAGING_PROFILE_ID: ${{ secrets.SONATYPE_STAGING_PROFILE_ID }} | |
| SDK_VERSION: ${{ github.event.release.tag_name }} | |
| - name: Publish Release to Maven Central | |
| if: "!contains(github.event.release.tag_name, '-rc')" | |
| run: | | |
| echo "Publishing Release Version ${{ github.event.release.tag_name}}" | |
| ./gradlew publishAllPublicationsToSonatypeRepository closeAndReleaseSonatypeStagingRepository | |
| env: | |
| OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }} | |
| OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }} | |
| SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }} | |
| SIGNING_PASSWORD: ${{ secrets.SIGNING_PASSWORD }} | |
| SIGNING_SECRET_KEY_RING_FILE: ${{ secrets.SIGNING_SECRET_KEY_RING_FILE }} | |
| SONATYPE_STAGING_PROFILE_ID: ${{ secrets.SONATYPE_STAGING_PROFILE_ID }} | |
| SDK_VERSION: ${{ github.event.release.tag_name }} |
🤖 Prompt for AI Agents
templates/kmp/.github/workflows/publish.yml lines 36-44: the step uses GitHub
expressions inside a bash if incorrectly (producing literal "if true"/"if
false"); fix by either moving the contains(...) expression to the step-level if:
attribute so the step runs only for RC vs release, or keep a single step but
change the bash conditional to compare the expression string to "true" (e.g. use
a test like [ "<expression>" = "true" ]), then run the appropriate gradle
commands in the branches.
| android:roundIcon="@mipmap/ic_launcher_round" | ||
| android:supportsRtl="true" | ||
| android:theme="@style/Theme.{{ spec.title | caseUcfirst }}KMPSDK" | ||
| android:usesCleartextTraffic="true"> |
There was a problem hiding this comment.
Critical security risk: cleartext traffic enabled.
Setting android:usesCleartextTraffic="true" allows unencrypted HTTP connections, exposing user data to interception and man-in-the-middle attacks. This should not be enabled in production builds.
If cleartext traffic is required for development/testing only, consider using build variants:
- android:usesCleartextTraffic="true">
+ android:usesCleartextTraffic="false"
+ android:networkSecurityConfig="@xml/network_security_config">Then create res/xml/network_security_config.xml with debug-only cleartext domains if needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| android:usesCleartextTraffic="true"> | |
| android:usesCleartextTraffic="false" | |
| android:networkSecurityConfig="@xml/network_security_config"> |
🤖 Prompt for AI Agents
In templates/kmp/androidApp/src/main/AndroidManifest.xml.twig around line 12,
android:usesCleartextTraffic="true" enables unencrypted HTTP and must be removed
or set to "false" for production; change the manifest to not enable cleartext
globally, and implement a debug-only configuration instead by adding a network
security config XML (res/xml/network_security_config.xml) that whitelists any
required development domains and reference it only in the debug build (use build
variants/manifest placeholders or a debug AndroidManifest merge) so production
builds never allow cleartext traffic.
| <action android:name="android.intent.action.VIEW" /> | ||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <data android:scheme="appwrite-callback-6070749e6acd4" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Hardcoded callback scheme should be parameterized.
The callback scheme appwrite-callback-6070749e6acd4 appears to be hardcoded. For a reusable SDK template, this should be configurable via a template variable to allow different projects to use unique callback schemes.
Apply this change to parameterize the scheme:
- <data android:scheme="appwrite-callback-6070749e6acd4" />
+ <data android:scheme="{{ callback_scheme | default('appwrite-callback-' ~ sdk.namespace) }}" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <data android:scheme="appwrite-callback-6070749e6acd4" /> | |
| <data android:scheme="{{ callback_scheme | default('appwrite-callback-' ~ sdk.namespace) }}" /> |
🤖 Prompt for AI Agents
In templates/kmp/androidApp/src/main/AndroidManifest.xml.twig around line 26,
the android:scheme value is hardcoded as "appwrite-callback-6070749e6acd4";
replace it with a template variable (e.g. {{ callback_scheme }} or
project-specific name) so the scheme is configurable when the template is
rendered. Ensure the variable is documented or has a sensible default in the
template context, and update any template usage or README to pass the chosen
callback_scheme when generating the project.
| binding.oAuth.setOnClickListener{ | ||
| viewModel.oAuthLogin(activity as ComponentActivity) | ||
| } |
There was a problem hiding this comment.
Use safe cast for activity.
The unsafe cast activity as ComponentActivity could throw ClassCastException if the activity is not a ComponentActivity. Use requireActivity() with a safe cast instead.
Apply this diff:
binding.oAuth.setOnClickListener{
- viewModel.oAuthLogin(activity as ComponentActivity)
+ (requireActivity() as? ComponentActivity)?.let { activity ->
+ viewModel.oAuthLogin(activity)
+ }
}🤖 Prompt for AI Agents
In
templates/kmp/androidApp/src/main/java/io/package/android/ui/accounts/AccountsFragment.kt.twig
around lines 54 to 56, the code unsafely casts activity to ComponentActivity
which can throw ClassCastException; change to use requireActivity() with a safe
cast and early return or error handling if the cast fails (e.g., val
componentActivity = requireActivity() as? ComponentActivity ?: return) and then
call viewModel.oAuthLogin(componentActivity) so the fragment won’t crash when
the host activity isn’t a ComponentActivity.
| "appwrite-callback-6070749e6acd4://demo.appwrite.io/auth/oauth2/success", | ||
| "appwrite-callback-6070749e6acd4://demo.appwrite.io/auth/oauth2/failure" | ||
| ) |
There was a problem hiding this comment.
Parameterize the OAuth callback URIs.
Both callback URIs embed the sample project ID (6070749e6acd4), so generated apps that use any other Appwrite project cannot complete OAuth. Template these values from the configured project or delegate to the client’s WebAuthComponent helpers instead of hard-coding the sample IDs.
🤖 Prompt for AI Agents
In
templates/kmp/androidApp/src/main/java/io/package/android/ui/accounts/AccountsViewModel.kt.twig
around lines 84 to 86, the OAuth callback URIs currently embed the hard-coded
sample project ID (6070749e6acd4); update the template to build these URIs from
the configured Appwrite project ID (or call the client’s WebAuthComponent
helper) instead of hard-coding the ID so generated apps use the correct project;
ensure both the success and failure URIs are produced from the same templated
value/helper and that any necessary import or parameter is added to the
template.
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
Outdated
Show resolved
Hide resolved
templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig
Show resolved
Hide resolved
templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig
Show resolved
Hide resolved
templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig
Show resolved
Hide resolved
| writeToFile(Query.equal("released", listOf(true))) | ||
| writeToFile(Query.equal("title", listOf("Spiderman", "Dr. Strange"))) | ||
| writeToFile(Query.notEqual("title", "Spiderman")) | ||
| writeToFile(Query.lessThan("releasedYear", 1990)) | ||
| writeToFile(Query.greaterThan("releasedYear", 1990)) | ||
| writeToFile(Query.search("name", "john")) | ||
| writeToFile(Query.isNull("name")) | ||
| writeToFile(Query.isNotNull("name")) | ||
| writeToFile(Query.between("age", 50, 100)) | ||
| writeToFile(Query.between("age", 50.5, 100.5)) | ||
| writeToFile(Query.between("name", "Anna", "Brad")) | ||
| writeToFile(Query.startsWith("name", "Ann")) | ||
| writeToFile(Query.endsWith("name", "nne")) | ||
| writeToFile(Query.select(listOf("name", "age"))) | ||
| writeToFile(Query.orderAsc("title")) | ||
| writeToFile(Query.orderDesc("title")) | ||
| writeToFile(Query.cursorAfter("my_movie_id")) | ||
| writeToFile(Query.cursorBefore("my_movie_id")) | ||
| writeToFile(Query.limit(50)) | ||
| writeToFile(Query.offset(20)) | ||
| writeToFile(Query.contains("title", listOf("Spider"))) | ||
| writeToFile(Query.contains("labels", listOf("first"))) | ||
| writeToFile( | ||
| Query.or( | ||
| listOf( | ||
| Query.equal("released", listOf(true)), | ||
| Query.lessThan("releasedYear", 1990) | ||
| ) | ||
| ) | ||
| ) | ||
| writeToFile( | ||
| Query.and( | ||
| listOf( | ||
| Query.equal("released", listOf(false)), | ||
| Query.greaterThan("releasedYear", 2015) | ||
| ) | ||
| ) | ||
| ) | ||
|
|
||
| // Permission & Roles helper tests | ||
| writeToFile(Permission.read(Role.any())) | ||
| writeToFile(Permission.write(Role.user(ID.custom("userid")))) | ||
| writeToFile(Permission.create(Role.users())) | ||
| writeToFile(Permission.update(Role.guests())) | ||
| writeToFile(Permission.delete(Role.team("teamId", "owner"))) | ||
| writeToFile(Permission.delete(Role.team("teamId"))) | ||
| writeToFile(Permission.create(Role.member("memberId"))) | ||
| writeToFile(Permission.update(Role.users("verified"))) | ||
| writeToFile(Permission.update(Role.user(ID.custom("userid"), "unverified"))) | ||
| writeToFile(Permission.create(Role.label("admin"))) | ||
|
|
||
| // ID helper tests | ||
| writeToFile(ID.unique()) | ||
| writeToFile(ID.custom("custom_id")) | ||
|
|
There was a problem hiding this comment.
Restore full query helper coverage to match Base assertions.
expectedOutput includes Base::QUERY_HELPER_RESPONSES, but this test only writes the first handful of helper strings. Everything from orderRandom() through the geospatial helpers is never appended to result.txt, so the comparison against Base::QUERY_HELPER_RESPONSES will fail every run. Please emit the remaining helper outputs (or drop them from expectedOutput), otherwise the Android KMP test can’t pass.
writeToFile(Query.orderAsc("title"))
writeToFile(Query.orderDesc("title"))
+ writeToFile(Query.orderRandom())
writeToFile(Query.cursorAfter("my_movie_id"))
writeToFile(Query.cursorBefore("my_movie_id"))
writeToFile(Query.limit(50))
writeToFile(Query.offset(20))
writeToFile(Query.contains("title", listOf("Spider")))
writeToFile(Query.contains("labels", listOf("first")))
+ writeToFile(Query.notContains("title", listOf("Spider")))
+ writeToFile(Query.notSearch("name", "john"))
+ writeToFile(Query.notBetween("age", 50, 100))
+ writeToFile(Query.notStartsWith("name", "Ann"))
+ writeToFile(Query.notEndsWith("name", "nne"))
+ writeToFile(Query.lessThan("\$createdAt", "2023-01-01"))
+ writeToFile(Query.greaterThan("\$createdAt", "2023-01-01"))
+ writeToFile(Query.between("\$createdAt", "2023-01-01", "2023-12-31"))
+ writeToFile(Query.lessThan("\$updatedAt", "2023-01-01"))
+ writeToFile(Query.greaterThan("\$updatedAt", "2023-01-01"))
+ writeToFile(Query.between("\$updatedAt", "2023-01-01", "2023-12-31"))
+ writeToFile(Query.distanceEqual("location", listOf(listOf(listOf(40.7128, -74.0), listOf(40.7128, -74.0)), 1000, true)))
+ writeToFile(Query.distanceEqual("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceNotEqual("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceNotEqual("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceGreaterThan("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceGreaterThan("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceLessThan("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.distanceLessThan("location", listOf(listOf(40.7128, -74.0), 1000, true)))
+ writeToFile(Query.intersects("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.notIntersects("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.crosses("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.notCrosses("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.overlaps("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.notOverlaps("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.touches("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.notTouches("location", listOf(40.7128, -74.0)))
+ writeToFile(Query.contains("location", listOf(listOf(40.7128, -74.0), listOf(40.7128, -74.0))))
+ writeToFile(Query.notContains("location", listOf(listOf(40.7128, -74.0), listOf(40.7128, -74.0))))
+ writeToFile(Query.equal("location", listOf(listOf(40.7128, -74.0), listOf(40.7128, -74.0))))
+ writeToFile(Query.notEqual("location", listOf(listOf(40.7128, -74.0), listOf(40.7128, -74.0))))
writeToFile(
Query.or(
listOf(
Query.equal("released", listOf(true)),🤖 Prompt for AI Agents
In tests/languages/kmp/Tests.kt around lines 255 to 309, the test writes only a
subset of query/permission/ID helper outputs but expectedOutput includes
Base::QUERY_HELPER_RESPONSES (which contains many more entries from
orderRandom() through geospatial helpers); restore full coverage by adding
writeToFile calls for the remaining helper helpers in the same pattern
(orderRandom(), orderNatural(), the rest of ordering, cursor/limit/offset
variants if any, contains/containsAny/containsAll variants, and all geospatial
helpers like near/within/geoBoundingBox, plus any missing permission/role
helpers) so the file emits every string included in Base::QUERY_HELPER_RESPONSES
(or alternatively remove those extra entries from expectedOutput if
intentionally reducing coverage).
| if (method.uppercase() == "GET") { | ||
| filteredParams.forEach { (key, value) -> | ||
| when (value) { | ||
| is List<*> -> value.forEach { item -> | ||
| parameter("${key}[]", item.toString()) | ||
| } | ||
|
|
||
| else -> parameter(key, value.toString()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify array parameter serialization and object conversion.
Line 167 serializes list parameters as "key[]" query parameters, and line 170 converts all other values to strings using toString(). Both approaches should be verified:
- Confirm that the API expects array parameters in the
"key[]"format. - For complex objects (non-primitives),
toString()may produce invalid or unexpected query parameter values.
Run the following script to check how array parameters are used in method definitions:
🏁 Script executed:
#!/bin/bash
# Search for methods with array parameters to understand expected format
rg -nP --type-add 'spec:*.{json,yaml,yml}' --type=spec -C3 'type.*array|array.*type' Length of output: 5402
I need to gather more information about how the API parameters are actually serialized. Let me verify the implementation details and test expectations.
Let me search the codebase for test files and actual usage patterns to better understand the expected parameter format.
Let me run a proper search on the repository:
🏁 Script executed:
#!/bin/bash
# Search for tests related to parameters or BaseClient
find . -type f -name "*.kt" -o -name "*.kts" | xargs grep -l "parameter\|BaseClient\|z\[\]" 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for test files that validate query parameter serialization
fd -e ".kt" "test" --type f | xargs grep -l "parameter.*\[\]" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for parameter usage and array handling in tests
rg -n "parameter.*\[\]|collectionFormat" --type=kt --type=jsonLength of output: 26
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for the template file and any related test files
rg -n "key\[\]|collectionFormat|multi" --type=json --type=yaml -C2
Length of output: 2397
🏁 Script executed:
#!/bin/bash
# Find and examine the template file mentioned in the review
find . -name "BaseClient.kt.twig" -o -name "*.twig" | head -20Length of output: 1283
🏁 Script executed:
#!/bin/bash
# Search for test expectations around array parameters
rg -n "z\[\]" --type-add 'template:*.twig' --type=template -C3Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search specifically for the kmp BaseClient template
find . -path "*kmp*" -name "BaseClient.kt.twig"
Length of output: 74
🏁 Script executed:
#!/bin/bash
# List all files in kmp templates directory
find . -path "*kmp*" -type f | grep -E "(BaseClient|Client)" | head -20Length of output: 542
🏁 Script executed:
#!/bin/bash
# Search for the kmp template directory structure
find . -type d -name "kmp" Length of output: 37
---
🏁 Script executed:
```shell
#!/bin/bash
# View the relevant section of the template file
sed -n '160,175p' ./templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig
Length of output: 632
🏁 Script executed:
#!/bin/bash
# Search for test files that validate parameter serialization
find ./tests/languages/kmp -type f | head -30Length of output: 30
🏁 Script executed:
#!/bin/bash
# Search for tests that check array parameters or query params
rg -n "z\[\]|parameter|array" ./tests/languages/kmp -C3Length of output: 7652
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for mock server or request validation code
find ./mock-server -type f -name "*.php" | head -20
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Search for how array parameters are validated in mock server
rg -n "collectionFormat|multi|\[\]" ./mock-server -C2Length of output: 5634
🏁 Script executed:
#!/bin/bash
# Look for expected query parameter format in the codebase
rg -n "z=|z\[\]" ./tests -C3Length of output: 0
[/web_search]
🌐 Web query:
Ktor HTTP client parameter function multiple values array query string
💡 Result:
Short answer: call parameter/append repeatedly or use a Parameters/ParametersBuilder that accepts multiple values.
Examples:
-
Repeated parameter() calls (builds ?tag=a&tag=b):
client.get("...") {
url {
parameter("tag", "a")
parameter("tag", "b")
}
}
(parameter() appends a single query parameter). [1][2] -
Use URLBuilder.parameters (ParametersBuilder) directly:
client.get("...") {
url {
parameters.append("tag", "a")
parameters.append("tag", "b")
}
}
Or build once from a list:
url { parameters += Parameters.build { appendAll("tag", listOf("a","b")) } }
Or construct Parameters from pairs / list: Parameters.build { append("a","b1"); append("a","b2") } or parametersOf("a", listOf("b1","b2")).formUrlEncode() to see encoding. [1][3]
Notes:
- Ktor encodes parameters by default; repeated keys produce multiple key=value entries in the query string (server-side can read them as a list). [1][3]
Sources:
- Ktor — Making requests / query parameters. [1]
- Ktor API — HttpRequestBuilder.parameter(). [2]
- Ktor — Parameters / ParametersBuilder / formUrlEncode examples. [3]
Fix array parameter serialization format to match API specification.
The template uses parameter("${key}[]", item.toString()) which generates query parameters like ?z[]=val1&z[]=val2. However, the API specification defines "collectionFormat": "multi" for array parameters, which expects ?z=val1&z=val2 format.
The fix is to remove the [] suffix from the parameter name:
is List<*> -> value.forEach { item ->
parameter(key, item.toString())
}Calling parameter() multiple times with the same key automatically creates the repeated query parameter format that Ktor sends as ?key=val1&key=val2, matching the collectionFormat: "multi" specification.
The toString() conversion for other values is acceptable since the spec only defines string types within arrays.
🤖 Prompt for AI Agents
In templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig
around lines 163 to 172, array query parameters are serialized as key[] (e.g.,
?z[]=a&z[]=b) which doesn't match the API spec's collectionFormat: "multi";
update the List handling to call parameter with the plain key for each item
(i.e., remove the "[]" suffix) so repeated parameter(key, item.toString()) calls
produce ?key=val1&key=val2 as required.
| var result: Map<*, *> = mapOf<String, Any>() | ||
| while (offset < size) { | ||
| val bytesToRead = if (size - offset < CHUNK_SIZE) { | ||
| (size - offset).toInt() | ||
| } else { | ||
| CHUNK_SIZE.toInt() | ||
| } | ||
| val chunk = data.readByteArray(bytesToRead) | ||
|
|
||
| httpClient.submitFormWithBinaryData( | ||
| url = endpoint + path, | ||
| formData = formData { | ||
| filteredParams.forEach { (key, value) -> | ||
| when (value) { | ||
| is List<*> -> value.forEach { item -> | ||
| append("${key}[]", item.toString()) | ||
| } | ||
|
|
||
| else -> append(key, value.toString()) | ||
| } | ||
| } | ||
| append( | ||
| paramName, | ||
| chunk, | ||
| Headers.build { | ||
| append(HttpHeaders.ContentType, input.mimeType) | ||
| append( | ||
| HttpHeaders.ContentDisposition, | ||
| "filename=${input.filename.quote()}" | ||
| ) | ||
| } | ||
| ) | ||
| } | ||
| ) { | ||
| header( | ||
| HttpHeaders.ContentRange, | ||
| "bytes $offset-${((offset + CHUNK_SIZE) - 1).coerceAtMost(size - 1)}/$size" | ||
| ) | ||
| combinedHeaders.forEach { (key, value) -> | ||
| header(key, value) | ||
| } | ||
| if (chunksUploaded != 0L) { | ||
| header("x-{{ spec.title | caseLower }}-id", result["\$id"].toString().removeSurrounding("\"")) | ||
| } | ||
| }.let { response -> | ||
| result = handleResponse(response, Map::class, getSerializer(Map::class)) | ||
| offset += bytesToRead | ||
| chunksUploaded++ | ||
| onProgress?.invoke( | ||
| UploadProgress( | ||
| id = result["\$id"].toString(), | ||
| progress = offset.coerceAtMost(size).toDouble() / size * 100, | ||
| sizeUploaded = offset.coerceAtMost(size), | ||
| chunksTotal = result["chunksTotal"].toString().toInt(), | ||
| chunksUploaded = result["chunksUploaded"].toString().toInt(), | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Multiple unsafe map accesses without null checks.
Lines 332, 340, and 343-344 access map fields without null checks:
- Line 332:
result["\$id"] - Line 340:
result["\$id"] - Lines 343-344:
result["chunksTotal"],result["chunksUploaded"]
These could throw exceptions if the response structure is unexpected.
Apply this diff to add proper null-safety:
if (chunksUploaded != 0L) {
- header("x-{{ spec.title | caseLower }}-id", result["\$id"].toString().removeSurrounding("\""))
+ result["\$id"]?.toString()?.removeSurrounding("\"")?.let { id ->
+ header("x-{{ spec.title | caseLower }}-id", id)
+ }
}
}.let { response ->
result = handleResponse(response, Map::class, getSerializer(Map::class))
offset += bytesToRead
chunksUploaded++
onProgress?.invoke(
UploadProgress(
- id = result["\$id"].toString(),
+ id = result["\$id"]?.toString() ?: "",
progress = offset.coerceAtMost(size).toDouble() / size * 100,
sizeUploaded = offset.coerceAtMost(size),
- chunksTotal = result["chunksTotal"].toString().toInt(),
- chunksUploaded = result["chunksUploaded"].toString().toInt(),
+ chunksTotal = result["chunksTotal"]?.toString()?.toIntOrNull() ?: 0,
+ chunksUploaded = result["chunksUploaded"]?.toString()?.toIntOrNull() ?: 0,
)
)
}| fun JsonObjectBuilder.addToJsonObject(key: String, value: Any?) { | ||
| when (value) { | ||
| is String -> put(key, value) | ||
| is Number -> put(key, value) | ||
| is Boolean -> put(key, value) | ||
| is List<*> -> { | ||
| val jsonArray = getJsonArray(value) | ||
| put(key, jsonArray) | ||
| } | ||
|
|
||
| is Enum<*> -> put(key, JsonPrimitive(value.name)) | ||
| null -> put(key, JsonNull) | ||
| else -> put(key, JsonPrimitive(value.toString())) | ||
| } | ||
| } | ||
|
|
||
| fun getJsonArray(items: List<Any?>): JsonArray { | ||
| return JsonArray(items.map { item -> | ||
| when (item) { | ||
| is String -> JsonPrimitive(item) | ||
| is Number -> JsonPrimitive(item) | ||
| is Boolean -> JsonPrimitive(item) | ||
| is Enum<*> -> JsonPrimitive(item.name) | ||
| null -> JsonNull | ||
| is List<*> -> getJsonArray(item) | ||
| else -> JsonPrimitive(item.toString()) | ||
| } | ||
| }) | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fallback to toString() for complex objects risks data corruption.
The else branches at lines 72 and 85 use toString() as a fallback for unknown types. For complex objects without a meaningful toString() implementation, this produces non-JSON strings (e.g., "SomeClass@1a2b3c4d") that cannot be deserialized and may corrupt data.
Consider one of these approaches:
- Throw an exception for unsupported types to fail fast:
- else -> put(key, JsonPrimitive(value.toString()))
+ else -> throw IllegalArgumentException("Unsupported type for JSON serialization: ${value::class}")- Serialize complex objects properly using the Json instance:
- else -> put(key, JsonPrimitive(value.toString()))
+ else -> put(key, json.encodeToJsonElement(DynamicLookupSerializer, value))Apply the same fix to line 85 in getJsonArray.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun JsonObjectBuilder.addToJsonObject(key: String, value: Any?) { | |
| when (value) { | |
| is String -> put(key, value) | |
| is Number -> put(key, value) | |
| is Boolean -> put(key, value) | |
| is List<*> -> { | |
| val jsonArray = getJsonArray(value) | |
| put(key, jsonArray) | |
| } | |
| is Enum<*> -> put(key, JsonPrimitive(value.name)) | |
| null -> put(key, JsonNull) | |
| else -> put(key, JsonPrimitive(value.toString())) | |
| } | |
| } | |
| fun getJsonArray(items: List<Any?>): JsonArray { | |
| return JsonArray(items.map { item -> | |
| when (item) { | |
| is String -> JsonPrimitive(item) | |
| is Number -> JsonPrimitive(item) | |
| is Boolean -> JsonPrimitive(item) | |
| is Enum<*> -> JsonPrimitive(item.name) | |
| null -> JsonNull | |
| is List<*> -> getJsonArray(item) | |
| else -> JsonPrimitive(item.toString()) | |
| } | |
| }) | |
| } | |
| fun JsonObjectBuilder.addToJsonObject(key: String, value: Any?) { | |
| when (value) { | |
| is String -> put(key, value) | |
| is Number -> put(key, value) | |
| is Boolean -> put(key, value) | |
| is List<*> -> { | |
| val jsonArray = getJsonArray(value) | |
| put(key, jsonArray) | |
| } | |
| is Enum<*> -> put(key, JsonPrimitive(value.name)) | |
| null -> put(key, JsonNull) | |
| else -> throw IllegalArgumentException("Unsupported type for JSON serialization: ${value::class}") | |
| } | |
| } | |
| fun getJsonArray(items: List<Any?>): JsonArray { | |
| return JsonArray(items.map { item -> | |
| when (item) { | |
| is String -> JsonPrimitive(item) | |
| is Number -> JsonPrimitive(item) | |
| is Boolean -> JsonPrimitive(item) | |
| is Enum<*> -> JsonPrimitive(item.name) | |
| null -> JsonNull | |
| is List<*> -> getJsonArray(item) | |
| else -> throw IllegalArgumentException("Unsupported type for JSON serialization: ${item::class}") | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
In
templates/kmp/shared/src/commonMain/kotlin/io/package/extensions/JsonExtensions.kt.twig
around lines 60 to 88, the fallback branches use value.toString() /
item.toString(), which can produce non-JSON or non-deserializable
representations for complex objects; replace these fallbacks by either
(preferably) serializing unknown objects properly with the kotlinx.serialization
Json instance (e.g., call Json.encodeToJsonElement(serializerOrDefault, value)
or Json.encodeToJsonElement(value) for polymorphic/reflective serialization) so
complex types become proper JsonElements, or if serialization cannot be
guaranteed, throw an IllegalArgumentException indicating the unsupported type to
fail fast; apply the same change to both the addToJsonObject else branch and the
getJsonArray else branch so neither uses toString() as the fallback.
…cookie storage assignment
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig (1)
12-19: Address the outstanding context caching concern.While using
.applicationContext(line 19) prevents the typical memory leak scenario mentioned in the previous review, the reviewer specifically asked whether we can avoid caching the context altogether. Consider extracting the needed values during initialization:
packageName(used in lines 29, 41)appVersion(used in line 42)- DataStore file path (used in line 23)
This refactor would address the reviewer's concern:
actual class Client constructor( context: Context, endpoint: String = "{{spec.endpoint}}", endpointRealtime: String? = null, selfSigned: Boolean = false, ) : BaseClient<Client>(endpoint, endpointRealtime) { actual override val coroutineContext = Job() + Dispatchers.Default - private val appContext = context.applicationContext + private val packageName: String + private val appVersion: String + private val cookieStoragePath: okio.Path private val dataStoreManager = DataStoreManager( PreferenceDataStoreFactory.createWithPath ( - produceFile = { appContext.filesDir.resolve("appwriteCookies.preferences_pb").absolutePath.toPath() } + produceFile = { cookieStoragePath } )) val dataStoreCookieStorage = DataStoreCookieStorage(dataStoreManager) - private val appVersion by lazy { + init { + val appContext = context.applicationContext + packageName = appContext.packageName + cookieStoragePath = appContext.filesDir.resolve("appwriteCookies.preferences_pb").absolutePath.toPath() + try { val pInfo = appContext.packageManager.getPackageInfo(appContext.packageName, 0) - return@lazy pInfo.versionName + appVersion = pInfo.versionName } catch (e: PackageManager.NameNotFoundException) { - e.printStackTrace() - return@lazy "" + appVersion = "" + // Consider proper logging instead of printStackTrace } - } - init { httpClient = createHttpClient(selfSigned, dataStoreCookieStorage) headers = mutableMapOf( "content-type" to "application/json", - "origin" to "{{ spec.title | caseLower }}-android://${appContext.packageName}", + "origin" to "{{ spec.title | caseLower }}-android://${packageName}", "user-agent" to "${packageName}/${appVersion}, ${System.getProperty("http.agent")}",
🧹 Nitpick comments (1)
templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig (1)
27-35: Improve error handling.Using
printStackTrace()on line 32 is not ideal for production code. Consider using a proper logging framework to ensure errors are captured and monitored appropriately.} catch (e: PackageManager.NameNotFoundException) { - e.printStackTrace() + // Log the error using your logging framework + // e.g., Log.w("Client", "Failed to get package info", e) return@lazy "" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig(1 hunks)
🔇 Additional comments (2)
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig (2)
36-38: Previous issue resolved.The guard that prevented the first assignment has been removed, and
cookieStorageis now assigned unconditionally. This fixes the critical bug flagged in the previous review.
198-208: Ensure valid presentation anchor fallback.If no key window is found, returning an empty
UIWindow()(line 207) may cause the authentication session to fail. Verify this scenario is handled or throw an exception to surface the issue early.Test this edge case or add defensive error handling:
override fun presentationAnchorForWebAuthenticationSession(session: ASWebAuthenticationSession): ASPresentationAnchor { val windows = UIApplication.sharedApplication.windows val count = windows.size for (i in 0 until count) { val window = windows[i] as? UIWindow if (window?.isKeyWindow() == true) { return window } } // Consider: throw IllegalStateException("No key window available for authentication") return UIWindow() }
| endpointRealtime: String? = null, | ||
| selfSigned: Boolean = false, | ||
| ) : BaseClient<Client>(endpoint, endpointRealtime) { | ||
| actual override val coroutineContext = Job() + Dispatchers.Default |
There was a problem hiding this comment.
Potential resource leak: Job not cancelled.
The Job() created here should be cancelled when the Client is no longer needed to prevent resource leaks. Consider adding a cleanup method (e.g., close() or dispose()) that cancels the job.
fun close() {
coroutineContext.cancel()
httpClient.close()
}🤖 Prompt for AI Agents
In templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig
around line 18, the coroutineContext is created with a bare Job() which is never
cancelled; change to store the Job in a field (e.g., val job = Job()) and
compose coroutineContext = job + Dispatchers.Default, then add a public cleanup
method (e.g., fun close() or override fun close() if implementing Closeable)
that calls job.cancel() and also httpClient.close() to release resources and
avoid leaks.
| actual fun setSelfSigned(value: Boolean): Client { | ||
| httpClient = createHttpClient(value, dataStoreCookieStorage) | ||
| return this | ||
| } |
There was a problem hiding this comment.
Close the old httpClient before replacing it.
When setSelfSigned recreates the httpClient, the previous instance is replaced without being closed. This can leak connections and other resources managed by Ktor's HttpClient.
actual fun setSelfSigned(value: Boolean): Client {
+ httpClient.close()
httpClient = createHttpClient(value, dataStoreCookieStorage)
return this
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| actual fun setSelfSigned(value: Boolean): Client { | |
| httpClient = createHttpClient(value, dataStoreCookieStorage) | |
| return this | |
| } | |
| actual fun setSelfSigned(value: Boolean): Client { | |
| httpClient.close() | |
| httpClient = createHttpClient(value, dataStoreCookieStorage) | |
| return this | |
| } |
🤖 Prompt for AI Agents
In templates/kmp/shared/src/androidMain/kotlin/io/package/Client.android.kt.twig
around lines 55–58, the method recreates httpClient without closing the previous
instance; before assigning the new client call close() on the existing
httpClient (safely handling nullability and catching any exceptions—for example
wrap in runCatching or try/catch) to release resources, then assign the new
client and return this.
| callbackURL != null -> { | ||
| val callbackUrlString = callbackURL.absoluteString ?: "" | ||
| handleIncomingCookie(callbackUrlString) | ||
| pending?.onComplete?.invoke(Result.success(callbackUrlString)) | ||
| if (pending?.continuation?.isActive == true) { | ||
| pending.continuation.resume(callbackUrlString) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix double-resume race condition.
The continuation is resumed twice when a callback URL is received:
handleIncomingCookie(line 69) →onCallback(line 177) →continuation.resume(line 112)- Then the completion handler resumes again at line 72
This will crash with IllegalStateException: Already resumed. Either remove the resume at lines 71-72 or refactor handleIncomingCookie to not call onCallback in this flow.
Apply this diff to prevent double-resume:
callbackURL != null -> {
val callbackUrlString = callbackURL.absoluteString ?: ""
handleIncomingCookie(callbackUrlString)
- pending?.onComplete?.invoke(Result.success(callbackUrlString))
- if (pending?.continuation?.isActive == true) {
- pending.continuation.resume(callbackUrlString)
- }
+ // Note: handleIncomingCookie calls onCallback which resumes the continuation
}🤖 Prompt for AI Agents
In
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
around lines 67-74, the continuation is being resumed twice when a callback URL
arrives (once via handleIncomingCookie -> onCallback path and again here),
causing IllegalStateException: Already resumed; remove the explicit
continuation.resume call in this block (the check and resume on lines 71-72) and
keep invoking pending?.onComplete?.invoke(Result.success(callbackUrlString)) so
the resume happens only via the established onCallback flow.
| val rawDomain = cookieParts["domain"] ?: return | ||
| val key = cookieParts["key"] ?: return | ||
| val secret = cookieParts["secret"] ?: return | ||
|
|
||
| val domain = if (rawDomain.isNotEmpty()) rawDomain.substring(1) else rawDomain |
There was a problem hiding this comment.
Verify domain parsing logic.
The code unconditionally strips the first character from rawDomain (line 134), assuming it's a leading dot. If the domain doesn't start with a dot (e.g., from malformed query parameters), this produces an incorrect domain.
Consider:
- val domain = if (rawDomain.isNotEmpty()) rawDomain.substring(1) else rawDomain
+ val domain = rawDomain.removePrefix(".")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val rawDomain = cookieParts["domain"] ?: return | |
| val key = cookieParts["key"] ?: return | |
| val secret = cookieParts["secret"] ?: return | |
| val domain = if (rawDomain.isNotEmpty()) rawDomain.substring(1) else rawDomain | |
| val rawDomain = cookieParts["domain"] ?: return | |
| val key = cookieParts["key"] ?: return | |
| val secret = cookieParts["secret"] ?: return | |
| val domain = rawDomain.removePrefix(".") |
🤖 Prompt for AI Agents
In
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
around lines 130 to 134, the domain parsing unconditionally strips the first
character from rawDomain which corrupts domains that do not begin with a dot;
update the logic to only remove a leading dot (e.g., check
rawDomain.startsWith(".") or use removePrefix(".")) and also trim whitespace and
guard against rawDomain being just "." so you preserve the original rawDomain
when no leading dot is present.
| expires = cookieParts["expires"]?.let { | ||
| null // You may need to parse the date properly | ||
| }, |
There was a problem hiding this comment.
Implement cookie expiration parsing.
Cookie expires is always null (line 144), so cookies never expire via the expires attribute. This may cause stale authentication state or security issues if long-lived cookies aren't cleaned up.
Parse the ISO-8601 or HTTP-date format from cookieParts["expires"]:
expires = cookieParts["expires"]?.let {
// Parse using NSDateFormatter or kotlinx-datetime
parseHttpDate(it)
}🤖 Prompt for AI Agents
In
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
around lines 143-145, the expires field is always set to null; replace the
placeholder with real parsing of cookieParts["expires"] by attempting to parse
both HTTP-date and ISO-8601 formats (e.g., using
NSDateFormatter/ISO8601DateFormatter on iOS or kotlinx-datetime on common code),
convert the parsed date to the expected expires type, and return null only if
parsing fails; ensure parsing is tolerant of common HTTP-date variants and time
zones and does not throw on invalid input.
templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/StringCollectionSerializer.kt.twig (1)
1-26: Filename typo flagged in previous review.templates/kmp/shared/src/iosMain/kotlin/io/package/cookies/IosCookieStorage.kt.twig (1)
185-185: HttpOnly flag not preserved during cookie restoration.Line 185 uses the raw string
"HttpOnly"which NSHTTPCookie ignores, causing restored cookies to lose their HttpOnly protection. This is a security regression.templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig (2)
163-172: Array parameter serialization format mismatch with API specification.Lines 167, 241, and 305 serialize array parameters using
"${key}[]"format (e.g.,?z[]=val1&z[]=val2), but the API specification defines"collectionFormat": "multi"which expects?z=val1&z=val2format without the[]suffix.
332-344: Unsafe map accesses without null-safety checks.Lines 332, 340, 343, and 344 access map fields without null checks, which could throw exceptions if the response structure is unexpected:
- Line 332:
result["\$id"]- Line 340:
result["\$id"]- Lines 343-344:
result["chunksTotal"]andresult["chunksUploaded"]templates/kmp/shared/build.gradle.kts.twig (1)
16-16: Guard against missing SDK_VERSION env var.
System.getenv("SDK_VERSION")returnsnullin local builds, causingproject.ext["PUBLISH_VERSION"].toString()on line 29 to fail with an NPE. Provide a fallback to the template version.Apply this diff:
- set("PUBLISH_VERSION", System.getenv("SDK_VERSION")) + set("PUBLISH_VERSION", System.getenv("SDK_VERSION") ?: "{{ sdk.version }}")
🧹 Nitpick comments (3)
templates/kmp/shared/src/androidMain/kotlin/io/package/models/InputFile.android.kt.twig (1)
4-4: Remove unused import.The
RequiresApiannotation is no longer used after removing the annotation from the function. Consider removing this import for cleaner code.-import androidx.annotation.RequiresApitemplates/kmp/shared/build.gradle.kts.twig (2)
8-8: Consider moving the AtomicFU version to the version catalog.The AtomicFU plugin version is hardcoded while other plugins use the version catalog. For consistency and easier maintenance, move this version to
templates/kmp/gradle/libs.versions.toml.Apply this diff to use the version catalog:
- id("org.jetbrains.kotlinx.atomicfu") version "0.29.0" + alias(libs.plugins.kotlinx.atomicfu)Then add the plugin reference to the version catalog file.
99-99: Move Firebase BOM version to the version catalog.The Firebase BOM version is hardcoded in the build script. For consistency with other dependencies and easier maintenance, move this version to
templates/kmp/gradle/libs.versions.toml.Apply this diff:
- implementation(project.dependencies.platform("com.google.firebase:firebase-bom:33.6.0")) + implementation(project.dependencies.platform(libs.firebase.bom))Then add the BOM reference to the version catalog file with version
33.6.0.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/SDK/Language/KMP.php(1 hunks)templates/kmp/gradle/wrapper/gradle-wrapper.properties(1 hunks)templates/kmp/shared/build.gradle.kts.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/SerializableCookie.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig(1 hunks)templates/kmp/shared/src/androidMain/kotlin/io/package/models/InputFile.android.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig(1 hunks)templates/kmp/shared/src/commonMain/kotlin/io/package/serializers/StringCollectionSerializer.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/cookies/IosCookieStorage.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig(1 hunks)templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- templates/kmp/gradle/wrapper/gradle-wrapper.properties
- templates/kmp/shared/src/iosMain/kotlin/io/package/webInterface/UrlParser.ios.kt.twig
- templates/kmp/shared/src/androidMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig
- templates/kmp/shared/src/iosMain/kotlin/io/package/WebAuthComponent.ios.kt.twig
- templates/kmp/shared/src/androidMain/kotlin/io/package/cookies/SerializableCookie.kt.twig
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/KMP.php (2)
src/SDK/Language.php (2)
Language(5-279)toPascalCase(115-118)src/SDK/Language/Kotlin.php (6)
Kotlin(8-651)getName(13-16)getFilters(475-500)getPropertyType(536-560)getReturnType(502-526)getFiles(302-473)
🪛 PHPMD (2.15.0)
src/SDK/Language/KMP.php
44-44: Avoid unused parameters such as '$spec'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (4)
templates/kmp/shared/src/androidMain/kotlin/io/package/models/InputFile.android.kt.twig (1)
9-17: Android API compatibility issue resolved.The previous concern about the
@RequiresApiannotation has been addressed. The implementation now uses a runtime API level check, allowing the function to work on all Android API levels while still leveragingFiles.probeContentType()when available (API 26+). The fallback toURLConnection.guessContentTypeFromName()ensures compatibility with older devices.templates/kmp/shared/src/iosMain/kotlin/io/package/extensions/OAuth2Extensions.kt.twig (1)
62-72: LGTM - Validation logic correctly fixed.The null-check validation now correctly uses
||to ensure bothkeyandsecretare non-null before constructing the Cookie. This prevents NPEs and aligns with the intended authentication flow.templates/kmp/shared/src/commonMain/kotlin/io/package/BaseClient.kt.twig (1)
283-284: LGTM - Safe cast implemented correctly.The unsafe cast has been properly fixed with safe nullable casting and a fallback to
0L. This preventsClassCastExceptionwhen the response structure is unexpected.templates/kmp/shared/build.gradle.kts.twig (1)
182-199: LGTM! Android configuration is correct.The
compileSdkhas been properly set to 35 (the latest public SDK), and the rest of the Android configuration (minSdk, Java 17 compatibility, test options) looks good.
| // Arrays of primitives | ||
| if (($property['type'] ?? '') === 'array') { | ||
| $itemType = $property['array']['type'] ?? 'string'; | ||
| $mapper = match ($itemType) { | ||
| 'integer' => 'it.jsonPrimitive.long', | ||
| 'number' => 'it.jsonPrimitive.double', | ||
| 'boolean' => 'it.jsonPrimitive.boolean', | ||
| default => 'it.jsonPrimitive.content', | ||
| }; | ||
| if ($property['required']) { | ||
| return "$key!!.jsonArray.map { $mapper }"; | ||
| } | ||
| return "$key?.jsonArray?.map { $mapper }"; | ||
| } | ||
|
|
||
| // Objects (kept as JsonElement/Any via @Contextual) | ||
| if (($property['type'] ?? '') === 'object') { | ||
| return $key; | ||
| } |
There was a problem hiding this comment.
Reintroduce nested-model deserialization logic.
Properties that declare a sub_schema (e.g., arrays of nested models such as Collection.rules) now hit the “arrays of primitives” branch, so the generated KMP code ends up mapping each element to JsonPrimitive.content. That produces a List<String>/JsonElement where the templates expect List<YourSubModel>, causing type mismatches or crashes at runtime. Please mirror the Kotlin generator’s handling for sub-schema objects before the primitive branches so we instantiate the proper model types.
One way to address this is to add a dedicated sub-schema branch ahead of the primitive array block:
@@
- // Arrays of primitives
- if (($property['type'] ?? '') === 'array') {
+ // Nested models (objects or arrays of objects)
+ if (isset($property['sub_schema']) && !empty($property['sub_schema'])) {
+ $nestedClass = $this->toPascalCase($property['sub_schema']);
+ $genericSuffix = $this->hasGenericType($property['sub_schema'], $spec) ? '<T>' : '';
+
+ if (($property['type'] ?? '') === self::TYPE_ARRAY) {
+ $mapper = $property['required']
+ ? "$key!!.jsonArray.map { $nestedClass$genericSuffix.fromJsonObject(it.jsonObject) }"
+ : "$key?.jsonArray?.map { $nestedClass$genericSuffix.fromJsonObject(it.jsonObject) }";
+ return $mapper;
+ }
+
+ return $property['required']
+ ? "$nestedClass$genericSuffix.fromJsonObject($key!!.jsonObject)"
+ : "$key?.jsonObject?.let { $nestedClass$genericSuffix.fromJsonObject(it) }";
+ }
+
+ // Arrays of primitives
+ if (($property['type'] ?? '') === 'array') {
$itemType = $property['array']['type'] ?? 'string';(Adjust the factory method naming to whatever the generated models expose—fromJsonObject, from, etc.)
Committable suggestion skipped: line range outside the PR's diff.
| protected function getReturnType(array $method, array $spec, string $namespace, string $generic = 'T', bool $withGeneric = true): string | ||
| { | ||
| if ($method['type'] === 'webAuth') { | ||
| return 'Bool'; | ||
| } | ||
| return parent::getReturnType($method, $spec, $namespace, $generic, $withGeneric); | ||
| } |
There was a problem hiding this comment.
Use the Kotlin Boolean type for webAuth methods.
Bool isn’t a valid Kotlin type (that’s Swift); this will break compilation for every generated webAuth method. The KMP code should return Boolean, just like the base Kotlin generator.
if ($method['type'] === 'webAuth') {
- return 'Bool';
+ return 'Boolean';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected function getReturnType(array $method, array $spec, string $namespace, string $generic = 'T', bool $withGeneric = true): string | |
| { | |
| if ($method['type'] === 'webAuth') { | |
| return 'Bool'; | |
| } | |
| return parent::getReturnType($method, $spec, $namespace, $generic, $withGeneric); | |
| } | |
| protected function getReturnType(array $method, array $spec, string $namespace, string $generic = 'T', bool $withGeneric = true): string | |
| { | |
| if ($method['type'] === 'webAuth') { | |
| return 'Boolean'; | |
| } | |
| return parent::getReturnType($method, $spec, $namespace, $generic, $withGeneric); | |
| } |
🤖 Prompt for AI Agents
In src/SDK/Language/KMP.php around lines 101 to 107, the getReturnType override
returns the Swift type "Bool" for webAuth methods which is invalid in Kotlin;
change the return value to the Kotlin type "Boolean" so webAuth methods match
the base Kotlin generator's return type and compile correctly.
| alias(libs.plugins.kotlinx.serialization) | ||
| id("org.jetbrains.kotlinx.atomicfu") version "0.29.0" | ||
| id("maven-publish") | ||
| id("signing") |
There was a problem hiding this comment.
🧩 Analysis chain
Verify signing configuration for Maven Central publishing.
The signing plugin is declared but no signing { } configuration block is present. Maven Central requires signed artifacts. Confirm that signing is configured elsewhere (e.g., in a parent script or convention plugin) or add the signing configuration here.
Run this script to search for signing configuration:
🏁 Script executed:
#!/bin/bash
# Search for signing configuration blocks in the repository
ast-grep --pattern $'signing {
$$$
}'
# Also check for signing-related setup in other gradle files
rg -n 'sign\(' -g '*.kts' -g '*.gradle'Length of output: 0
I need to broaden the search with different patterns and approaches, since the initial search produced no results. Let me investigate further:
I need to conduct a broader search to understand the signing configuration context better, since the initial patterns returned no results:
🏁 Script executed:
#!/bin/bash
# Broader search for signing-related configuration
# Check for signing with various patterns and whitespace variations
rg -n -i 'signing\s*\{' -g '*.kts' -g '*.gradle' -g '*.properties' -g '*.twig'
# Check for signing plugin configuration in DSL
rg -n 'id.*signing' -g '*.kts' -g '*.gradle' -g '*.twig'
# Look for parent build scripts and settings
echo "=== Checking settings.gradle(.kts) ==="
find . -name 'settings.gradle*' -type f | head -5
# Check for convention plugins or build configuration
echo "=== Checking for convention plugins ==="
find . -path '*/convention*' -o -path '*/buildSrc*' | head -10
# Search for Maven Central or publishing related config
rg -n 'MavenCentral\|publishToMavenCentral\|signing' -g '*.kts' -g '*.gradle' | head -20Length of output: 369
🏁 Script executed:
#!/bin/bash
# Broader search for signing-related configuration across all relevant files
echo "=== Searching for signing patterns (relaxed) ==="
rg -n -i 'signing' -g '*.kts' -g '*.gradle' -g '*.twig' -C 2
echo "=== Checking gradle.properties files ==="
find . -name 'gradle.properties' -type f -exec grep -l 'sign' {} \;
echo "=== Looking for build configuration structure ==="
find . -type d \( -name 'buildSrc' -o -name 'build-logic' -o -name 'convention' \) | head -5
echo "=== Checking settings file ==="
find . -maxdepth 2 -name 'settings.gradle*' -type f
echo "=== Search in all gradle/twig files for publish or maven config ==="
rg -n 'publish|maven|sonatype' -i -g '*.kts' -g '*.gradle' -g '*.twig' | head -15Length of output: 6483
I need to see the complete KMP file to determine if signing configuration exists:
🏁 Script executed:
#!/bin/bash
# View the complete KMP build.gradle.kts.twig file
echo "=== Full KMP build.gradle.kts.twig file ==="
cat templates/kmp/shared/build.gradle.kts.twig
# Also check if KMP applies external scripts like the other templates
echo -e "\n=== Check for script imports in KMP ==="
rg -n 'apply|include|script' templates/kmp/shared/build.gradle.kts.twigLength of output: 7406
Let me check for any root-level or parent build scripts that might handle signing configuration globally:
🏁 Script executed:
#!/bin/bash
# Check for root build.gradle files and any parent configuration
echo "=== Root-level gradle files ==="
find . -maxdepth 1 -name 'build.gradle*' -type f
echo "=== Root gradle.properties ==="
find . -maxdepth 1 -name 'gradle.properties' -type f
echo "=== Check if KMP shares parent build config ==="
find . -path '*/kmp/*' -name 'build.gradle*' -o -path '*/kmp/*' -name 'gradle.properties'
echo "=== Search for any gradle plugins applied at root or shared level ==="
rg -n 'signing\|id.*signing' --type gradle --type propertiesLength of output: 381
Let me check the parent KMP build file and gradle properties:
🏁 Script executed:
#!/bin/bash
# Check parent KMP build file
echo "=== Parent KMP build.gradle.kts.twig ==="
cat templates/kmp/build.gradle.kts.twig
echo -e "\n=== KMP gradle.properties ==="
cat templates/kmp/gradle.properties
echo -e "\n=== Search for signing in KMP directory ==="
rg -n 'signing' templates/kmp/ -g '*.gradle*' -g '*.properties'Length of output: 982
Add the signing configuration block to complete Maven Central publishing setup.
The signing plugin is declared on line 10 but no signing { } configuration block exists in this file or parent/sibling build scripts. Unlike the Kotlin and Android templates (which include signing configuration in their publish scripts), the KMP template is missing this critical component. Maven Central requires signed artifacts.
Add the signing configuration block with:
signing {
sign publishing.publications
}Ensure signing credentials are provided via environment variables or properties matching the Kotlin/Android template pattern (e.g., SIGNING_KEY_ID, SIGNING_PASSWORD, SIGNING_SECRET_KEY_RING_FILE).
🤖 Prompt for AI Agents
In templates/kmp/shared/build.gradle.kts.twig around line 10, the signing plugin
is applied but there is no signing { } configuration block; add a signing
configuration that signs the publishing.publications (i.e., configure signing to
sign publishing.publications) and ensure the signing credentials are read from
environment variables or Gradle properties following the Kotlin/Android template
pattern (e.g., SIGNING_KEY_ID, SIGNING_PASSWORD, SIGNING_SECRET_KEY_RING_FILE)
so artifacts can be accepted by Maven Central.
| username = System.getenv("OSSRH_USERNAME") | ||
| password = System.getenv("OSSRH_PASSWORD") |
There was a problem hiding this comment.
🧩 Analysis chain
Verify OSSRH credentials handling in local builds.
The credentials pull from environment variables without fallback. Confirm that local builds (where OSSRH_USERNAME and OSSRH_PASSWORD are unset) either skip publishing or gracefully handle missing credentials.
Run this script to check if there are any guards or conditional publishing logic:
🏁 Script executed:
#!/bin/bash
# Check for credential handling patterns in related files
rg -n -C3 'OSSRH_USERNAME|OSSRH_PASSWORD|credentials' -g '*.yml' -g '*.yaml' -g '*.kts'Length of output: 0
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and show its content around lines 175-176
find . -name "build.gradle.kts.twig" -type f | head -5Length of output: 131
🏁 Script executed:
#!/bin/bash
# Search for OSSRH patterns without file type restrictions
rg -n 'OSSRH_USERNAME|OSSRH_PASSWORD' --type-list | head -20
rg -n 'OSSRH_USERNAME|OSSRH_PASSWORD'Length of output: 1103
🏁 Script executed:
#!/bin/bash
# Look for publishing or credentials configuration more broadly
rg -n 'credentials|publishing' -A2 -B2Length of output: 3535
🏁 Script executed:
#!/bin/bash
# Check for gradle.properties or environment variable handling
find . -name "gradle.properties" -o -name "*.gradle" -o -name "*.gradle.kts" | head -20Length of output: 406
🏁 Script executed:
#!/bin/bash
# Search for task configurations that might condition publishing
rg -n 'task|publishing\s*{|repositories\s*{' -g '*.kts' -g '*.twig' -C1Length of output: 137
🏁 Script executed:
#!/bin/bash
# Show full context of credentials block and surrounding code in KMP template
cat -n templates/kmp/shared/build.gradle.kts.twig | sed -n '160,180p'Length of output: 752
🏁 Script executed:
#!/bin/bash
# Check for ext property definitions or gradle.properties in KMP
cat templates/kmp/gradle.propertiesLength of output: 399
🏁 Script executed:
#!/bin/bash
# Show full credentials and repositories blocks in KMP
cat -n templates/kmp/shared/build.gradle.kts.twig | sed -n '128,180p'Length of output: 2272
🏁 Script executed:
#!/bin/bash
# Compare the three credential patterns side by side
echo "=== KMP Template (lines 175-176) ===" && \
rg -n -A1 'username = System.getenv' templates/kmp/shared/build.gradle.kts.twig && \
echo -e "\n=== Kotlin Template (lines 18-19) ===" && \
rg -n -A1 'ossrhUsername' templates/kotlin/scripts/setup.gradle && \
echo -e "\n=== Android Template (lines 18-19) ===" && \
rg -n -A1 'ossrhUsername' templates/android/scripts/publish-config.gradleLength of output: 855
🏁 Script executed:
#!/bin/bash
# Search for signing configuration in KMP template
rg -n 'signing\s*{' templates/kmp/shared/build.gradle.kts.twig -A10Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check if there are any task conditionals or onlyIf statements in KMP
rg -n 'onlyIf|enabled|gradle.taskGraph' templates/kmp/shared/build.gradle.kts.twig -C2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the beginning of the KMP build file for imports and plugins
head -50 templates/kmp/shared/build.gradle.kts.twigLength of output: 1935
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about credential handling
rg -n 'SNAPSHOT|publish|release|local' templates/kmp/shared/build.gradle.kts.twig -B1 -A1Length of output: 845
Add fallback to OSSRH credentials to prevent null values in local builds.
The KMP template directly assigns System.getenv() results to credentials without fallback, unlike the Kotlin and Android templates which use the Elvis operator (?:) with initialized defaults. When OSSRH_USERNAME and OSSRH_PASSWORD environment variables are unset (common in local dev builds), credentials will be null and publishing will fail silently or with unclear errors.
Fix: Add fallback values matching the pattern in templates/kotlin/scripts/setup.gradle and templates/android/scripts/publish-config.gradle:
- Initialize
ext["ossrhUsername"]andext["ossrhPassword"]to empty strings - Use
System.getenv("OSSRH_USERNAME") ?: ext["ossrhUsername"]instead of directSystem.getenv()call
Lines 175–176 in templates/kmp/shared/build.gradle.kts.twig
🤖 Prompt for AI Agents
In templates/kmp/shared/build.gradle.kts.twig around lines 175-176, the OSSRH
credentials are set directly from System.getenv(...) which can be null in local
builds; initialize ext["ossrhUsername"] and ext["ossrhPassword"] to empty
strings (matching the other templates) and replace the direct System.getenv
calls with fallbacks using the Elvis operator, e.g.
System.getenv("OSSRH_USERNAME") ?: ext["ossrhUsername"] and
System.getenv("OSSRH_PASSWORD") ?: ext["ossrhPassword"], so credentials are
never null during local builds.
…existing function implementations
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig (1)
81-81: Minor formatting inconsistency.This method is formatted on a single line while others use multi-line formatting. Consider matching the style used in surrounding methods for consistency.
</review_comment_end -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig(1 hunks)
🔇 Additional comments (4)
templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig (4)
9-14: LGTM!The class structure is well-designed with appropriate use of
@Contextualfor handling polymorphic value types in kotlinx.serialization, and nullable types for optional parameters.
15-20: LGTM!The
toStringoverride correctly uses kotlinx.serialization to produce a JSON representation.
260-302: LGTM!The date helper methods provide a clean API by delegating to the base comparison operators with the special
$createdAtand$updatedAtattributes. The dollar signs are correctly escaped.
329-362: Nested list structure is correct and intentional.The test cases confirm the implementation matches the expected API format. The methods correctly wrap the parameters in
listOf(listOf(values, distance, meters))to create the proper structure for the Query API, supporting both simple coordinates likelistOf(40.7128, -74)and complex coordinate lists.
| fun or(queries: List<String>) = Query("or", null, queries.map { it.fromJson<Query>() }).toString() | ||
|
|
||
| /** | ||
| * Combine multiple queries using logical AND operator. | ||
| * | ||
| * @param queries The list of query strings to combine. | ||
| * @returns The query string. | ||
| */ | ||
| fun and(queries: List<String>) = Query("and", null, queries.map { it.fromJson<Query>() }).toString() |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the necessity of deserializing query strings.
Both or() and and() methods deserialize query strings back to Query objects (it.fromJson<Query>()) before re-serializing. This round-trip could be inefficient and may fail if the input strings aren't valid JSON Query objects.
Please verify:
- Is this round-trip necessary for API compatibility, or can query strings be passed directly in the values list?
- Should there be error handling for invalid query strings?
🏁 Script executed:
#!/bin/bash
# Search for tests or documentation that show how or/and methods are used
rg -n -C3 '\bor\s*\(|\band\s*\(' --type=kotlin -g '!**/build/**'Length of output: 2811
I can see the tests are passing serialized Query objects (strings) to or() and and(). Let me check the Query data class definition to understand if the round-trip is necessary.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Query data class definition to check constructor parameter types
rg -n -A10 'data class Query' --type=kotlin -g '!**/build/**' | head -50
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Look for Query class in generated files and check constructor
fd -e 'Query.kt' -g '!**/build/**' | head -20
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the Query.kt.twig template file to understand constructor and class structure
cat 'templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig' | head -150
Length of output: 5474
---
🏁 Script executed:
```shell
#!/bin/bash
# Read lines around 310-318 to see or/and method implementations
sed -n '300,330p' 'templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig'
Length of output: 1339
Add error handling for invalid query strings in or() and and() methods.
The deserialization round-trip is intentional and necessary—it ensures nested queries serialize as Query objects within the JSON structure. However, the current implementation lacks error handling: if a query string isn't valid JSON, fromJson<Query>() will throw an uncaught exception.
Add try-catch with a descriptive error message or validate inputs before deserialization.
🤖 Prompt for AI Agents
In templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig around
lines 310 to 318, the or() and and() helpers call queries.map {
it.fromJson<Query>() } with no error handling so an invalid JSON string will
throw uncaught exceptions; wrap the deserialization in safe handling (e.g., use
runCatching / try-catch inside the map), and on failure throw a clear
IllegalArgumentException (or similar) that includes which method (or/and), the
index or value of the offending query string and a short reason, or
alternatively validate the string before attempting fromJson and aggregate
failures into the thrown error so callers get a descriptive message rather than
a raw parse exception.
| private fun parseValue(value: Any): List<Any> { | ||
| return when (value) { | ||
| is List<*> -> value as List<Any> | ||
| else -> listOf(value) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix unchecked cast that may cause runtime exceptions.
Line 444 performs an unchecked cast value as List<Any> which can fail at runtime. The Kotlin compiler typically warns about this because it cannot verify type safety at compile time.
Apply this diff to use a safe cast or suppress the warning if the cast is guaranteed safe:
private fun parseValue(value: Any): List<Any> {
return when (value) {
- is List<*> -> value as List<Any>
+ is List<*> -> @Suppress("UNCHECKED_CAST") (value as List<Any>)
else -> listOf(value)
}
}Alternatively, if you want to be more defensive:
private fun parseValue(value: Any): List<Any> {
return when (value) {
- is List<*> -> value as List<Any>
+ is List<*> -> value.filterNotNull()
else -> listOf(value)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun parseValue(value: Any): List<Any> { | |
| return when (value) { | |
| is List<*> -> value as List<Any> | |
| else -> listOf(value) | |
| } | |
| } | |
| private fun parseValue(value: Any): List<Any> { | |
| return when (value) { | |
| is List<*> -> @Suppress("UNCHECKED_CAST") (value as List<Any>) | |
| else -> listOf(value) | |
| } | |
| } |
🤖 Prompt for AI Agents
In templates/kmp/shared/src/commonMain/kotlin/io/package/Query.kt.twig around
lines 442 to 447, the unchecked cast `value as List<Any>` can throw at runtime;
replace it with a safe cast and defensive conversion such as using `value as?
List<*>` and then mapping/typing its elements to List<Any> (e.g. filter/map to
Any or throw a clear exception) or return an empty list if the cast fails; do
not use an unchecked `as` cast or rely on suppression unless you can absolutely
guarantee the input type.
4 participants
What does this PR do?
Adds a KMP SDK
Test Plan
Run KMPAndroid14Java17Test
Test OAuth2, cookies, and iOS file uploads using custom project in Android Studio
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
yes
Summary by CodeRabbit
New Features
Tests