Is your feature request related to a problem? Please describe.
AtCommandExecutorContext models a connection's identity — atSign, keys, config this overlaps with the requirements
for porting pluggable cyrptography. Currently this is only used by AuthenticationCommands and EnrollCommands.
Every other command re-declares atSign and keys as loose parameters, it would be better if there was consistency.
Also AtClientImpl holds its own copies of the same three values.
The same identity is therefore constructed twice per client from one source, AND the analysis for this issue revealed a (see below).
Bug
Every client built with withMonitoring(true) sends a from: command with no clientId and no client
version metadata, so monitoring clients are unidentifiable to the atServer.
AtClients:69 passes the raw config to Notifications.monitor.
AtCommandExecutors:76 builds the context with createClientConfig(config), which adds clientId
and everything in client-config.properties (:120-135).
AtCommandExecutors:85 — defaultIfNotSet(onReady, createOnReady(context)) — means an explicit
onReady replaces the context-based one, so sendFrom never runs and the challenge stays null.
Notifications.monitor therefore calls the 4-arg authenticateWithPkam
(AuthenticationCommands:92-98), which issues its own from: at :124 with the unenriched map.
Both paths still issue exactly one from: and one pkam: — this is a lost-metadata defect, not a
duplicate-authentication one. A single shared context removes the second path.
Describe the solution you'd like
Standardize on (AtCommandExecutor, AtCommandExecutorContext, …) as the first two arguments of every
command method, and create exactly one AtCommandExecutorContext per connection in
AtClients.createAtClient, passing it to both AtCommandExecutors.builder() and
AtClientImpl.builder(). It cannot be created by AtClientImpl, which receives an already-built
executor — and AtCommandExecutors needs the context to wire onReady. Existing arities stay as
one-line delegates so no caller outside the SDK breaks.
This is a signature and wiring change only: no new types, no behaviour change beyond the fix below.
Additional context
pluggable cryptography. This refactor is a prerequisite for it — the CryptographyProvider
interface will take (AtCommandExecutor, AtCommandExecutorContext, …) — but it lands separately so
that change stays reviewable.
Verification
mvn --batch-mode clean install. Command tests change at their call sites only; the asserted command
strings must be byte-identical, with one deliberate exception: the from: emitted on the monitoring
path now carries clientId and the version properties.
Is your feature request related to a problem? Please describe.
AtCommandExecutorContextmodels a connection's identity —atSign,keys,configthis overlaps with the requirementsfor porting pluggable cyrptography. Currently this is only used by
AuthenticationCommandsandEnrollCommands.Every other command re-declares
atSignandkeysas loose parameters, it would be better if there was consistency.Also
AtClientImplholds its own copies of the same three values.The same identity is therefore constructed twice per client from one source, AND the analysis for this issue revealed a (see below).
Bug
Every client built with
withMonitoring(true)sends afrom:command with noclientIdand no clientversion metadata, so monitoring clients are unidentifiable to the atServer.
AtClients:69passes the rawconfigtoNotifications.monitor.AtCommandExecutors:76builds the context withcreateClientConfig(config), which addsclientIdand everything in
client-config.properties(:120-135).AtCommandExecutors:85—defaultIfNotSet(onReady, createOnReady(context))— means an explicitonReadyreplaces the context-based one, sosendFromnever runs and the challenge stays null.Notifications.monitortherefore calls the 4-argauthenticateWithPkam(
AuthenticationCommands:92-98), which issues its ownfrom:at:124with the unenriched map.Both paths still issue exactly one
from:and onepkam:— this is a lost-metadata defect, not aduplicate-authentication one. A single shared context removes the second path.
Describe the solution you'd like
Standardize on
(AtCommandExecutor, AtCommandExecutorContext, …)as the first two arguments of everycommand method, and create exactly one
AtCommandExecutorContextper connection inAtClients.createAtClient, passing it to bothAtCommandExecutors.builder()andAtClientImpl.builder(). It cannot be created byAtClientImpl, which receives an already-builtexecutor — and
AtCommandExecutorsneeds the context to wireonReady. Existing arities stay asone-line delegates so no caller outside the SDK breaks.
This is a signature and wiring change only: no new types, no behaviour change beyond the fix below.
Additional context
pluggable cryptography. This refactor is a prerequisite for it — the
CryptographyProviderinterface will take
(AtCommandExecutor, AtCommandExecutorContext, …)— but it lands separately sothat change stays reviewable.
Verification
mvn --batch-mode clean install. Command tests change at their call sites only; the asserted commandstrings must be byte-identical, with one deliberate exception: the
from:emitted on the monitoringpath now carries
clientIdand the version properties.