Croway commented on PR #29286:
URL: https://github.com/apache/camel/pull/29286#issuecomment-6682964086
@davsclaus thanks for the review. I pushed three commits that handle the
points below.
**1. The `readLock` enum lists only `none`, `rename` and `changed`**
(4da66546099b)
This works with the inherited `@UriParam`: the subclass declares the option
again. `EndpointSchemaGeneratorMojo.findClassProperties` reads the declared
fields of the endpoint class first, then those of each superclass
(`EndpointSchemaGeneratorMojo.java:1311` and `:1422`). It adds an endpoint
option only when the model has no option of that name yet (`:1607`), so the
subclass's declaration wins. `@UriParam` can only go on a field
(`UriParam.java:31`), so the option has to be declared as a field. Camel
already does this for the `delay` option of `ScheduledPollEndpoint`:
`MailEndpoint` (CAMEL-8933), `SlackEndpoint` (CAMEL-18177),
`AbstractTwitterEndpoint` and `SnmpEndpoint` each declare a private field with
their own `@UriParam`, plus a setter override that calls `super`.
- `RemoteFileEndpoint` (ftp, ftps, sftp, mina-sftp, azure-files) and
`SmbEndpoint` declare `readLock` again with `enums = VALID_READ_LOCKS`. That is
the `"none,rename,changed"` constant the error message also uses. The
description covers only these three read locks. `setReadLock` calls
`super.setReadLock`, so the process strategy factories still read the value
from `GenericFileEndpoint`. The generated configurers and URI factories do not
change. scp is producer-only and does not list the option.
- `false` and `off` are not in the enum. file and smb list exactly the
values their check accepts, and neither of them has these aliases. On the
remote endpoints `false` and `off` are still accepted, only so that existing
routes keep working. The catalog validation reported them as invalid before
this PR too, because they were not among the 8 values.
- Regenerated: the six component json files, their catalog copies, and the
six Endpoint DSL factories (only the `readLock` javadoc changes).
- Tests: added `readLockListsOnlyTheSupportedReadLocks` to
`RemoteFileEndpointReadLockTest` (ftp, ftps, sftp),
`MinaSftpEndpointReadLockTest` and `FilesEndpointReadLockTest`, plus a new
`SmbEndpointReadLockTest`. They read the json through
`CatalogCamelContext.getComponentParameterJsonSchema`, the same way
`TwitterTimelineEndpointTest` does. Before the change all 6 fail with
`expected: <[none, rename, changed]> but was: <[none, markerFile, fileLock,
rename, changed, idempotent, idempotent-changed, idempotent-rename]>`. After
the change they pass.
- Catalog validation, checked with `validateEndpointProperties`: on the
rebuilt catalog, `readLock=markerFile` (ftp, smb), `idempotent` (sftp),
`fileLock` (mina-sftp) and `idempotent-changed` (azure-files) are now invalid
enum values. With the catalog from before this change they passed validation.
`file:inbox?readLock=markerFile` still passes.
- Upgrade guide: one more paragraph says what the docs, the catalog and the
Endpoint DSL now list, and what the validation now reports.
**2. [Question] Endpoint DSL methods are removed**: no change. These options
never did anything on these endpoints, so deprecating them first would only
keep builder methods that have no effect for one more release. The upgrade
guide lists the removed options per component and says to remove the calls.
Endpoint URIs that set them still resolve.
**3. [Nit] smb refuses `false` and `off`**: agreed, a later tidy-up. This PR
keeps the smb runtime behaviour as it is. For smb it only narrows the enum
metadata to what smb accepts, and `readLockCheck` now uses the same constant as
the enum.
**4. [Nit] `Set.of` constant** (2fef591ccf74): `RemoteFileEndpoint` now
checks the value against `Set.of("none", "rename", "changed", "false", "off")`.
The String constant is kept for the error message and the enum.
**5. [Nit] Upgrade-guide heading** (99e2f10b8ff0): the heading now lists the
artifacts `camel-ftp, camel-mina-sftp, camel-azure-files, camel-smb`, and the
text says that `ftp`, `ftps` and `sftp` are all in `camel-ftp`.
The unit and integration tests of camel-ftp, camel-mina-sftp,
camel-azure-files, camel-smb and camel-jsch pass.
_Claude Code on behalf of Federico Mariani_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
