-
Notifications
You must be signed in to change notification settings - Fork 26
feat: renable orbot and use status #678
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
Conversation
- imported strong ok http client from netcipher due to R.class conflicts with dependencies - Refactored SaveClient to set/unset orbot proxy - added dependency injection for saveclient and tor status repository - update general settings to include tor connection status with some fakery for user experience when orbot is flaky - refactored GDrive implementation to use basic API with SaveClient - added a broadcast reciever for Orbot status, updates the tor repository
Conflicts: app/build.gradle app/src/main/java/net/opendasharchive/openarchive/SaveApp.kt app/src/main/java/net/opendasharchive/openarchive/core/di/CoreModule.kt app/src/main/java/net/opendasharchive/openarchive/features/media/ReviewActivity.kt app/src/main/java/net/opendasharchive/openarchive/features/settings/GeneralSettingsActivity.kt app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt app/src/main/java/net/opendasharchive/openarchive/services/webdav/WebDavConduit.kt app/src/main/java/net/opendasharchive/openarchive/upload/UploadService.kt app/src/main/java/net/opendasharchive/openarchive/util/Prefs.kt app/src/main/res/layout/activity_main.xml app/src/main/res/xml/prefs_general.xml build.gradle gradle/wrapper/gradle-wrapper.properties settings.gradle
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.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
@ryjen we went through the code. Thank You. Can you please remove any untested / incomplete / optimisations that doesn't directly impact tor working as next branch is expected to have only stable and tested code. You can park the changes in a different branch and raise the PR for the ones that are stable for the moment. |
Hi @ryjen , Hope you’re doing well. I checked out your
I even tried marking the release buildType as debuggable (isDebuggable = true), but it still fails the same way. It seems related to NetCipher’s receiver setup—I found a GitLab issue from last year on the same Save app here: Could you share how you built and ran your changes in debug mode? Any details on your setup or tweaks you used would be really helpful. Thanks a lot for your time! |
avoid reciever, use callback
running is slow for debugging
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.
Pull Request Overview
This PR refactors the network client to support dependency injection and Tor proxying, re-enables Orbot integration, and surfaces Tor status in the settings UI.
- Introduces a
SaveClient
as an injectableCall.Factory
using NetCipher for Tor support - Adds DI modules for Tor (
torModule
) and registersSaveClient
inservicesModule
- Updates the settings screen to show Tor connection status and an “Open Orbot” preference
- Simplifies
IaConduit
and other services to use the injectedSaveClient
- Imports a local copy of
StrongOkHttpClientBuilder
to avoid library conflicts
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
app/src/main/java/net/opendasharchive/openarchive/services/tor/Module.kt | Adds Koin module for Tor repository and view model |
app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt | Refactors uploads to use injected SaveClient |
app/src/main/java/net/opendasharchive/openarchive/services/SaveClient.kt | Rewrites network client to implement Call.Factory and Tor logic |
app/src/main/java/net/opendasharchive/openarchive/services/Module.kt | Registers SaveClient and includes torModule |
app/src/main/java/net/opendasharchive/openarchive/features/settings/SettingsFragment.kt | Displays Tor status and adds “Open Orbot” preference |
app/src/main/java/net/opendasharchive/openarchive/features/settings/OpenOrbotPreference.java | New preference to launch Orbot |
app/src/main/java/net/opendasharchive/openarchive/SaveApp.kt | Always initializes NetCipher client and requests Tor start |
app/build.gradle.kts | Updates signing config and comments out Tor libs |
app/src/main/java/info/guardianproject/netcipher/client/StrongOkHttpClientBuilder.java | Adds local copy of NetCipher builder to avoid resource conflicts |
Comments suppressed due to low confidence (1)
app/src/main/java/net/opendasharchive/openarchive/features/media/ReviewActivity.kt:336
- After building the Picasso request,
.into(imageView)
is missing, so the video thumbnail won’t be rendered. Add.into(imageView)
at the end of the chain.
.load(VideoRequestHandler.SCHEME_VIDEO + ":" + media.originalFilePath)
if (!status.hasExtra(OrbotHelper.EXTRA_STATUS)) { | ||
status.putExtra(OrbotHelper.EXTRA_STATUS, OrbotHelper.STATUS_OFF) | ||
} | ||
override fun onEnabled(statusIntent: Intent?) { |
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.
The result of strongBuilder.applyTo(okBuilder, statusIntent) is not assigned back to okBuilder, so proxy settings will not be applied. Consider reassigning: okBuilder = strongBuilder.applyTo(okBuilder, statusIntent)
.
Copilot uses AI. Check for mistakes.
@@ -84,7 +84,7 @@ android { | |||
signingConfigs { | |||
getByName("debug") { | |||
val props = loadLocalProperties() | |||
storeFile = file(props["STOREFILE"] as? String ?: "") | |||
storeFile = props["STOREFILE"]?.let { file(it) } |
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.
[nitpick] If STOREFILE
is missing, storeFile
will be left null and could break the debug signing config. Consider providing a fallback file or failing early with a clear error.
storeFile = props["STOREFILE"]?.let { file(it) } | |
val storeFilePath = props["STOREFILE"] ?: throw IllegalArgumentException("STOREFILE is missing in local.properties or environment variables.") | |
storeFile = file(storeFilePath) |
Copilot uses AI. Check for mistakes.
@@ -80,14 +80,14 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { | |||
// Ignored. Not used here. | |||
} | |||
|
|||
private suspend fun OkHttpClient.uploadContent(fileName: String, mimeType: String) { | |||
private fun SaveClient.uploadContent(fileName: String, mimeType: String) { |
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.
This method performs network I/O but isn’t marked suspend
or dispatched on an IO thread. Consider making it suspend
or wrapping its body in an IO dispatcher to avoid blocking.
Copilot uses AI. Check for mistakes.
NOTE: you can ignore the extra GDrive files, I experimented using the direct API so that SaveClient with a Tor proxy could better be used. It is untested and probably incomplete. If you would prefer i exclude let me know.