From 3e4357c02b5691f86ea76bfc24e6ef5e0b468b45 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Wed, 24 Jun 2026 15:58:58 +0200 Subject: [PATCH 01/14] Auto-flags multi-param serialise as :synthetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DSL auto-applied :synthetic when a parse block (or mapper #parse) declared 2+ parameters, but did not do the same for the symmetric multi-param serialise case. Both shapes produce an attribute whose value does not live at @model_attributes[model_name]; only the merge side was being marked. Until now the flag had no readers in lib, so the asymmetry was invisible. It becomes visible the moment any code path branches on synthetic? — applying the flag now keeps that future code correct. --- lib/rest_easy/resource.rb | 2 ++ spec/rest_easy/resource_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index eb44e60..e414cd0 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -250,6 +250,7 @@ def attr(name_or_mapping, *args, &block) serialise_params = serialise_block.parameters.select { |ptype, _| ptype == :opt || ptype == :req } if serialise_params.length > 1 + flags << :synthetic unless flags.include?(:synthetic) target_fields = serialise_params.map { |_, pname| pname } end elsif block @@ -284,6 +285,7 @@ def attr(name_or_mapping, *args, &block) if serialise_block params = serialise_block.parameters.select { |ptype, _| ptype == :opt || ptype == :req } if params.length > 1 + flags << :synthetic unless flags.include?(:synthetic) target_fields = params.map { |_, pname| pname } end end diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 44ac7e9..4cf7779 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -484,6 +484,11 @@ def self.serialise(street, city) expect(attr_def.target_fields).to eq([:street, :city]) end + it "auto-detects synthetic from mapper serialise parameter count" do + attr_def = @resource_class.all_attribute_definitions[:address] + expect(attr_def.synthetic?).to be true + end + it "gathers model values by param names for serialise" do instance = @resource_class.stub(street: "Main St", city: "Stockholm", address: "ignored") serialised = instance.serialise @@ -749,6 +754,20 @@ def self.serialise(street, city) attr_def = resource_class.all_attribute_definitions[:address] expect(attr_def.target_fields).to eq([:street, :city]) end + + it "auto-detects synthetic from serialise block parameter count" do + resource_class = Class.new(described_class) do + attr :street, String + attr :city, String + + attr :address, String do + serialise { |street, city| "#{street}, #{city}" } + end + end + + attr_def = resource_class.all_attribute_definitions[:address] + expect(attr_def.synthetic?).to be true + end end # ── Resource-level hooks ─────────────────────────────────────────────── From 00233fe22194296f9050f9eac2b4c3e797799137 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Mon, 15 Jun 2026 14:16:36 +0200 Subject: [PATCH 02/14] Enforces :required on serialise as well as parse Previously the flag raised MissingAttributeError only when an API response omitted the field; it was silently ignored on save, letting incomplete payloads reach the backend. Serialise now raises before any HTTP request when a required attribute is nil, with the attribute name on the exception. --- CHANGELOG.md | 11 +++ README.md | 28 +++---- lib/rest_easy/resource.rb | 4 + spec/rest_easy/resource_spec.rb | 129 ++++++++++++++++++++++++++++++-- 4 files changed, 152 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92023c3..b31c133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## [Unreleased] +### Fixed + +- **`:required` is now enforced on serialise as well as parse.** Previously + the flag raised `MissingAttributeError` only when an API response omitted + the field; it was silently ignored when sending (`save`), letting + incomplete payloads reach the backend and surface as a generic + `RequestError`. The flag now also fires at `serialise` time (and therefore + on `save`) when a required attribute is `nil`, before any HTTP request, + with the attribute name on the exception (`#attribute_name`). Inbound + behavior is unchanged — `parse` still raises on missing required fields. + ## [1.3.1] - 2026-05-27 ### Fixed diff --git a/README.md b/README.md index 17cf3aa..88ef41f 100644 --- a/README.md +++ b/README.md @@ -92,15 +92,15 @@ end ### Available settings -| Setting | Default | Description | -|----------------------------------|----------------------------|--------------------------------------------------------------------------------------------| -| `authentication` | `Auth::Null.new` | Authentication strategy | -| `base_url` | `"https://example.com"` | Base URL for all requests | -| `conversions.json_attributes` | `:PascalCase` | Naming convention for JSON response/request fields | -| `conversions.query_parameters` | `nil` (no transformation) | Naming convention for query parameter keys | +| Setting | Default | Description | +|----------------------------------|----------------------------|-------------------------------------------------------------------------------------------------| +| `authentication` | `Auth::Null.new` | Authentication strategy | +| `base_url` | `"https://example.com"` | Base URL for all requests | +| `conversions.json_attributes` | `:PascalCase` | Naming convention for JSON response/request fields | +| `conversions.query_parameters` | `nil` (no transformation) | Naming convention for query parameter keys | | `log_bodies` | `false` | When `true`, request/response bodies are logged. Off by default to avoid leaking domain secrets | -| `logger` | `nil` | When set, attaches Faraday's logger middleware and writes HTTP request/response details to it | -| `max_retries` | `3` | Retry count on request failure | +| `logger` | `nil` | When set, attaches Faraday's logger middleware and writes HTTP request/response details to it | +| `max_retries` | `3` | Retry count on request failure | ### Logging HTTP traffic @@ -326,12 +326,12 @@ attr [:tax_url, '@urlTaxReductionList'], String, :read_only ### Flags -| Flag | Effect | -|--------------|----------------------------------------------------------| -| `:required` | Raises `MissingAttributeError` if absent in API response | -| `:optional` | Documents that the field may be absent (default) | -| `:read_only` | Excluded from serialisation (not sent back to the API) | -| `:key` | Marks the unique identifier for CRUD operations | +| Flag | Effect | +|--------------|------------------------------------------------------------------------| +| `:required` | Raises `MissingAttributeError` if missing on `save` or in API response | +| `:optional` | Documents that the field may be absent (default) | +| `:read_only` | Excluded from serialisation (not sent back to the API) | +| `:key` | Marks the unique identifier for CRUD operations | ```ruby key :id, Integer, :read_only diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index e414cd0..717f886 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -595,6 +595,10 @@ def serialise next if attr_def.read_only? value = @model_attributes[attr_def.model_name] + if attr_def.required? && !attr_def.synthetic? && value.nil? + raise MissingAttributeError.new(attr_def.model_name) + end + if attr_def.target_fields.any? # Multi-param serialise: gather model values by param names, splat into block model_values = attr_def.target_fields.map { |fn| @model_attributes[fn] } diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 4cf7779..42efbfd 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -320,14 +320,131 @@ def serialise(model_name) end describe "attribute flags" do - it "supports :required flag" do - resource_class = Class.new(described_class) do - attr :name, String, :required + describe ":required flag" do + let(:resource_class) do + Class.new(described_class) do + attr :name, String, :required + attr :amount, Float + end end - expect { - resource_class.parse({}) - }.to raise_error(RestEasy::MissingAttributeError) + context "on parse (inbound)" do + it "raises MissingAttributeError when a :required attribute is absent from the API response" do + expect { + resource_class.parse({}) + }.to raise_error(RestEasy::MissingAttributeError) + end + + it "names the missing attribute on the error" do + expect { + resource_class.parse({}) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:name) + } + end + end + + context "on stub (build)" do + it "does not raise when a :required attribute is omitted" do + expect { + resource_class.stub(amount: 100.0) + }.not_to raise_error + end + + it "does not raise when a :required attribute is set to nil" do + expect { + resource_class.stub(name: nil, amount: 100.0) + }.not_to raise_error + end + end + + context "on serialise (outbound)" do + it "raises MissingAttributeError when a :required attribute is nil" do + instance = resource_class.stub(amount: 100.0) + + expect { + instance.serialise + }.to raise_error(RestEasy::MissingAttributeError) + end + + it "names the missing attribute on the error" do + instance = resource_class.stub(amount: 100.0) + + expect { + instance.serialise + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:name) + } + end + + it "does not raise when all :required attributes are present" do + instance = resource_class.stub(name: "Acme", amount: 100.0) + + expect { instance.serialise }.not_to raise_error + end + + it "raises when a parsed instance has been mutated to clear a :required attribute" do + instance = resource_class.parse({ "Name" => "Acme", "Amount" => 100.0 }) + mutated = instance.update(name: nil) + + expect { + mutated.serialise + }.to raise_error(RestEasy::MissingAttributeError) + end + + it "does not enforce :required on :read_only attributes (never sent)" do + read_only_required = Class.new(described_class) do + key :id, Integer, :read_only, :required + attr :name, String + end + + instance = read_only_required.parse({ "Id" => 1, "Name" => "Acme" }) + + expect { instance.serialise }.not_to raise_error + end + end + + context "on save (no HTTP traffic on incomplete payload)" do + before(:all) do + module RequiredFlagTestApi + extend RestEasy + + configure do |config| + config.base_url = "https://api.example.com/v1" + end + end + + class RequiredFlagTestApi::Invoice < RestEasy::Resource + configure do + path "invoices" + end + + key :document_number, Integer, :read_only + attr :customer_number, String, :required + attr :amount, Float + end + end + + after(:all) do + Object.send(:remove_const, :RequiredFlagTestApi) + end + + it "raises MissingAttributeError before issuing an HTTP request" do + instance = RequiredFlagTestApi::Invoice.stub(amount: 300.0) + + allow(RequiredFlagTestApi::Invoice).to receive(:post) + allow(RequiredFlagTestApi::Invoice).to receive(:put) + + expect { + RequiredFlagTestApi::Invoice.save(instance) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:customer_number) + } + + expect(RequiredFlagTestApi::Invoice).not_to have_received(:post) + expect(RequiredFlagTestApi::Invoice).not_to have_received(:put) + end + end end it "supports :optional flag" do From 3ca19eda7f741ab3e0574d14c6ecd31c5cade9be Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Thu, 25 Jun 2026 08:52:22 +0200 Subject: [PATCH 03/14] Enforces :required on synthetic attributes For attributes with multi-param parse or serialise, :required was a silent no-op: the parse-side check sat inside the non-source_fields branch, and the serialise-side check explicitly bypassed any attr flagged :synthetic. The flag's intent ("don't accept partial data") was unenforceable for the very shapes most likely to depend on it. Semantic: marking a synthetic attribute :required means all of its underlying fields must be present. * Merge (multi-param parse): all source API fields must be in the response on parse; if round-trippable, the merged model value must additionally be non-nil at serialise time. * Combine (multi-param serialise): all named model attributes must be non-nil at serialise time. Parse-side check does not apply. * Split (single-param bare block extracting from one API field): the underlying API field must be present on parse. :read_only continues to skip the serialise-time check, matching the single-field semantics. Documents the previously-undocumented combine pattern (multi-param serialise) in the README, alongside merge and split. --- CHANGELOG.md | 16 +++++ README.md | 27 ++++++++- lib/rest_easy/resource.rb | 15 ++++- spec/rest_easy/resource_spec.rb | 100 ++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b31c133..e8d00bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,22 @@ on `save`) when a required attribute is `nil`, before any HTTP request, with the attribute name on the exception (`#attribute_name`). Inbound behavior is unchanged — `parse` still raises on missing required fields. +- **`:required` is now enforced on synthetic attributes (merge / combine / + split patterns).** Previously the flag was silently a no-op for any + attribute with a multi-parameter parse or serialise block. It now + enforces presence on all underlying API or model fields: a merge + attribute requires every source API field on parse; a combine attribute + requires every named model attribute on serialise; a split attribute + requires the underlying API field on parse. `:read_only` continues to + skip the serialise-time check. + +### Changed + +- **The DSL auto-applies `:synthetic` to multi-parameter serialise blocks** + (and mapper `serialise` methods), mirroring the existing behavior for + multi-parameter parse. Previously only the parse side was tagged, + leaving the flag inconsistent with its intent of marking attributes whose + storage shape diverges from the standard one-slot layout. ## [1.3.1] - 2026-05-27 diff --git a/README.md b/README.md index 88ef41f..f7c5bb8 100644 --- a/README.md +++ b/README.md @@ -328,7 +328,7 @@ attr [:tax_url, '@urlTaxReductionList'], String, :read_only | Flag | Effect | |--------------|------------------------------------------------------------------------| -| `:required` | Raises `MissingAttributeError` if missing on `save` or in API response | +| `:required` | Raises `MissingAttributeError` if absent in the API response (on parse) or `nil` at serialise time (`save`, `to_api`). See [Merge](#merge-pattern--many-api-fields-into-one-model-attribute), [Combine](#combine-pattern--many-model-attributes-into-one-api-field), and [Split](#split-pattern--one-api-field-into-many-model-attributes) patterns for how this applies to synthetic attributes. | | `:optional` | Documents that the field may be absent (default) | | `:read_only` | Excluded from serialisation (not sent back to the API) | | `:key` | Marks the unique identifier for CRUD operations | @@ -430,6 +430,29 @@ end attr :full_name, String, FullNameMapper ``` +Marking a merge attribute `:required` enforces that **all** underlying API fields are present in the response on parse. If the attribute is also round-trippable (not `:read_only`), the merged model value must additionally be non-`nil` at serialise time. + +Note that string-concatenation merges like the `full_name` example above are inherently lossy on the round-trip — splitting `"Hans Erik Nilsson"` back into `FirstName`/`LastName` is ambiguous. Such attributes should normally be `:read_only`. Round-trippable merges work when the model representation preserves the structure (e.g. a tuple, a `Money` value object, or a `Date` built from year/month/day components). + +### Combine pattern — many model attributes into one API field + +The dual of merge: when the serialise method takes multiple parameters, RestEasy gathers the values from the corresponding model attributes and passes them in. The block returns the single combined API value: + +```ruby +attr :street, String +attr :city, String + +attr :address, String do + serialise { |street, city| "#{street}, #{city}" } +end +``` + +The parameter names (`street`, `city`) name model attributes; the framework reads them from the instance and splats them into the block. The block's return value is written under the attribute's API name (`Address`). + +This also works with mapper objects whose `serialise` method takes multiple parameters. + +Marking a combine attribute `:required` enforces that **all** named model attributes are non-`nil` at serialise time. The parse side does not apply — combine attributes don't read from a single API field on the way in, so users typically populate the underlying model attributes directly. + ### Split pattern — one API field into many model attributes Use a bare block with a parameter to extract from a single API field: @@ -446,6 +469,8 @@ end The parameter name (`address`) determines which API field to read from. +Marking a split attribute `:required` enforces that the underlying API field is present on parse. Since split attributes typically aren't sent back to the API as a single field, mark them `:read_only` to skip the serialise-time check. + ### Ignoring fields Tell RestEasy to silently skip API fields you don't need: diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index 717f886..c7e1c3a 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -595,8 +595,14 @@ def serialise next if attr_def.read_only? value = @model_attributes[attr_def.model_name] - if attr_def.required? && !attr_def.synthetic? && value.nil? - raise MissingAttributeError.new(attr_def.model_name) + if attr_def.required? + missing = if attr_def.target_fields.any? + attr_def.target_fields.any? { |fn| @model_attributes[fn].nil? } + else + value.nil? + end + + raise MissingAttributeError.new(attr_def.model_name) if missing end if attr_def.target_fields.any? @@ -689,6 +695,11 @@ def init_from_api(api_data, extra_meta = {}) api_key = convention.serialise(field_name) api_data[api_key] end + + if attr_def.required? && raw_values.any?(&:nil?) + raise MissingAttributeError.new(model_name) + end + @model_attributes[model_name] = attr_def.parse_value(*raw_values) else raw_value = api_data[attr_def.api_name] diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 42efbfd..8a12d55 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -404,6 +404,106 @@ def serialise(model_name) end end + context "on a synthetic merge attribute (many API → one model)" do + let(:merge_class) do + Class.new(described_class) do + attr :first_name, String + attr :last_name, String + + attr :full_name, String, :required do + parse { |first_name, last_name| "#{first_name} #{last_name}" } + serialise { |full_name| full_name.split(' ', 2) } + end + end + end + + it "raises on parse when any source field is absent" do + expect { + merge_class.parse({ "FirstName" => "Alice" }) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:full_name) + } + end + + it "does not raise on parse when all source fields are present" do + expect { + merge_class.parse({ "FirstName" => "Alice", "LastName" => "Smith" }) + }.not_to raise_error + end + + it "raises on serialise when the merged value is nil" do + instance = merge_class.stub(first_name: "Alice", last_name: "Smith") + + expect { + instance.serialise + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:full_name) + } + end + + it "does not raise on serialise when the merged value is present" do + instance = merge_class.stub(full_name: "Alice Smith") + + expect { instance.serialise }.not_to raise_error + end + end + + context "on a synthetic combine attribute (many model → one API)" do + let(:combine_class) do + Class.new(described_class) do + attr :street, String + attr :city, String + + attr :address, String, :required do + serialise { |street, city| "#{street}, #{city}" } + end + end + end + + it "raises on serialise when any target field is nil" do + instance = combine_class.stub(street: "Main St") + + expect { + instance.serialise + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:address) + } + end + + it "does not raise on serialise when all target fields are present" do + instance = combine_class.stub(street: "Main St", city: "Stockholm") + + expect { instance.serialise }.not_to raise_error + end + end + + context "on a :read_only synthetic attribute (derivational)" do + let(:derivational_class) do + Class.new(described_class) do + attr :first_name, String + attr :last_name, String + + attr :full_name, String, :read_only, :required do |first_name, last_name| + "#{first_name} #{last_name}" + end + end + end + + it "still enforces :required at parse time when a source field is absent" do + expect { + derivational_class.parse({ "FirstName" => "Alice" }) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:full_name) + } + end + + it "does not enforce :required at serialise time (never sent)" do + instance = derivational_class.parse({ "FirstName" => "Alice", "LastName" => "Smith" }) + + expect { instance.serialise }.not_to raise_error + end + end + context "on save (no HTTP traffic on incomplete payload)" do before(:all) do module RequiredFlagTestApi From a15bde47f8b94176b29de117dfa98bf2672bc442 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 08:35:08 +0200 Subject: [PATCH 04/14] Skips inbound API field for combine attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A combine attribute's api_name (e.g. "Address" for a model that builds the value from :street + :city on serialise) does not exist on the API side by design. The previous code still looked up api_data[api_name], stored the value at @model_attributes[model_name], and ran the standard :required check — none of which make sense for a synthetic attribute. If the API ever did return the shadowed name, the framework would store it and then overwrite it during serialise from target_fields, leaving instance.address and the outbound payload in contradiction. Three consequences: * init_from_api now branches on attr_def.synthetic? for non-source_fields attrs and stores nil, skipping the lookup and the :required raise. * The debug-mode "expected API field but missing" warning also skips combine attrs, since their absence is the documented contract. * The combine context gains a spec asserting that an inbound value for the shadowed API field is ignored. --- CHANGELOG.md | 9 +++++++++ lib/rest_easy/resource.rb | 7 +++++++ spec/rest_easy/resource_spec.rb | 16 ++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d00bf..10c8efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,15 @@ multi-parameter parse. Previously only the parse side was tagged, leaving the flag inconsistent with its intent of marking attributes whose storage shape diverges from the standard one-slot layout. +- **Combine attributes no longer read from their `api_name` on parse.** + A combine attribute's API name does not correspond to a real inbound + field by design — the value is built from `target_fields` at serialise + time. The previous code looked up `api_data[api_name]`, stored it at + the model slot, and ran the standard `:required` check, raising + spuriously when the field was absent (the documented case). Inbound + values for the shadowed API key are now ignored and the model slot is + set to `nil`, avoiding the contradiction where `instance.address` could + return one value while serialise would overwrite it with another. ## [1.3.1] - 2026-05-27 diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index c7e1c3a..b037f91 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -701,6 +701,11 @@ def init_from_api(api_data, extra_meta = {}) end @model_attributes[model_name] = attr_def.parse_value(*raw_values) + elsif attr_def.synthetic? + # Combine pattern: the attribute's api_name does not exist on the + # API side by design — the value is built from target_fields at + # serialise time. Nothing inbound to read or validate. + @model_attributes[model_name] = nil else raw_value = api_data[attr_def.api_name] @@ -737,6 +742,8 @@ def init_from_api(api_data, extra_meta = {}) # Warn about declared attributes missing from the API response klass.all_attribute_definitions.each do |model_name, attr_def| next if attr_def.required? # already raises + # Combine attrs have no inbound api_name; absence is expected. + next if attr_def.synthetic? && attr_def.source_fields.empty? api_keys_to_check = if attr_def.source_fields.any? attr_def.source_fields.map { |sf| convention.serialise(sf) } diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 8a12d55..8a5df8c 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -475,6 +475,22 @@ def serialise(model_name) expect { instance.serialise }.not_to raise_error end + + it "does not raise on parse when the combined API field is absent" do + expect { + combine_class.parse({ "Street" => "Main St", "City" => "Stockholm" }) + }.not_to raise_error + end + + it "ignores any inbound value for the combined API field (no shadowing)" do + instance = combine_class.parse({ + "Street" => "Main St", + "City" => "Stockholm", + "Address" => "Should be ignored" + }) + + expect(instance.address).to be_nil + end end context "on a :read_only synthetic attribute (derivational)" do From 3ef0bec9831f69b585ed7c14a8386097871132c4 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 09:50:09 +0200 Subject: [PATCH 05/14] Clarifies :required treats explicit null as missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A :required attribute means "must carry a value", and explicit null in the API response is treated as missing — the same as an omitted key. Rationale: * :required + null is semantically odd; if null is acceptable, the attribute is :optional, not :required. * Stricter behavior catches upstream API regressions more loudly (a field that suddenly goes null is a signal worth raising on). * Symmetric with the serialise-side check, which already raises on nil. * Loosening later is easy; tightening after consumers depend on the loose behavior would be a breaking change. Adds a spec pinning the behavior so future refactors don't accidentally loosen it. Clarifies the README flags table and the parse-time CHANGELOG entry. No code change. --- CHANGELOG.md | 3 +++ README.md | 2 +- spec/rest_easy/resource_spec.rb | 8 ++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10c8efe..479d5e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ on `save`) when a required attribute is `nil`, before any HTTP request, with the attribute name on the exception (`#attribute_name`). Inbound behavior is unchanged — `parse` still raises on missing required fields. + A field is treated as missing whether the API response omits the key + entirely or includes it as explicit `null`; both are rejected as + incomplete data. - **`:required` is now enforced on synthetic attributes (merge / combine / split patterns).** Previously the flag was silently a no-op for any attribute with a multi-parameter parse or serialise block. It now diff --git a/README.md b/README.md index f7c5bb8..2503c12 100644 --- a/README.md +++ b/README.md @@ -328,7 +328,7 @@ attr [:tax_url, '@urlTaxReductionList'], String, :read_only | Flag | Effect | |--------------|------------------------------------------------------------------------| -| `:required` | Raises `MissingAttributeError` if absent in the API response (on parse) or `nil` at serialise time (`save`, `to_api`). See [Merge](#merge-pattern--many-api-fields-into-one-model-attribute), [Combine](#combine-pattern--many-model-attributes-into-one-api-field), and [Split](#split-pattern--one-api-field-into-many-model-attributes) patterns for how this applies to synthetic attributes. | +| `:required` | Raises `MissingAttributeError` if missing or explicitly `null` in the API response (on parse) or `nil` at serialise time (`save`, `to_api`). Omitted keys and explicit `null` are treated identically — a required attribute must carry a value. See [Merge](#merge-pattern--many-api-fields-into-one-model-attribute), [Combine](#combine-pattern--many-model-attributes-into-one-api-field), and [Split](#split-pattern--one-api-field-into-many-model-attributes) patterns for how this applies to synthetic attributes. | | `:optional` | Documents that the field may be absent (default) | | `:read_only` | Excluded from serialisation (not sent back to the API) | | `:key` | Marks the unique identifier for CRUD operations | diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 8a5df8c..366063e 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -342,6 +342,14 @@ def serialise(model_name) expect(e.attribute_name).to eq(:name) } end + + it "raises when a :required attribute is explicitly null in the API response" do + expect { + resource_class.parse({ "Name" => nil, "Amount" => 100.0 }) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:name) + } + end end context "on stub (build)" do From aebb2fe3a655cbbaa33e4c3a65097c5d1798010e Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 09:57:11 +0200 Subject: [PATCH 06/14] Extracts Attribute#validate_required! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The :required check appeared inline in three places: both branches of init_from_api and the serialise loop. Each restated the same logic ("if required and any value is nil, raise MissingAttributeError"), making future refactors (e.g. tweaking the message, adding new flags like :nullable, changing the predicate to use Hash#key?) require edits in multiple sites with subtle drift opportunities. Centralizes the check on the attribute itself: attr_def.validate_required!(*values) The method is a no-op for non-required attrs and raises with the attribute name when any value is nil. Behavior-preserving — all 202 existing specs continue to pass. Also moves the serialise-loop required check from a leading pre-check to per-branch calls, eliminating the duplicated target_fields gather (previously computed once for the required check and once for the actual serialisation). --- lib/rest_easy/attribute.rb | 7 +++++++ lib/rest_easy/resource.rb | 21 +++++---------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/rest_easy/attribute.rb b/lib/rest_easy/attribute.rb index c1a93ac..4de3a80 100644 --- a/lib/rest_easy/attribute.rb +++ b/lib/rest_easy/attribute.rb @@ -36,6 +36,13 @@ def synthetic? @flags.include?(:synthetic) end + def validate_required!(*values) + return unless required? + return if values.none?(&:nil?) + + raise RestEasy::MissingAttributeError.new(model_name) + end + def coerce(value) @type[value] rescue Dry::Types::ConstraintError, Dry::Types::CoercionError => e diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index b037f91..a91904f 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -595,21 +595,13 @@ def serialise next if attr_def.read_only? value = @model_attributes[attr_def.model_name] - if attr_def.required? - missing = if attr_def.target_fields.any? - attr_def.target_fields.any? { |fn| @model_attributes[fn].nil? } - else - value.nil? - end - - raise MissingAttributeError.new(attr_def.model_name) if missing - end - if attr_def.target_fields.any? # Multi-param serialise: gather model values by param names, splat into block model_values = attr_def.target_fields.map { |fn| @model_attributes[fn] } + attr_def.validate_required!(*model_values) result[attr_def.api_name] = attr_def.serialise_value(*model_values) elsif attr_def.source_fields.any? + attr_def.validate_required!(value) serialised = attr_def.serialise_value(value) if serialised.is_a?(::Array) # Array return: zip with source field API names @@ -625,6 +617,7 @@ def serialise result[attr_def.api_name] = serialised end else + attr_def.validate_required!(value) result[attr_def.api_name] = attr_def.serialise_value(value) end end @@ -696,9 +689,7 @@ def init_from_api(api_data, extra_meta = {}) api_data[api_key] end - if attr_def.required? && raw_values.any?(&:nil?) - raise MissingAttributeError.new(model_name) - end + attr_def.validate_required!(*raw_values) @model_attributes[model_name] = attr_def.parse_value(*raw_values) elsif attr_def.synthetic? @@ -709,9 +700,7 @@ def init_from_api(api_data, extra_meta = {}) else raw_value = api_data[attr_def.api_name] - if raw_value.nil? && attr_def.required? - raise MissingAttributeError.new(model_name) - end + attr_def.validate_required!(raw_value) if raw_value.nil? @model_attributes[model_name] = nil From adafa2317d69748693ca6fbd32e02cf5cb55ab68 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:03:40 +0200 Subject: [PATCH 07/14] Adds update-path coverage to :required save spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing save spec only exercised the create path (stubbed instance → new? → POST). The accompanying assertion that PUT was not called was vacuously true since save never dispatched to it. A future regression that only broke the update path (parse → mutate → PUT) would have left the suite green. Adds a sibling spec that parses an instance to non-new?, mutates a :required field to nil, and asserts save raises before PUT is issued. Renames the original spec for symmetry ("raises before POSTing a new record" / "raises before PUTing an updated record"). --- spec/rest_easy/resource_spec.rb | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 366063e..6e6e520 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -553,7 +553,7 @@ class RequiredFlagTestApi::Invoice < RestEasy::Resource Object.send(:remove_const, :RequiredFlagTestApi) end - it "raises MissingAttributeError before issuing an HTTP request" do + it "raises before POSTing a new record" do instance = RequiredFlagTestApi::Invoice.stub(amount: 300.0) allow(RequiredFlagTestApi::Invoice).to receive(:post) @@ -568,6 +568,27 @@ class RequiredFlagTestApi::Invoice < RestEasy::Resource expect(RequiredFlagTestApi::Invoice).not_to have_received(:post) expect(RequiredFlagTestApi::Invoice).not_to have_received(:put) end + + it "raises before PUTing an updated record" do + instance = RequiredFlagTestApi::Invoice.parse({ + "DocumentNumber" => 1, + "CustomerNumber" => "CUST-001", + "Amount" => 300.0 + }) + mutated = instance.update(customer_number: nil) + + allow(RequiredFlagTestApi::Invoice).to receive(:post) + allow(RequiredFlagTestApi::Invoice).to receive(:put) + + expect { + RequiredFlagTestApi::Invoice.save(mutated) + }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:customer_number) + } + + expect(RequiredFlagTestApi::Invoice).not_to have_received(:post) + expect(RequiredFlagTestApi::Invoice).not_to have_received(:put) + end end end From de5d4755a0e58ba8046a94b44f65b1829116c10e Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:05:35 +0200 Subject: [PATCH 08/14] Tightens two :required serialise specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two improvements: * The ":read_only attributes are not enforced" spec stubbed an :id-less instance by parsing { "Id" => 1, ... }. Since :id was always non-nil, the spec passed regardless of whether the :read_only short-circuit ran — the :required check would never have raised either way. Switches to stub(name: "Acme") with no :id, so the spec actually exercises the read_only-takes-priority invariant. * The "mutated to clear" spec asserted that serialise raises but not that update(name: nil) actually applied. A future regression in update that silently dropped nil-valued writes would have produced a passing test for the wrong reason. Adds expect(mutated.name).to be_nil before the raise expectation. --- spec/rest_easy/resource_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 6e6e520..b02dcdf 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -395,6 +395,8 @@ def serialise(model_name) instance = resource_class.parse({ "Name" => "Acme", "Amount" => 100.0 }) mutated = instance.update(name: nil) + expect(mutated.name).to be_nil + expect { mutated.serialise }.to raise_error(RestEasy::MissingAttributeError) @@ -406,7 +408,9 @@ def serialise(model_name) attr :name, String end - instance = read_only_required.parse({ "Id" => 1, "Name" => "Acme" }) + # No :id supplied. If :read_only didn't short-circuit the + # :required check, the stub instance would raise on serialise. + instance = read_only_required.stub(name: "Acme") expect { instance.serialise }.not_to raise_error end From af5c13dbc673f8e4768df0bbb777ffb3c58626a3 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:07:50 +0200 Subject: [PATCH 09/14] Restores original settings-table padding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier work on this branch widened the Description column in the Available settings table to fit the longer Flags-table descriptions that landed at the same time. That re-padded every settings row, polluting git blame on rows whose content was untouched. Reverts the settings-table whitespace to match main exactly. The Flags-table widening stays — it's load-bearing for the longer :required description. --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 2503c12..ee2b45f 100644 --- a/README.md +++ b/README.md @@ -92,15 +92,15 @@ end ### Available settings -| Setting | Default | Description | -|----------------------------------|----------------------------|-------------------------------------------------------------------------------------------------| -| `authentication` | `Auth::Null.new` | Authentication strategy | -| `base_url` | `"https://example.com"` | Base URL for all requests | -| `conversions.json_attributes` | `:PascalCase` | Naming convention for JSON response/request fields | -| `conversions.query_parameters` | `nil` (no transformation) | Naming convention for query parameter keys | +| Setting | Default | Description | +|----------------------------------|----------------------------|--------------------------------------------------------------------------------------------| +| `authentication` | `Auth::Null.new` | Authentication strategy | +| `base_url` | `"https://example.com"` | Base URL for all requests | +| `conversions.json_attributes` | `:PascalCase` | Naming convention for JSON response/request fields | +| `conversions.query_parameters` | `nil` (no transformation) | Naming convention for query parameter keys | | `log_bodies` | `false` | When `true`, request/response bodies are logged. Off by default to avoid leaking domain secrets | -| `logger` | `nil` | When set, attaches Faraday's logger middleware and writes HTTP request/response details to it | -| `max_retries` | `3` | Retry count on request failure | +| `logger` | `nil` | When set, attaches Faraday's logger middleware and writes HTTP request/response details to it | +| `max_retries` | `3` | Retry count on request failure | ### Logging HTTP traffic From 2b9cf7e03d0074b25c934e8df21b30d85f4d39a9 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:31:35 +0200 Subject: [PATCH 10/14] Introduces Attribute#combine? predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sites in init_from_api had inline copies of the same compound check ("synthetic? && source_fields.empty?") to detect combine attributes — the parse-loop elsif branch and the debug-warn skip. A future change to what "combine" means (e.g., the target_fields-based refactor we've been discussing) would have to update both. Adds Attribute#combine? returning synthetic? && source_fields.empty? and switches both call sites to use it. The debug-warn loop's earlier "already raises" comment was misleading for combine + required attrs (they no longer raise, they're skipped via this predicate); rewrites the comment to describe both skip categories accurately. Also removes a dead `value = @model_attributes[attr_def.model_name]` assignment at the top of the serialise loop that was unused by the target_fields branch. Inlines it in the two branches that need it. --- lib/rest_easy/attribute.rb | 7 +++++++ lib/rest_easy/resource.rb | 14 ++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/rest_easy/attribute.rb b/lib/rest_easy/attribute.rb index 4de3a80..9fce661 100644 --- a/lib/rest_easy/attribute.rb +++ b/lib/rest_easy/attribute.rb @@ -36,6 +36,13 @@ def synthetic? @flags.include?(:synthetic) end + # A combine attribute is built from multiple model fields on serialise + # (target_fields), with no inbound source from its own api_name on parse. + # See the "Combine pattern" section of the README. + def combine? + synthetic? && @source_fields.empty? + end + def validate_required!(*values) return unless required? return if values.none?(&:nil?) diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index a91904f..c9600ff 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -593,7 +593,6 @@ def serialise # Serialise all attributes klass.all_attribute_definitions.each do |_model_name, attr_def| next if attr_def.read_only? - value = @model_attributes[attr_def.model_name] if attr_def.target_fields.any? # Multi-param serialise: gather model values by param names, splat into block @@ -601,6 +600,7 @@ def serialise attr_def.validate_required!(*model_values) result[attr_def.api_name] = attr_def.serialise_value(*model_values) elsif attr_def.source_fields.any? + value = @model_attributes[attr_def.model_name] attr_def.validate_required!(value) serialised = attr_def.serialise_value(value) if serialised.is_a?(::Array) @@ -617,6 +617,7 @@ def serialise result[attr_def.api_name] = serialised end else + value = @model_attributes[attr_def.model_name] attr_def.validate_required!(value) result[attr_def.api_name] = attr_def.serialise_value(value) end @@ -692,7 +693,7 @@ def init_from_api(api_data, extra_meta = {}) attr_def.validate_required!(*raw_values) @model_attributes[model_name] = attr_def.parse_value(*raw_values) - elsif attr_def.synthetic? + elsif attr_def.combine? # Combine pattern: the attribute's api_name does not exist on the # API side by design — the value is built from target_fields at # serialise time. Nothing inbound to read or validate. @@ -728,11 +729,12 @@ def init_from_api(api_data, extra_meta = {}) end end - # Warn about declared attributes missing from the API response + # Warn about declared attributes missing from the API response. + # Combine attrs have no inbound api_name by design; non-combine + # required attrs already raised in the parse loop above. klass.all_attribute_definitions.each do |model_name, attr_def| - next if attr_def.required? # already raises - # Combine attrs have no inbound api_name; absence is expected. - next if attr_def.synthetic? && attr_def.source_fields.empty? + next if attr_def.combine? + next if attr_def.required? api_keys_to_check = if attr_def.source_fields.any? attr_def.source_fields.map { |sf| convention.serialise(sf) } From 4c3cb3eeec909a4960af90eb60dbd483840b8b35 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:36:49 +0200 Subject: [PATCH 11/14] Warns when combine attribute also defines a parse block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A combine pattern (multi-parameter `serialise`, no multi-parameter `parse`) has no inbound API field — its api_name does not exist on the API side. If the user also defines an explicit `parse` block in the DSL, that block was silently ignored: init_from_api skips the combine branch without ever calling parse_value. That silent ignore is a footgun. A user mis-wiring a combine attr (or experimenting with hybrid shapes) would never get feedback that their parse logic is dead code. Emits a warning at attribute-declaration time when target_fields is populated, source_fields is empty, and an explicit parse_block was supplied via the DSL block form. The message names the attribute, shows the target_fields it gathers from, and suggests removing the parse block or restructuring. Mapper-form combine is intentionally NOT covered by the warning — the mapper interface requires a `parse` method, so its presence is structural rather than user intent. The README documents the limitation for both forms. --- README.md | 2 ++ lib/rest_easy/resource.rb | 12 +++++++++ spec/rest_easy/resource_spec.rb | 44 +++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/README.md b/README.md index ee2b45f..92b689e 100644 --- a/README.md +++ b/README.md @@ -453,6 +453,8 @@ This also works with mapper objects whose `serialise` method takes multiple para Marking a combine attribute `:required` enforces that **all** named model attributes are non-`nil` at serialise time. The parse side does not apply — combine attributes don't read from a single API field on the way in, so users typically populate the underlying model attributes directly. +Combine attributes do not run a `parse` block. If you declare one alongside a multi-parameter `serialise`, RestEasy emits a warning at load time — the parse block would be silently ignored otherwise. If you need to read from an API field on parse, declare a separate attribute for it (or restructure as a merge pattern). + ### Split pattern — one API field into many model attributes Use a bare block with a parameter to extract from a single API field: diff --git a/lib/rest_easy/resource.rb b/lib/rest_easy/resource.rb index c9600ff..ddf8c6f 100644 --- a/lib/rest_easy/resource.rb +++ b/lib/rest_easy/resource.rb @@ -289,6 +289,18 @@ def attr(name_or_mapping, *args, &block) target_fields = params.map { |_, pname| pname } end end + + # Combine pattern (multi-param serialise, no multi-param parse) + # has no inbound api_name on parse. If the user also wrote an + # explicit `parse` block, it will be silently ignored — warn so + # the inconsistency is visible at load time. + if target_fields.any? && source_fields.empty? && parse_block + warn "RestEasy: :#{attribute_model_name} declares a combine pattern " \ + "(serialise from #{target_fields.inspect}) and also defines a parse block. " \ + "Combine attributes have no inbound API field to read, so the parse block " \ + "will not run. Remove the parse block, or restructure the declaration if you " \ + "intended to read from the API." + end end end diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index b02dcdf..8c73664 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -1034,6 +1034,50 @@ def self.serialise(street, city) attr_def = resource_class.all_attribute_definitions[:address] expect(attr_def.synthetic?).to be true end + + it "warns when a combine attribute also defines an explicit parse block" do + expect { + Class.new(described_class) do + attr :street, String + attr :city, String + + attr :address, String do + parse { |val| val.to_s.upcase } + serialise { |street, city| "#{street}, #{city}" } + end + end + }.to output( + /:address declares a combine pattern.*parse block.*will not run/m + ).to_stderr + end + + it "does not warn when the combine attribute has no explicit parse block" do + expect { + Class.new(described_class) do + attr :street, String + attr :city, String + + attr :address, String do + serialise { |street, city| "#{street}, #{city}" } + end + end + }.not_to output(/declares a combine pattern/).to_stderr + end + + it "does not warn for mapper-form combine attributes" do + mapper = Module.new do + def self.parse(raw_value) = raw_value + def self.serialise(street, city) = "#{street}, #{city}" + end + + expect { + Class.new(described_class) do + attr :street, String + attr :city, String + attr :address, String, mapper + end + }.not_to output(/declares a combine pattern/).to_stderr + end end # ── Resource-level hooks ─────────────────────────────────────────────── From a9e7d356a8c9479ee34808849ed6d7b0eaef0914 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:37:49 +0200 Subject: [PATCH 12/14] Adds spec coverage for validate_required! and combine + :read_only Two gaps surfaced in self-review: * Attribute had no unit specs at all. Adds spec/rest_easy/attribute_spec.rb covering validate_required! edge cases: not-required is a no-op, required raises on any nil, multi-value calls behave correctly, and the empty-splat case is documented as "nothing to check" rather than "missing" so future call sites with conditionally empty arrays don't spuriously raise. * No spec exercised combine + :read_only. The combination is plausible (e.g. an audit-trail attribute the API returns but we never want to send back). Adds two specs verifying that serialise is skipped via the :read_only short-circuit and that parse leaves the model slot nil regardless of any inbound shadow value. Also adds Attribute#combine? predicate specs alongside. --- spec/rest_easy/attribute_spec.rb | 75 ++++++++++++++++++++++++++++++++ spec/rest_easy/resource_spec.rb | 23 ++++++++++ 2 files changed, 98 insertions(+) create mode 100644 spec/rest_easy/attribute_spec.rb diff --git a/spec/rest_easy/attribute_spec.rb b/spec/rest_easy/attribute_spec.rb new file mode 100644 index 0000000..7671771 --- /dev/null +++ b/spec/rest_easy/attribute_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +RSpec.describe RestEasy::Attribute do + describe "#validate_required!" do + let(:type) { RestEasy::Types::Coercible::String } + + context "when the attribute is not :required" do + let(:attr) { described_class.new(model_name: :name, api_name: "Name", type:) } + + it "does not raise even when the value is nil" do + expect { attr.validate_required!(nil) }.not_to raise_error + end + + it "does not raise with multiple nil values" do + expect { attr.validate_required!(nil, nil) }.not_to raise_error + end + end + + context "when the attribute is :required" do + let(:attr) { described_class.new(model_name: :name, api_name: "Name", type:, flags: [:required]) } + + it "does not raise when the single value is non-nil" do + expect { attr.validate_required!("Acme") }.not_to raise_error + end + + it "raises when the single value is nil" do + expect { attr.validate_required!(nil) }.to raise_error(RestEasy::MissingAttributeError) { |e| + expect(e.attribute_name).to eq(:name) + } + end + + it "does not raise when all of multiple values are non-nil" do + expect { attr.validate_required!("Alice", "Smith") }.not_to raise_error + end + + it "raises when any of multiple values is nil" do + expect { attr.validate_required!("Alice", nil) }.to raise_error(RestEasy::MissingAttributeError) + expect { attr.validate_required!(nil, "Smith") }.to raise_error(RestEasy::MissingAttributeError) + end + + it "does not raise when called with no values (treats empty splat as nothing-to-check)" do + # Documented behavior: callers must pass the values they intend to + # validate. An empty splat is treated as "nothing to check" rather + # than as "missing" so that future call sites with conditionally + # empty arrays don't spuriously raise. + expect { attr.validate_required! }.not_to raise_error + end + end + end + + describe "#combine?" do + let(:type) { RestEasy::Types::Coercible::String } + + it "is true when synthetic with no source_fields" do + attr = described_class.new( + model_name: :address, api_name: "Address", type:, + flags: [:synthetic], target_fields: [:street, :city] + ) + expect(attr.combine?).to be true + end + + it "is false when synthetic with source_fields (merge / split)" do + attr = described_class.new( + model_name: :full_name, api_name: "FullName", type:, + flags: [:synthetic], source_fields: [:first_name, :last_name] + ) + expect(attr.combine?).to be false + end + + it "is false for a non-synthetic standard attribute" do + attr = described_class.new(model_name: :name, api_name: "Name", type:) + expect(attr.combine?).to be false + end + end +end diff --git a/spec/rest_easy/resource_spec.rb b/spec/rest_easy/resource_spec.rb index 8c73664..4ea73cf 100644 --- a/spec/rest_easy/resource_spec.rb +++ b/spec/rest_easy/resource_spec.rb @@ -505,6 +505,29 @@ def serialise(model_name) end end + context "on a :read_only combine attribute" do + let(:read_only_combine) do + Class.new(described_class) do + attr :street, String + attr :city, String + + attr :address, String, :read_only, :required do + serialise { |street, city| "#{street}, #{city}" } + end + end + end + + it "does not enforce :required at serialise time (never sent)" do + instance = read_only_combine.stub + expect { instance.serialise }.not_to raise_error + end + + it "leaves the model slot nil on parse regardless of inbound value" do + instance = read_only_combine.parse({ "Address" => "ignored" }) + expect(instance.address).to be_nil + end + end + context "on a :read_only synthetic attribute (derivational)" do let(:derivational_class) do Class.new(described_class) do From c01a6baf83a233f66fdc5c76ab2fe81bd6bad683 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:38:27 +0200 Subject: [PATCH 13/14] Rewords split section :required guidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier wording — "split attributes typically aren't sent back to the API as a single field, mark them :read_only to skip the serialise-time check" — collapsed two distinct behaviors into one ambiguous claim. Split attributes ARE serialised; they go out under their own model api_names (Street, City), not recombined into the inbound source field (Address). Rewrites to make the serialise direction explicit, then offers two recovery patterns for users whose API does not accept the parts as independent top-level fields: :read_only (skip serialisation) or an after_serialise hook (reconstruct the source field). --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 92b689e..34bf734 100644 --- a/README.md +++ b/README.md @@ -471,7 +471,9 @@ end The parameter name (`address`) determines which API field to read from. -Marking a split attribute `:required` enforces that the underlying API field is present on parse. Since split attributes typically aren't sent back to the API as a single field, mark them `:read_only` to skip the serialise-time check. +Marking a split attribute `:required` enforces that the underlying API field is present on parse — in the example above, the API response must include `Address`. + +On serialise, split attributes are emitted under their own api_names (here, `Street` and `City`) rather than being recombined into the original source field. If the API does not accept the parts as independent top-level fields, mark them `:read_only` to keep them out of the outbound payload entirely; alternatively, reconstruct the source field in an `after_serialise` hook. ### Ignoring fields From c6467b242bb11971ba1edc94affd41680a71d1d5 Mon Sep 17 00:00:00 2001 From: Hannes Elvemyr Date: Fri, 26 Jun 2026 10:52:27 +0200 Subject: [PATCH 14/14] Tightens Attribute#combine? to require target_fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous definition (synthetic? && source_fields.empty?) would classify an attribute as combine if the user passed :synthetic as an explicit flag without any block — e.g. `attr :foo, String, :synthetic`. With no target_fields, treating that as combine wrongly skips the api_name lookup in init_from_api and silently sets the model slot to nil, hiding any inbound value. Combine is structurally defined by target_fields (the model fields the serialise block gathers from) plus the absence of source_fields (no inbound merge / split). Using the structural predicate instead of the flag avoids the edge case without relying on synthetic? being applied consistently. Adds a spec for the explicit-synthetic-flag edge case so the classification stays correct under future refactors. --- lib/rest_easy/attribute.rb | 2 +- spec/rest_easy/attribute_spec.rb | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/rest_easy/attribute.rb b/lib/rest_easy/attribute.rb index 9fce661..ef9cc2b 100644 --- a/lib/rest_easy/attribute.rb +++ b/lib/rest_easy/attribute.rb @@ -40,7 +40,7 @@ def synthetic? # (target_fields), with no inbound source from its own api_name on parse. # See the "Combine pattern" section of the README. def combine? - synthetic? && @source_fields.empty? + @target_fields.any? && @source_fields.empty? end def validate_required!(*values) diff --git a/spec/rest_easy/attribute_spec.rb b/spec/rest_easy/attribute_spec.rb index 7671771..57b18e6 100644 --- a/spec/rest_easy/attribute_spec.rb +++ b/spec/rest_easy/attribute_spec.rb @@ -51,7 +51,7 @@ describe "#combine?" do let(:type) { RestEasy::Types::Coercible::String } - it "is true when synthetic with no source_fields" do + it "is true when target_fields is populated and source_fields is empty" do attr = described_class.new( model_name: :address, api_name: "Address", type:, flags: [:synthetic], target_fields: [:street, :city] @@ -59,7 +59,7 @@ expect(attr.combine?).to be true end - it "is false when synthetic with source_fields (merge / split)" do + it "is false when source_fields is also populated (merge / split)" do attr = described_class.new( model_name: :full_name, api_name: "FullName", type:, flags: [:synthetic], source_fields: [:first_name, :last_name] @@ -71,5 +71,17 @@ attr = described_class.new(model_name: :name, api_name: "Name", type:) expect(attr.combine?).to be false end + + it "is false when :synthetic is set but no target_fields are present" do + # Edge case: user explicitly passes :synthetic as a flag without a + # multi-param block. The attribute is not actually combine-shaped — + # treating it as such would incorrectly skip the api_name lookup + # on parse and silently set the model slot to nil. + attr = described_class.new( + model_name: :foo, api_name: "Foo", type:, + flags: [:synthetic] + ) + expect(attr.combine?).to be false + end end end