Make FTP server a separate module#4602
Draft
TranceLove wants to merge 12 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modularizes the FTP server feature by extracting it from the app module into a dedicated ftpserver library, coupled through a new server-core abstraction module to support future pluggable server implementations.
Changes:
- Added new Gradle modules:
server-core(server abstractions/registry) andftpserver(FTP implementation: engine, service/receiver base classes, filesystem adapters, UI, resources). - Updated
appintegration to use the new FTP module (newAppFtpService/AppFtpReceiver, updated fragment/tile/notification to referenceFtpPreferences/FtpServerEngine). - Added unit tests for FTP custom commands (AVBL/FEAT/PWD) and updated Kotlin stdlib version.
Reviewed changes
Copilot reviewed 62 out of 63 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle | Includes the new server-core and ftpserver modules. |
| server-core/** | Introduces server abstractions (types, preferences/notification interfaces, registry, events) and module Gradle config. |
| ftpserver/** | New FTP module: engine/service base, filesystem implementations, UI/resources, and unit tests. |
| app/** | Wires the app to the new FTP module (service/receiver, fragment, tile, notification) and adds a WebDAV certificate callable/test. |
| gradle/libs.versions.toml | Updates Kotlin stdlib version used by the project. |
| file_operations/src/test/** | Adds/updates a (currently empty/commented) test helper class. |
| app/play/release/output-metadata.json | Adds a generated build artifact (should likely not be committed). |
| plan-modularizeFtpServer.prompt.md | Planning documentation for the modularization work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TranceLove
force-pushed
the
feature/ftpserver-module
branch
9 times, most recently
from
640126d to
502d11a
Compare
TranceLove
force-pushed
the
feature/ftpserver-module
branch
from
a60a1de to
94a6a98
Compare
Contributor
|
Verified in Android 16 with daily FTP usage. |
EmmanuelMess
requested changes
Apr 18, 2026
Member
EmmanuelMess
left a comment
EmmanuelMess
left a comment
There was a problem hiding this comment.
I still need to check some files and all the tests.
Also, I am not quite understanding the separation of concerns between app, ftpserver and server-core or if the server-core functionality is actually completely already in use.
| * | ||
| * Each server module should implement this to provide its components. | ||
| */ | ||
| interface ServerProvider { |
Member
There was a problem hiding this comment.
Very similar to FileServer (see comment there).
Member
There was a problem hiding this comment.
This should be in some llm specific folder somewhere. See https://github.com/EmmanuelMess/Multistep-DEVS-Simulator .claude and spec files folders for ideas.
Member
There was a problem hiding this comment.
Also its current structure might not be useful, useful information would be:
- restrictions applied to the llm in code creation
- specfic set of specifications passed, initially and during refinement. Including UI/UX design files and diagrams
- a review of code of llm, and changed that were manually applied later
| │ ├── ServerPreferences.kt | ||
| │ └── ServerRegistry.kt | ||
|
|
||
| ftpserver/ |
Member
There was a problem hiding this comment.
If possible this is better as a UML diagram of classes and such, not a semi formatted list. See https://github.com/EmmanuelMess/Multistep-DEVS-Simulator/blob/master/design/class%20diagrams/class%20diagram.svg for an example.
EmmanuelMess
added
the
PR-Requested-Changes
TranceLove
force-pushed
the
feature/ftpserver-module
branch
from
94a6a98 to
b22a5e2
Compare
- Replace EventBus with kotlinx.coroutines - FtpServerFragment update code to recommended
This enables future integration of other file services more easier.
SshAuthenticationTaskTest.prepareSshConnectionPool() was storing a raw SSHClient Mockito mock directly into NetCopyClientConnectionPool.connections without wrapping it in SSHClientImpl. This caused a ClassCastException when shutdown() was called in AbstractSftpServerTest.tearDown(), since the lambda tried to cast the raw SSHClient (which is not a NetCopyClient) to NetCopyClient. The unhandled exception from the fire-and-forget runInBackground call was then rethrown as OnErrorNotImplementedException, leaving connections un-cleared. On the next test's setUp() call, the stale connection from SshAuthenticationTaskTest was still present in the pool. When validate() found it invalid and tried to reconnect via createSshClient(url), it required a DB lookup that found no matching entry (because the SshAuthenticationTaskTest connection used a different URL), causing FilesOnSshdTest.testListFilesWithSpecialChars to time out waiting for SFTP results. Fixes: - Wrap mock SSHClient in SSHClientImpl in prepareSshConnectionPool() so the connections map always holds proper NetCopyClient instances - Add tearDown() to SshAuthenticationTaskTest to reset sshClientFactory and clear the connections map after each test, preventing state leakage - Add sshClientFactory reset in AbstractSftpServerTest.setUp() as an additional safeguard against contamination from any preceding test class - Minor: refactor FilesOnSshdTest to use SAM lambda syntax for OnFileFound callbacks; extend atMost timeout to 120 seconds for testListFilesWithSpecialChars
To complete the registry base approach
To prompt users to disable battery optimization for better network I/O performance when running FTP server. Fixes TeamAmaze#4603
TranceLove
force-pushed
the
feature/ftpserver-module
branch
from
b22a5e2 to
9fa4a9a
Compare
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
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.
Description
Make FTP server a separate module, coupled via another server-core module, for facilitating integrations in the future.
Issue tracker
Automatic tests
Manual tests
Build tasks success
Successfully running following tasks on local:
./gradlew assembledebug./gradlew spotlessCheck