From eb25eb04890096c2c9b6bba98de474c54e228448 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Sat, 29 Nov 2025 10:18:50 +1000 Subject: [PATCH 1/4] ADR 28: adopt cqs terminology --- custom-words.txt | 1 + .../adr/0008-server-CQRS-pattern.md | 15 ++- .../adr/0028-server-adopt-cqs-terminology.md | 90 +++++++++++++++ docs/architecture/adr/index.mdx | 2 +- docs/architecture/server/index.md | 108 ++++++++++-------- 5 files changed, 165 insertions(+), 51 deletions(-) create mode 100644 docs/architecture/adr/0028-server-adopt-cqs-terminology.md diff --git a/custom-words.txt b/custom-words.txt index aaf6cc11e..26c938b0d 100644 --- a/custom-words.txt +++ b/custom-words.txt @@ -12,6 +12,7 @@ clickjacking codebases CODEOWNERS CQRS +CQS CTAP2 deinitializer deinitializers diff --git a/docs/architecture/adr/0008-server-CQRS-pattern.md b/docs/architecture/adr/0008-server-CQRS-pattern.md index 49ff3737b..e9b779096 100644 --- a/docs/architecture/adr/0008-server-CQRS-pattern.md +++ b/docs/architecture/adr/0008-server-CQRS-pattern.md @@ -1,14 +1,27 @@ --- adr: "0008" -status: Accepted +status: Superseded date: 2022-07-15 tags: [server] +superseded_by: "0028" --- # 0008 - Server: Adopt CQRS +:::note Superseded by ADR-0028 + +This ADR originally established "CQRS" as the terminology for our pattern. However, +[ADR-0028](./0028-server-adopt-cqs-terminology.md) clarifies that our implementation actually +follows the simpler CQS (Command Query Separation) pattern rather than full CQRS. While the +architectural decision to use commands and queries remains valid and in effect, the terminology has +been updated to more accurately reflect our implementation. + +The guidance in this ADR is still relevant, but it should be read in conjunction with ADR-0028. + +::: + ## Context and problem statement In Bitwarden Server, we currently use an `<>Service` pattern to act on our entities. These diff --git a/docs/architecture/adr/0028-server-adopt-cqs-terminology.md b/docs/architecture/adr/0028-server-adopt-cqs-terminology.md new file mode 100644 index 000000000..73683c0dc --- /dev/null +++ b/docs/architecture/adr/0028-server-adopt-cqs-terminology.md @@ -0,0 +1,90 @@ +--- +adr: "0028" +status: "Proposed" +date: 2025-11-29 +tags: [server] +--- + +# 0028 - Server: Clarify CQS vs CQRS Terminology + + + +## Context and problem statement + +In [ADR-0008](./0008-server-CQRS-pattern.md), we adopted what we called "CQRS" (Command Query +Responsibility Segregation) for the server architecture. However, upon reflection and after several +years of implementation, it has become clear that our actual implementation does not match the full +scope and complexity of CQRS as defined in industry literature. + +**CQRS** is a comprehensive architectural pattern that separates the read and write models at the +data storage level. A full CQRS implementation typically includes: + +- Separate data models for reads and writes +- Event sourcing +- Eventual consistency between read and write stores +- Complex synchronization mechanisms +- Often, separate databases or data stores for queries vs commands + +**CQS** (Command Query Separation) is a simpler, more focused principle that states: + +- Commands change state but don't return data (or only return operation results) +- Queries return data but don't change state +- Each operation should have a single responsibility + +Our implementation follows CQS principles: we break up large service classes into smaller command +and query classes, but we do not maintain separate data models or storage layers. This is exactly +what CQS aims to achieve - smaller, more focused classes that are easier to test and maintain. + +The terminology mismatch has caused confusion for developers joining the project, as they research +CQRS and find it describes a much more complex architecture than what we've actually implemented. +This creates an unnecessary learning curve and misaligned expectations. + +## Considered options + +- **Keep CQRS terminology** - Continue using CQRS terminology despite the mismatch. This is + difficult to justify. + +- **Adopt CQS terminology** - Update our documentation to use CQS terminology, which accurately + describes what we've implemented. This provides clarity for new developers and aligns our + documentation with our actual implementation. + +- **Implement full CQRS** - Actually implement the full CQRS pattern with separate read/write + models. This would be a large architectural change that has not been proposed and which is out of + scope in any case. This ADR is focused on aligning our documentation with our current practices. + +## Decision outcome + +Chosen option: **Adopt CQS terminology**. + +The terminology change better reflects our implementation and reduces confusion. It also +acknowledges that what we've built what we needed: a pragmatic solution to break up large service +classes without the additional complexity of full CQRS. + +### Positive consequences + +- **Clarity for new developers** - Developers can research CQS and find documentation that matches + our implementation +- **Accurate documentation** - Our architecture documentation reflects what we actually built +- **Reduced complexity** - We're not implying architectural complexity we haven't implemented +- **Better expectations** - Team members understand the scope and scale of what we're maintaining + +### Negative consequences + +- **Historical confusion** - Older discussions, PRs, and code comments may still reference "CQRS" +- **Name change overhead** - Some mental adjustment needed for developers familiar with the old + terminology + +### Migration plan + +1. Update this documentation to use CQS terminology, particularly the + [Server Architecture](../server/index.md) page. +1. Update ADR-0008 status to "Superseded" with a reference to this ADR. Add a note referencing this + ADR. +1. No code changes required - class names like `CreateCipherCommand` remain appropriate as "command" + and "query" are common to both patterns + +## Further reading + +- [Martin Fowler on CQS](https://martinfowler.com/bliki/CommandQuerySeparation.html) - a succinct + high level summary of our approach +- [ADR-0008: Server CQRS Pattern](./0008-server-CQRS-pattern.md) (superseded by this ADR) diff --git a/docs/architecture/adr/index.mdx b/docs/architecture/adr/index.mdx index b6c780bf7..10f220e9a 100644 --- a/docs/architecture/adr/index.mdx +++ b/docs/architecture/adr/index.mdx @@ -112,7 +112,7 @@ work. **Example scenarios:** - A technology choice has been agreed upon (e.g., "Use Jest for testing") -- A pattern has been established as the standard approach (e.g., "Use CQRS for server architecture") +- A pattern has been established as the standard approach (e.g., "Use CQS for server architecture") - A decision is complete and being followed across teams ### Rejected diff --git a/docs/architecture/server/index.md b/docs/architecture/server/index.md index bfd8533b7..09555e8b8 100644 --- a/docs/architecture/server/index.md +++ b/docs/architecture/server/index.md @@ -4,22 +4,34 @@ sidebar_position: 6 # Server Architecture -## CQRS ([ADR-0008](../adr/0008-server-CQRS-pattern.md)) +## Command Query Separation (CQS) -Our server architecture uses the the Command and Query Responsibility Segregation (CQRS) pattern. +Our server architecture uses the Command and Query Separation (CQS) pattern. The main goal of this pattern is to break up large services focused on a single entity (e.g. -`CipherService`) and move towards smaller, reusable classes based on actions or tasks (e.g. -`CreateCipher`). In the future, this may enable other benefits such as enqueuing commands for -execution, but for now the focus is on having smaller, reusable chunks of code. +`CipherService`, which handles everything to do with a cipher) and move towards smaller classes +based on discrete actions or user flows (e.g. `CreateCipherCommand`, which handles only creating a +cipher). This results in smaller classes with fewer interdependencies that are easier to change and +test. + +:::note + +Previously this pattern was referred to as Command Query Responsibility Segregation (CQRS). However, +CQRS is a far more complex architecture with larger impacts than we wanted or needed. Our +implementation in practice more closely resembled CQS, which has a more modest (but still effective) +aim of simply breaking up large classes. This documentation has been rewritten to better reflect our +practice and therefore refers only to CQS. + +::: ### Commands vs. queries -**Commands** are write operations, e.g. `RotateOrganizationApiKeyCommand`. They should never read -from the database. +**Commands** are write operations, e.g. `RotateOrganizationApiKeyCommand`. They change the state of +the system. They may return data about the operation (e.g. the updated object or an error message), +but otherwise should not be used to return data to its caller. -**Queries** are read operations, e.g. `GetOrganizationApiKeyQuery`. They should never write to the -database. +**Queries** are read operations, e.g. `GetOrganizationApiKeyQuery`. They should only return a value +and should never change the state of the system. The database is the most common data source we deal with, but others are possible. For example, a query could also get data from a remote server. @@ -28,18 +40,17 @@ Each query or command should have a single responsibility. For example: delete a file, rotate an API key. They are designed around verbs or actions (e.g. `RotateOrganizationApiKeyCommand`), not domains or entities (e.g. `ApiKeyService`). -### Writing commands or queries +Which you use will often follow the HTTP verb: a POST operation will generally call a command, +whereas a GET operation will generally call a query. -A simple query may just be a repository call to fetch data from the database. (We already use -repositories, and this is not what we're concerned about here.) However, more complex queries can -require additional logic around the repository call, which will require their own class. Commands -always need their own class. +Teams have wide discretion in how they structure their commands and queries. -The class, interface and public method should be named after the action. For example: +### Structure of a command -```csharp -namespace Bit.Core.OrganizationFeatures.OrganizationApiKeys; +A command is just a class. The class, interface and public method should be named after the action. +For example: +```csharp public class RotateOrganizationApiKeyCommand : IRotateOrganizationApiKeyCommand { public async Task RotateApiKeyAsync(OrganizationApiKey organizationApiKey) @@ -49,51 +60,43 @@ public class RotateOrganizationApiKeyCommand : IRotateOrganizationApiKeyCommand } ``` -The query/command should only expose public methods that run the complete action. It should not have +The command should only expose public methods that run the complete action. It should not have public helper methods. -The directory structure and namespaces should be organized by feature. Interfaces should be stored -in a separate sub-folder. For example: - -```text - Core/ - └── OrganizationFeatures/ - └── OrganizationApiKeys/ - ├── Interfaces/ - │ └── IRotateOrganizationApiKeyCommand.cs - └── RotateOrganizationApiKeyCommand.cs -``` +A command will usually follow these steps: -### Maintaining the command/query distinction +1. Fetch additional data required to process the request (if required) +1. Validate the request +1. Perform the action (state change) +1. Perform any side effects (e.g. sending emails or push notifications) +1. Return information about the outcome to the user (e.g. an error message or the successfully + created or updated object) -By separating read and write operations, CQRS encourages us to maintain loose coupling between -classes. There are two golden rules to follow when using CQRS in our codebase: +If you have complex validation logic, it can be useful to move it to a separate validator class. +This makes the validator and the command easier to understand, test and maintain. -- **Commands should never read and queries should never write** -- **Commands and queries should never call each other** +Some teams have defined their own request and result objects to pass data to and from commands and +validators. This is optional but can be useful to avoid primitive obsession and have strongly typed +interfaces. -Both of these lead to tight coupling between classes, reduce opportunities for code re-use, and -conflate the command/query distinction. +### Structure of a query -You can generally avoid these problems by: +A simple query may not require its own class if it is appropriately encapsulated by a single +database call. In that case, the "query" is just a repository method. -- writing your commands so that they receive all the data they need in their arguments, rather than - fetching the data themselves -- calling queries and commands sequentially (one after the other), passing the results along the - call chain +However, more complex queries can require additional logic in addition to the repository call. In +this case, it is appropriate to define a separate query class. -For example, if we need to update an API key for an organization, it might be tempting to have an -`UpdateApiKeyCommand` which fetches the current API key and then updates it. However, we can break -this down into two separate queries/commands, which are called separately: +A query is just a class. The class, interface and public method should be named after the data being +queried. For example: ```csharp -var currentApiKey = await _getOrganizationApiKeyQuery.GetOrganizationApiKeyAsync(orgId); -await _rotateOrganizationApiKeyCommand.RotateApiKeyAsync(currentApiKey); +public interface IGetOrganizationApiKeyQuery +{ + Task GetOrganizationApiKeyAsync(Guid organizationId, OrganizationApiKeyType organizationApiKeyType); +} ``` -This has unit testing benefits as well - instead of having lengthy "arrange" phases where you mock -query results, you can simply supply different argument values using the `Autodata` attribute. - ### Avoid [primitive obsession](https://refactoring.guru/smells/primitive-obsession) Where practical, your commands and queries should take and return whole objects (e.g. `User`) rather @@ -103,3 +106,10 @@ than individual properties (e.g. `userId`). Lots of optional parameters can quickly become difficult to work with. Instead, consider using method overloading to provide different entry points into your command or query. + +## Further reading + +- [ADR-0028: Server CQS Terminology](../adr/0028-server-adopt-cqs-terminology.md) - Clarifies why we + use CQS instead of CQRS terminology +- [ADR-0008: Server CQRS Pattern](../adr/0008-server-CQRS-pattern.md) - Original architectural + decision (superseded by ADR-0028) From 9083ddd1d1603d2a50bf05719e505160e069cf8e Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Sat, 29 Nov 2025 11:22:26 +1000 Subject: [PATCH 2/4] claude feedback --- .../adr/0028-server-adopt-cqs-terminology.md | 4 ++-- docs/architecture/server/index.md | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/architecture/adr/0028-server-adopt-cqs-terminology.md b/docs/architecture/adr/0028-server-adopt-cqs-terminology.md index 73683c0dc..168da0ebf 100644 --- a/docs/architecture/adr/0028-server-adopt-cqs-terminology.md +++ b/docs/architecture/adr/0028-server-adopt-cqs-terminology.md @@ -57,8 +57,8 @@ This creates an unnecessary learning curve and misaligned expectations. Chosen option: **Adopt CQS terminology**. The terminology change better reflects our implementation and reduces confusion. It also -acknowledges that what we've built what we needed: a pragmatic solution to break up large service -classes without the additional complexity of full CQRS. +acknowledges that we built what we needed: a pragmatic solution to break up large service classes +without the additional complexity of full CQRS. ### Positive consequences diff --git a/docs/architecture/server/index.md b/docs/architecture/server/index.md index 09555e8b8..3cfafb73c 100644 --- a/docs/architecture/server/index.md +++ b/docs/architecture/server/index.md @@ -6,13 +6,12 @@ sidebar_position: 6 ## Command Query Separation (CQS) -Our server architecture uses the Command and Query Separation (CQS) pattern. +Our server architecture uses the Command Query Separation (CQS) pattern. The main goal of this pattern is to break up large services focused on a single entity (e.g. `CipherService`, which handles everything to do with a cipher) and move towards smaller classes -based on discrete actions or user flows (e.g. `CreateCipherCommand`, which handles only creating a -cipher). This results in smaller classes with fewer interdependencies that are easier to change and -test. +based on discrete actions (e.g. `CreateCipherCommand`, which handles only creating a cipher). This +results in smaller classes with fewer interdependencies that are easier to change and test. :::note @@ -27,8 +26,8 @@ practice and therefore refers only to CQS. ### Commands vs. queries **Commands** are write operations, e.g. `RotateOrganizationApiKeyCommand`. They change the state of -the system. They may return data about the operation (e.g. the updated object or an error message), -but otherwise should not be used to return data to its caller. +the system. They may return data about the operation result (e.g. the updated object or an error +message), but otherwise should not be used to return data to its caller. **Queries** are read operations, e.g. `GetOrganizationApiKeyQuery`. They should only return a value and should never change the state of the system. From d433d5bc90e9952a7e31e1e3053ac8e73040b738 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Tue, 2 Dec 2025 10:24:52 +1000 Subject: [PATCH 3/4] Consolidate ADR-0028 into ADR-0008 Updates ADR-0008 to use CQS terminology and removes the separate ADR-0028. Also fixes references in server/index.md and angular.md. --- .../adr/0008-server-CQRS-pattern.md | 40 +++++---- .../adr/0028-server-adopt-cqs-terminology.md | 90 ------------------- docs/architecture/server/index.md | 6 +- docs/contributing/code-style/web/angular.md | 2 +- 4 files changed, 24 insertions(+), 114 deletions(-) delete mode 100644 docs/architecture/adr/0028-server-adopt-cqs-terminology.md diff --git a/docs/architecture/adr/0008-server-CQRS-pattern.md b/docs/architecture/adr/0008-server-CQRS-pattern.md index e9b779096..69dceb6e5 100644 --- a/docs/architecture/adr/0008-server-CQRS-pattern.md +++ b/docs/architecture/adr/0008-server-CQRS-pattern.md @@ -1,24 +1,20 @@ --- adr: "0008" -status: Superseded +status: Accepted date: 2022-07-15 tags: [server] -superseded_by: "0028" --- -# 0008 - Server: Adopt CQRS +# 0008 - Server: Adopt CQS -:::note Superseded by ADR-0028 +:::note Terminology clarification -This ADR originally established "CQRS" as the terminology for our pattern. However, -[ADR-0028](./0028-server-adopt-cqs-terminology.md) clarifies that our implementation actually -follows the simpler CQS (Command Query Separation) pattern rather than full CQRS. While the -architectural decision to use commands and queries remains valid and in effect, the terminology has -been updated to more accurately reflect our implementation. - -The guidance in this ADR is still relevant, but it should be read in conjunction with ADR-0028. +This ADR originally used "CQRS" (Command Query Responsibility Segregation) terminology. However, our +implementation more accurately reflects CQS (Command Query Separation), which is a simpler pattern +focused on breaking up large service classes rather than separating read/write data models. The +content has been updated to use the more accurate CQS terminology. ::: @@ -43,27 +39,26 @@ method parameters, which goes against our typical DI pattern. - **`<>Services`** -- Discussed above - **Queries and Commands** -- Fundamentally our problem is that the `<>Service` name encapsulates absolutely anything you can do with that entity and excludes any code reuse across - different entities. The CQRS pattern creates classes based on the action being taken on the - entity. This naturally limits the classes scope and allows for reuse should two entities need to - implement the same command behavior. - https://docs.microsoft.com/en-us/azure/architecture/patterns/cqrs + different entities. The CQS pattern creates classes based on the action being taken on the entity. + This naturally limits the classes scope and allows for reuse should two entities need to implement + the same command behavior. - **Small Feature-based services** -- This design would break `<>Service` into `<>Service`, but ultimately runs into the same problems. As a feature grows, this service would become bloated and tightly coupled to other services. ## Decision outcome -Chosen option: **Queries and Commands** +Chosen option: **Queries and Commands (CQS pattern)** -Commands seem all-around the better decision to the incumbent. We gain code reuse and limit class -scope. In addition, we have an iterative path to a full-blown CQRS pipeline, with queued work. +Commands seem all-around the better decision to the incumbent, primarily because it limits class +scope. Queries are already basically done through repositories and/or services, but would require some restructuring to be obvious. ## Transition plan -We will gradually transition to a CQRS pattern over time. If a developer is making changes that use +We will gradually transition to a CQS pattern over time. If a developer is making changes that use or affect a service method, they should consider whether it can be extracted to a query/command, and include it in their work as tech debt. @@ -72,3 +67,10 @@ up all at once. It's acceptable to refactor "one level deep" and then leave othe are. This may result in the new query/command still being somewhat coupled with other service methods. This is acceptable during the transition phase, but these interdependencies should be removed over time. + +## Further reading + +- [Server Architecture](../server/index.md) - Practical guide on implementing CQS in the server + codebase +- [Martin Fowler on CQS](https://martinfowler.com/bliki/CommandQuerySeparation.html) - High-level + summary of the CQS principle diff --git a/docs/architecture/adr/0028-server-adopt-cqs-terminology.md b/docs/architecture/adr/0028-server-adopt-cqs-terminology.md deleted file mode 100644 index 168da0ebf..000000000 --- a/docs/architecture/adr/0028-server-adopt-cqs-terminology.md +++ /dev/null @@ -1,90 +0,0 @@ ---- -adr: "0028" -status: "Proposed" -date: 2025-11-29 -tags: [server] ---- - -# 0028 - Server: Clarify CQS vs CQRS Terminology - - - -## Context and problem statement - -In [ADR-0008](./0008-server-CQRS-pattern.md), we adopted what we called "CQRS" (Command Query -Responsibility Segregation) for the server architecture. However, upon reflection and after several -years of implementation, it has become clear that our actual implementation does not match the full -scope and complexity of CQRS as defined in industry literature. - -**CQRS** is a comprehensive architectural pattern that separates the read and write models at the -data storage level. A full CQRS implementation typically includes: - -- Separate data models for reads and writes -- Event sourcing -- Eventual consistency between read and write stores -- Complex synchronization mechanisms -- Often, separate databases or data stores for queries vs commands - -**CQS** (Command Query Separation) is a simpler, more focused principle that states: - -- Commands change state but don't return data (or only return operation results) -- Queries return data but don't change state -- Each operation should have a single responsibility - -Our implementation follows CQS principles: we break up large service classes into smaller command -and query classes, but we do not maintain separate data models or storage layers. This is exactly -what CQS aims to achieve - smaller, more focused classes that are easier to test and maintain. - -The terminology mismatch has caused confusion for developers joining the project, as they research -CQRS and find it describes a much more complex architecture than what we've actually implemented. -This creates an unnecessary learning curve and misaligned expectations. - -## Considered options - -- **Keep CQRS terminology** - Continue using CQRS terminology despite the mismatch. This is - difficult to justify. - -- **Adopt CQS terminology** - Update our documentation to use CQS terminology, which accurately - describes what we've implemented. This provides clarity for new developers and aligns our - documentation with our actual implementation. - -- **Implement full CQRS** - Actually implement the full CQRS pattern with separate read/write - models. This would be a large architectural change that has not been proposed and which is out of - scope in any case. This ADR is focused on aligning our documentation with our current practices. - -## Decision outcome - -Chosen option: **Adopt CQS terminology**. - -The terminology change better reflects our implementation and reduces confusion. It also -acknowledges that we built what we needed: a pragmatic solution to break up large service classes -without the additional complexity of full CQRS. - -### Positive consequences - -- **Clarity for new developers** - Developers can research CQS and find documentation that matches - our implementation -- **Accurate documentation** - Our architecture documentation reflects what we actually built -- **Reduced complexity** - We're not implying architectural complexity we haven't implemented -- **Better expectations** - Team members understand the scope and scale of what we're maintaining - -### Negative consequences - -- **Historical confusion** - Older discussions, PRs, and code comments may still reference "CQRS" -- **Name change overhead** - Some mental adjustment needed for developers familiar with the old - terminology - -### Migration plan - -1. Update this documentation to use CQS terminology, particularly the - [Server Architecture](../server/index.md) page. -1. Update ADR-0008 status to "Superseded" with a reference to this ADR. Add a note referencing this - ADR. -1. No code changes required - class names like `CreateCipherCommand` remain appropriate as "command" - and "query" are common to both patterns - -## Further reading - -- [Martin Fowler on CQS](https://martinfowler.com/bliki/CommandQuerySeparation.html) - a succinct - high level summary of our approach -- [ADR-0008: Server CQRS Pattern](./0008-server-CQRS-pattern.md) (superseded by this ADR) diff --git a/docs/architecture/server/index.md b/docs/architecture/server/index.md index 3cfafb73c..0382ef0a5 100644 --- a/docs/architecture/server/index.md +++ b/docs/architecture/server/index.md @@ -108,7 +108,5 @@ method overloading to provide different entry points into your command or query. ## Further reading -- [ADR-0028: Server CQS Terminology](../adr/0028-server-adopt-cqs-terminology.md) - Clarifies why we - use CQS instead of CQRS terminology -- [ADR-0008: Server CQRS Pattern](../adr/0008-server-CQRS-pattern.md) - Original architectural - decision (superseded by ADR-0028) +- [ADR-0008: Server CQS Pattern](../adr/0008-server-CQRS-pattern.md) - Architectural decision to + adopt CQS for breaking up large service classes diff --git a/docs/contributing/code-style/web/angular.md b/docs/contributing/code-style/web/angular.md index cf547ba5a..165ed03ad 100644 --- a/docs/contributing/code-style/web/angular.md +++ b/docs/contributing/code-style/web/angular.md @@ -212,7 +212,7 @@ component "Organization Reports Module" { @enduml ``` -## Reactivity ([ADR-0003](../../../architecture/adr/0003-observable-data-services.md) & ADR-0028) +## Reactivity ([ADR-0003](../../../architecture/adr/0003-observable-data-services.md)) We make heavy use of reactive programming using [Angular Signals][signals] & [RxJS][rxjs]. Generally components should always derive their state reactively from services whenever possible. From 6246617e6ef7e01562dc43712e382d378a4f2468 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Thu, 4 Dec 2025 09:37:23 +1000 Subject: [PATCH 4/4] PR feedback --- docs/architecture/server/index.md | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/docs/architecture/server/index.md b/docs/architecture/server/index.md index 0382ef0a5..d885a16c3 100644 --- a/docs/architecture/server/index.md +++ b/docs/architecture/server/index.md @@ -8,26 +8,15 @@ sidebar_position: 6 Our server architecture uses the Command Query Separation (CQS) pattern. -The main goal of this pattern is to break up large services focused on a single entity (e.g. -`CipherService`, which handles everything to do with a cipher) and move towards smaller classes -based on discrete actions (e.g. `CreateCipherCommand`, which handles only creating a cipher). This +We adopted this pattern in order to break up large services focused on a single entity (e.g. +`CipherService`) into smaller classes based on discrete actions (e.g. `CreateCipherCommand`). This results in smaller classes with fewer interdependencies that are easier to change and test. -:::note - -Previously this pattern was referred to as Command Query Responsibility Segregation (CQRS). However, -CQRS is a far more complex architecture with larger impacts than we wanted or needed. Our -implementation in practice more closely resembled CQS, which has a more modest (but still effective) -aim of simply breaking up large classes. This documentation has been rewritten to better reflect our -practice and therefore refers only to CQS. - -::: - ### Commands vs. queries **Commands** are write operations, e.g. `RotateOrganizationApiKeyCommand`. They change the state of -the system. They may return data about the operation result (e.g. the updated object or an error -message), but otherwise should not be used to return data to its caller. +the system. They may have no return value, or may return the operation result only (e.g. the updated +object or an error message). **Queries** are read operations, e.g. `GetOrganizationApiKeyQuery`. They should only return a value and should never change the state of the system. @@ -42,8 +31,6 @@ file, rotate an API key. They are designed around verbs or actions (e.g. Which you use will often follow the HTTP verb: a POST operation will generally call a command, whereas a GET operation will generally call a query. -Teams have wide discretion in how they structure their commands and queries. - ### Structure of a command A command is just a class. The class, interface and public method should be named after the action. @@ -65,10 +52,10 @@ public helper methods. A command will usually follow these steps: 1. Fetch additional data required to process the request (if required) -1. Validate the request -1. Perform the action (state change) -1. Perform any side effects (e.g. sending emails or push notifications) -1. Return information about the outcome to the user (e.g. an error message or the successfully +2. Validate the request +3. Perform the action (state change) +4. Perform any side effects (e.g. sending emails or push notifications) +5. Return information about the outcome to the user (e.g. an error message or the successfully created or updated object) If you have complex validation logic, it can be useful to move it to a separate validator class.