Conversation
niveathika
added
the
Skip GraalVM Check
There was a problem hiding this comment.
Pull request overview
This PR introduces a new onFileDelete method as a replacement for the deprecated onFileDeleted method in the FTP listener service. The key change is the method signature: onFileDelete is invoked once per deleted file with a single string parameter, while onFileDeleted receives all deleted files as a string[] array in a single invocation.
- Added new
onFileDeleteremote function that processes deletions one file at a time - Deprecated the existing
onFileDeletedmethod with appropriate compiler warnings - Updated task scheduling to use
task:Listenerinstead oftask:scheduleJobRecurByFrequency
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java | Added constant for the new ON_FILE_DELETE_REMOTE_FUNCTION |
| native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java | Updated method lookup to support both onFileDelete and onFileDeleted |
| native/src/main/java/io/ballerina/stdlib/ftp/server/FtpListener.java | Implemented processDeletionCallback to route to appropriate handler based on parameter type, added processFileDeleteCallback for per-file invocation |
| compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java | Added new error codes and messages for onFileDelete validation, updated existing error messages to reference new method |
| compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java | Added validation to prevent using both methods simultaneously and ensure at least one deletion handler exists |
| compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpFileDeletedValidator.java | Refactored to support validation of both method types with appropriate parameter checks |
| compiler-plugin-tests/src/test/resources/ballerina_sources/valid_content_service_7/* | Added test case for valid onFileDelete usage |
| compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_content_service_22-27/* | Added test cases for various onFileDelete validation scenarios |
| compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java | Added 6 new test methods to validate onFileDelete behavior and error handling |
| gradle.properties | Updated stdlibTaskVersion from 2.7.0 to 2.11.0 |
| docs/spec/spec.md | Updated specification to document onFileDelete instead of onFileDeleted |
| changelog.md | Added entry documenting the deprecation and new method |
| ballerina/tests/listener_on_file_deleted_test.bal | Updated tests to use onFileDelete with per-file invocation pattern |
| ballerina/listener_endpoint.bal | Refactored task scheduling to use task:Listener API instead of job-based scheduling |
| ballerina/client_endpoint.bal | Moved deprecation notices in documentation comments |
| ballerina/caller.bal | Moved deprecation notices in documentation comments |
| ballerina/Dependencies.toml | Updated dependencies to reflect new task library version and added test dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # + path - The resource path | ||
| # + content - Content to be written to the file in server | ||
| # + return - `()` or else an `ftp:Error` if failed to establish the communication with the FTP server | ||
| # # Deprecated: Use the format specific put methods(`putJson`, `putXml`, `putCsv`, `putBytes`, `putText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| # + path - The resource path | ||
| # + content - Content to be written to the file in server | ||
| # + return - `()` or else an `ftp:Error` if failed to establish the communication with the FTP server | ||
| # # Deprecated: Use the format specific put methods(`putJson`, `putXml`, `putCsv`, `putBytes`, `putText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| # ```ballerina | ||
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = client->get(path); | ||
| # ``` | ||
| # | ||
| # + path - The resource path | ||
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors | ||
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| # ```ballerina | |
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = client->get(path); | |
| # ``` | |
| # | |
| # + path - The resource path | |
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors | |
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. | |
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. | |
| # ```ballerina | |
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = client->get(path); | |
| # ``` | |
| # | |
| # + path - The resource path | |
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors |
| # + content - Content to be written to the file in server | ||
| # + compressionType - Type of the compression to be used if the file should be compressed before uploading | ||
| # + return - `()` or else an `ftp:Error` if failed to establish the communication with the FTP server | ||
| # # Deprecated: Use the format specific put methods(`putJson`, `putXml`, `putCsv`, `putBytes`, `putText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| # ```ballerina | ||
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = caller->get(path); | ||
| # ``` | ||
| # | ||
| # + path - The resource path | ||
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors | ||
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| # ```ballerina | |
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = caller->get(path); | |
| # ``` | |
| # | |
| # + path - The resource path | |
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors | |
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. | |
| # # Deprecated: Use the format specific get methods(`getJson`, `getXml`, `getCsv`, `getBytes`, `getText`) instead. | |
| # ```ballerina | |
| # stream<byte[] & readonly, io:Error?>|ftp:Error channel = caller->get(path); | |
| # ``` | |
| # | |
| # + path - The resource path | |
| # + return - A byte stream from which the file can be read or `ftp:Error` in case of errors |
| # + content - Content to be written to the file in server | ||
| # + compressionType - Type of the compression to be used if the file should be compressed before uploading | ||
| # + return - `()` or else an `ftp:Error` if failed to establish the communication with the FTP server | ||
| # # Deprecated: Use the format specific put methods(`putJson`, `putXml`, `putCsv`, `putBytes`, `putText`) instead. |
There was a problem hiding this comment.
The comment formatting for the deprecation notice is inconsistent with the rest of the documentation. The deprecation notice should appear before the code example, not after the return parameter description. This pattern is inconsistent with how deprecation is handled in other methods in the same file.
| - [Added missing Commons VFS configuration parameters](https://github.com/ballerina-platform/ballerina-library/issues/8369) | ||
| - Add functionality for streaming put methods in ftp:Client | ||
| - Add functionality for type based put and get in ftp:Caller | ||
| - Deprecate `onFileDeleted` method and introduced `onFileDelete` |
There was a problem hiding this comment.
Grammar issue in the changelog entry. The verb tense is inconsistent: "Deprecate" should be changed to "Deprecated" to match the past tense used in "introduced". The entry should read: "Deprecated onFileDeleted method and introduced onFileDelete"
| - Deprecate `onFileDeleted` method and introduced `onFileDelete` | |
| - Deprecated `onFileDeleted` method and introduced `onFileDelete` |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1518 +/- ##
============================================
+ Coverage 81.70% 81.83% +0.12%
- Complexity 588 792 +204
============================================
Files 40 55 +15
Lines 2892 3485 +593
Branches 468 595 +127
============================================
+ Hits 2363 2852 +489
- Misses 337 381 +44
- Partials 192 252 +60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
niveathika
force-pushed
the
cluster
branch
2 times, most recently
from
13ca981 to
9d70cb9
Compare
|
1 participant
Purpose
Examples
Checklist