-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
[adr] BiDi low-level definitions are generated from a shared spec model without orchestration #17701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+101
β0
Closed
[adr] BiDi low-level definitions are generated from a shared spec model without orchestration #17701
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6d22642
[docs] add BiDi generated protocol layer design decision record
titusfortner 4b3f61d
[docs] number BiDi ADR record by PR and link discussion
titusfortner a49e5ab
[docs] set ADR title and discussion link to PR 17701
titusfortner 67b42c9
[docs] retitle ADR: definitions generated without orchestration
titusfortner 6ce1f6e
[docs] clarify generated modules may call single commands; orchestratβ¦
titusfortner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| # 17701. BiDi's low-level definitions are generated from a shared spec model without orchestration | ||
|
|
||
| - Status: Proposed | ||
| - Discussion: https://github.com/SeleniumHQ/selenium/pull/17701 | ||
|
|
||
| ## Context | ||
|
|
||
| The WebDriver BiDi specification is defined in CDDL (Concise Data Definition Language). | ||
| A binding's BiDi support spans four layers, from the wire up: | ||
|
|
||
| - **Transport / session substrate** β the connection, sending each command and correlating its | ||
| response by the envelope id, and delivering inbound event frames upward. Domain-blind. | ||
| - **Low-level definitions** β the types, command shapes, and event shapes the spec defines, | ||
| including the id types (a subscription, an intercept) and the commands that produce and consume | ||
| them. | ||
| - **Orchestration** β the code that composes the definitions into capabilities and manages their | ||
| lifecycle: storing a subscription to unsubscribe later, mapping an intercept id to a handler, | ||
| matching an event to its registered callback, and wrapping events into the objects a handler | ||
| receives. | ||
| - **High-level API** β the protocol-neutral, idiomatic capabilities users program against. | ||
|
|
||
| Bindings differ today in how they produce the definitions and how cleanly they separate them from | ||
| the orchestration above: | ||
|
|
||
| | Binding | Current behavior | | ||
| |------------|------------------| | ||
| | Java | Hand-written (~143 module classes); definitions and orchestration on the same class; separate protocol-neutral high-level (`RemoteNetwork`). | | ||
| | Python | Generated from CDDL; orchestration injected into the generated classes via an enhancements manifest. | | ||
| | Ruby | Hand-written low-level; orchestration fused with the high-level API. | | ||
| | .NET | Hand-written module classes. | | ||
| | JavaScript | Generated from CDDL; orchestration injected into the generated classes via an enhancements manifest. | | ||
|
|
||
| They also differ on source of truth β the CDDL spec or the existing implementation β and on whether | ||
| they share one model or each interpret CDDL independently. | ||
|
|
||
| This record decides the low-level definitions layer β what it is, where it comes from, and how the | ||
| layers around it relate to it. This ADR assumes #17670 is accepted and that BiDi is an internal | ||
| implementation. | ||
|
|
||
| ## Decision | ||
|
|
||
| **1. The spec is the oracle, through one shared model.** The definitions are generated from a | ||
|
titusfortner marked this conversation as resolved.
|
||
| single shared, binding-neutral projection of the spec β not reconstructed from the existing | ||
| implementation, and not parsed independently per binding. Because the spec, not the existing code, | ||
| defines the shape, the generated definitions need not match the existing implementation β in API | ||
| shape or byte-for-byte. The shared model is the one place the spec is interpreted and normalized, so | ||
| bindings stay consistent with each other; each still emits its own language-idiomatic code from it. | ||
|
|
||
| **2. The generated definitions are read-only data, not the actionable surface.** Where a generated | ||
| object reaches a user β for example, the request object inside a network handler β it is immutable, | ||
|
titusfortner marked this conversation as resolved.
Outdated
|
||
| informational data; users act through the orchestration wrapper, not on the generated object. | ||
|
|
||
| **3. Orchestration stays out of the low-level definitions.** The definitions carry the spec β types, | ||
| commands, and events β and nothing that coordinates them: subscription lifecycle, event dispatch, | ||
| mapping ids to handlers, and the objects handed to a handler live in a separate layer that imports | ||
| the definitions, not spliced into the generated classes (e.g. through an enhancements manifest). A | ||
| thin, stateless convenience over a single command is a lesser matter; what must not land here is | ||
|
titusfortner marked this conversation as resolved.
Outdated
|
||
| coordination. The definitions stay a projection of the spec, so regenerating them never disturbs the | ||
| layer that imports them, and they depend only on the transport's send-and-deliver interface. | ||
|
|
||
| ## Considered options | ||
|
|
||
| - **Keep hand-maintaining the definitions** β each binding writes and updates the protocol types, | ||
| commands, and events by hand, with no generation. | ||
| - The same protocol is hand-maintained separately in every binding, so they drift apart β the | ||
| inconsistency this record exists to prevent. | ||
| - Every spec change is a manual edit repeated in each binding, with no shared source of truth. | ||
|
|
||
| - **Put the orchestration in the generated low-level class (e.g. via an enhancements manifest)** β | ||
| splice the coordination β subscription lifecycle, dispatch, handler wrapping β into the generated | ||
| classes alongside the spec types, as Python and JavaScript do today. | ||
| - Couples two layers that change for different reasons: a spec update and a coordination change | ||
| touch the same artifact, and regenerating risks the coordination. | ||
| - Coordination is harder to find, review, and type-check when it lives inside generated output. | ||
| - Thin conveniences are not the concern β the objection is coordination in the low-level layer. | ||
|
|
||
| - **Derive the definitions from the existing implementation** β generate to | ||
| reproduce the current shape. | ||
| - Treats the existing implementation as the source of truth instead of the spec, carrying its | ||
| inconsistencies forward. | ||
| - Nothing supported depends on the generated definitions, so they need not match it. | ||
|
|
||
| - **Generate each binding independently from CDDL** β every binding interprets the spec on its own | ||
|
titusfortner marked this conversation as resolved.
Outdated
|
||
| (or hand-writes), with no shared model. | ||
| - Each binding normalizes types, names, and gap handling separately, so the bindings drift apart | ||
| over time β the cross-binding inconsistency this record exists to prevent. | ||
| - A shared, binding-neutral model avoids the drift while still letting each binding emit | ||
| language-idiomatic code from it: the source model is shared, the generated definitions are | ||
| per-language. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - The supported API depends only on the wrapper, not on the generated definitions' | ||
| shape, so regenerating from a changed spec does not change what users program against. | ||
| - The generated definitions can live in their own namespace that the higher layers migrate onto, | ||
| and the existing implementation is retired. | ||
| - Orchestration and the high-level API are checked-in source, navigable and reviewable, and | ||
| regenerating the definitions never touches them. | ||
| - A binding that today combines the layers in one class (with injected orchestration and | ||
| enhancements) splits them: the generated definitions move to their own namespace, which the | ||
| orchestration and high-level API import. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.