113 lines
4.1 KiB
Markdown
113 lines
4.1 KiB
Markdown
|
|
# ADR-001: No `listAll()` Method in Documents Client
|
||
|
|
|
||
|
|
**Status:** Accepted
|
||
|
|
**Date:** 2026-01-16
|
||
|
|
**Context:** Response to client feature request (REQ-2 in suggested-improvements.md)
|
||
|
|
|
||
|
|
## Context
|
||
|
|
|
||
|
|
A client integration team requested a `listAll()` method that would:
|
||
|
|
1. Return all documents regardless of status in a single call
|
||
|
|
2. Support filtering by status codes
|
||
|
|
3. Optionally auto-paginate to fetch all pages
|
||
|
|
|
||
|
|
The Binect REST API exposes two distinct endpoints for listing documents:
|
||
|
|
- `GET /documents` - Returns shippable documents (status 2)
|
||
|
|
- `GET /documents/errors` - Returns erroneous documents (status 7)
|
||
|
|
|
||
|
|
There is no single API endpoint that returns documents across all statuses.
|
||
|
|
|
||
|
|
## Decision
|
||
|
|
|
||
|
|
We will **not** implement a `listAll()` method on the `DocumentsClient` class.
|
||
|
|
|
||
|
|
## Rationale
|
||
|
|
|
||
|
|
### 1. Violates Core Design Principle: 1:1 Semantic Mapping
|
||
|
|
|
||
|
|
The SDK's architecture mandates a "1:1 semantic mapping to REST endpoints" (CLAUDE.md). A `listAll()` method would need to:
|
||
|
|
- Call multiple API endpoints (`/documents` and `/documents/errors`)
|
||
|
|
- Merge and deduplicate results
|
||
|
|
- Present a unified interface that doesn't exist in the underlying API
|
||
|
|
|
||
|
|
This creates abstraction that obscures actual API behavior.
|
||
|
|
|
||
|
|
### 2. Violates Transparency Principle
|
||
|
|
|
||
|
|
The design constraint "Transparency over abstraction: Developers must reason about actual API calls" means developers should understand exactly which API calls are being made. A `listAll()` method hides the fact that multiple calls occur, making it harder to:
|
||
|
|
- Debug network issues
|
||
|
|
- Understand rate limiting implications
|
||
|
|
- Reason about data freshness (documents may change between calls)
|
||
|
|
|
||
|
|
### 3. Incomplete Coverage Cannot Be Fixed
|
||
|
|
|
||
|
|
Even if we implemented `listAll()`, we cannot retrieve documents in all statuses:
|
||
|
|
- Status 1 (IN_PREPARATION) - No list endpoint
|
||
|
|
- Status 3 (PRODUCTION_QUEUE) - No list endpoint
|
||
|
|
- Status 4 (PRINTING) - No list endpoint
|
||
|
|
- Status 5 (SENT) - No list endpoint
|
||
|
|
- Status 6 (CANCELED) - No list endpoint
|
||
|
|
|
||
|
|
A method named `listAll()` that doesn't actually list all documents would be misleading.
|
||
|
|
|
||
|
|
### 4. Convenience Layer Alternative Exists
|
||
|
|
|
||
|
|
Developers who need to combine results can do so explicitly:
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
import { fetchAllPages } from '@binect/js';
|
||
|
|
|
||
|
|
// Explicit composition - developer knows exactly what's happening
|
||
|
|
const [shippable, erroneous] = await Promise.all([
|
||
|
|
fetchAllPages((limit, offset) => client.documents.list({ limit, offset })),
|
||
|
|
fetchAllPages((limit, offset) => client.documents.listErrors({ limit, offset })),
|
||
|
|
]);
|
||
|
|
|
||
|
|
const allAvailable = [...shippable, ...erroneous];
|
||
|
|
```
|
||
|
|
|
||
|
|
This approach:
|
||
|
|
- Makes the multiple API calls explicit
|
||
|
|
- Allows parallel execution for better performance
|
||
|
|
- Gives developers control over error handling per endpoint
|
||
|
|
- Doesn't pretend to offer functionality the API doesn't support
|
||
|
|
|
||
|
|
## Consequences
|
||
|
|
|
||
|
|
### Positive
|
||
|
|
- SDK remains a transparent wrapper over the API
|
||
|
|
- No false promises about "all documents" when API doesn't support it
|
||
|
|
- Developers understand the actual API limitations
|
||
|
|
- Easier to maintain as SDK behavior matches API exactly
|
||
|
|
|
||
|
|
### Negative
|
||
|
|
- Developers must write slightly more code to combine results
|
||
|
|
- May be perceived as less convenient than competing SDKs
|
||
|
|
|
||
|
|
## Alternatives Considered
|
||
|
|
|
||
|
|
### Alternative 1: Add `listAll()` to Convenience Layer (helpers.ts)
|
||
|
|
|
||
|
|
We considered adding a helper function rather than a method:
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
export async function listAllDocuments(client: BinectClient): Promise<Document[]>
|
||
|
|
```
|
||
|
|
|
||
|
|
**Rejected because:** Even as a helper, the name "listAll" is misleading since it cannot list documents in statuses 1, 3, 4, 5, or 6. A more honest name would be `listShippableAndErroneous()`, which provides little value over explicit composition.
|
||
|
|
|
||
|
|
### Alternative 2: Add Status Filter to Existing Methods
|
||
|
|
|
||
|
|
We considered adding a status filter parameter to `list()`:
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
await client.documents.list({ status: [2, 7] });
|
||
|
|
```
|
||
|
|
|
||
|
|
**Rejected because:** The API doesn't support this. We would be inventing client-side filtering semantics that don't map to any API capability.
|
||
|
|
|
||
|
|
## References
|
||
|
|
|
||
|
|
- CLAUDE.md - Design Constraints section
|
||
|
|
- suggested-improvements.md - REQ-2: Add Method to List All Documents
|