Skip to content

Standardize command signatures on (AtCommandExecutor, AtCommandExecutorContext) and share one context per connection #428

Description

@akafredperry

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:85defaultIfNotSet(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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions