-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Added service contract technical guidelines #3421
Changes from 10 commits
a14ae2d
62d65a9
1527d6c
ae0b146
d9786b8
9b63185
361769c
c9c0d3d
793075f
a3fe869
ee3d95e
f57c274
c2102ee
4de91aa
0327977
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| group: coding-standards | ||
| title: Magento technical vision | ||
| --- | ||
|
|
||
| The Magento technical vision is a collection of documents that describe the desired state of Magento product. | ||
|
|
||
| Each individual technical vision document relates to a set of modules logically grouped together. Individual documents typically describe a component's place in the system, its architecture, extension scenarios. and a list of invariants that must be preserved at all times during component refactoring or extension to ensure consistency. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| group: coding-standards | ||
| title: Magento technical vision | ||
| --- | ||
|
|
||
| The Magento technical vision is a collection of documents that describe the desired state of Magento product. | ||
|
|
||
| Each individual technical vision document relates to a set of modules logically grouped together. Individual documents typically describe a component's place in the system, its architecture, extension scenarios. and a list of invariants that must be preserved at all times during component refactoring or extension to ensure consistency. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -532,7 +532,77 @@ class View extends Template | |
|
|
||
| ### 6.4. Service Contracts (Application) layer | ||
|
|
||
| We are reviewing this section and will publish it soon. | ||
| 6.4.1. Location of API interfaces | ||
|
|
||
| 6.4.1.1. Service contract interfaces SHOULD be placed in separate API modules. All other modules will depend on the API module, while implementations could be easily swapped via `di.xml`. API modules must have `Api` suffix in their names. For example, if a module is named `MyModule`, its APIs SHOULD be declared in a module named `MyModuleApi`. | ||
|
keharper marked this conversation as resolved.
Outdated
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.1.2. Service interfaces which should be exposed as web APIs MUST be placed under the `MyModuleApi/Api` directory. Service data interfaces MUST be placed under `MyModuleApi/Api/Data`. Directories under `MyModuleApi/Api` SHOULD NOT be nested. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.1.3. All other APIs, including explicit extension points like Chain or Composite implementations, MUST be placed under `MyModuleApi/Model`. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.2. Service Interface Structure | ||
|
|
||
| 6.4.2.1. Methods that have similar MUST serve similar purposes across different services, but they still MAY have different signatures. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing a word after first "similar"? |
||
|
|
||
| 6.4.2.2. Service contracts SHOULD NOT be used for read scenarios on the storefront, GraphQL SHOULD be used for storefront scenarios instead. Check out [web API technical vision]({{ page.baseurl }}/coding-standards/technical-vision/webapi.html) for more details. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.2.3. Each service interface SHOULD declare a single public method. An interface name SHOULD reflect the task/action to be performed. For example, `Magento\InventoryApi\Api\StockSourceLinksDeleteInterface::execute(array $links)`. The only exception is a Repository API which MAY be added for convenience and MUST be limited to singular CRUD operations and `getList($searchCriteria)`. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.3. Service Method Signature | ||
|
|
||
| 6.4.3.1. Strict typing is enforced for Service and Data interfaces located under `MyCompany/MyModuleApi/Api`. Only the following types are allowed: | ||
|
|
||
| * Scalar types: `string` (including Date and DateTime); `int`; `float`; `boolean` | ||
|
|
||
| * Data interfaces | ||
|
|
||
| * One-dimensional indexed arrays of scalars or data interfaces: for example `string[]`, `\MyCompany\MyModuleApi\Api\Data\SomeInterface[]`. Hash maps (associative arrays) are not supported. | ||
|
|
||
| * Nullable scalars or data interfaces: for example `string|null`. Using just `null` is prohibited. | ||
|
|
||
| * `void` | ||
|
|
||
| 6.4.3.2. Service contracts SHOULD support batch data processing. For example, an entity persisting method SHOULD accept an array of entities to persist instead of a single entity. Customizations via plugins SHOULD be adjusted respectively. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.3.3. Batch retrieval operations MUST accept `SearchCriteriaInterface` and return `SearchResultInterface` to support pagination. | ||
|
|
||
| 6.4.3.4. Batch operations that modify state MUST accept an array of entities and return a response object which contains: | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| * An array of successfully processed items | ||
|
|
||
| * An array of items with retriable errors | ||
|
|
||
| * An array of items with non-retriable errors | ||
|
|
||
| 6.4.3.5. Batch operations that modify state SHOULD be implemented in the most performant manner and SHOULD NOT load modified entities to generate response. | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should also mention void as an allowed type
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might not be supported by the web APIs, but I think this is the desired state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we fixed that for Web API framework in MSI scope and delivered to 2.3.0 Magento release So that now all MSI services modifying data exposing through the web API return void <route url="/V1/inventory/source-items" method="POST">
<service class="Magento\InventoryApi\Api\SourceItemsSaveInterface" method="execute"/>
<resources>
<resource ref="Magento_InventoryApi::source"/>
</resources>
</route>declare(strict_types=1);
namespace Magento\InventoryApi\Api;
/**
* Service method for source items save multiple
* Performance efficient API
*
* Used fully qualified namespaces in annotations for proper work of WebApi request parser
*
* @api
*/
interface SourceItemsSaveInterface
{
/**
* Save Multiple Source item data
*
* @param \Magento\InventoryApi\Api\Data\SourceItemInterface[] $sourceItems
* @return void
* @throws \Magento\Framework\Exception\InputException
* @throws \Magento\Framework\Validation\ValidationException
* @throws \Magento\Framework\Exception\CouldNotSaveException
*/
public function execute(array $sourceItems): void;
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @maghamed how would REST and SOAP responses look like in this case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paliarush for example, like this https://devdocs.magento.com/guides/v2.3/rest/tutorials/inventory/create-sources.html so, just an empty array returned in accordance with https://jsonapi.org/ standard : |
||
| 6.4.3.6. Asynchronous invocation of the command services SHOULD be supported by the web API framework. | ||
|
|
||
| 6.4.3.7. Operation UUID MAY be provided by the client during service invocation. UUID MUST allow the client to get the operation status information. | ||
|
|
||
| 6.4.3.8. Data objects returned by service contracts SHOULD be fully loaded to ensure consistency. | ||
|
|
||
| 6.4.4. Service Implementation | ||
|
|
||
| 6.4.4.1. Service data interfaces SHOULD extend from `Magento\Framework\Api\ExtensibleDataInterface`. The only exception is when extensibility is not desired, like in case of value-objects. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.4.2. Extensible data interfaces MUST NOT form hierarchies. If interface `MyInterface` extends `ExtensibleDataInterface`, there must be no interfaces extending `MyInterface`. Otherwise a list of extension attributes will be shared for all extensible interfaces in the hierarchy. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.4.3. Service implementations and plugins MUST NOT rely on storage-specific integrity features, such as foreign key constraints. | ||
|
|
||
| 6.4.4.4. Replace strategy SHOULD be used to persist main entity fields/attributes, child entities, and relation links. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace strategy should be "replacement strategy", unless "replace strategy" is a technical term |
||
|
|
||
| 6.4.4.5. During update operations, Web APIs using the`PATCH` HTTP method and all action controllers that accept entities SHOULD pre-load them first, merge request data on top of that, and provide full data to service. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| 6.4.4.6. If a service method needs to modify the argument, the original argument object MUST NOT be modified and its copy SHOULD be modified instead. | ||
|
|
||
| 6.4.4.7. Services SHOULD NOT apply ACL rules to methods or returned data. | ||
|
|
||
| 6.4.4.8. If multiple data scopes available, within one service call one entity SHOULD be persisted only for one specific data scope. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rewrite line 6.4.4.8 as follows: If multiple data scopes are available, at least one service call must contain a persisted entity for a specific data scope. or, if this more accurate: ... each service call must contain a minimum of one persisted entity for a specified data scope. |
||
|
|
||
| 6.4.4.9. Service contracts SHOULD NOT apply presentation layer formatting to the returned data. | ||
|
|
||
| 6.4.4.10. Service data interfaces MUST NOT contain any business logic. They SHOULD represent a container of data transferable over the wire. All the business logic SHOULD be moved to services. | ||
|
keharper marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## 7. Configuration | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../../v2.1/coding-standards/technical-vision/index.md |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../../v2.2/coding-standards/technical-vision/index.md |
Uh oh!
There was an error while loading. Please reload this page.