Import / Export / Share Container Configs #232
Conversation
WalkthroughAdds deep-link support and import/export/share of container configurations: manifest intent-filters, a new ContainerConfigIO utility for serialization/links/share-codes, MainActivity early deep-link import emitting a new AndroidEvent.ConfigImported, UI changes for file/share import/export, and a versionCode bump. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant System as Android System / Browser
participant Manifest as AndroidManifest
participant MA as MainActivity
participant ConfigIO as ContainerConfigIO
participant Event as AndroidEvent
participant UI as App UI
User->>System: Tap deep-link (gamenative://... or https://gamenative.app/...)
System->>Manifest: Resolve intent to MainActivity
Manifest->>MA: Start with intent(uri)
MA->>MA: handleLaunchIntent()
MA->>ConfigIO: importFromDeepLink(uri)
alt valid config
ConfigIO-->>MA: ContainerData
MA->>Event: Emit ConfigImported(ContainerData)
Event-->>UI: Apply/configure UI state
else no config / invalid
ConfigIO-->>MA: null
MA->>MA: Continue existing launch-intent flow
end
sequenceDiagram
actor User
participant UI as ContainerConfigDialog
participant ConfigIO as ContainerConfigIO
participant System as Android System
User->>UI: Click "Import" (From File)
UI->>System: Launch file picker
System-->>UI: Return file URI
UI->>ConfigIO: importFromFile(uri)
ConfigIO-->>UI: ContainerData or null
UI->>UI: Show success toast / error toast
User->>UI: Click "Import" (From Share Code)
UI->>UI: Show input dialog
User->>UI: Submit share code
UI->>ConfigIO: importFromShareCode(code)
ConfigIO-->>UI: ContainerData or null
UI->>UI: Show success toast / error toast
User->>UI: Click "Export"
UI->>System: Launch save dialog
System-->>UI: Return target URI
UI->>ConfigIO: exportToFile(targetUri)
ConfigIO-->>UI: success/failure
UI->>UI: Show toast
User->>UI: Click "Share"
UI->>ConfigIO: createShareMessageIntent(container)
ConfigIO-->>UI: Intent with share text
UI->>System: Launch chooser
System->>User: Share via chosen app
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (3)
app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt (2)
25-25: Remove unused constant.The
MIME_TYPE_JSONconstant is defined but never used in this file. Consider removing it or using it in theexportToFileor file picker operations if appropriate.
366-405: Consider extracting common compression logic.The compression and encoding logic is duplicated across
createWebLink,createDeepLink, andcreateShareCode. Consider extracting this into a private helper function to improve maintainability.Example refactor:
private fun compressAndEncode(jsonString: String, flags: Int = Base64.URL_SAFE or Base64.NO_WRAP): String { val compressed = ByteArrayOutputStream().use { byteStream -> GZIPOutputStream(byteStream).use { gzipStream -> gzipStream.write(jsonString.toByteArray()) } byteStream.toByteArray() } return Base64.encodeToString(compressed, flags) } fun createWebLink(container: Container): String { val encoded = compressAndEncode(exportToJson(container)) return "https://gamenative.app/config?data=$encoded" } fun createDeepLink(container: Container): String { val encoded = compressAndEncode(exportToJson(container)) return "gamenative://config?data=$encoded" }app/src/main/java/app/gamenative/MainActivity.kt (1)
191-210: Consider user feedback for import failures.The deep link import logic is well-structured, but when import fails (line 206), only a log message is generated. Consider showing a user-facing error message (Toast or dialog) to inform users that the config link was invalid or could not be processed.
Example:
val containerData = ContainerConfigIO.importFromDeepLink(uri) if (containerData != null) { Timber.i("[ConfigImport]: Successfully imported config from link: ${containerData.name}") - // Store imported config for user to apply - // You can emit an event here to show a dialog or navigate to container creation PluviaApp.events.emit(AndroidEvent.ConfigImported(containerData)) } else { Timber.e("[ConfigImport]: Failed to parse config from link") + lifecycleScope.launch { + Toast.makeText(this@MainActivity, "Failed to import configuration from link", Toast.LENGTH_LONG).show() + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/app/gamenative/MainActivity.kt(2 hunks)app/src/main/java/app/gamenative/events/AndroidEvent.kt(2 hunks)app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt(7 hunks)app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
app/src/main/java/com/winlator/container/Container.java (1)
Container(22-937)
🔇 Additional comments (7)
app/src/main/java/app/gamenative/events/AndroidEvent.kt (1)
4-4: LGTM!The new
ConfigImportedevent follows the established pattern in this file and correctly carries the importedContainerDatapayload.Also applies to: 22-22
app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt (3)
247-288: LGTM with minor observation.The share intent functions are well-structured. Note that line 276 contains an emoji that may not render correctly in all contexts, but this is acceptable for user-facing messaging.
297-356: LGTM!The share code generation and parsing logic is well-implemented with proper compression, versioning, and flexible input handling.
413-432: LGTM!The deep link parsing correctly handles the URL query parameter and decompression.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (3)
752-808: LGTM!The share code import dialog is well-designed with clear instructions, validation, and user feedback.
837-941: LGTM!The top bar actions provide a clear and intuitive interface for import, export, and sharing operations. The dropdown for import options and the automatic filename generation are good UX touches.
173-217: Missinglanguagefield in export Container.The temporary Container created for export (lines 173-217) is missing the
languagefield, which is present in the share button's Container (line 926). This will cause exported configs to lose language settings.Apply this diff:
isEmulateKeyboardMouse = config.emulateKeyboardMouse isForceDlc = config.forceDlc + language = config.language // Set controller emulation bindings if presentLikely an incorrect or invalid review comment.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
Outdated
Show resolved
Hide resolved
Phr3d13
changed the title
| val imported = ContainerConfigIO.importFromFile(context, it) | ||
| if (imported != null) { | ||
| config = imported | ||
| Toast.makeText(context, "Configuration imported successfully", Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
Would recommend putting this into the Strings.xml
| config = imported | ||
| Toast.makeText(context, "Configuration imported successfully", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| Toast.makeText(context, "Failed to import configuration", Toast.LENGTH_LONG).show() |
| */ | ||
| fun exportToJson(container: Container): String { | ||
| val configData = JSONObject().apply { | ||
| put("version", CONTAINER_CONFIG_VERSION) |
There was a problem hiding this comment.
I think it'd be worth us putting in the app version too.
| * @param jsonString JSON string containing the container config | ||
| * @return ContainerData object if import succeeded, null otherwise | ||
| */ | ||
| fun importFromJson(jsonString: String): ContainerData? { |
There was a problem hiding this comment.
As a general question, is it worth putting in some input sanitation here? Might not be required, but worth asking.
|
Idea is great, execution looks solid. |
|
I think this commit addresses all suggestions / issues. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/main/res/values/strings.xml (1)
126-129: Good additions, but consider adding more string resources.The new string resources are well-named and clear. However,
ContainerConfigDialog.ktcontains several hardcoded strings that should be moved here for consistency and localization:
- Line 161: "Configuration imported successfully"
- Line 178: "Configuration exported successfully"
- Line 180: "Failed to export configuration"
- Line 744: "Configuration imported successfully" (duplicate)
- Line 748: "Invalid share code or link"
Consider adding these to
strings.xml:<string name="failed_to_import_config">Failed to import configuration</string> <string name="share_config">Share Config</string> + <string name="config_imported_success">Configuration imported successfully</string> + <string name="config_exported_success">Configuration exported successfully</string> + <string name="config_export_failed">Failed to export configuration</string> + <string name="invalid_share_code">Invalid share code or link</string>app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
154-183: Move hardcoded strings to string resources.The launchers correctly use
toTempContainer()andContainerConfigIOfunctions, but several toast messages are hardcoded instead of using string resources:
- Line 161: "Configuration imported successfully"
- Line 178: "Configuration exported successfully"
- Line 180: "Failed to export configuration"
Line 163 correctly uses
R.string.failed_to_import_config, but the success messages should follow the same pattern.Based on learnings (phobos665's suggestion to use strings.xml).
Apply this diff after adding the suggested strings to
strings.xml:if (imported != null) { config = imported - Toast.makeText(context, "Configuration imported successfully", Toast.LENGTH_SHORT).show() + Toast.makeText(context, context.getString(R.string.config_imported_success), Toast.LENGTH_SHORT).show() } else { Toast.makeText(context, context.getString(R.string.failed_to_import_config), Toast.LENGTH_LONG).show() }val success = ContainerConfigIO.exportToFile(context, tempContainer, it) if (success) { - Toast.makeText(context, "Configuration exported successfully", Toast.LENGTH_SHORT).show() + Toast.makeText(context, context.getString(R.string.config_exported_success), Toast.LENGTH_SHORT).show() } else { - Toast.makeText(context, "Failed to export configuration", Toast.LENGTH_LONG).show() + Toast.makeText(context, context.getString(R.string.config_export_failed), Toast.LENGTH_LONG).show() }
709-765: Consider externalizing dialog strings for localization.The share code import dialog functions correctly but contains multiple hardcoded strings. While the core functionality works, consider moving these to
strings.xmlfor consistency and localization support:
- "Import from Share Code"
- "Paste the share code or link you received:"
- "Accepts: GN1:..., gamenative://..., or https://gamenative.app/..."
- "Share Code or Link"
- "Configuration imported successfully" (line 744)
- "Invalid share code or link" (line 748)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/build.gradle.kts(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt(7 hunks)app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/AndroidManifest.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt (1)
app/src/main/java/com/winlator/container/Container.java (1)
Container(22-937)
🔇 Additional comments (11)
app/build.gradle.kts (1)
54-54: LGTM! Version code increment aligns with new features.The version code bump from 4 to 5 is appropriate for the import/export/share functionality being added in this PR.
app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt (8)
18-31: LGTM! Well-defined security constants.The security limits (MAX_JSON_SIZE_BYTES, MAX_STRING_LENGTH, etc.) are appropriate safeguards against resource exhaustion and malicious inputs. The version constant enables future backward compatibility.
37-99: Excellent refactoring! Addresses past review feedback.The
toTempContainerextension function successfully eliminates the code duplication that was flagged in previous reviews between export and share flows. The mapping is comprehensive and includes all necessary fields (FEXCore settings, Wine registry, controller bindings).
105-251: Strong security measures implemented.The sanitization functions provide comprehensive protection against common attacks:
- Path traversal prevention (removes
..patterns)- Command injection mitigation (removes shell metacharacters)
- Environment variable validation (enforces
key=valueformat)- Length limits on all inputs
This directly addresses phobos665's question about input sanitation from previous reviews.
Based on learnings (phobos665's feedback about input sanitation).
284-380: Comprehensive export with version tracking.The export function includes all necessary metadata (version, appVersion, timestamp) and comprehensively maps all container fields including FEXCore settings, Wine registry entries, and optional configurations. This addresses previous feedback about including the app version and ensuring all fields are exported.
Based on learnings (phobos665's suggestion to include app version).
389-528: Robust import with security measures.The import functions implement multiple layers of protection:
- Size limits to prevent resource exhaustion (MAX_JSON_SIZE_BYTES)
- Version validation with forward compatibility warnings
- Comprehensive input sanitization via
sanitizeContainerData- Sensible defaults from Container constants
All fields are properly mapped including those mentioned in past reviews.
587-758: LGTM! Share and deep link functionality is well-implemented.The share code and deep link functions provide multiple user-friendly import methods with proper security measures:
- GZIP compression reduces share code size
- Decompression bomb protection validates both compressed and decompressed sizes
- Support for multiple formats (GN1:, gamenative://, https://) enhances usability
- Base64 URL_SAFE encoding for web links
769-841: Complete field mapping in launch intent.The
createLaunchIntentfunction now comprehensively maps all ContainerData fields to JSON, including FEXCore settings, Wine registry entries, paths, and controller bindings. This addresses the previous review feedback about incomplete field mapping.
849-854: LGTM! Filename generation is well-implemented.The filename sanitization and timestamp formatting create readable, safe filenames for exported configs.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
3-88: LGTM! Imports are appropriate for the new functionality.The imports support file picker integration, sharing, and the new ContainerConfigIO utility. The
toTempContainerimport confirms usage of the refactored helper function.
794-860: Excellent integration! Duplicate code eliminated.The top bar actions correctly use
toTempContainer()at lines 174 and 846, successfully eliminating the code duplication flagged in previous reviews. The integration withContainerConfigIOis clean and the share button correctly uses the string resource.Minor: Consider externalizing "Import", "Export", "From File", and "From Share Code" strings for consistency, though button labels are often acceptable as inline text.
| fun createShareMessageIntent(container: Container, gameName: String = ""): Intent { | ||
| val webLink = createWebLink(container) | ||
| val configCode = createShareCode(container) | ||
| val displayName = gameName.ifEmpty { container.name } | ||
|
|
||
| val message = buildString { | ||
| appendLine("🎮 GameNative Config: $displayName") | ||
| appendLine() | ||
| appendLine("Settings: ${container.graphicsDriver}, ${container.dxWrapper}, Wine ${container.wineVersion}") | ||
| appendLine() | ||
| appendLine("� Click to import:") | ||
| appendLine(webLink) | ||
| appendLine() | ||
| appendLine("📋 Or paste this code in GameNative → Import → From Share Code:") | ||
| appendLine(configCode) | ||
| } | ||
|
|
||
| return Intent(Intent.ACTION_SEND).apply { | ||
| type = "text/plain" | ||
| putExtra(Intent.EXTRA_TEXT, message) | ||
| putExtra(Intent.EXTRA_SUBJECT, "GameNative Config: $displayName") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix corrupted emoji character.
Line 566 contains a corrupted character (�) that appears to be an encoding issue. This should be a clickable link emoji to match the style of lines 562 (🎮) and 569 (📋).
Apply this diff to use a proper link emoji:
- appendLine("� Click to import:")
+ appendLine("🔗 Click to import:")🤖 Prompt for AI Agents
In app/src/main/java/app/gamenative/utils/ContainerConfigIO.kt around lines 556
to 578, the message string contains a corrupted character on line 566; replace
the corrupted character (`�`) with a proper link emoji (e.g. "🔗") so the line
reads like "🔗 Click to import:", and ensure the file is saved with UTF-8
encoding so the emoji persists correctly.
| if (isConfigLink) { | ||
| Timber.d("[ConfigImport]: Received config import link: ${uri.scheme}://${uri.host}${uri.path}") | ||
| val containerData = ContainerConfigIO.importFromDeepLink(uri) | ||
| if (containerData != null) { | ||
| Timber.i("[ConfigImport]: Successfully imported config from link: ${containerData.name}") | ||
| // Store imported config for user to apply | ||
| // You can emit an event here to show a dialog or navigate to container creation | ||
| PluviaApp.events.emit(AndroidEvent.ConfigImported(containerData)) | ||
| } else { | ||
| Timber.e("[ConfigImport]: Failed to parse config from link") | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
I'd do this the other way around to do an early return pattern.
If(!thing) { TImber.e(...) return };
... rest of logic
|
Code looks solid, happy to give approval pending some testing I can do tonight. |
|
I will prune the changes here and merge |
3 participants
As the title says.
Summary by CodeRabbit