-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add library documentation #7950
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
Merged
+161
β0
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
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
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,157 @@ | ||
| # Bitwarden library shape | ||
|
|
||
| This document is the canonical shape a library under `src/Libraries/` is expected | ||
| to follow. New libraries and reviewers can measure against it. See | ||
| [ADR-0031](https://contributing.bitwarden.com/architecture/adr/adopt-minimal-apis) | ||
| and [ADR-0032](https://contributing.bitwarden.com/architecture/adr/break-up-core) | ||
| for the motivating decisions. | ||
|
|
||
| ## Public surface | ||
|
|
||
| Types in a library are `internal` by default. A type is only `public` when a | ||
| consumer outside the library legitimately needs it, and every `public` type | ||
| carries a documented contract: the guarantees a consumer can rely on and the | ||
| invariants callers must uphold. | ||
|
|
||
| The smallest reasonable public surface is two extension methods: | ||
|
|
||
| - `AddFoo(this IServiceCollection services)` β registers everything the library | ||
| needs to run. | ||
| - `MapFooEndpoints(this IEndpointRouteBuilder builder)` β attaches the library's | ||
| HTTP endpoints. | ||
|
|
||
| Many libraries never need more than these two entry points. Settings classes, | ||
| repository interfaces, and domain types become `public` only when a host or | ||
| another library must interact with them directly. | ||
|
|
||
| ## Intentional public interface | ||
|
|
||
| ### Settings | ||
|
|
||
| A library declares its own strongly-typed options class rather than extending | ||
| `GlobalSettings`. The options class is `public` so hosts can bind it and | ||
| consumers can inject `IOptions<FooSettings>`. The host β not the library β | ||
| owns the configuration section name and the binding call. | ||
|
|
||
| ```csharp | ||
| public sealed class FooSettings | ||
| { | ||
| public TimeSpan BarExpiration { get; set; } = TimeSpan.FromHours(4); | ||
| } | ||
|
|
||
| // In the library β no configuration knowledge inside AddFoo. | ||
| public static IServiceCollection AddFoo(this IServiceCollection services) | ||
| { | ||
| // Library consumes IOptions<FooSettings>; host binds it. | ||
| return services; | ||
| } | ||
|
|
||
| // In the host. | ||
| services.Configure<FooSettings>(configuration.GetSection("Foo")); | ||
| services.AddFoo(); | ||
| ``` | ||
|
|
||
| Document the expected configuration shape β defaults, validation rules β on the | ||
| settings class itself with XML doc comments. | ||
|
|
||
| ### Endpoints | ||
|
|
||
| Endpoints are minimal APIs defined inside the library. The library exposes a | ||
| single `MapFooEndpoints` extension on `IEndpointRouteBuilder`. The host owns | ||
| the route prefix; the library still owns the concerns that describe *what* it | ||
| is β authorization policies, tags, endpoint filters, versioning β and attaches | ||
| them to an empty `MapGroup("")` so every endpoint inside the library inherits | ||
| them uniformly. | ||
|
|
||
| ```csharp | ||
| public static RouteGroupBuilder MapFooEndpoints(this IEndpointRouteBuilder builder) | ||
| { | ||
| var group = builder.MapGroup("") | ||
| .WithTags("Foo") | ||
| .RequireAuthorization(); | ||
|
|
||
| group.MapGet("/{id}", GetFooAsync); | ||
| group.MapPost("/", CreateFooAsync); | ||
|
|
||
| return group; | ||
| } | ||
|
|
||
| // In the host β host owns "/foo" and nothing else. | ||
| app.MapGroup("/foo").MapFooEndpoints(); | ||
| ``` | ||
|
|
||
| Return the library's own `RouteGroupBuilder` rather than the outer builder so | ||
| the host can chain further group-scoped configuration onto exactly what the | ||
| library mapped. | ||
|
|
||
| A library may also add a non-empty inner `MapGroup` when it needs a sub-prefix | ||
| shared by all of its endpoints. That is a deliberate choice on top of the empty | ||
| group, not a replacement for the host's outer prefix. | ||
|
|
||
| Endpoint handlers and endpoint filters live inside the library and stay `internal` | ||
| unless another library needs them. | ||
|
|
||
| ### Repositories | ||
|
|
||
| A library owns its data access end-to-end: the repository interface, the Dapper | ||
| implementation for MSSQL, and the Entity Framework Core implementations for | ||
| PostgreSQL, MySQL, and SQLite. All of it lives inside the library. | ||
|
|
||
| The repository interface is `internal` when only the library consumes it. | ||
| `AddFoo` chooses the correct implementation based on the host's configured | ||
| database provider β the same dual-ORM pattern used elsewhere in the repo. If | ||
| the library does not need persistence, it has no data layer at all. | ||
|
|
||
| ```csharp | ||
| public static IServiceCollection AddFoo(this IServiceCollection services) | ||
| { | ||
| services.AddVaultDatabase(); | ||
|
|
||
| services.TryAddSingleton<DapperFooRepository>(); | ||
| services.TryAddSingleton<EntityFrameworkFooRepository>(); | ||
| services.TryAddSingleton<IFooRepository>(sp => | ||
| { | ||
| var vaultDatabaseSettings = sp.GetRequiredService<IOptions<VaultDatabaseSettings>>().Value; | ||
|
|
||
| if (vaultDatabaseSettings.PrefersDapper()) | ||
| { | ||
| return sp.GetRequiredService<DapperFooRepository>(); | ||
| } | ||
|
|
||
| return sp.GetRequiredService<EntityFrameworkFooRepository>(); | ||
| }); | ||
|
|
||
| return services; | ||
| } | ||
| ``` | ||
|
|
||
| ## Composing your feature | ||
|
|
||
| `AddFoo` is where the library's internal services are wired together. Follow | ||
| the existing DI conventions: | ||
|
|
||
| - Register services with `TryAdd*` so hosts can pre-register test doubles or | ||
| alternative implementations (ADR-0026). | ||
| - Register interfaces against internal implementations. External consumers only | ||
| see the interfaces the library chose to expose. | ||
|
|
||
| `MapFooEndpoints` composes the library's endpoints under an inner | ||
| `RouteGroupBuilder`. Attach cross-cutting concerns β authorization policies, | ||
| endpoint filters, tags, API versioning β to the group so every endpoint in the | ||
| library inherits them uniformly. | ||
|
|
||
| ## Depending on other features | ||
|
|
||
| Libraries interact with each other **only through public surface**. If | ||
| Library A needs behavior from Library B, it consumes B's public interfaces via | ||
| DI or calls B's public extension methods. Reaching into another library's | ||
| `internal` types is not allowed; if you find you need one, that is a | ||
| cross-team conversation with the owning team, not a hidden coupling to | ||
| work around. | ||
|
|
||
| `Core` is a transitional exception. A library may depend on `Core` while the | ||
| break-up is in progress, but it must document what it took from `Core` β the | ||
| specific types, services, or settings β so those pieces can be prioritized for | ||
| extraction. Treat every `Core` reference as a debt entry, not a permanent | ||
| choice. New libraries should sit below `Core` in the dependency graph and | ||
| avoid growing new dependencies on it wherever possible. | ||
Oops, something went wrong.
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.