-
-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure version files are created at same path as store #101
Conversation
Hi @JanTie Thanks for the contribution! Little curious as to how this failed to create the version file before this patch. Could you share how the store was created? |
What i tried to say in the original ticket is: As in all these cases there is no directory defined for the version path, the system will always try to create the file on the current PWD of the system. On Android devices however that directory will always be read-only, causing apps trying to make use of versioned KStores to crash immediately. I must say i did not have the time to verify the results of this PR yet. Just wanted to point out the underlying issue for now. |
Hi @JanTie Thanks for the detailed explanation. It does make sense why it would fail in some scenarios. However, I'm unsure how the change to Perhaps a better way to handle this would be to let the codec manage the version file instead, so that you can create this file wherever it needs to go. For example, something like public class VersionedCodec<T: @Serializable Any>(
private val file: Path,
private val version: Int = 0,
private val json: Json,
private val serializer: KSerializer<T>,
private val migration: Migration<T>,
private val versionFile: Path = "$$file.version".toPath() // TODO: Save to file metadata instead
): Codec<T> { .. } Any thoughts on this approach? |
I added your feedback. I also provided the exact same default in the PS: Sry for minor reformats, i guess my autoformat kicked in. We may want to use some sort of code formatting tool at some point. I can revert these changes if needed. |
Hi @JanTie Can we also update the API? you can do this by running |
updated the api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done 👍 Thank you very much for your contribution
Addresses #99
I did not have the time yet to verify the result. However, these changes should pretty much ensure that the version file will always be created at the same location as the store itself, having the ".version" suffix.
Therefore, if the targeted directory is write-accessable on android, there shall be no more errors during the creation of such versioned stores.