-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Make FTP server a separate module #4602
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
Open
TranceLove
wants to merge
15
commits into
TeamAmaze:release/4.0
Choose a base branch
from
TranceLove:feature/ftpserver-module
base: release/4.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,035
−1,878
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
bac9274
FTP server minor improvements
TranceLove 4182891
Migrate ftpserver to separate module
TranceLove 5e23bf0
Move tests to ftpserver module and deprecate classes in app module
TranceLove 82380f2
More moves
TranceLove 8c74c63
Changes per PR feedback and fix Codacy errors
TranceLove 4611cae
Remove redundant classes after the move
TranceLove 6a3ad95
Fix cross-test contamination causing FilesOnSshdTest regression
TranceLove d3ec5ca
Changes per PR feedback (again)
TranceLove 6310285
Wire FtpServerProvider into ServerRegistry
TranceLove f2294e0
Migrate from FtpNotification to FtpServerNotification.kt
TranceLove e5037c9
Add battery optimization prompt
TranceLove 3893a89
Changes per PR feedback
TranceLove d37c452
Consolidate FTP fragment ownership in app
TranceLove bda8869
Changes per PR feedback
TranceLove d4e5527
Made FtpCommandMessageProvider an individual interface
TranceLove File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,252 @@ | ||
| ## Plan: Modularize FTP Server Functionality for Pluggable Server Implementations | ||
|
|
||
| The goal is to extract FTP server functionality from the `app` module into a dedicated module, enabling future pluggable implementations (SSH server, WebDAV server, etc.). Currently, FTP-related code is tightly coupled with the app module, including UI (`FtpServerFragment`), services (`FtpService`, `FtpReceiver`, `FtpTileService`), notifications (`FtpNotification`), filesystem factories, and resources. | ||
|
|
||
| ### Implementation Progress | ||
|
|
||
| #### ✅ Completed Steps | ||
|
|
||
| 1. **Define a server abstraction layer in a new core module** — Created `server-core` module with: | ||
| - `FileServer.kt` - Base interface for server implementations | ||
| - `ServerPreferences.kt` - Interface for server preferences management | ||
| - `ServerNotification.kt` - Interface for notification handling | ||
| - `ServerEvent.kt` - Sealed class for server state events | ||
| - `ServerRegistry.kt` - Registry for discovering and managing server providers | ||
|
|
||
| 2. **Restructure the existing `ftpserver` module** — Updated module to depend on `server-core` instead of `app`. Created: | ||
| - `service/FtpServerEngine.kt` - Core FTP server lifecycle management | ||
| - `service/FtpServerService.kt` - Abstract Android Service for FTP | ||
| - `service/FtpReceiver.kt` - Abstract BroadcastReceiver for FTP start/stop | ||
| - `service/FtpEventBus.kt` - Event bus using Kotlin Flow | ||
| - `service/FtpPreferences.kt` - FTP-specific preferences | ||
| - `service/FtpCipherSuites.kt` - SSL cipher configuration | ||
| - `service/FtpCommandFactoryFactory.kt` - Custom command factory | ||
|
|
||
| 3. **Move FTP filesystem factories to the ftpserver module** — Created in `ftpserver/filesystem/`: | ||
| - `AndroidFileSystemFactory.kt` | ||
| - `AndroidFtpFileSystemView.kt` | ||
| - `AndroidFtpFile.kt` | ||
| - `RootFileSystemFactory.kt` | ||
| - `RootFileSystemView.kt` | ||
| - `RootFtpFile.kt` | ||
| - `commands/AVBL.kt`, `FEAT.kt`, `PWD.kt` | ||
|
|
||
| 4. **Created FtpServerProvider** — Implementation of `ServerProvider` interface in `FtpServerProvider.kt` | ||
|
|
||
| 5. **Extract FTP UI components to the ftpserver module** — ✅ Completed: | ||
| - `ui/BaseFtpServerFragment.kt` - Abstract base fragment for FTP UI | ||
| - `ui/FtpServerNotification.kt` - Notification handler implementation | ||
| - `res/layout/fragment_ftp.xml` - FTP fragment layout | ||
| - `res/menu/ftp_server_menu.xml` - Menu resources | ||
| - `res/values/strings.xml` - FTP strings (with `ftpmod_` prefix to avoid collisions) | ||
| - `res/values/colors.xml` - Color resources | ||
| - `res/drawable/` - Icon drawables | ||
|
|
||
| 6. **Update AndroidManifest and app module integration** — ✅ Partially completed: | ||
| - Added abstract FtpServerService and FtpReceiver to ftpserver manifest | ||
| - App module still uses its own FtpService, FtpReceiver implementations (coexisting during transition) | ||
|
|
||
| #### 🔄 Remaining Steps | ||
|
|
||
| 7. **Migration of app module FTP code** — ✅ Completed: | ||
| - Created `AppFtpService.kt` - Concrete implementation extending `FtpServerService` | ||
| - Created `AppFtpReceiver.kt` - Concrete implementation extending `FtpReceiver` | ||
| - Updated `FtpServerFragment.kt` to use ftpserver module classes (`FtpPreferences`, `FtpServerEngine`, `FtpServerEvent`, `FtpEventBus`) | ||
| - Updated `FtpTileService.kt` to use ftpserver module classes | ||
| - Updated `FtpNotification.java` to use ftpserver module classes | ||
| - Updated `AndroidManifest.xml` to register `AppFtpService` and `AppFtpReceiver` | ||
| - Added `@JvmStatic` annotations to `FtpPreferences` for Java interop | ||
| - Old `FtpService.kt` and `FtpReceiver.kt` are now deprecated (can be removed in future) | ||
|
|
||
| 8. **Move FTP tests to the ftpserver module** — ✅ Completed: | ||
| - Created `commands/LogMessageFilter.kt` - Test utility for capturing FTP responses | ||
| - Created `commands/AbstractFtpserverCommandTest.kt` - Base test class (plain JUnit) | ||
| - Created `commands/AVBLCommandTest.kt` - 8 tests for AVBL command | ||
| - Created `commands/PWDCommandTest.kt` - 3 tests for PWD command | ||
| - Created `commands/FEATCommandTest.kt` - 1 test for FEAT command | ||
| - Total: 12 tests, all passing | ||
| - Uses mixed mocking approach: MockK for most mocks, Mockito for `java.io.File` (better final class support) | ||
|
|
||
| ### Module Structure Created | ||
|
|
||
| ``` | ||
| server-core/ | ||
| ├── build.gradle | ||
| ├── src/main/ | ||
| │ ├── AndroidManifest.xml | ||
| │ └── java/com/amaze/filemanager/server/ | ||
| │ ├── FileServer.kt | ||
| │ ├── ServerEvent.kt | ||
| │ ├── ServerNotification.kt | ||
| │ ├── ServerPreferences.kt | ||
| │ └── ServerRegistry.kt | ||
|
|
||
| ftpserver/ | ||
|
TranceLove marked this conversation as resolved.
|
||
| ├── build.gradle | ||
| ├── src/main/ | ||
| │ ├── AndroidManifest.xml | ||
| │ ├── java/com/amaze/filemanager/ftpserver/ | ||
| │ │ ├── FtpServerProvider.kt | ||
| │ │ ├── commands/ | ||
| │ │ │ ├── AVBL.kt | ||
| │ │ │ ├── FEAT.kt | ||
| │ │ │ └── PWD.kt | ||
| │ │ ├── filesystem/ | ||
| │ │ │ ├── AndroidFileSystemFactory.kt | ||
| │ │ │ ├── AndroidFtpFile.kt | ||
| │ │ │ ├── AndroidFtpFileSystemView.kt | ||
| │ │ │ ├── RootFileSystemFactory.kt | ||
| │ │ │ ├── RootFileSystemView.kt | ||
| │ │ │ └── RootFtpFile.kt | ||
| │ │ ├── service/ | ||
| │ │ │ ├── FtpCipherSuites.kt | ||
| │ │ │ ├── FtpCommandFactoryFactory.kt | ||
| │ │ │ ├── FtpEventBus.kt | ||
| │ │ │ ├── FtpPreferences.kt | ||
| │ │ │ ├── FtpReceiver.kt | ||
| │ │ │ ├── FtpServerEngine.kt | ||
| │ │ │ └── FtpServerService.kt | ||
| │ │ └── ui/ | ||
| │ │ ├── BaseFtpServerFragment.kt | ||
| │ │ └── FtpServerNotification.kt | ||
| │ └── res/ | ||
| │ ├── drawable/ | ||
| │ │ ├── ic_clear_all.xml | ||
| │ │ ├── ic_eye_grey600_24dp.xml | ||
| │ │ ├── ic_ftp_dark.xml | ||
| │ │ └── ic_ftp_light.xml | ||
| │ ├── layout/ | ||
| │ │ └── fragment_ftp.xml | ||
| │ ├── menu/ | ||
| │ │ └── ftp_server_menu.xml | ||
| │ └── values/ | ||
| │ ├── colors.xml | ||
| │ └── strings.xml | ||
| ├── src/test/ | ||
| │ ├── java/com/amaze/filemanager/ftpserver/commands/ | ||
| │ │ ├── AbstractFtpserverCommandTest.kt | ||
| │ │ ├── AVBLCommandTest.kt | ||
| │ │ ├── FEATCommandTest.kt | ||
| │ │ ├── LogMessageFilter.kt | ||
| │ │ └── PWDCommandTest.kt | ||
| │ └── resources/mockito-extensions/ | ||
| │ └── org.mockito.plugins.MockMaker | ||
| ``` | ||
|
|
||
| ### FTP Server Class Diagram | ||
|
|
||
| ```mermaid | ||
| classDiagram | ||
| direction LR | ||
|
|
||
| namespace server_core { | ||
| class FileServer { | ||
| <<interface>> | ||
| } | ||
| class ServerPreferences { | ||
| <<interface>> | ||
| } | ||
| class ServerNotification { | ||
| <<interface>> | ||
| } | ||
| class ServerEvent { | ||
| <<sealed>> | ||
| } | ||
| class ServerRegistry | ||
| class ServerProvider { | ||
| <<interface>> | ||
| } | ||
| } | ||
|
|
||
| namespace ftpserver_service { | ||
| class FtpServerProvider | ||
| class FtpServerEngine | ||
| class FtpServerService { | ||
| <<abstract>> | ||
| } | ||
| class FtpReceiver { | ||
| <<abstract>> | ||
| } | ||
| class FtpEventBus | ||
| class FtpPreferences | ||
| class FtpCommandFactoryFactory | ||
| class FtpCipherSuites | ||
| } | ||
|
|
||
| namespace ftpserver_ui { | ||
| class BaseFtpServerFragment { | ||
| <<abstract>> | ||
| } | ||
| class FtpServerNotification | ||
| } | ||
|
|
||
| namespace ftpserver_filesystem { | ||
| class AndroidFileSystemFactory | ||
| class AndroidFtpFileSystemView | ||
| class AndroidFtpFile | ||
| class RootFileSystemFactory | ||
| class RootFileSystemView | ||
| class RootFtpFile | ||
| } | ||
|
|
||
| namespace ftpserver_commands { | ||
| class AVBL | ||
| class FEAT | ||
| class PWD | ||
| } | ||
|
|
||
| namespace app_integration { | ||
| class AppFtpService | ||
| class AppFtpReceiver | ||
| class FtpServerFragment | ||
| class FtpTileService | ||
| class FtpNotification | ||
| } | ||
|
|
||
| FtpServerProvider ..|> ServerProvider | ||
| FtpServerProvider ..> ServerPreferences | ||
| FtpServerProvider ..> FileServer | ||
| ServerRegistry --> ServerProvider : registers | ||
|
|
||
| FtpServerService --> FtpServerEngine : owns | ||
| FtpServerService --> FtpEventBus : publishes | ||
| FtpReceiver --> FtpServerEngine : start/stop | ||
| FtpServerEngine ..> FtpPreferences : reads config | ||
| FtpServerEngine ..> FtpCommandFactoryFactory : command factory | ||
| FtpCommandFactoryFactory ..> AVBL | ||
| FtpCommandFactoryFactory ..> FEAT | ||
| FtpCommandFactoryFactory ..> PWD | ||
| FtpServerEngine ..> FtpCipherSuites | ||
|
|
||
| FtpServerEngine ..> AndroidFileSystemFactory | ||
| FtpServerEngine ..> RootFileSystemFactory | ||
| AndroidFileSystemFactory --> AndroidFtpFileSystemView | ||
| AndroidFtpFileSystemView --> AndroidFtpFile | ||
| RootFileSystemFactory --> RootFileSystemView | ||
| RootFileSystemView --> RootFtpFile | ||
|
|
||
| FtpServerNotification ..|> ServerNotification | ||
| BaseFtpServerFragment --> FtpServerNotification | ||
| BaseFtpServerFragment ..> FtpEventBus : observes | ||
| BaseFtpServerFragment ..> FtpPreferences | ||
| BaseFtpServerFragment ..> FtpServerEngine | ||
| BaseFtpServerFragment ..> ServerEvent | ||
|
|
||
| AppFtpService --|> FtpServerService | ||
| AppFtpReceiver --|> FtpReceiver | ||
| FtpServerFragment --|> BaseFtpServerFragment | ||
| FtpTileService ..> FtpServerEngine | ||
| FtpTileService ..> FtpEventBus | ||
| FtpNotification ..> FtpServerNotification | ||
| ``` | ||
|
|
||
| ### Further Considerations | ||
|
|
||
| 1. **Dependency inversion** — ✅ Done: `ftpserver` now depends on `server-core`, and `app` depends on both. | ||
|
|
||
| 2. **Feature module vs library module** — Implemented as library module (simpler approach). | ||
|
|
||
| 3. **Preference storage** — ✅ Done: `ServerPreferences` interface created in server-core, implemented in `FtpServerProvider`. | ||
|
|
||
| 4. **String resources** — Used `ftpmod_` prefix for all ftpserver module strings to avoid collision with app module's existing strings during transition period. | ||
|
|
||
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
Oops, something went wrong.
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.
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 should be in some llm specific folder somewhere. See https://github.com/EmmanuelMess/Multistep-DEVS-Simulator
.claudeandspec filesfolders for ideas.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.
Also its current structure might not be useful, useful information would be: