Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR replaces the previous in-app document scanning implementation (DocumentScan/AppScan) with the FairScan library, removing the legacy scanning UI/worker pipeline and wiring scanning via FairScan intents. It also adds CI plumbing to build FairScan from source and publish it to mavenLocal() until an upstream artifact is available.
Changes:
- Swap
:appscan/ DocumentScan integration for FairScan dependency resolution (currently viamavenLocal()+ composite action). - Remove legacy DocumentScan activities, ViewModel, workers, DI wiring, and related resources/strings.
- Harden URI “sensitive source” detection and add unit tests to cover FairScan-embedded authorities.
| File | Description |
|---|---|
| settings.gradle.kts | Adds mavenLocal() (group-filtered) for FairScan and removes :appscan from the build. |
| gradle/libs.versions.toml | Drops DocumentScanner coordinates; introduces FairScan version/catalog entry. |
| gradle.properties | Disables AGP unique manifest package enforcement to work around LiteRT packaging conflict. |
| appscan/src/main/java/com/nextcloud/appscan/ScanPageContract.kt | Removes AppScan contract (module elimination). |
| appscan/src/main/java/com/nextcloud/appscan/AppScanActivity.kt | Removes AppScan activity (module elimination). |
| appscan/src/main/AndroidManifest.xml | Removes appscan manifest (module elimination). |
| appscan/build.gradle.kts | Removes appscan module Gradle build file. |
| app/src/test/java/com/owncloud/android/ui/helpers/UriUploaderTest.kt | Adds unit tests for updated sensitive-URI logic (including FairScan embedded authority). |
| app/src/test/java/com/nextcloud/client/jobs/BackgroundJobFactoryTest.kt | Updates factory test after removing PDF generation use case. |
| app/src/main/res/values/strings.xml | Removes strings only used by the legacy document scan UI/export flow. |
| app/src/main/res/menu/activity_document_scan.xml | Removes legacy document scan menu. |
| app/src/main/res/layout/document_page_item.xml | Removes legacy scan page item layout. |
| app/src/main/res/layout/dialog_scan_export_type.xml | Removes legacy export type dialog layout. |
| app/src/main/res/layout/activity_document_scan.xml | Removes legacy document scan activity layout. |
| app/src/main/java/com/owncloud/android/ui/helpers/UriUploader.kt | Replaces substring check with authority-based checks for content:// URIs; exposes for testing. |
| app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java | Replaces internal DocumentScan launch with FairScan intent fallback logic. |
| app/src/main/java/com/owncloud/android/ui/fragment/OCFileListBottomSheetDialog.kt | Simplifies scan action wiring to always use “scan from app” path. |
| app/src/main/java/com/owncloud/android/ui/fragment/OCFileListBottomSheetActions.kt | Removes legacy scan methods and availability flag; updates docstring. |
| app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt | Removes PDF-generation background work API/implementation. |
| app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManager.kt | Removes startPdfGenerateAndUploadWork from the interface. |
| app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt | Removes PDF generation worker creation path and dependency. |
| app/src/main/java/com/nextcloud/client/documentscan/GeneratePdfFromImagesWork.kt | Removes legacy WorkManager PDF generation worker. |
| app/src/main/java/com/nextcloud/client/documentscan/GeneratePDFUseCase.kt | Removes legacy “images to PDF” use case. |
| app/src/main/java/com/nextcloud/client/documentscan/DocumentScanViewModel.kt | Removes legacy scan/export UI state machine. |
| app/src/main/java/com/nextcloud/client/documentscan/DocumentScanActivity.kt | Removes legacy document scan activity. |
| app/src/main/java/com/nextcloud/client/documentscan/DocumentPageListAdapter.kt | Removes legacy scan page list adapter. |
| app/src/main/java/com/nextcloud/client/documentscan/AppScanOptionalFeature.kt | Removes optional-feature wrapper (no longer needed). |
| app/src/main/java/com/nextcloud/client/di/ViewModelModule.kt | Removes DocumentScanViewModel binding. |
| app/src/main/java/com/nextcloud/client/di/VariantModule.kt | Removes reflection-based variant module for :appscan. |
| app/src/main/java/com/nextcloud/client/di/ComponentsModule.java | Removes Dagger injector for DocumentScanActivity. |
| app/src/main/java/com/nextcloud/client/di/AppComponent.kt | Removes VariantModule from component modules list. |
| app/src/main/AndroidManifest.xml | Removes DocumentScanActivity declaration; adjusts permission merge behavior. |
| app/src/androidTestGplay/java/com/nextcloud/client/di/VariantModuleTest.kt | Removes tests covering now-deleted VariantModule/AppScanOptionalFeature behavior. |
| app/src/androidTestGeneric/java/com/nextcloud/client/di/VariantModuleTest.kt | Removes tests covering now-deleted VariantModule/AppScanOptionalFeature behavior. |
| app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.kt | Updates test wiring after removing scan actions/optional feature injection. |
| app/src/androidTest/java/com/nextcloud/client/documentscan/GeneratePDFUseCaseTest.kt | Removes tests for deleted PDF generation use case. |
| app/build.gradle.kts | Replaces variant-scoped :appscan dependency with FairScan library dependency. |
| .github/workflows/unit-tests.yml | Publishes FairScan to mavenLocal() before running unit tests. |
| .github/workflows/qa.yml | Publishes FairScan to mavenLocal() before QA build. |
| .github/workflows/garm.yml | Publishes FairScan to mavenLocal() before emulator/KVM steps. |
| .github/workflows/detectWrongSettings.yml | Publishes FairScan to mavenLocal() before running “Detect SNAPSHOT”. |
| .github/workflows/check.yml | Publishes FairScan to mavenLocal() before running matrix checks. |
| .github/workflows/assembleFlavors.yml | Publishes FairScan to mavenLocal() before assembling flavors. |
| .github/workflows/analysis.yml | Publishes FairScan to mavenLocal() before analysis job steps. |
| .github/actions/publish-fairscan-to-maven-local/action.yml | Adds composite action to checkout FairScan and publishToMavenLocal for CI consumption. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| default: "phillab/FairScan" | ||
| ref: | ||
| description: "FairScan ref to build. See the 'repository' input comment: temporary." | ||
| required: false | ||
| default: "ph/offer_as_library" |
There was a problem hiding this comment.
thanks. As it is a draft, this is deliberate for now. Will be adjusted before merging.
e5b2df2 to
bb1aac9
Compare
📱 QA build
The QA build installs alongside a released Nextcloud app, so you can keep Downloading the file requires a GitHub account, so open this link on the |
| return uploadPath + displayName | ||
| } | ||
|
|
||
| private fun isSensitiveUri(uri: Uri): Boolean = uri.toString().contains(activity.packageName) |
There was a problem hiding this comment.
TODO: This change actually points towards a bad data flow. I must rather revert it and solve the file handover differently
Dead code once nothing calls startPdfGenerateAndUploadWork(): this job only existed to combine DocumentScanActivity's scanned pages into one PDF for upload, and that flow is being replaced by an embedded FairScan, which produces the finished PDF itself. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
DocumentScanActivity's own page-collection UI (backed by the Hazzatur Document-Scanning-Android-SDK via AppScanOptionalFeature) is being replaced by an embedded FairScan, which handles its own multi-page capture and PDF export. Removing it here together with its only caller keeps the tree buildable; the cross-app "launch FairScan if installed separately" fallback in OCFileListBottomSheetDialog is now the sole scan path until the embedded replacement lands. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
Nothing references AppScanOptionalFeature/VariantModule's reflection lookup of com.nextcloud.appscan.ScanPageContract anymore, so the Hazzatur Document-Scanning-Android-SDK dependency and the :appscan module it lived in go too. The JitPack repo in settings.gradle.kts stays - other dependencies still resolve through it. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
FairScan will replace the old scanner component with state-of-the-art CV and UI. This is WIP, because FairScan is (not yet) available on a remote registry. So it is only included via local maven, but nothing actually ensures that the build machine actually has FairScan build locally, via ./gradlew publishToMavenLocal Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
This is WIP, because in the final integration, FairScan should be available on a remote package registry and doesn't need to be build locally. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
Ran `./gradlew --write-verification-metadata pgp,sha256 --export-keys help` as per https://developer.android.com/build/dependency-verification#bootstrap_the_checksum_and_signature_data
Ran `./gradlew --write-verification-metadata sha256 -Dorg.gradle.dependency.verification=lenient help && ./gradlew --export-keys`
…talled A separately installed FairScan wins over the built-in scanner, so users can choose to install (a newer version of) FairScan themselves. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
Route both the external-app and embedded FairScan scan results through a single field-level ActivityResultLauncher. The upload uses FileUploadHelper directly, instead of routing back to FileDisplayActivity's requestUploadOfContentFromApps() because: 1. The suggested new pattern is ActivityResultLauncher, which didn't go well with the intent-based pattern of FileDisplayActivity. As a reference, AlbumItemsFragment also does its own upload. 2. FileDisplayActivity uses UriUploader which explicitly prevents local files from being uploaded, cf. isSensitiveUri(). But in case of the document scanning, the file is created internally, in the /fairscan folder, and uploaded from there. Removed the now-unused REQUEST_CODE__SELECT_CONTENT_FROM_APPS_AUTO_RENAME, which was originally introduced for renaming the external FairScan file. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Philipp Hasper <vcs@hasper.info>
79e48fa to
9e710b0
Compare
|
stable22-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/17710-IT-stable22-17-46 |
|
stable35-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/17710-IT-stable35-17-46 |
|
master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/17710-IT-master-17-47 |


Important note: this is not yet working out-of-the-box, because FairScan isn't yet available as library.
Eventually, it will be available upstream, but for testing, you'll need to:
git clone https://github.com/phillab/fairscan -b ph/offer_as_librarycd fairscan && ./gradlew publishToMavenLocalOr you just download the QA apk from the comment further below.
TODO:
🖼️ Screenshots
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)