Skip to content

docs: add architecture and design document#209

Open
rjayasinghe wants to merge 5 commits into
mainfrom
docs/architecture-document
Open

docs: add architecture and design document#209
rjayasinghe wants to merge 5 commits into
mainfrom
docs/architecture-document

Conversation

@rjayasinghe

Copy link
Copy Markdown
Contributor

Summary

  • Adds docs/architecture.md documenting the plugin's overall design
  • Covers the CAP Java messaging core abstractions the plugin builds on (AbstractMessagingService, BrokerConnectionProvider, BrokerConnection)
  • Describes the boot flow, two-binding model (AEM broker + validation service), AMQP/OAuth2 connection mechanics, SEMP v2 management plane, validation handshake, and configuration surface
  • Includes a comparison table against cds-feature-enterprise-messaging
  • Lists caveats and edge cases identified during analysis

Test plan

  • Review document for accuracy against current source
  • Verify all file path and line number references are still valid

Adds docs/architecture.md describing how the plugin integrates with the
CAP Java messaging core, including boot flow, AMQP/OAuth2 connection
mechanics, SEMP v2 management plane, validation handshake, and a
comparison with cds-feature-enterprise-messaging.
@hyperspace-insights

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


Add Architecture & Design Documentation for cds-feature-advanced-event-mesh

Documentation

📄 Added a comprehensive architecture and design document for the cds-feature-advanced-event-mesh plugin, providing a detailed technical reference for developers working with or maintaining the plugin.

Changes

  • docs/architecture.md: New document covering the full architecture of the plugin, including:
    • TL;DR overview: Describes the plugin as a ~1.1k LoC CAP Java plugin bridging SAP Advanced Event Mesh (Solace PubSub+) into CAP's messaging abstraction.
    • CAP Java messaging stack: Explains the three core types (AbstractMessagingService, BrokerConnection, BrokerConnectionProvider) the plugin builds upon.
    • Module map: Outlines the package structure and responsibilities of each class.
    • Boot flow: Step-by-step description of startup — OAuth2 supplier registration, binding discovery, service materialization, and outbox wrapping.
    • Two-binding model: Documents the AEM broker binding (AMQP + SEMP v2) and the validation service binding (handshake), including their expected shapes and auth mechanisms.
    • AMQP connection plane: Details the Qpid + XOAUTH2 SASL setup and token refresh strategy.
    • SEMP v2 management plane: Covers queue/subscription CRUD operations and the skipManagement escape hatch.
    • Inbound topic extraction & outbound prefixing: Explains broker-specific topic addressing differences.
    • Validation handshake: Describes the one-shot compliance check before the first publish.
    • Comparison table vs. cds-feature-enterprise-messaging: Highlights key differences in auth, multi-tenancy, namespace handling, and complexity.
    • Configuration surface: Documents plugin-specific properties (skipManagement, subaccountId).
    • Caveats & edge cases: Flags fragile areas such as endpoint iteration order, null topic handling, and mandatory validation binding.

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.22.5

  • Output Template: Default Template
  • File Content Strategy: Full file content
  • LLM: anthropic--claude-4.6-sonnet
  • Event Trigger: pull_request.opened
  • Summary Prompt: Default Prompt
  • Correlation ID: 1b85d086-6078-4143-8ea1-647b64b48c57

@hyperspace-insights hyperspace-insights Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document is well-written and a valuable addition to the repository, but it contains several inaccuracies where the prose diverges from the actual source code — notably around getAemEndpoint's guard logic, the bytes-message branch in getMessageTopic, the payload shape of the validation handshake, a non-obvious race condition on the volatile Boolean validation guard, and the deferred/conditional nature of the validation binding's mandatory requirement. Please address the flagged comments before merging.

PR Bot Information

Version: 1.22.5

  • File Content Strategy: Full file content
  • Event Trigger: pull_request.opened
  • LLM: anthropic--claude-4.6-sonnet
  • Correlation ID: 1b85d086-6078-4143-8ea1-647b64b48c57

Comment thread docs/architecture.md
Comment thread docs/architecture.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread docs/architecture.md Outdated
- Clarify that the validation binding throw is deferred to
  createMessagingService (line 159), not services(); silently ignored
  if no AEM bindings are present
- Add missing bytes-message branch to getMessageTopic description
  (lines 153-162, both text and binary facades)
- Note that validate() is called before the topic:// prefix in
  emitTopicMessage (line 145 before 146)
- Correct validation payload: only the host component of managementUri
  is sent as hostName, not the full URI
- Flag that aemBrokerValidated is a boxed Boolean with an unsynchronized
  check-then-act, allowing concurrent validation calls
- Correct getAemEndpoint() fragility: key guard passes but iterator
  returns wrong value if insertion order is unexpected
@rjayasinghe rjayasinghe requested review from dimamost and t-bonk June 11, 2026 09:16

@t-bonk t-bonk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all it LGTM. There is only one change that crucial. Also please remove all line references as the line numbers might change when fixing issue or adding features.

Comment thread docs/architecture.md Outdated
| Binding | Tag/Name | Purpose | Auth |
|----------------|---------------------------|--------------------------------------------------------------------------|-----------------------------------------------------------------|
| AEM broker | `advanced-event-mesh` | AMQP+JMS data plane *and* SEMP management plane | OAuth2 client_credentials against IAS (`authentication-service`) |
| Validation | `aem-validation-service` | One-time handshake confirming the VMR is provisioned for this subaccount | OAuth2 against XSUAA (`handshake.oa2`) |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal of this validation is to check whether the AEM service is resold by SAP (reply with HTTP 20x) or the customer sourced it directly from Solace (HTTP > 399).

Comment thread docs/architecture.md Outdated

`AemValidationClient.validate(managementUri, subaccountId)` POSTs `{"hostName": <host extracted from managementUri>, "subaccountId"?: ...}` to the validation service's handshake endpoint (the destination base URI from the validation binding). Only the *host* component of `managementUri` is sent, not the full URI. It runs at most once per service instance — guarded by the `aemBrokerValidated` volatile `Boolean` flag in `AemMessagingService.validate()` (line 165), and only on the first publish (lazily via `emitTopicMessage`). A 200 confirms this BTP subaccount is entitled to this VMR. Anything else → `ServiceException`, no message is sent.

This is a compliance / anti-misconfiguration check: it prevents an app from being mistakenly bound to a broker in the wrong subaccount.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong! See above

Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Address review feedback from @t-bonk on PR #209: the validation
handshake verifies the AEM broker was resold through SAP (2xx) vs.
sourced directly from Solace (>=400), not that the VMR is provisioned
for a specific subaccount.
@rjayasinghe rjayasinghe deployed to Codescans July 1, 2026 14:47 — with GitHub Actions Active
@rjayasinghe rjayasinghe requested a review from t-bonk July 1, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants