feat: unify importers#3436
Conversation
mostafaNazari702
changed the title
8cf92f3 to
2a88257
Compare
|
Next steps after review can concern with registering the handlers or getting rid of the "Local" and "Remote" concept by merging both into one. A local and remote source of patches therefore becomes identical in shape and behaviour. |
7e3cd9a to
9ea6088
Compare
|
Need review on my commits merge local and remote sources into one + provide the content resolver through the koin graph |
| // Imported files are copied into app storage and their origin is not recorded. | ||
| is SourceInfo.Local -> Uri.parse("rvp:${file.absolutePath}") | ||
| // Imported files are copied into app storage, the source points at that copy. | ||
| is SourceInfo.Local -> Uri.fromFile(file) |
There was a problem hiding this comment.
No distinction of local, api, or remote should exist
There was a problem hiding this comment.
The persistence needs to get rid of it as well, and instead a migration should be added to migrate the old schema to the new
There was a problem hiding this comment.
This is already fixed in 602a23c, it was pushed after this review, right before the "mostafa requested a review from oSuMATrix, "store sources as plain URLs and migrate old rows on boot".
The RoomSource sealed class is gone now, we just store a plain Uri in the entities and the converter is simply Uri.parse() / toString().
Old rows get migrated when the app starts in SourceManager.loadFromDb(): uid 0 gets updated to the configured default URL and old local rows without a scheme get rewritten to file:// URLs that point to their private copy.
The migration is safe to run more than once and the schema identity hash didnt change because the only thing that changed was the converter.
| private fun Throwable.toValidationMessage() = when (asSourceException()) { | ||
| // wtf is this? this data is not a bundle, at least something! | ||
| is UnsupportedRemoteSourceException -> app.getString(R.string.remote_source_url_unsupported) | ||
| is UnsupportedSourceException -> app.getString(R.string.remote_source_url_unsupported) |
There was a problem hiding this comment.
References to remote still exist.
There was a problem hiding this comment.
Fixed.
validateUrl doesn't go through HttpService anymore, It now resolves the protocol through the handler mapjust like everything else so SourceManager no longer has any dependency on HTTP.
I also renamed the string resources from remote_source_url_* to source_url_*, and setEndpoint is now to setUrl.
The only place that still uses the old local/remote naming is ImportSourceDialog that's getting replaced by the Figma redesign anyway so i didn't think it was worth renaming a bunch of translated strings right before removing them. I can clean it up anyway, but are you truely sure you'd rather have it cleaned up?
2 participants
No description provided.