[codex] Preserve photo-picker fallback extension#533
Merged
Conversation
Owner
Author
|
@codex review this |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the Android photo-picker regression behind issue #526.
On Android photo-picker URIs, FileKit tries to replace synthetic picker names like
18.jpgwith the real MediaStore display name. That is the right first choice because it preserves the user-visible filename when the lookup succeeds. The problem is the failure path: when the MediaStore query is blocked or returns no row, simply surfacing the synthetic picker display name restores the extension but reintroduces the ambiguity that the stable fallback was meant to avoid. Different picker URIs can still be distinct files even when they expose the same synthetic display name pattern, so reusing that value directly is not a safe fallback.The root cause was that the fallback strategy mixed together two separate concerns: preserving uniqueness and preserving the extension. The provider/id-based fallback solved the uniqueness problem, while the synthetic picker name still contained useful extension metadata. Treating the whole synthetic name as the final fallback discarded the stability guarantees; treating only the provider/id fallback as final discarded the extension.
This change keeps the existing priority order. FileKit still prefers the real MediaStore display name when it can resolve it, and still returns non-synthetic openable display names directly. When the URI is a photo-picker URI and only a synthetic picker display name is available, the code now keeps the stable provider/id-based fallback name and appends only the synthetic extension if one exists. That produces names such as
photopicker-18.jpg, which remain stable and distinct while allowingPlatformFile.extensionto stay populated.The Android host tests were updated to cover the intended contract: MediaStore lookup failures now resolve to a stable fallback with the original extension, the no-display-name case still falls back to the stable provider/id name without an extension, and multiple synthetic picker names still resolve to distinct stable names.
Validation used
./gradlew :filekit-core:testAndroidHostTest --tests 'io.github.vinceglb.filekit.PlatformFileAndroidTest'and./gradlew :filekit-core:check, both of which passed.