diff --git a/CHANGELOG.md b/CHANGELOG.md index bbd3211..658c283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## [Unreleased] +- **Breaking:** reading an attribute that is absent from an API payload now + raises `Booqable::MissingAttribute` (a `NoMethodError` subclass) instead of + silently returning nil. - Support Ruby 4.0 ## [1.2.1] - 2026-06-10 diff --git a/README.md b/README.md index 818ded5..36bf0bb 100644 --- a/README.md +++ b/README.md @@ -406,6 +406,33 @@ customer = client.parse_resource(payload) `deserialize_resource` is available as an alias for `parse_resource`. +## Strict attribute reads + +Reading an attribute that is absent from the API payload raises +`Booqable::MissingAttribute` instead of silently returning nil, so typos and +renamed API fields fail loudly: + +```ruby +customer.name # => "John Doe" +customer.full_name # raises Booqable::MissingAttribute (key absent from payload) +``` + +An attribute that is present in the payload with a null value still returns +nil — only absent keys raise: + +```ruby +order.customer # => nil when the payload contains "customer": null +``` + +`Booqable::MissingAttribute` subclasses `NoMethodError`, so generic rescues +keep working. To probe for an attribute that may be absent, use hash-style +access or `key?`: + +```ruby +order[:customer] # => nil when the key is absent (lenient probe) +order.key?(:customer) # => false when the key is absent +``` + ## Advanced usage ### Custom middleware diff --git a/lib/booqable.rb b/lib/booqable.rb index 67f03a7..ba22a5e 100644 --- a/lib/booqable.rb +++ b/lib/booqable.rb @@ -12,6 +12,7 @@ require_relative "booqable/version" require_relative "booqable/rate_limit" require_relative "booqable/error" +require_relative "booqable/strict_attributes" require_relative "booqable/oauth_client" require_relative "booqable/middleware/base" require_relative "booqable/middleware/raise_error" diff --git a/lib/booqable/error.rb b/lib/booqable/error.rb index 081b69c..e989c09 100644 --- a/lib/booqable/error.rb +++ b/lib/booqable/error.rb @@ -421,6 +421,50 @@ class ServiceUnavailable < ServerError; end # and body matches 'read-only' class ReadOnlyMode < ServerError; end + # Raised when reading an attribute that is absent from an API resource + # + # Sawyer normally answers reads of absent attributes with nil, which turns + # typos and renamed API fields (e.g. `time_zone` vs `default_timezone`) + # into silent data bugs. Resources created by this gem raise this error + # instead — see {Booqable::StrictAttributes}. + # + # Attributes that are present in the payload with a null value still + # return nil; only reads of keys that are absent from the payload raise. + # + # This inherits from NoMethodError (not Booqable::Error) because an absent + # attribute is a programming error in the caller, not an API failure: + # + # * generic `rescue NoMethodError` / `rescue StandardError` code keeps working + # * ActiveSupport's `resource.try(:foo)` stays a safe probe: it returns nil + # without calling, since respond_to? is false for absent attributes + # (the bang variant `try!(:foo)` raises) + # * hash-style access stays a lenient, explicit probe: `resource[:foo]` + # returns nil for absent keys because Sawyer rescues NoMethodError there + class MissingAttribute < NoMethodError + # Initialize a new MissingAttribute error + # + # @param attribute_name [Symbol] Name of the absent attribute that was read + # @param resource [Sawyer::Resource] Resource the attribute was read from + def initialize(attribute_name, resource) + super(build_message(attribute_name, resource), attribute_name) + end + + private + + def build_message(attribute_name, resource) + message = +"undefined attribute `#{attribute_name}` for #{resource_description(resource)}. " + message << "The attribute is absent from the API payload " + message << "(attributes present with a null value return nil). " + message << "Available attributes: #{resource.attrs.keys.sort.join(", ")}" + message + end + + def resource_description(resource) + type = resource.attrs[:type] + type.is_a?(String) ? "a Booqable #{type} resource" : "a Booqable resource" + end + end + # Raised when Booqable configuration is invalid class ConfigArgumentError < ArgumentError; end diff --git a/lib/booqable/http.rb b/lib/booqable/http.rb index 16b51d3..ce63c89 100644 --- a/lib/booqable/http.rb +++ b/lib/booqable/http.rb @@ -243,15 +243,17 @@ def faraday_builder # Get or create the Sawyer agent for API requests # - # Returns a memoized Sawyer::Agent configured with the API endpoint, - # serializer, and optional logging. Sawyer handles the low-level HTTP - # communication and response parsing. + # Returns a memoized Booqable::SawyerAgent configured with the API + # endpoint, serializer, and optional logging. Sawyer handles the + # low-level HTTP communication and response parsing. Resources created + # through this agent raise {Booqable::MissingAttribute} when an absent + # attribute is read (see {Booqable::StrictAttributes}). # - # @return [Sawyer::Agent] HTTP agent instance + # @return [Booqable::SawyerAgent] HTTP agent instance # @api private def agent - @agent ||= Sawyer::Agent.new(api_endpoint, - sawyer_options) do |agent| + @agent ||= Booqable::SawyerAgent.new(api_endpoint, + sawyer_options) do |agent| agent.response :logger, logger, bodies: true if logger end end diff --git a/lib/booqable/resource_parser.rb b/lib/booqable/resource_parser.rb index 14832cd..e228b56 100644 --- a/lib/booqable/resource_parser.rb +++ b/lib/booqable/resource_parser.rb @@ -69,11 +69,12 @@ def parse # # The agent URL is a placeholder - we don't make any HTTP requests. # We just need the agent to create Sawyer::Resource objects that - # provide dot-notation attribute access. + # provide dot-notation attribute access. Using Booqable::SawyerAgent + # makes attribute reads strict (see {Booqable::StrictAttributes}). # - # @return [Sawyer::Agent] + # @return [Booqable::SawyerAgent] def sawyer_agent - @sawyer_agent ||= Sawyer::Agent.new("https://example.com") do |http| + @sawyer_agent ||= Booqable::SawyerAgent.new("https://example.com") do |http| http.headers[:content_type] = "application/json" end end diff --git a/lib/booqable/strict_attributes.rb b/lib/booqable/strict_attributes.rb new file mode 100644 index 0000000..feda9e4 --- /dev/null +++ b/lib/booqable/strict_attributes.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Booqable + # Sawyer::Agent used for every resource this gem creates + # + # Behaves exactly like Sawyer::Agent. It exists as a marker so that + # {Booqable::StrictAttributes} can tell resources created by this gem + # apart from resources created by other Sawyer-based gems + # loaded in the same process. + class SawyerAgent < Sawyer::Agent + end + + # Strict attribute reads for resources created by this gem + # + # Sawyer::Resource#method_missing silently answers reads of absent + # attributes with nil. That turns typos and renamed API fields + # (e.g. `time_zone` vs `default_timezone`) into silent data bugs. + # For resources created by a {Booqable::SawyerAgent}, this module raises + # {Booqable::MissingAttribute} instead — both for plain reads + # (`resource.foo`) and predicate reads (`resource.foo?`). + # + # Attributes that ARE present in the payload with a null value still + # return nil — only reads of keys that are absent from the payload raise. + # + # Everything else about Sawyer::Resource is unchanged: attribute writes + # (`resource.foo = 1`) still define new attributes, hash-style access + # (`resource[:foo]`) stays a lenient probe returning nil for absent keys, + # and `to_h`/`to_attrs`, `key?`, `dig`, `fetch`, enumeration, marshaling, + # and respond_to? semantics all behave as before. Resources created by + # other gems' Sawyer agents are unaffected. + module StrictAttributes + # Matches plain attribute reads (`foo`) and predicate reads (`foo?`). + # Setters (`foo=`) stay permitted since Sawyer allows adding attributes. + ATTRIBUTE_READ_PATTERN = /\A([a-z0-9_]+)(\?)?\z/i + + # Raise {Booqable::MissingAttribute} for reads of absent attributes + # on strict resources; defer to Sawyer's behavior for everything else. + def method_missing(method, *) + attr_name = booqable_missing_attribute_read(method) + raise Booqable::MissingAttribute.new(attr_name, self) if attr_name + + super + end + + private + + # Returns the attribute name when +method+ is a read of an attribute + # that is absent from the payload of a strict resource, nil otherwise + # + # @param method [Symbol] the method name passed to #method_missing + # @return [Symbol, nil] + def booqable_missing_attribute_read(method) + # _agent/_fields are Sawyer::Resource's public attr_readers — the bare `agent`/`fields` + # spellings are SPECIAL_METHODS resolved inside method_missing, so calling those from + # this hook (which method_missing invokes) would recurse infinitely. + return nil unless _agent.is_a?(Booqable::SawyerAgent) + + match = ATTRIBUTE_READ_PATTERN.match(method.to_s) + return nil unless match + + attr_name = match[1].to_sym + return nil if _fields.include?(attr_name) + return nil if match[2].nil? && Sawyer::Resource::SPECIAL_METHODS.include?(match[1]) + + attr_name + end + end +end + +Sawyer::Resource.prepend(Booqable::StrictAttributes) diff --git a/spec/booqable/resource_proxy_spec.rb b/spec/booqable/resource_proxy_spec.rb index c7dea73..dbb0734 100644 --- a/spec/booqable/resource_proxy_spec.rb +++ b/spec/booqable/resource_proxy_spec.rb @@ -193,7 +193,9 @@ expect(orders).to be_an(Array) expect(orders.length).to be > 0 order = orders.first - if order.customer + # Hash-style access is the lenient probe: this cassette's list payload + # has no customer key, and a strict read would raise MissingAttribute. + if order[:customer] expect(order.customer).not_to be_an(Array) expect(order.customer).to respond_to(:id) expect(order.customer).to respond_to(:name) diff --git a/spec/booqable/strict_attributes_spec.rb b/spec/booqable/strict_attributes_spec.rb new file mode 100644 index 0000000..f203b0c --- /dev/null +++ b/spec/booqable/strict_attributes_spec.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +require "json" + +describe Booqable::StrictAttributes do + let(:payload) do + { + "data" => { + "id" => "order-123", + "type" => "orders", + "attributes" => { + "status" => "reserved", + "customer" => nil + }, + "relationships" => { + "lines" => { + "data" => [ + { "id" => "line-1", "type" => "lines" }, + { "id" => "line-2", "type" => "lines" } + ] + } + } + }, + "included" => [ + { "id" => "line-1", "type" => "lines", "attributes" => { "quantity" => 2 } }, + { "id" => "line-2", "type" => "lines", "attributes" => { "quantity" => 5 } } + ] + } + end + + let(:resource) { Booqable::ResourceParser.parse(payload) } + + describe "reading a missing attribute" do + it "raises Booqable::MissingAttribute" do + expect { resource.customer_name }.to raise_error(Booqable::MissingAttribute) + end + + it "names the missing attribute in the message" do + expect { resource.customer_name } + .to raise_error(Booqable::MissingAttribute, /customer_name/) + end + + it "names the resource type in the message" do + expect { resource.customer_name } + .to raise_error(Booqable::MissingAttribute, /orders/) + end + + it "lists the available attributes in the message" do + expect { resource.customer_name } + .to raise_error(Booqable::MissingAttribute, /status/) + end + + it "is a NoMethodError, so generic rescues still work" do + expect { resource.customer_name }.to raise_error(NoMethodError) + end + + it "exposes the attribute name on the error" do + expect { resource.customer_name } + .to raise_error(Booqable::MissingAttribute) { |error| expect(error.name).to eq(:customer_name) } + end + + it "raises for predicate-style reads of missing attributes" do + expect { resource.customer_name? } + .to raise_error(Booqable::MissingAttribute, /customer_name/) + end + + it "raises when a missing attribute is read with arguments" do + expect { resource.customer_name("argument") } + .to raise_error(Booqable::MissingAttribute) + end + + it "raises on nested resources" do + expect { resource.lines.first.nonexistent } + .to raise_error(Booqable::MissingAttribute, /nonexistent/) + end + + it "falls back to a plain NoMethodError for non-attribute methods" do + expect { resource.grab! } + .to raise_error(NoMethodError) { |error| expect(error).not_to be_a(Booqable::MissingAttribute) } + end + + it "keeps returning the value when method_missing is reached for a present field" do + expect(resource.method_missing(:status)).to eq("reserved") + end + end + + describe "reading a present-but-null attribute" do + it "returns nil" do + expect(resource.customer).to be_nil + end + + it "returns false for predicate-style reads" do + expect(resource.customer?).to be(false) + end + end + + describe "Sawyer machinery that must keep working" do + it "keeps to_h / to_attrs working" do + expect(resource.to_h).to include(id: "order-123", status: "reserved", customer: nil) + expect(resource.to_attrs[:lines].first).to eq(id: "line-1", type: "lines", quantity: 2) + end + + it "keeps key? working" do + expect(resource.key?(:customer)).to be(true) + expect(resource.key?(:customer_name)).to be(false) + end + + it "keeps hash-style access lenient as an explicit probe" do + expect(resource[:status]).to eq("reserved") + expect(resource[:customer_name]).to be_nil + end + + it "keeps dig and fetch working" do + expect(resource.dig(:status)).to eq("reserved") + expect(resource.dig(:customer_name)).to be_nil + expect { resource.fetch(:customer_name) }.to raise_error(KeyError) + end + + it "keeps enumeration working" do + expect(resource.map { |key, _value| key }).to include(:id, :type, :status, :customer) + end + + it "keeps respond_to? semantics" do + expect(resource.respond_to?(:status)).to be(true) + expect(resource.respond_to?(:customer_name)).to be(false) + end + + it "keeps attribute writes working, including new attributes" do + resource.brand_new = "value" + expect(resource.brand_new).to eq("value") + end + + it "keeps hash-style writes working" do + resource[:status] = "started" + expect(resource.status).to eq("started") + end + + it "keeps Sawyer special methods working" do + expect(resource.fields).to include(:status) + expect(resource.agent).to be_a(Sawyer::Agent) + expect(resource.rels).to be_a(Sawyer::Relation::Map) + end + + it "keeps marshaling working" do + restored = Marshal.load(Marshal.dump(resource)) + expect(restored.status).to eq("reserved") + expect(restored.to_h).to eq(resource.to_h) + end + end + + describe "scoping" do + it "does not change behavior of Sawyer resources created by other agents" do + other_agent = Sawyer::Agent.new("https://example.com") do |http| + http.headers[:content_type] = "application/json" + end + other_resource = Sawyer::Resource.new(other_agent, { name: "octocat" }) + + expect(other_resource.nonexistent).to be_nil + end + end + + describe "resources returned from HTTP requests" do + it "raises MissingAttribute on resources parsed from API responses" do + body = { + data: { + id: "company-1", + type: "companies", + attributes: { name: "Demo", default_timezone: "Europe/Amsterdam" } + } + }.to_json + + stub_get("companies/company-1") + .to_return(status: 200, body: body, headers: { content_type: "application/json" }) + + company = api_key_client.get("companies/company-1").data + + expect(company.default_timezone).to eq("Europe/Amsterdam") + expect { company.time_zone } + .to raise_error(Booqable::MissingAttribute, /time_zone/) + end + end +end