diff --git a/CHANGELOG.md b/CHANGELOG.md index 980f07c..531f962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Document uploads: `upload_client`, `upload_invoice`, `upload_expense`, and + thirteen more, one for each resource with an `/{id}/upload` route. They + take a `Vec` and an optional `is_public`, and return the + refreshed record. Uploads are never retried, since a replay could attach + the file twice. This is the only way the API creates a document, so there + is no `create_document`. Enables reqwest's `multipart` feature, which adds + `mime`, `mime_guess`, and `unicase` and leaves the MSRV at 1.88. +- `FileUpload` and `Error::InvalidUpload`. An upload with no files, a nameless + file, or an unparseable MIME type is refused locally, because the server + answers an empty upload with a success that changed nothing. - A declared minimum supported Rust version of **1.88**, verified by running the test suite against it. The floor comes from the dependency graph (`reqwest` → `url` → `idna` → `icu_*`), not from this crate's own code. @@ -17,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 release. The advisory job fails visibly rather than silently; it is kept non-blocking through branch protection rather than `continue-on-error`. - Wrappers for every one of the API's 34 `Route::resource` groups, plus - `users` — 265 methods. See `COVERAGE.md` for the per-endpoint inventory. + `users` — 284 methods (including document uploads). See `COVERAGE.md` for the per-endpoint inventory. - `COVERAGE.md`, a detailed record of what is wrapped, what is not, and why, checked against `routes/api.php` on the `v5-stable` branch. - Retry with full-jitter exponential backoff, configurable through diff --git a/CLAUDE.md b/CLAUDE.md index 9e81625..3e482d9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,6 +161,18 @@ changes, doc tweaks) does not need an entry. elsewhere without checking `routes/api.php`. In particular the official PHP SDK puts bulk `download` on *every* resource, which is wrong; only PDF-rendering entities support it. +- `upload_methods!` generates `upload_{x}` for the 16 resources with a + `/{id}/upload` route; documents can only be created this way, because + `DocumentController::store` is an empty stub upstream (hence no + `create_document`). Three things are easy to get wrong, all verified + against the upstream source: the request is a **`POST` with + `_method=PUT`** (PHP does not parse multipart on a real `PUT`, and the + spec's `_method=POST` would 405); `is_public` goes out as **`1`/`0`** + (Laravel's strict `boolean` rule rejects the string `"true"`); and uploads + are **never retried**, because reqwest's multipart body is not cloneable + and `execute` gives an un-cloneable request exactly one attempt — a test + pins this, since nothing in the types would notice if it changed. Don't + "fix" any of the three. - Non-CRUD endpoints (documents, reports, charts, search, statics, emails, refunds, ping/health) are hand-written in the last `impl` block rather than macro-generated, because their request and response shapes are all @@ -224,16 +236,16 @@ changes, doc tweaks) does not need an entry. ## Coverage **All 34 of the API's `Route::resource` groups are wrapped**, plus `users` -(which isn't one). 265 methods: 30 resources with CRUD + bulk, 3 with CRUD -only, 4 with entity actions, and 25 hand-written. +(which isn't one). 284 methods: 30 resources with CRUD + bulk, 3 with CRUD +only, 4 with entity actions, 16 with document uploads, and 28 hand-written. Plus the non-CRUD surface: documents, entity actions and PDF downloads for the four action-capable resources, `report`, `chart`, `search`, `statics`, `send_email`, `refund_payment`, `ping`, and `health_check`. **`COVERAGE.md` is the detailed inventory** — every wrapped endpoint, every -gap, and the reason for each gap. The main remaining gaps are multipart -uploads (needs a reqwest feature), a handful of per-resource extras +gap, and the reason for each gap. The main remaining gaps are a handful of +per-resource extras (`clients/{id}/purge`, `company_gateways/{id}/test`, …), and the integration/auth routes. diff --git a/COVERAGE.md b/COVERAGE.md index 19fe74a..3726fac 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -18,7 +18,7 @@ declares 34 `Route::resource` groups and 274 explicit route declarations > exists or what it returns. **All 34 `Route::resource` groups are now wrapped**, plus `users` (which is -not a resource group). 265 methods in total. +not a resource group). 284 methods in total. ## Resources with full CRUD + bulk @@ -123,6 +123,54 @@ same reason: the response isn't JSON. Convenience wrappers: `mark_invoice_sent` and `mark_invoice_paid`. +## Document uploads + +`PUT /{resource}/{id}/upload` exists for 16 resources, each getting +`upload_{singular}(id, files, is_public)` that returns the refreshed record: + +| Resource | Method | +| --- | --- | +| Clients | `upload_client` | +| Companies | `upload_company` | +| Credits | `upload_credit` | +| Expenses | `upload_expense` | +| Group Settings | `upload_group_setting` | +| Invoices | `upload_invoice` | +| Payments | `upload_payment` | +| Products | `upload_product` | +| Projects | `upload_project` | +| Purchase Orders | `upload_purchase_order` | +| Quotes | `upload_quote` | +| Recurring Expenses | `upload_recurring_expense` | +| Recurring Invoices | `upload_recurring_invoice` | +| Recurring Quotes | `upload_recurring_quote` | +| Tasks | `upload_task` | +| Vendors | `upload_vendor` | + +This is the *only* way the API creates a document: +`DocumentController::store` is an empty stub upstream, so `Document` has +`list`, `get`, `update`, `delete`, `download`, and `bulk`, but no +`create_document`, and there will not be one. + +Three details that are easy to get wrong, all verified against the upstream +source rather than the published spec: + +- **The request is a `POST` carrying `_method=PUT`.** The route is + registered as `PUT`, but PHP does not parse a multipart body on a `PUT`, so + the files would never arrive. Laravel's kernel turns the spoofed method back + into a `PUT` before routing. The OpenAPI spec documents `POST` with + `_method=POST`, which would 405 against these routes. +- **`is_public` is sent as `1`/`0`.** Laravel's `boolean` rule accepts only + `true`, `false`, `0`, `1`, `'0'`, and `'1'` under strict comparison, so the + string `"true"` fails validation. +- **Uploads are never retried**, even on a 429. An upload adds a document, so + it is not idempotent whatever its verb, and a replay after a lost response + would attach the file twice. + +The server enforces file type and size limits; a refusal comes back as a 422 +`Error::Api` with the per-field `errors` map. Files are held in memory while +uploading. + ## Other wrapped endpoints | Endpoint | Method | @@ -166,14 +214,13 @@ that case the body is a short JSON message instead of CSV. ## Not covered -### Blocked on a dependency change +### Nothing is blocked on a dependency -**Multipart uploads** — `PUT /{resource}/{id}/upload` exists for clients, -companies, credits, expenses, group_settings, invoices, payments, products, -projects, purchase_orders, quotes, recurring_expenses, recurring_invoices, -recurring_quotes, tasks, and vendors. These need reqwest's `multipart` -feature, which isn't currently enabled. This is the main reason `Document` -has no `create_document`. +Multipart uploads used to be, and are now wrapped — see [Document +uploads](#document-uploads). One upload route is deliberately left out: +`PUT /edocument/upload` is not a document attachment. It imports an expense +from an e-document (`ExpenseController::edocument`), and is better treated as +an import endpoint alongside the others under *Import/export* below. ### Resource groups not wrapped diff --git a/Cargo.lock b/Cargo.lock index fc8c8dd..85398e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -600,6 +600,22 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "mime" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" + +[[package]] +name = "mime_guess" +version = "2.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7c44f8e672c00fe5308fa235f821cb4198414e1c77935c1ab6948d3fd78550e" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "mio" version = "1.2.4" @@ -776,6 +792,7 @@ dependencies = [ "base64 0.22.1", "bytes", "futures-core", + "futures-util", "http", "http-body", "http-body-util", @@ -784,6 +801,7 @@ dependencies = [ "hyper-util", "js-sys", "log", + "mime_guess", "native-tls", "percent-encoding", "pin-project-lite", @@ -1163,6 +1181,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "unicase" +version = "2.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "357cc3acc6a036009fd6c973ed009037c732d60d0b4f6c673e9041497482a28f" + [[package]] name = "unicode-ident" version = "1.0.27" diff --git a/Cargo.toml b/Cargo.toml index d9f3c67..0ff9fe1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ license = "MIT" repository = "https://github.com/awkawb1/invoiceninja-sdk" [dependencies] -reqwest = { version = "0.12", default-features = false, features = ["json", "native-tls"] } +reqwest = { version = "0.12", default-features = false, features = ["json", "native-tls", "multipart"] } serde = { version = "1", features = ["derive"] } serde_json = "1" thiserror = "2" diff --git a/GOALS.md b/GOALS.md index 18db010..2a31ecc 100644 --- a/GOALS.md +++ b/GOALS.md @@ -141,11 +141,16 @@ All 34 `Route::resource` groups are wrapped. Full coverage is the target, so `COVERAGE.md`'s gap list is a to-do list, not a permanent boundary. Roughly in order of value over effort: -1. **Multipart uploads** — 16 `PUT /{resource}/{id}/upload` routes plus - `edocument/upload`. Blocked only on enabling reqwest's `multipart` - feature. This is also what makes `create_document` possible, so it - unblocks the one obviously missing piece of an otherwise complete - resource. +**Done:** multipart document uploads, for the 16 resources that have the +route (`upload_client`, `upload_invoice`, …). This earlier claimed it would +also make `create_document` possible, which was wrong: upstream +`DocumentController::store` is an empty stub, and an upload is the *only* +way a document is created. There is no `create_document` and there will not +be one. `edocument/upload` was left out because it imports an expense from +an e-document rather than attaching a file, so it belongs with item 6. + +Still to do: + 2. **Per-resource extras** — mechanical, and they reuse existing patterns: `webhooks/{id}/retry`, `tasks/sort`, `designs/set/default`, `client_gateway_tokens/{id}/setAsDefault`, `subscriptions/steps`, the @@ -159,9 +164,10 @@ in order of value over effort: 5. **Public invitation-key downloads** — eight routes that take no auth token, so they do not fit the authenticated request path and need a different entry point. -6. **Import/export and the integrations** — Stripe, QuickBooks, einvoice - (17 routes), Yodlee, Nordigen, calendar. The largest block and the least - SDK-shaped; worth doing last and possibly in pieces. +6. **Import/export and the integrations** — `/import`, `/export`, + `edocument/upload`, Stripe, QuickBooks, einvoice (17 routes), Yodlee, + Nordigen, calendar. The largest block and the least SDK-shaped; worth + doing last and possibly in pieces. Keep `COVERAGE.md` current as these land — it names exact counts, so it goes stale quietly. @@ -178,9 +184,12 @@ These are genuinely undecided. Deciding one is a change to this file. eleven chart endpoints share no response shape. Per-chart structs would be eleven small models of unstable payloads; unclear whether that earns its keep. -- **Large responses are buffered entirely into memory.** PDF downloads and - CSV report exports go through `Vec`. Streaming would be better for - big exports but changes those signatures. +- **Large bodies are buffered entirely into memory.** PDF downloads and CSV + report exports come back as `Vec`, and an uploaded `FileUpload` holds + its whole contents until the request finishes. Streaming would be better + for big files in both directions but changes those signatures — and for + uploads it would also change the retry story, which currently depends on + the multipart body being un-cloneable. - **`Error::PaginationLimit` has no test**, because reaching it needs 10,001 mock round trips. Making `MAX_PAGES` injectable purely for a test may be worse than the gap. diff --git a/README.md b/README.md index 4580bf7..11b1f03 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ IN_API_TOKEN=... cargo run --example list_clients ## Coverage -Every one of the API's 34 resource groups, plus users — 265 methods. Each +Every one of the API's 34 resource groups, plus users — 284 methods. Each resource has `list`, `list_all`, `get`, `create`, `update`, `delete`, and (where the API offers one) `bulk`: @@ -158,15 +158,33 @@ let pdf: Vec = client.download_invoice("abc123").await?; let merged: Vec = client.download_invoices(&["abc123", "def456"]).await?; ``` -Reports return their raw export body (CSV for most of them), and the -schemaless endpoints decode into any type you choose: +Documents are attached through the record they belong to, since that is the +only way the API creates one. Sixteen resources support it, as +`upload_client`, `upload_invoice`, `upload_expense`, and so on: + +```rust +use invoiceninjars::FileUpload; + +let receipt = FileUpload::new("receipt.pdf", std::fs::read("receipt.pdf")?) + .mime("application/pdf"); + +// `Some(true)` makes the document visible to the client in the portal. +let expense = client.upload_expense("abc123", vec![receipt], Some(false)).await?; +``` + +Files are held in memory while uploading, and an upload is never retried, +since a replay after a lost response would attach the file twice. + +Reports return their raw export body (CSV for most of them). `statics`, +`search`, and `health_check` return typed results; `chart` has no common +response shape, so it decodes into any type you choose: ```rust use serde_json::{json, Value}; let csv: Vec = client.report("invoices", &json!({"date_range": "this_year"})).await?; let totals: Value = client.chart("totals_v2", &json!({})).await?; -let statics: Value = client.statics().await?; +let statics = client.statics().await?; ``` Add more resources by adding a model under `src/models/` and a diff --git a/src/client.rs b/src/client.rs index 2b08900..188ee26 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,7 +1,7 @@ use std::time::Duration; use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC}; -use reqwest::{header, Method, StatusCode}; +use reqwest::{header, multipart, Method, StatusCode}; use serde::{de::DeserializeOwned, Serialize}; use crate::error::{ApiErrorBody, Error, Result}; @@ -14,6 +14,7 @@ use crate::models::{ TaskScheduler, TaskStatus, TaxRate, User, Vendor, Webhook, }; use crate::retry::{self, RetryPolicy}; +use crate::upload::FileUpload; /// API root of the hosted Invoice Ninja app. pub const HOSTED_BASE_URL: &str = "https://invoicing.co/api/v1"; @@ -220,8 +221,16 @@ impl InvoiceNinjaClient { /// configured [`RetryPolicy`]. /// /// Every helper goes through here, so retrying is uniform. The request is - /// cloned before each attempt; a body that cannot be cloned (none of ours - /// today, since all payloads are in-memory JSON) simply gets one attempt. + /// cloned before each attempt; a body that cannot be cloned simply gets + /// one attempt. + /// + /// Multipart uploads rely on that. `reqwest` builds their body as a + /// stream, so `try_clone` returns `None` and they are never replayed — + /// not even on a 429. That is wanted: an upload *adds* a document, so it + /// is not idempotent whatever its HTTP verb, and a replay after a lost + /// response would attach the file twice. A test pins this, because + /// nothing in the types would stop a future change from making the body + /// cloneable and silently breaking it. /// /// Note what is *not* retried: a `POST` that fails with a 5xx or times /// out is surfaced, because the server may already have applied it and a @@ -545,6 +554,44 @@ impl InvoiceNinjaClient { Self::decode(response).await } + /// Attaches files to a record through its `/upload` route. + /// + /// The route is registered as `PUT`, but PHP does not parse a multipart + /// body on a `PUT`, so the files would never arrive. The working form is + /// a `POST` carrying `_method=PUT`, which Laravel's kernel turns back + /// into a `PUT` before routing. The published OpenAPI spec documents + /// `POST` with `_method=POST`, which would 405 against these routes. + /// + /// `is_public` goes out as `1`/`0`: Laravel's `boolean` rule accepts only + /// `true`, `false`, `0`, `1`, `'0'` and `'1'`, so the string `"true"` + /// fails validation. + async fn upload_files( + &self, + path: &str, + files: Vec, + is_public: Option, + ) -> Result { + // The server answers an upload with no files with a success that + // changed nothing, which is exactly the quiet failure to avoid. + if files.is_empty() { + return Err(Error::InvalidUpload("no files to upload".into())); + } + + let mut form = multipart::Form::new().text("_method", "PUT"); + if let Some(is_public) = is_public { + form = form.text("is_public", if is_public { "1" } else { "0" }); + } + for file in files { + form = form.part("documents[]", file.into_part()?); + } + + let response = self + .execute(self.req(Method::POST, path).multipart(form)) + .await?; + let body: ItemResponse = Self::decode(response).await?; + Ok(body.data) + } + /// Issues a request whose body is discarded, failing on non-2xx. async fn get_discard(&self, path: &str) -> Result<()> { self.get_raw(path).await.map(|_| ()) @@ -609,6 +656,34 @@ macro_rules! resource_methods { }; } +/// Generates `upload_{x}`, which attaches files through +/// `PUT /{resource}/{id}/upload`. Sixteen resources expose this route; check +/// `routes/api.php` before adding one. +/// +/// The upload route is how a document is created at all — +/// `DocumentController::store` is an empty stub upstream, which is why there +/// is no `create_document`. +macro_rules! upload_methods { + ($upload:ident, $path:literal, $model:ty) => { + /// Attaches files to the record and returns it as refreshed by the + /// server. `is_public` sets whether the documents are visible to the + /// client in the portal; `None` leaves the server default. + /// + /// The server enforces file type and size limits, which surface as a + /// 422 [`Error::Api`](crate::Error::Api). Uploads are never retried, + /// since a replay could attach the file twice. + pub async fn $upload( + &self, + id: &str, + files: Vec, + is_public: Option, + ) -> Result<$model> { + self.upload_files(&format!("{}/{}/upload", $path, seg(id)?), files, is_public) + .await + } + }; +} + /// Generates the `GET /{resource}/{id}/{action}` helpers. Only invoices, /// quotes, credits, and purchase orders expose this route. macro_rules! entity_action_methods { @@ -977,6 +1052,33 @@ impl InvoiceNinjaClient { .await } + // --- document uploads ----------------------------------------------- + + upload_methods!(upload_client, "clients", ClientModel); + upload_methods!(upload_company, "companies", Company); + upload_methods!(upload_credit, "credits", Credit); + upload_methods!(upload_expense, "expenses", Expense); + upload_methods!(upload_group_setting, "group_settings", GroupSetting); + upload_methods!(upload_invoice, "invoices", Invoice); + upload_methods!(upload_payment, "payments", Payment); + upload_methods!(upload_product, "products", Product); + upload_methods!(upload_project, "projects", Project); + upload_methods!(upload_purchase_order, "purchase_orders", PurchaseOrder); + upload_methods!(upload_quote, "quotes", Quote); + upload_methods!( + upload_recurring_expense, + "recurring_expenses", + RecurringExpense + ); + upload_methods!( + upload_recurring_invoice, + "recurring_invoices", + RecurringInvoice + ); + upload_methods!(upload_recurring_quote, "recurring_quotes", RecurringQuote); + upload_methods!(upload_task, "tasks", Task); + upload_methods!(upload_vendor, "vendors", Vendor); + // --- entity actions with non-entity responses ----------------------- /// Converts an invoice into a purchase order. Separate from diff --git a/src/error.rs b/src/error.rs index fcf108d..988e205 100644 --- a/src/error.rs +++ b/src/error.rs @@ -54,6 +54,14 @@ pub enum Error { #[error("`{0}` cannot be used as a url path segment")] InvalidPathSegment(String), + /// An upload was refused before any request was sent: no files, a file + /// with no name, or a MIME type that does not parse. + /// + /// Raised locally because the server would otherwise answer an empty + /// upload with a success that changed nothing. + #[error("invalid upload: {0}")] + InvalidUpload(String), + /// A `list_all_*` call kept receiving full pages past the point where any /// plausible result set would have ended. /// diff --git a/src/lib.rs b/src/lib.rs index 6833e15..b3c64a6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,9 @@ mod client; mod error; pub mod models; mod retry; +mod upload; pub use client::{InvoiceNinjaClient, DEFAULT_TIMEOUT, HOSTED_BASE_URL}; pub use error::{ApiErrorBody, Error, Result}; pub use retry::RetryPolicy; +pub use upload::FileUpload; diff --git a/src/upload.rs b/src/upload.rs new file mode 100644 index 0000000..8becba6 --- /dev/null +++ b/src/upload.rs @@ -0,0 +1,70 @@ +use std::fmt; + +use reqwest::multipart::Part; + +use crate::error::{Error, Result}; + +/// A file to attach to a record with the `upload_*` methods. +/// +/// The contents are held in memory, so a large file costs its full size in +/// RAM for as long as the upload takes. Streaming uploads are an open +/// question in `GOALS.md`. +/// +/// ``` +/// use invoiceninjars::FileUpload; +/// +/// let file = FileUpload::new("receipt.pdf", b"%PDF-1.4 ...".to_vec()) +/// .mime("application/pdf"); +/// ``` +#[derive(Clone)] +pub struct FileUpload { + file_name: String, + bytes: Vec, + mime: Option, +} + +impl FileUpload { + /// `file_name` is what the server records and shows against the + /// document, so it should carry the right extension. + pub fn new(file_name: impl Into, bytes: impl Into>) -> Self { + Self { + file_name: file_name.into(), + bytes: bytes.into(), + mime: None, + } + } + + /// Sets the part's `Content-Type`. Optional: without it the part carries + /// none, and the server decides from the file's contents. + pub fn mime(mut self, mime: impl Into) -> Self { + self.mime = Some(mime.into()); + self + } + + pub(crate) fn into_part(self) -> Result { + if self.file_name.trim().is_empty() { + return Err(Error::InvalidUpload( + "a file needs a non-empty file name".into(), + )); + } + + let part = Part::bytes(self.bytes).file_name(self.file_name); + match self.mime { + Some(mime) => part + .mime_str(&mime) + .map_err(|_| Error::InvalidUpload(format!("`{mime}` is not a valid MIME type"))), + None => Ok(part), + } + } +} + +/// Elides the contents: the derived impl would print the entire file. +impl fmt::Debug for FileUpload { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FileUpload") + .field("file_name", &self.file_name) + .field("len", &self.bytes.len()) + .field("mime", &self.mime) + .finish() + } +} diff --git a/tests/client_requests.rs b/tests/client_requests.rs index 7e63ca6..8343343 100644 --- a/tests/client_requests.rs +++ b/tests/client_requests.rs @@ -1176,3 +1176,277 @@ async fn no_budget_means_only_max_retries_bounds_the_call() { .expect_err("still fails"); assert_eq!(server.received_requests().await.unwrap().len(), 5); } + +// --- document uploads --------------------------------------------------- + +use invoiceninjars::FileUpload; + +fn pdf(name: &str) -> FileUpload { + FileUpload::new(name, b"%PDF-1.4 fake".to_vec()).mime("application/pdf") +} + +fn body_text(request: &wiremock::Request) -> String { + String::from_utf8_lossy(&request.body).into_owned() +} + +async fn upload_ok_server(route: &str) -> MockServer { + let server = MockServer::start().await; + Mock::given(path(route)) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "data": { "id": "abc" } }))) + .mount(&server) + .await; + server +} + +/// The route is registered as PUT, but PHP does not parse multipart on a PUT, +/// so the files would never arrive. The working form is POST with a +/// `_method=PUT` field, which Laravel turns back into a PUT before routing. +#[tokio::test] +async fn upload_is_a_post_that_spoofs_put_with_documents_as_an_array() { + let server = upload_ok_server("/clients/abc/upload").await; + + let updated = client(&server) + .upload_client("abc", vec![pdf("receipt.pdf")], None) + .await + .expect("upload should succeed"); + assert_eq!(updated.id.as_deref(), Some("abc"), "returns the entity"); + + let sent = &server.received_requests().await.unwrap()[0]; + assert_eq!( + sent.method.as_str(), + "POST", + "a real PUT would drop the files" + ); + let content_type = sent.headers.get("content-type").unwrap().to_str().unwrap(); + assert!( + content_type.starts_with("multipart/form-data; boundary="), + "got {content_type}" + ); + assert_eq!( + sent.headers.get("x-api-token").unwrap().to_str().unwrap(), + "test-token" + ); + + let body = body_text(sent); + assert!(body.contains("name=\"_method\"\r\n\r\nPUT"), "body: {body}"); + assert!( + body.contains("name=\"documents[]\"; filename=\"receipt.pdf\""), + "body: {body}" + ); + assert!(body.contains("Content-Type: application/pdf")); + assert!( + body.contains("%PDF-1.4 fake"), + "file bytes must be in the body" + ); +} + +/// Laravel's `boolean` rule accepts only true, false, 0, 1, '0' and '1' under +/// strict comparison, so the string "true" fails validation. +#[tokio::test] +async fn is_public_is_sent_as_one_or_zero_never_true_false() { + for (flag, expected) in [ + (Some(true), Some("1")), + (Some(false), Some("0")), + (None, None), + ] { + let server = upload_ok_server("/invoices/abc/upload").await; + client(&server) + .upload_invoice("abc", vec![pdf("a.pdf")], flag) + .await + .expect("upload should succeed"); + + let body = body_text(&server.received_requests().await.unwrap()[0]); + match expected { + Some(value) => assert!( + body.contains(&format!("name=\"is_public\"\r\n\r\n{value}\r\n")), + "is_public={flag:?} should send {value}, body: {body}" + ), + None => assert!( + !body.contains("is_public"), + "an unset flag must be omitted, not defaulted, body: {body}" + ), + } + assert!(!body.contains("true") && !body.contains("false")); + } +} + +#[tokio::test] +async fn several_files_become_several_document_parts() { + let server = upload_ok_server("/expenses/abc/upload").await; + client(&server) + .upload_expense("abc", vec![pdf("one.pdf"), pdf("two.pdf")], None) + .await + .expect("upload should succeed"); + + let body = body_text(&server.received_requests().await.unwrap()[0]); + assert_eq!(body.matches("name=\"documents[]\"").count(), 2); + assert!(body.contains("filename=\"one.pdf\"") && body.contains("filename=\"two.pdf\"")); +} + +/// An upload with nothing in it is answered with a success that changed +/// nothing; that is caught locally instead. +#[tokio::test] +async fn invalid_uploads_are_rejected_before_any_request() { + let server = upload_ok_server("/clients/abc/upload").await; + let c = client(&server); + + let none = c.upload_client("abc", vec![], None).await.unwrap_err(); + assert!(matches!(none, Error::InvalidUpload(_)), "got {none:?}"); + + let nameless = c + .upload_client("abc", vec![FileUpload::new(" ", b"x".to_vec())], None) + .await + .unwrap_err(); + assert!( + matches!(nameless, Error::InvalidUpload(_)), + "got {nameless:?}" + ); + + let bad_mime = c + .upload_client( + "abc", + vec![FileUpload::new("a.txt", b"x".to_vec()).mime("not a mime")], + None, + ) + .await + .unwrap_err(); + assert!( + matches!(bad_mime, Error::InvalidUpload(_)), + "got {bad_mime:?}" + ); + + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test] +async fn upload_ids_get_the_same_path_protection() { + let server = upload_ok_server("/clients/x/upload").await; + let err = client(&server) + .upload_client("..", vec![pdf("a.pdf")], None) + .await + .unwrap_err(); + assert!(matches!(err, Error::InvalidPathSegment(_)), "got {err:?}"); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +/// An upload ADDS a document, so it is not idempotent whatever the HTTP verb. +/// A replay after a lost response would attach the file twice. This holds +/// only because reqwest's multipart body cannot be cloned, which nothing in +/// the types enforces, so it is pinned here for 5xx and 429 alike. +#[tokio::test] +async fn uploads_are_never_replayed() { + for status in [500u16, 503, 429] { + let server = MockServer::start().await; + Mock::given(path("/clients/abc/upload")) + .respond_with(ResponseTemplate::new(status)) + .mount(&server) + .await; + + let err = client(&server) + .with_retry_policy(quick_retry()) + .upload_client("abc", vec![pdf("a.pdf")], None) + .await + .expect_err("the failure should surface"); + assert!( + matches!(err, Error::Api { status: s, .. } if s == status), + "got {err:?}" + ); + assert_eq!( + server.received_requests().await.unwrap().len(), + 1, + "a {status} on an upload must not be retried" + ); + } +} + +#[tokio::test] +async fn upload_validation_errors_keep_the_field_map() { + let server = MockServer::start().await; + Mock::given(path("/clients/abc/upload")) + .respond_with(ResponseTemplate::new(422).set_body_json(json!({ + "message": "The given data was invalid.", + "errors": { "documents.0": ["The documents.0 must be a file of type: pdf."] } + }))) + .mount(&server) + .await; + + let err = client(&server) + .upload_client("abc", vec![pdf("a.exe")], None) + .await + .unwrap_err(); + match err { + Error::Api { status, errors, .. } => { + assert_eq!(status, 422); + assert!(errors.contains_key("documents.0")); + } + other => panic!("expected Error::Api, got {other:?}"), + } +} + +#[test] +fn file_upload_debug_never_prints_the_contents() { + let file = FileUpload::new("secret.pdf", b"CONFIDENTIAL-CONTENTS".to_vec()); + let rendered = format!("{file:?}"); + assert!(!rendered.contains("CONFIDENTIAL"), "got {rendered}"); + assert!(rendered.contains("secret.pdf") && rendered.contains("21")); +} + +/// The sixteen methods come from one macro, so a typo in a path or a wrong +/// resource name would only show up here. +#[tokio::test] +async fn every_upload_method_targets_its_own_route() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "data": {} }))) + .mount(&server) + .await; + let c = client(&server); + let f = || vec![pdf("a.pdf")]; + + c.upload_client("i", f(), None).await.unwrap(); + c.upload_company("i", f(), None).await.unwrap(); + c.upload_credit("i", f(), None).await.unwrap(); + c.upload_expense("i", f(), None).await.unwrap(); + c.upload_group_setting("i", f(), None).await.unwrap(); + c.upload_invoice("i", f(), None).await.unwrap(); + c.upload_payment("i", f(), None).await.unwrap(); + c.upload_product("i", f(), None).await.unwrap(); + c.upload_project("i", f(), None).await.unwrap(); + c.upload_purchase_order("i", f(), None).await.unwrap(); + c.upload_quote("i", f(), None).await.unwrap(); + c.upload_recurring_expense("i", f(), None).await.unwrap(); + c.upload_recurring_invoice("i", f(), None).await.unwrap(); + c.upload_recurring_quote("i", f(), None).await.unwrap(); + c.upload_task("i", f(), None).await.unwrap(); + c.upload_vendor("i", f(), None).await.unwrap(); + + let paths: Vec = server + .received_requests() + .await + .unwrap() + .iter() + .map(|r| r.url.path().to_string()) + .collect(); + assert_eq!( + paths, + [ + "clients", + "companies", + "credits", + "expenses", + "group_settings", + "invoices", + "payments", + "products", + "projects", + "purchase_orders", + "quotes", + "recurring_expenses", + "recurring_invoices", + "recurring_quotes", + "tasks", + "vendors", + ] + .map(|resource| format!("/{resource}/i/upload")) + ); +}