From c672b3b027c301b4569b097d7deee7d05db7fc2e Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:02:46 +0200 Subject: [PATCH 01/13] Docs: add prioritised roadmap from deep codebase review --- ROADMAP.md | 516 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 516 insertions(+) create mode 100644 ROADMAP.md diff --git a/ROADMAP.md b/ROADMAP.md new file mode 100644 index 0000000..679a31b --- /dev/null +++ b/ROADMAP.md @@ -0,0 +1,516 @@ +# Roadmap + +Deep review of `rspec-json_api` 1.5.0 (master at `87fddee`), done on 2026-09-03. + +## How this was produced + +- Read every file in the repository: library, generators, specs, gemspec, Gemfile and lockfile, appraisal gemfiles, CI workflow, RuboCop config, README and CHANGELOG. +- Ran the suite and linter on Ruby 4.0.1 with the committed `Gemfile.lock`: 63 examples, 0 failures; RuboCop 29 files, no offenses. +- Ran `bundle outdated` and `bundle-audit check --update` against `Gemfile.lock`. +- Ran a throwaway probe script that feeds edge-case schemas through `RSpec::JsonApi::Matchers::MatchJsonSchema`. Every "Confirmed" bug below quotes the exact input, so it can be reproduced in `bin/console`. + +Each item is marked either **Confirmed** (reproduced or directly visible in the code) or **Suggestion** (a judgement call, an assumption, or a design proposal). Priorities follow the impact on a consumer's test suite: Critical means a valid test run crashes or a wrong response passes; High means correctness or supply-chain problems that are cheap to fix; Medium means real friction; Low means polish. + +## Top ten by priority and impact + +| # | Item | Priority | Effort | Status | +|---|------|----------|--------|--------| +| 1.1 | List schemas crash with `NoMethodError` when the actual value is not an array | Critical | S | Confirmed | +| 1.2 | Elements of exact arrays skip the key-structure guard (false positives) | High | S | Confirmed | +| 1.3 | `Types::URI` is unanchored, so a URI buried in prose passes | High | S | Confirmed | +| 1.4 | Matcher raises `TypeError` for `nil` or already-parsed input | High | S | Confirmed | +| 2.1 | Drop `railties` and `rspec-rails` from the runtime dependencies | High | M | Confirmed | +| 2.3 | Move the dev lockfile past 70+ open advisories | High | S | Confirmed | +| 3.1 | Report the failing key path instead of a one-line `inspect` diff | High | L | Suggestion | +| 5.1 | Close the spec gaps that let the crash paths ship | High | M | Confirmed | +| 4.1 | Optional keys and nullable values in the schema DSL | High | M | Suggestion | +| 5.2 | CI matrix exercises almost no Rails code | Medium | M | Confirmed | + +## 1. Bugs to fix + +### 1.1 List schemas crash when the actual value is not an array + +Priority: Critical. Category: Correctness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/schema_match.rb:97-111`. `compare_typed_array` and `compare_interface_array` call `actual_value.all?` without checking the receiver. Reproduced against 1.5.0: + +```ruby +match_json_schema({ notes: [String] }).matches?('{"notes":"x"}') # NoMethodError: undefined method 'all?' for String +match_json_schema({ notes: [String] }).matches?('{"notes":null}') # NoMethodError: undefined method 'all?' for nil +match_json_schema({ items: [{ id: Integer }] }).matches?('{"items":null}') # NoMethodError +match_json_schema({ items: [{ tags: [String] }] }).matches?('{"items":[{"tags":"b"}]}') # NoMethodError +``` + +**Problem and impact.** An API that returns `null` or a scalar where a list is expected is the everyday failure this gem exists to catch. Instead of a red example with a message, the consumer's suite errors out. Nested cases (probe 4) crash from inside `compare_interface_array`, so interface arrays are affected too. The 1.5.0 fix for `dig` raising `TypeError` on scalars (commit `c30eb5f`) covered objects but left the array branches with the same class of bug. + +**Recommended solution.** Guard at the top of `compare_array`: `return false unless actual_value.is_a?(Array)`. Add one spec per crash input above. This also makes probe 4 fail cleanly instead of raising. + +**Dependencies and risks.** None. Pure fix. + +### 1.2 Elements of exact arrays skip the key-structure guard + +Priority: High. Category: Correctness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/schema_match.rb:114-120`. `compare_exact_array` routes Hash elements through `compare`, not `match`, so `same_key_structure?` never runs for them. `compare` unions the key paths of both sides and compares value by value, and `nil == nil` is true. Reproduced: + +```ruby +# extra null-valued key in an element: passes +match_json_schema({ c: [{ id: Integer }, { id: Integer }] }) + .matches?('{"c":[{"id":1,"x":null},{"id":2}]}') # => true + +# missing key whose schema allows blank: passes inside the array... +match_json_schema({ c: [{ id: Integer, n: -> { { type: String, allow_blank: true } } }, { id: Integer }] }) + .matches?('{"c":[{"id":1},{"id":2}]}') # => true + +# ...but the same shape is rejected at the top level +match_json_schema({ n: -> { { type: String, allow_blank: true } } }).matches?('{}') # => false +``` + +**Problem and impact.** The README promises "match_json_schema always require full keys match". Inside an exact array that promise does not hold: a response with unexpected null fields, or with fields missing, passes. Commit `a79e487` fixed exactly this for interface arrays in 1.5.0 and left the exact-array branch untouched. + +**Recommended solution.** In `compare_exact_array`, call `match(actual_value[index], elem)` for Hash elements (as `compare_interface_array` already does). Add the two inputs above as failing specs first. + +**Dependencies and risks.** Behaviour change: suites that relied on the lax check will start failing, correctly. Note it under "Fixed" in the CHANGELOG. Pairs naturally with 3.2, which touches the same method. + +### 1.3 `Types::URI` is unanchored + +Priority: High. Category: Correctness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/types/uri.rb:6` uses `URI::DEFAULT_PARSER.make_regexp`, which has no `\A`/`\z` anchors. `compare_regexp` (`schema_match.rb:78-80`) uses `match?`, so any substring match wins: + +```ruby +match_json_schema({ u: RSpec::JsonApi::Types::URI }) + .matches?('{"u":"not a uri but see http://example.com ok"}') # => true +``` + +`Types::EMAIL` (`URI::MailTo::EMAIL_REGEXP`) is anchored and rejects the equivalent input, and `Types::UUID` was anchored in 1.5.0 (commit `ba59f62`) for the same reason. URI was missed. + +**Problem and impact.** A field that should hold a URL accepts free text as long as a URL appears somewhere in it. For a type that ships as a built-in, that is a silent correctness hole. + +**Recommended solution.** `URI = /\A#{URI::DEFAULT_PARSER.make_regexp}\z/`. Add a spec with a URI embedded in prose and one with leading whitespace. Consider a stricter `Types::URL` (http/https only) as a separate built-in, since `make_regexp` also accepts `mailto:` and `urn:` (see 4.4). + +**Dependencies and risks.** Strings with surrounding whitespace start failing; that is the intended behaviour. + +### 1.4 Matcher raises `TypeError` for `nil` or already-parsed input + +Priority: High. Category: Robustness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/matchers/match_json_schema.rb:25-33` rescues only `JSON::ParserError`. `JSON.parse(nil)` and `JSON.parse({})` raise `TypeError`: + +```ruby +match_json_schema({ id: String }).matches?(nil) # TypeError: no implicit conversion of nil into String +match_json_schema({ id: String }).matches?({ id: "x" }) # TypeError: no implicit conversion of Hash into String +``` + +**Problem and impact.** `expect(nil).to match_json_schema(...)` is a plausible outcome of a helper that returns `nil`, and passing `JSON.parse(response.body)` or `response.parsed_body` is what request specs naturally hand over. Both crash instead of failing with a message. + +**Recommended solution.** Short term: rescue `TypeError` alongside `JSON::ParserError` and set a `@parse_error` that `failure_message` prints ("expected a JSON String, got NilClass"). Longer term: accept Hash/Array input directly and objects that respond to `body` (see 4.3). + +**Dependencies and risks.** None for the rescue. Accepting parsed input is a feature decision (4.3). + +### 1.5 Regexp schema values ignore the value's type + +Priority: Medium. Category: Correctness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/schema_match.rb:78-80` and `constraints.rb:43` call `to_s` on the actual value before matching: + +```ruby +match_json_schema({ code: /\A\d+\z/ }).matches?('{"code":123}') # => true (an Integer) +match_json_schema({ code: /.*/ }).matches?('{"code":null}') # => true (nil.to_s == "") +``` + +**Problem and impact.** A regexp schema is the documented way to say "a string shaped like X". A number or `null` should not satisfy it. The `null` case is worse: a permissive regex accepts a missing value, which is what `allow_blank` exists to express explicitly. + +**Recommended solution.** `actual_value.is_a?(String) && expected_value.match?(actual_value)` in both places. Document that regexp schemas imply String. + +**Dependencies and risks.** Breaking for suites that regex-match numbers. Ship with 2.1 in a version that already carries a CHANGELOG "Changed" section; consider 2.0.0 for the combined set (see phasing). + +### 1.6 Misused Proc schemas raise raw Ruby errors + +Priority: Medium. Category: Robustness. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/constraints.rb:31-36` assumes the Proc returned a Hash; `schema_match.rb:82-84` calls the Proc with no arguments: + +```ruby +match_json_schema({ a: -> { true } }).matches?('{"a":1}') # NoMethodError: undefined method 'keys' for true +match_json_schema({ a: ->(v) { v > 1 } }).matches?('{"a":2}') # ArgumentError: wrong number of arguments (given 0, expected 1) +``` + +**Problem and impact.** Both shapes are what a first-time user reaches for. The README shows the correct form (`-> { { lambda: ... } }`) but the error a user gets does not point there. 1.5.0 added `ArgumentError` for unknown option keys; the non-Hash and arity cases were not covered. + +**Recommended solution.** In `validate!`, raise `ArgumentError, "schema Proc must return an options Hash, got TrueClass"` when the result is not a Hash. In `compare_proc`, either raise a clear error for arity 1, or treat an arity-1 lambda as a predicate (`condition.call(value)`), which is the more useful behaviour and a small feature. + +**Dependencies and risks.** If arity-1 lambdas become predicates, document it and add specs; it overlaps with 4.2. + +### 1.7 `have_no_content_spec.rb` never tests the `"{}"` case + +Priority: Low. Category: Test correctness. Effort: S. Status: Confirmed. + +**Evidence.** `spec/rspec/json_api/matchers/have_no_content_spec.rb:12-20` calls `let(:actual)` twice inside one context from a `%w[{} []].each` loop. The second `let` overrides the first, so both examples run with `"[]"`. Verified by printing `actual` from a copy of the block: both print `"[]"`. The `describe` string "match_empty_body matcher" is also stale; the matcher is `have_no_content`. + +**Problem and impact.** One of the two negative cases is untested and the documentation-format output shows two identically named examples. + +**Recommended solution.** Wrap each value in its own `context "when #{value} is given"`, and rename the top-level describe. + +### 1.8 `same_key_structure?` cannot tell which parent a nested hash belongs to + +Priority: Low. Category: Correctness (latent). Effort: S. Status: Suggestion. + +**Evidence.** `lib/rspec/json_api/traversal.rb:15-20` flattens nested keys into the parent's list (`{a: {c: 1}, b: 2}` becomes `[:a, [:c], :b]`) and `deep_sort` (`traversal.rb:42-46`) sorts by string, so parent association is lost: + +```ruby +RSpec::JsonApi::SchemaMatch.same_key_structure?({ a: 1, b: { c: "x" } }, { a: { c: String }, b: Integer }) # => true +``` + +**Problem and impact.** I could not turn this into an end-to-end false positive, because `compare` re-derives full key paths and catches the mismatch. But the guard is weaker than its name and comment claim, and a future refactor of `compare` could expose it. + +**Recommended solution.** Compare sorted `deep_key_paths` of both sides instead of `deep_sort(deep_keys(...))`. That removes `deep_keys` and `deep_sort` entirely. Folds into 3.6. + +## 2. Package and dependency updates + +### 2.1 Drop `railties` and `rspec-rails` from the runtime dependencies + +Priority: High. Category: Dependencies / supply chain. Effort: M. Status: Confirmed. + +**Evidence.** `rspec-json_api.gemspec:35-38` declares `activesupport`, `diffy`, `railties` and `rspec-rails` as runtime dependencies. `grep -rn "rspec\|Rails" lib` shows that nothing in `lib/rspec/` references `rspec-rails` or Rails; the only Rails references are the three generator classes under `lib/generators/`, and those are only ever loaded by Rails itself (it scans `lib/generators` of bundled gems). The matcher module (`lib/rspec/json_api/matchers.rb`) only needs `RSpec::Matchers` from `rspec-expectations`. + +**Problem and impact.** Every consumer pulls `actionpack`, `actionview`, `rack`, `rack-session`, `nokogiri`, `loofah`, `rails-html-sanitizer`, `crass`, `irb`, `rackup` and friends into their bundle to get a JSON matcher. In the current dev lockfile those transitive gems account for every one of the 70+ advisories `bundle-audit` reports (see 2.3). It also means a non-Rails project (Sinatra, Hanami, plain Rack) cannot use the gem without taking on `railties`. 1.5.0 already went from `rails` to `railties` (87 to 65 gems); this is the second step. + +**Recommended solution.** +- Runtime: `activesupport` (until 2.2 lands), `diffy`, `rspec-expectations ~> 3.0`. +- Development: `railties`, `rspec-rails`, `rake`, `rubocop`, plus `bundler-audit` and `simplecov` (5.1, 5.3). +- Keep `lib/generators/**` where it is. Rails finds it when both the gem and Rails are in the bundle; without Rails the files are never required. Add a comment in each generator saying so. +- README: state that the generators need Rails, the matchers do not. + +**Dependencies and risks.** A consumer who never listed `rspec-rails` in their own Gemfile and relied on this gem pulling it in would lose it. That is unlikely (rspec-rails is what people install first) but call it out in the CHANGELOG and bump to 2.0.0 together with 1.5 and 2.2. + +### 2.2 Replace the ActiveSupport `blank?`/`present?` extension with a local helper + +Priority: Medium. Category: Dependencies. Effort: S. Status: Confirmed (usage), Suggestion (removal). + +**Evidence.** `lib/rspec/json_api.rb:7` requires `active_support/core_ext/object/blank`. It is used in exactly two places: `constraints.rb:24` (`value.blank?`) and `schema_match.rb:36` (`actual.blank? && expected.present?`). + +**Problem and impact.** Two call sites carry `activesupport` plus its own tail (`concurrent-ruby`, `i18n`, `tzinfo`, `minitest`, `drb`, `bigdecimal`, `logger`, `securerandom`, ...). After 2.1 this would be the last heavyweight dependency; removing it leaves `diffy` and `rspec-expectations`. + +**Recommended solution.** A private `RSpec::JsonApi::Blank.blank?(value)` that reproduces the semantics that matter here: `nil`, `false`, empty String/Array/Hash, and whitespace-only strings. Write the specs first (the whitespace case is what `allow_blank` users depend on), then swap the two call sites. The `schema_match.rb:36` line also deserves a second look: with `same_key_structure?` already enforced by `match`, it only fires for the exact-array path, and after 1.2 it can probably go. + +**Dependencies and risks.** Requires 2.1 to be worthwhile. `blank?` on unusual objects (e.g. `BigDecimal`) is not relevant because input always comes from `JSON.parse`. + +### 2.3 Move the development lockfile past open security advisories + +Priority: High. Category: Security / dependencies. Effort: S. Status: Confirmed. + +**Evidence.** `bundle-audit check --update` on `Gemfile.lock` (2026-09-03). Grouped by gem, with the fix version the advisory database names: + +| Gem | Locked | Advisories | Fix | +|-----|--------|------------|-----| +| rack | 3.2.4 | 14 (five rated High) | >= 3.2.6 | +| nokogiri | 1.19.0 | 13 (one High) | >= 1.19.4 | +| activesupport | 8.1.2 | 3 (ReDoS, XSS, DoS in helpers) | >= 8.1.2.1 | +| actionpack / actionview | 8.1.2 | 1 each (XSS) | >= 8.1.2.1 | +| concurrent-ruby | 1.3.6 | 3 (one High) | >= 1.3.7 | +| loofah | 2.25.0 | 4 | >= 2.25.2 | +| crass | 1.0.6 | 4 | >= 1.0.7 | +| erb | 6.0.1 | 1 (High, deserialization guard bypass) | >= 6.0.4 | +| json | 2.18.0 | 2 | >= 2.19.9 | +| rails-html-sanitizer | 1.6.2 | 1 (XSS) | >= 1.7.1 | +| rack-session | 2.1.1 | 1 (session forgery) | >= 2.1.2 | + +`bundle outdated` shows `railties 8.1.3.1` is available, which drags the Rails 8.1.2.1+ fixes in. + +**Problem and impact.** None of these gems are used by the matcher at runtime and the lockfile does not bind consumers, so the direct exposure is the CI runner and contributor machines. Two things still make it worth doing now: `Gemfile.lock` is packaged inside the `.gem` (5.5), so a scanner pointed at the published artifact flags it, and the list is the concrete argument for 2.1. + +**Recommended solution.** `bundle update --conservative rack nokogiri railties concurrent-ruby loofah crass erb json rails-html-sanitizer rack-session`, run the suite, commit the lockfile. Then add `bundle-audit` to CI (5.3) so the list does not grow back quietly. + +**Dependencies and risks.** Low. All are patch-level within the ranges the Gemfile allows. + +### 2.4 Routine minor updates + +Priority: Low. Category: Dependencies. Effort: S. Status: Confirmed. + +**Evidence.** `bundle outdated`: rubocop 1.82.1 to 1.90.0 (allowed by `~> 1.65`), rake 13.3.1 to 13.4.2, rspec-rails 8.0.2 to 8.0.4, zeitwerk 2.7.4 to 2.8.3, i18n 1.14.8 to 1.15.2, rdoc 7.1.0 to 8.0.0. Two transitive majors are available and can wait: `parallel` 2.1.0 (via rubocop) and `diff-lcs` 2.0.0 (via rspec). + +**Recommended solution.** Fold into the same PR as 2.3. Newer RuboCop versions add cops under `NewCops: enable` (`.rubocop.yml:10`), so expect a few new offences to address. + +### 2.5 `Gemfile` duplicates gemspec dependencies with divergent constraints + +Priority: Low. Category: Dependencies / hygiene. Effort: S. Status: Confirmed. + +**Evidence.** `Gemfile:10-13` re-declares `activesupport`, `diffy` and `rspec-rails` that `gemspec` (line 8) already brings in, and `diffy` is `"~> 3.4"` in the Gemfile versus `">= 3.4.2"` in the gemspec. + +**Problem and impact.** Two places to keep in sync; the gemspec is the one consumers see. The appraisal gemfiles inherit the root Gemfile (`gemfiles/*.gemfile:5`), so the duplication propagates to the CI matrix. + +**Recommended solution.** Keep only `gemspec`, `rake`, `rubocop` (and the new dev tools) in the Gemfile; put the rest in `add_development_dependency` after 2.1. + +## 3. Code quality and architecture improvements + +### 3.1 Report the failing key path instead of a one-line `inspect` diff + +Priority: High. Category: Architecture / developer experience. Effort: L. Status: Suggestion. + +**Evidence.** `SchemaMatch.match` (`schema_match.rb:15-28`) returns a bare Boolean with no context. `MatchJsonSchema#failure_message` (`match_json_schema.rb:37-45`) prints `expected` and `actual` with `Hash#to_s` and hands the same two strings to Diffy (`match_json_schema.rb:58-60`). Actual output for a schema with one Proc and one type mismatch: + +``` +expected: {id: Integer, name: String, tags: [String], nested: {x: #}} + got: {id: "1", name: "n", tags: ["a"], nested: {x: 0}} + +Diff: +-{id: Integer, name: String, tags: [String], nested: {x: #}} +\ No newline at end of file ++{id: "1", name: "n", tags: ["a"], nested: {x: 0}} +\ No newline at end of file +``` + +**Problem and impact.** For a 40-key response the user gets two very long lines, a Proc address with a local file path, and no indication of which key failed or why (wrong type, extra key, missing key, constraint). The diff is a full-line replace, so Diffy adds nothing over the two lines above it. Diffy also shells out to `diff(1)` per failure. This is the single biggest day-to-day cost of using the gem. + +**Recommended solution.** Make `SchemaMatch` a single recursive walk that collects `Mismatch` records (path, reason, expected, actual) instead of returning `false` at the first miss. Render them as `at $.children[1].age: expected Integer, got String ("x")` and `at $.children[0]: unexpected key "x"`. Render schema values by name (`Integer`, `/\A\d+\z/`, `Proc(type: String, min: 1)`) and pretty-print the actual JSON. Diffy then becomes optional or goes away. This walk also replaces `same_key_structure?` plus `deep_key_paths` plus root-relative `dig_path` (see 5.7 and 3.6). + +**Dependencies and risks.** Largest item in the roadmap. Do it after 1.1, 1.2 and 3.2 have specs, so the rewrite has a safety net. It changes the failure text; that is not a public API, but mention it. + +### 3.2 `compare_exact_array` does not dispatch element schemas + +Priority: Medium. Category: Correctness / consistency. Effort: S. Status: Confirmed. + +**Evidence.** `schema_match.rb:118`: `elem.is_a?(Hash) ? compare(...) : compare_simple_value(...)`. Classes, Regexps, Procs and nested arrays as array elements are compared with `==`: + +```ruby +match_json_schema({ pair: [Integer, Integer] }).matches?('{"pair":[1,2]}') # => false +match_json_schema({ m: [[Integer]] }).matches?('{"m":[[1,2],[3]]}') # => false +``` + +**Problem and impact.** Fixed-length tuples and lists of lists cannot be expressed at all, and the failure is silent: the schema looks valid and simply never matches. + +**Recommended solution.** Dispatch every element through `compare_values` (and Hash elements through `match`, per 1.2). Document tuples and nested lists in the README once they work. + +**Dependencies and risks.** Same method as 1.2; do both in one change. + +### 3.3 Array schema dispatch is positional and ambiguous + +Priority: Medium. Category: Design. Effort: M. Status: Suggestion. + +**Evidence.** `schema_match.rb:86-94, 126-132`. `[X]` means "list of X" when `X` is a Class, "list of interface" when `X` is a Hash, and "exactly one element equal to X" otherwise. `[String, NilClass]` therefore means a two-element tuple, not a union. The README only documents the `[Class]` and `[INTERFACE]` forms. + +**Problem and impact.** Users cannot say "a list of UUID-typed strings" (`[Types::UUID]` is a one-element exact array containing a Regexp) or "a list of procs". Every new feature in section 4 will make the positional rules harder to explain. + +**Recommended solution.** Keep the two shorthand forms for compatibility and add explicit helpers on `RSpec::JsonApi` (or a `Schema` module users can include): `array_of(schema)`, `tuple(...)`, `one_of(...)`, `optional(schema)`, `nullable(schema)`. Internally represent them as small value objects that `compare_values` dispatches on. This is the natural place to hang 4.1, 4.2 and 4.8. + +**Dependencies and risks.** Design decision; agree the DSL before 3.1 so the mismatch renderer knows about the new node types. + +### 3.4 Top-level scalar, Regexp and Class schemas always fail + +Priority: Medium. Category: Correctness. Effort: S. Status: Confirmed. + +**Evidence.** `schema_match.rb:16`: `return false unless actual.instance_of?(expected.class)`. For `expected = String` the class is `Class`, so a String body never matches: + +```ruby +match_json_schema(String).matches?('"hello"') # => false +match_json_schema(/\Ahello\z/).matches?('"hello"') # => false +``` + +**Problem and impact.** Endpoints that return a bare string, number or boolean cannot be matched at all, and the reason is not obvious. + +**Recommended solution.** Route non-Hash, non-Array roots through `compare_values`; keep the `instance_of?` guard for Hash and Array roots only. + +### 3.5 Strictness rules differ by nesting level + +Priority: Low. Category: Consistency / documentation. Effort: S. Status: Confirmed. + +**Evidence.** Probe outputs in 1.2: `allow_blank` accepts `null` but not a missing key at the top level, while inside an exact array a missing key passes. Interface arrays behave like the top level. + +**Recommended solution.** After 1.2 the behaviour is uniform (strict everywhere). Write the rule down in the README: "every key in the schema must be present; `allow_blank` accepts `null` or empty, not absence; use `optional` (4.1) for absence." + +### 3.6 `Traversal` can be a single recursive helper + +Priority: Low. Category: Simplification. Effort: S. Status: Suggestion. + +**Evidence.** `traversal.rb:15-46`. `deep_keys` recurses with `respond_to?(:keys)` while `deep_key_paths` uses an explicit stack with `is_a?(Hash)` and a final `reverse`; `deep_sort` exists only to normalise `deep_keys` output. + +**Recommended solution.** One `each_path(hash) { |path, value| }` enumerator covers both uses (and fixes 1.8). If 3.1 lands, the module disappears altogether. + +### 3.7 `example_interface.rb` ships in the gem but is a test fixture + +Priority: Low. Category: Packaging / hygiene. Effort: S. Status: Confirmed. + +**Evidence.** `lib/rspec/json_api/interfaces/example_interface.rb` is not required by `lib/rspec/json_api.rb`; the only consumer is `spec/rspec/json_api/matchers/match_json_schema_spec.rb:3`. It is included in the built gem (verified by listing `rspec-json_api-1.5.0.gem`). + +**Recommended solution.** Move it to `spec/support/example_interface.rb`. The README example that mirrors it can stay as a code block. + +### 3.8 Generator namespace and `class_path` handling + +Priority: Low. Category: Maintainability. Effort: S. Status: Confirmed. + +**Evidence.** Generators live in `Rspec::JsonApi::Generators` (`lib/generators/**/*_generator.rb:3-5`), while the library is `RSpec::JsonApi`. This is deliberate: Thor derives the CLI namespace by snake-casing the constant, and `RSpec` would become `r_spec:json_api:install`. Nothing in the code says so. Separately, `InterfaceGenerator` and `TypeGenerator` interpolate only `file_name` (`interface_generator.rb:10,16`, `type_generator.rb:10,16`), so `rails g rspec:json_api:interface admin/user` writes `interfaces/user.rb` and a constant `USER`, silently dropping the namespace. + +**Recommended solution.** Add a two-line comment explaining the `Rspec` spelling. Either honour `class_path` in the output path and constant, or reject namespaced names with a clear message. Both need the generator specs from 5.1. + +### 3.9 `MatchJsonSchema` lacks `description` + +Priority: Low. Category: RSpec integration. Effort: S. Status: Confirmed. + +**Evidence.** Probe: `matcher.respond_to?(:description)` is false. RSpec's one-liner syntax (`it { is_expected.to match_json_schema(SCHEMA) }`) and `--format documentation` then fall back to a generic phrase. `failure_message_when_negated` (`match_json_schema.rb:50-52`) also has a doc comment that describes `self` as the return value, which is wrong. + +**Recommended solution.** Add `description` ("match JSON schema") and consider `RSpec::Matchers::Composable` so the matcher can be nested in `include` and `all`. Fix the comment. + +## 4. Feature proposals + +All items in this section are suggestions. + +### 4.1 Optional keys and nullable values + +Priority: High. Category: Schema DSL. Effort: M. + +**Evidence.** Today the only relaxation is `allow_blank` (`constraints.rb:24`), which accepts `null` or `""` but still requires the key to exist at the top level (probe in 1.2). There is no way to say "this key may be absent". + +**Problem and impact.** Paginated and conditional responses (`next_page` only on some pages, `deleted_at` only for deleted rows) force users to write two schemas or to loosen the whole response. + +**Recommended solution.** `optional(schema)` and `nullable(schema)` helpers (3.3), plus `optional: true` as a Proc option for people who prefer the Hash style. `optional` keys are excluded from the key-structure check when absent. + +**Dependencies and risks.** Needs 3.3 for the node types and 3.1 to report "missing required key" versus "unexpected key". + +### 4.2 Union types, a Boolean type, and `is_a?` semantics + +Priority: High. Category: Schema DSL. Effort: M. + +**Evidence.** JSON booleans have no single Ruby class, so the only ways to check one today are `inclusion: [true, false]` or a lambda (probe: both work but are undocumented). `type:` uses `instance_of?` (`constraints.rb:40`), so `type: Numeric` rejects `1` and `type: Float` rejects `1` even though JSON does not distinguish `1` from `1.0`. + +**Recommended solution.** `Types::BOOLEAN`, `one_of(Integer, Float)`, and `type:` accepting an Array of classes; switch the class check to `is_a?` so `Numeric` works (document it as a behaviour change). Also accept an arity-1 lambda directly as a predicate (1.6). + +### 4.3 Accept parsed JSON and response objects + +Priority: Medium. Category: Ergonomics. Effort: S. + +**Evidence.** `matches?` (`match_json_schema.rb:27`) only accepts a JSON String; a Hash raises (1.4). Request specs commonly have `response.parsed_body` or `JSON.parse(response.body)` at hand, and `have_no_content` (`have_no_content.rb:17`) has the same limitation. + +**Recommended solution.** If `actual` is a Hash or Array, deep-symbolise and use it; if it responds to `body`, call it; otherwise parse. Same for `have_no_content`. + +### 4.4 More built-in types + +Priority: Medium. Category: Types. Effort: S. + +**Evidence.** `lib/rspec/json_api/types/` has EMAIL, URI and UUID. `Types::URI` accepts any scheme (`mailto:`, `urn:`). + +**Recommended solution.** `Types::URL` (http/https), `Types::ISO8601_DATE`, `Types::ISO8601_DATETIME`, `Types::INTEGER_STRING`. Implement date types as lambdas around `Date.iso8601`/`Time.iso8601` rather than regexps so `2026-02-30` is rejected. + +### 4.5 Load user-defined types and interfaces automatically + +Priority: Medium. Category: Ergonomics. Effort: S. + +**Evidence.** `README.md:27-31` asks every user to paste two `Dir[...]` `require` loops into `rails_helper.rb`; the install generator (`install_generator.rb:9-11`) creates the directories but not the loader. + +**Recommended solution.** `RSpec::JsonApi.load_definitions(root = "spec/rspec/json_api")` that requires `types/*.rb` then `interfaces/*.rb` (interfaces reference types, so order matters), and have the install generator append the one-liner to `rails_helper.rb`. + +### 4.6 String keys in schemas + +Priority: Low. Category: Ergonomics. Effort: S. + +**Evidence.** `matches?` parses with `symbolize_names: true` (`match_json_schema.rb:27`), so `{ "id" => String }` never matches (probe). The README does not say keys must be symbols. + +**Recommended solution.** Deep-symbolise the schema keys once in `initialize`, or document the rule. Symbolising is friendlier and cheap. + +### 4.7 `have_no_content` failure message should show the body + +Priority: Low. Category: Developer experience. Effort: S. + +**Evidence.** `have_no_content.rb:26-37`: both messages are fixed strings; the user has to add a `puts` to see what came back. + +**Recommended solution.** Include a truncated `actual.inspect`, and accept response objects (4.3). + +### 4.8 Size constraints for lists + +Priority: Low. Category: Schema DSL. Effort: S. + +**Evidence.** `[String]` accepts `[]` (probe) and there is no way to require at least one element without a lambda. + +**Recommended solution.** `array_of(String, min: 1, max: 50)` on the 3.3 helpers. + +## 5. Other findings + +### 5.1 Testing gaps + +Priority: High. Category: Test debt. Effort: M. Status: Confirmed. + +**Evidence.** +- Every crash and false positive in section 1 lacks a spec; that is how they survived the 1.5.0 refactor. +- `Traversal` and `SchemaMatch` have no direct specs; they are exercised only through the matcher. +- `Constraints` has four examples (`constraints_spec.rb`); `inclusion`, `regex`, `lambda` and `max` are covered only via the matcher, and non-numeric `min`/`max` and Proc misuse are not covered at all. +- The three generators have zero tests. +- `match_json_schema_spec.rb` is 1,128 lines of nested `let` fixtures with 52 `include_examples` and 4 plain `it`s; adding a case means copying a 30-line block. +- `spec_helper.rb` does not enable `config.order = :random` or `config.warnings = true`. +- No coverage tooling. + +**Recommended solution.** Add specs for each section 1 input as the first commit of each fix. Add unit specs for `Constraints` and `SchemaMatch` with a table-driven style (`[schema, json, expected_result]` rows). Add generator specs with `Rails::Generators::TestCase` or the `ammeter` gem under the Rails appraisal jobs. Add SimpleCov with a floor. Turn on random ordering. + +### 5.2 The CI compatibility matrix exercises almost no Rails code + +Priority: Medium. Category: CI. Effort: M. Status: Confirmed. + +**Evidence.** `.github/workflows/main.yml:15-21` runs six Ruby/Rails pairs, but `spec/spec_helper.rb` requires only `rspec/json_api`, which in turn requires a single ActiveSupport file (`json_api.rb:7`). Nothing requires `rails` or `rspec-rails`, and the generators are never invoked. Each Rails job therefore tests `Object#blank?` against that Rails version. The 1.5.0 CHANGELOG describes the matrix as making "the advertised version support actually tested". + +**Problem and impact.** Six jobs of CI time for one line of coverage, and a false sense that Rails 6.1 through 8.1 compatibility is verified. Ruby 4.0 is also absent even though 1.5.0 shipped a Ruby 4.0 load fix (commit `8f7318a`), and the local lockfile was resolved on Ruby 4.0. + +**Recommended solution.** After 2.1, the runtime matrix only needs Ruby versions (3.2, 3.3, 3.4, 4.0) with the plain Gemfile. Keep one or two Rails appraisals that actually load `rspec-rails` and run the generator specs from 5.1 against a minimal dummy app. Add `permissions: contents: read` and a `concurrency` group while touching the file. + +### 5.3 Supply-chain checks are not automated + +Priority: Medium. Category: Security / DevOps. Effort: S. Status: Confirmed. + +**Evidence.** No `.github/dependabot.yml`; no `bundler-audit` step in the workflow; the workflow has no `permissions:` block (`main.yml`); `actions/checkout@v4` where v5 is current. + +**Recommended solution.** Dependabot for `bundler` and `github-actions` (weekly), a `bundle-audit check --update` job, least-privilege `permissions`, and the checkout bump. `rubygems_mfa_required` is already set in the gemspec (line 19), which is good. + +### 5.4 Release process + +Priority: Medium. Category: DevOps. Effort: M. Status: Confirmed. + +**Evidence.** Tags exist for v1.0.0, v1.0.1, v1.0.2, v1.1.0, v1.1.1 and v1.5.0 only; 1.2.x, 1.3.x and 1.4.0 were released without tags. `CHANGELOG.md` has entries for 1.5.0, 1.4.0 and 0.1.0 and nothing in between. Releases are manual (`rake release` from `bundler/gem_tasks`); a built `rspec-json_api-1.5.0.gem` sits in the working tree (gitignored). + +**Recommended solution.** A release workflow using RubyGems Trusted Publishing triggered by a `v*` tag, so publishing requires a tag and a green build. Backfill the missing tags from the version-bump commits and write short CHANGELOG entries from `git log` for 1.1 to 1.3.1. + +### 5.5 The gem packages repository tooling + +Priority: Low. Category: Packaging. Effort: S. Status: Confirmed. + +**Evidence.** `rspec-json_api.gemspec:23-25` includes every tracked file except `test/`, `spec/` and `features/`. Listing the built gem shows `.github/workflows/main.yml`, `.rubocop.yml`, `.gitattributes`, `.gitignore`, `.rspec`, `.ruby-version`, `Gemfile`, `Gemfile.lock`, `Rakefile`, `bin/console`, `bin/setup` and `gemfiles/*.gemfile` inside it. + +**Recommended solution.** `spec.files = Dir["lib/**/*"] + %w[LICENSE.txt README.md CHANGELOG.md]`. Drop `spec.bindir`/`spec.executables` (no `exe/` directory exists). + +### 5.6 Documentation + +Priority: Medium. Category: Documentation. Effort: S. Status: Confirmed. + +**Evidence.** `README.md`: +- Typos and grammar: "build-in" (lines 35, 114), "The gem allow users either to user build-in types or define owns" (line 113), "Proc match allows to customize schema according needs" (line 239), "The gem offers variety of possible matching methods" (line 146). +- Behaviours that are undocumented: invalid JSON yields a failed match (not an error); unknown Proc options raise `ArgumentError`; schema keys must be symbols; `type:` uses `instance_of?`; `[X]` semantics and the lack of tuples; top-level must be an object or array; how `allow_blank` interacts with missing keys. +- No supported Ruby/Rails matrix, even though the gemspec floor (Ruby 3.2, Rails 6.1.4.1) and the CI matrix define one. +- The name suggests the JSON:API specification (jsonapi.org); the gem is a general JSON-shape matcher. One sentence at the top would save readers a wrong assumption. +- No `CONTRIBUTING.md` or `SECURITY.md`; `bin/setup` and the toolchain (`.ruby-version` 3.2.2, Bundler 4.0.4 in the lockfile) are not mentioned anywhere. + +**Recommended solution.** Fix the prose, add a "Behaviour reference" section that answers the bullets above, add a support matrix, and a short contributing section. Update again when 3.3 and section 4 land. + +### 5.7 Performance + +Priority: Low. Category: Performance. Effort: folded into 3.1. Status: Suggestion. + +**Evidence.** For each object, `match` walks the tree for `same_key_structure?` (`schema_match.rb:30-33`), then `compare` walks both sides again for `deep_key_paths` and calls `dig_path` from the root for every leaf path (`schema_match.rb:35-62`), and this repeats for every element of every array. Diffy shells out to `diff(1)` on each failure. + +**Problem and impact.** Not measurable on typical API payloads; a few thousand keys would still finish in milliseconds. It is listed because 3.1 replaces all of it with a single walk, so no separate work is warranted. + +### 5.8 Observability + +Not applicable to a test-matcher gem in the usual sense. The equivalent concern is failure-message quality, which is 3.1 and 4.7. + +### 5.9 Local development setup + +Priority: Low. Category: Developer experience. Effort: S. Status: Confirmed. + +**Evidence.** `.ruby-version` pins 3.2.2, `Gemfile.lock` says `BUNDLED WITH 4.0.4` and lists `arm64-darwin-25` plus a `nokogiri` build for it, so the lockfile was last resolved on a newer Ruby than the one the repo declares. `mise.toml` is gitignored (commit `87fddee`). `bin/setup` only runs `bundle install`. + +**Recommended solution.** Decide on one declared development Ruby (3.4 or 4.0), regenerate the lockfile on it, and say in the README which Ruby and Bundler the lockfile expects. + +## Phased plan + +**Phase 1, patch release 1.5.1 (about a day).** 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5. All additive or pure fixes, each with a spec. + +**Phase 2, 2.0.0 (one to two weeks).** 1.2, 1.5, 3.2, 3.4, 3.9 (behaviour changes bundled in one CHANGELOG), 2.1 and 2.2 (dependency cut), 2.5, 3.7, 5.1 unit specs, 5.2 matrix rework, 5.6 documentation. Bump the major because 1.2, 1.5 and 2.1 can each break an existing suite. + +**Phase 3, 2.x (ongoing).** 3.3 DSL helpers, then 4.1, 4.2, 4.8 on top of them; 3.1 mismatch reporting once the DSL is settled; 4.3, 4.4, 4.5, 4.6, 4.7; 5.4 release automation; 1.8 and 3.6 disappear as part of 3.1. From ec61f689ad7a85f9298d2341d04680584c72b18d Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:03:52 +0200 Subject: [PATCH 02/13] Fix: fail instead of crashing when a list schema meets a non-array value --- lib/rspec/json_api/schema_match.rb | 8 ++- .../matchers/match_json_schema_spec.rb | 62 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/lib/rspec/json_api/schema_match.rb b/lib/rspec/json_api/schema_match.rb index 4fe51b2..3ada5a4 100644 --- a/lib/rspec/json_api/schema_match.rb +++ b/lib/rspec/json_api/schema_match.rb @@ -83,7 +83,13 @@ def compare_proc(actual_value, expected_value) Constraints.match(actual_value, expected_value.call) end + # A list schema only ever matches an actual Array. Without this guard the + # branches below call Array methods on whatever the response contained, so + # a null or a scalar where a list was expected raised NoMethodError + # instead of failing the match. def compare_array(actual_value, expected_value) + return false unless actual_value.is_a?(Array) + if simple_type?(expected_value) compare_typed_array(actual_value, expected_value) elsif interface?(expected_value) @@ -112,7 +118,7 @@ def compare_interface_array(actual_value, expected_value) # Any other array => element-by-element match, sizes must be equal. def compare_exact_array(actual_value, expected_value) - return false if actual_value&.size != expected_value&.size + return false if actual_value.size != expected_value.size expected_value.each_with_index.all? do |elem, index| elem.is_a?(Hash) ? compare(actual_value[index], elem) : compare_simple_value(actual_value[index], elem) diff --git a/spec/rspec/json_api/matchers/match_json_schema_spec.rb b/spec/rspec/json_api/matchers/match_json_schema_spec.rb index d189638..4bd6014 100644 --- a/spec/rspec/json_api/matchers/match_json_schema_spec.rb +++ b/spec/rspec/json_api/matchers/match_json_schema_spec.rb @@ -514,6 +514,68 @@ end end + context "when a list schema meets a non-array value" do + context "when a typed-array schema meets a scalar" do + let(:expected) do + { notes: [String] } + end + + let(:actual) do + { notes: "x" }.to_json + end + + include_examples "incorrect-match" + end + + context "when a typed-array schema meets null" do + let(:expected) do + { notes: [String] } + end + + let(:actual) do + { notes: nil }.to_json + end + + include_examples "incorrect-match" + end + + context "when an interface-array schema meets null" do + let(:expected) do + { items: [{ id: Integer }] } + end + + let(:actual) do + { items: nil }.to_json + end + + include_examples "incorrect-match" + end + + context "when a nested typed-array schema meets a scalar" do + let(:expected) do + { items: [{ tags: [String] }] } + end + + let(:actual) do + { items: [{ tags: "b" }] }.to_json + end + + include_examples "incorrect-match" + end + + context "when an exact-array schema meets a string of the same length" do + let(:expected) do + { tags: [1, 2] } + end + + let(:actual) do + { tags: "ab" }.to_json + end + + include_examples "incorrect-match" + end + end + context "when proc given" do context "when type comparison" do let(:expected) do From fdcf568e0f1fac73fc922c32e4d31f8dcd191d24 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:04:32 +0200 Subject: [PATCH 03/13] Fix: anchor URI type so a uri inside surrounding text no longer matches --- lib/rspec/json_api/types/uri.rb | 6 +++++- .../json_api/matchers/match_json_schema_spec.rb | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/rspec/json_api/types/uri.rb b/lib/rspec/json_api/types/uri.rb index 65e52d2..ad2924a 100644 --- a/lib/rspec/json_api/types/uri.rb +++ b/lib/rspec/json_api/types/uri.rb @@ -3,7 +3,11 @@ module RSpec module JsonApi module Types - URI = URI::DEFAULT_PARSER.make_regexp + # URI::DEFAULT_PARSER.make_regexp is unanchored, and comparison uses + # Regexp#match?, so on its own it accepts any string that merely contains + # a URI ("see https://example.com for details"). \A...\z holds the whole + # value to the pattern, the same way EMAIL and UUID already are anchored. + URI = /\A#{::URI::DEFAULT_PARSER.make_regexp}\z/ end end end diff --git a/spec/rspec/json_api/matchers/match_json_schema_spec.rb b/spec/rspec/json_api/matchers/match_json_schema_spec.rb index 4bd6014..9bd056d 100644 --- a/spec/rspec/json_api/matchers/match_json_schema_spec.rb +++ b/spec/rspec/json_api/matchers/match_json_schema_spec.rb @@ -386,6 +386,22 @@ include_examples "incorrect-match" end + + context "when a valid uri is embedded in surrounding text" do + let(:actual) do + { uri: "see https://example.com for details" }.to_json + end + + include_examples "incorrect-match" + end + + context "when a valid uri is surrounded by whitespace" do + let(:actual) do + { uri: " https://example.com " }.to_json + end + + include_examples "incorrect-match" + end end describe "uuid" do From 601962209fea29c80e8bcbfb8c4c17a7eade1b4d Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:06:11 +0200 Subject: [PATCH 04/13] Fix: fail with a typed message when actual is not a JSON String --- .../json_api/matchers/match_json_schema.rb | 36 ++++++++++++++++--- .../matchers/match_json_schema_spec.rb | 23 ++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/lib/rspec/json_api/matchers/match_json_schema.rb b/lib/rspec/json_api/matchers/match_json_schema.rb index 51626f6..a7b30af 100644 --- a/lib/rspec/json_api/matchers/match_json_schema.rb +++ b/lib/rspec/json_api/matchers/match_json_schema.rb @@ -24,17 +24,19 @@ def initialize(expected) # @return [Boolean] true if the actual JSON matches the expected schema, false otherwise. def matches?(actual) @diff = nil - @actual = JSON.parse(actual, symbolize_names: true) + @type_error = false + @actual = actual + + return false unless parse RSpec::JsonApi::SchemaMatch.match(@actual, expected) - rescue JSON::ParserError - @actual = actual - false end # Provides a failure message for when the JSON data does not match the expected schema. # @return [String] A descriptive message detailing the mismatch between expected and actual JSON. def failure_message + return type_error_message if @type_error + <<~MSG expected: #{expected} got: #{actual} @@ -46,18 +48,42 @@ def failure_message # Provides a failure message for when the JSON data matches the expected schema, but it was expected not to. # This is used in negative matchers. - # @return [self] Returns itself, but typically this method should be implemented to return a descriptive message + # @return [String] A descriptive message indicating the JSON was expected not to match the schema. def failure_message_when_negated + return type_error_message if @type_error + "expected the JSON data not to match the provided schema, but it did." end private + # Replaces @actual with its parsed form. Returns false when the input + # could not be parsed, leaving @actual as the raw value so the failure + # message can still show what came back. + # + # JSON.parse raises TypeError rather than JSON::ParserError when handed + # something that is not a String at all, such as nil or an already-parsed + # Hash. Both are easy mistakes to make in a request spec, so they fail + # the match instead of erroring the example out. + def parse + @actual = JSON.parse(@actual, symbolize_names: true) + true + rescue JSON::ParserError + false + rescue TypeError + @type_error = true + false + end + # The diff is only needed to render a failure message, so it is built # lazily and memoized rather than on every matches? call. def diff @diff ||= Diffy::Diff.new(expected, actual, context: 5) end + + def type_error_message + "expected a JSON String to match against the schema, got #{actual.class}" + end end end end diff --git a/spec/rspec/json_api/matchers/match_json_schema_spec.rb b/spec/rspec/json_api/matchers/match_json_schema_spec.rb index 9bd056d..00c0401 100644 --- a/spec/rspec/json_api/matchers/match_json_schema_spec.rb +++ b/spec/rspec/json_api/matchers/match_json_schema_spec.rb @@ -27,6 +27,29 @@ include_examples "incorrect-match" end + context "when actual is not a JSON String" do + it "fails instead of raising for nil" do + matcher = match_json_schema({ id: String }) + + expect { matcher.matches?(nil) }.not_to raise_error + expect(matcher.matches?(nil)).to be(false) + end + + it "fails instead of raising for an already-parsed Hash" do + matcher = match_json_schema({ id: String }) + + expect { matcher.matches?({ id: "x" }) }.not_to raise_error + expect(matcher.matches?({ id: "x" })).to be(false) + end + + it "names the offending type in the failure message" do + matcher = match_json_schema({ id: String }) + matcher.matches?(nil) + + expect(matcher.failure_message).to include("JSON String", "NilClass") + end + end + context "when the schema matches" do it "does not build a diff" do expect(Diffy::Diff).not_to receive(:new) From 0d38c4f5c6a2f3feaf68c4e66deade95a57f6546 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:07:30 +0200 Subject: [PATCH 05/13] Fix: raise a clear ArgumentError for misused schema Procs --- lib/rspec/json_api/constraints.rb | 2 ++ lib/rspec/json_api/schema_match.rb | 14 ++++++++++++++ spec/rspec/json_api/constraints_spec.rb | 5 +++++ .../json_api/matchers/match_json_schema_spec.rb | 12 ++++++++++++ 4 files changed, 33 insertions(+) diff --git a/lib/rspec/json_api/constraints.rb b/lib/rspec/json_api/constraints.rb index eb03abe..89634b8 100644 --- a/lib/rspec/json_api/constraints.rb +++ b/lib/rspec/json_api/constraints.rb @@ -29,6 +29,8 @@ def match(value, options) end def validate!(options) + raise ArgumentError, "schema Proc must return an options Hash, got #{options.class}" unless options.is_a?(Hash) + unknown = options.keys - SUPPORTED_OPTIONS return if unknown.empty? diff --git a/lib/rspec/json_api/schema_match.rb b/lib/rspec/json_api/schema_match.rb index 3ada5a4..a980350 100644 --- a/lib/rspec/json_api/schema_match.rb +++ b/lib/rspec/json_api/schema_match.rb @@ -79,10 +79,24 @@ def compare_regexp(actual_value, expected_value) expected_value.match?(actual_value.to_s) end + # A schema Proc describes the constraints for a value; it is called without + # arguments and must return the option Hash. A Proc that expects the value + # as an argument is a common misreading of the DSL, and calling it here + # would raise a bare "wrong number of arguments" from deep in the matcher. def compare_proc(actual_value, expected_value) + unless zero_arity?(expected_value) + raise ArgumentError, + "schema Proc must take no arguments; " \ + "write -> { { lambda: ->(value) { ... } } } to test the value itself" + end + Constraints.match(actual_value, expected_value.call) end + def zero_arity?(callable) + callable.parameters.none? { |type, _name| %i[req keyreq].include?(type) } + end + # A list schema only ever matches an actual Array. Without this guard the # branches below call Array methods on whatever the response contained, so # a null or a scalar where a list was expected raised NoMethodError diff --git a/spec/rspec/json_api/constraints_spec.rb b/spec/rspec/json_api/constraints_spec.rb index b35ff85..7f7f4a2 100644 --- a/spec/rspec/json_api/constraints_spec.rb +++ b/spec/rspec/json_api/constraints_spec.rb @@ -7,6 +7,11 @@ .to raise_error(ArgumentError, /Unsupported match option/) end + it "raises ArgumentError when the options are not a Hash" do + expect { described_class.match("value", true) } + .to raise_error(ArgumentError, /must return an options Hash/) + end + it "accepts a blank value when allow_blank is true" do expect(described_class.match(nil, value: "John", allow_blank: true)).to be(true) end diff --git a/spec/rspec/json_api/matchers/match_json_schema_spec.rb b/spec/rspec/json_api/matchers/match_json_schema_spec.rb index 00c0401..9c57016 100644 --- a/spec/rspec/json_api/matchers/match_json_schema_spec.rb +++ b/spec/rspec/json_api/matchers/match_json_schema_spec.rb @@ -50,6 +50,18 @@ end end + context "when a schema Proc is misused" do + it "raises ArgumentError when the Proc does not return an options Hash" do + expect { match_json_schema({ a: -> { true } }).matches?({ a: 1 }.to_json) } + .to raise_error(ArgumentError, /must return an options Hash/) + end + + it "raises ArgumentError when the Proc expects an argument" do + expect { match_json_schema({ a: ->(value) { value > 1 } }).matches?({ a: 2 }.to_json) } + .to raise_error(ArgumentError, /must take no arguments/) + end + end + context "when the schema matches" do it "does not build a diff" do expect(Diffy::Diff).not_to receive(:new) From db7a638d483a74e104852065a54d35cab7dbeaf5 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:08:01 +0200 Subject: [PATCH 06/13] Fix: give each have_no_content case its own context so all bodies run --- .../json_api/matchers/have_no_content_spec.rb | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/spec/rspec/json_api/matchers/have_no_content_spec.rb b/spec/rspec/json_api/matchers/have_no_content_spec.rb index 1b15853..d1e2bed 100644 --- a/spec/rspec/json_api/matchers/have_no_content_spec.rb +++ b/spec/rspec/json_api/matchers/have_no_content_spec.rb @@ -1,20 +1,25 @@ # frozen_string_literal: true -RSpec.describe "match_empty_body matcher" do - context "when empty value is given" do +RSpec.describe "have_no_content matcher" do + context "when an empty string is given" do let(:actual) { "" } - it "matches expected schema" do + it "matches" do expect(actual).to have_no_content end end - context "when non-empty value is given" do - %w[{} []].each do |actual_value| - let(:actual) { actual_value } + context "when a non-empty string is given" do + # Each value needs its own context: declaring let(:actual) more than once in + # a single context makes the last declaration win, so the earlier values + # would never be exercised. + ["{}", "[]", '{"id":1}', " "].each do |actual_value| + context "when the body is #{actual_value.inspect}" do + let(:actual) { actual_value } - it "matches expected schema" do - expect(actual).not_to have_no_content + it "does not match" do + expect(actual).not_to have_no_content + end end end end From ec965dc99dae55449bc1331652a2ee72a21f1a25 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:08:14 +0200 Subject: [PATCH 07/13] Deps: update locked gems past open security advisories --- Gemfile.lock | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index bfbf2fa..e1b3f95 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,9 +10,9 @@ PATH GEM remote: https://rubygems.org/ specs: - actionpack (8.1.2) - actionview (= 8.1.2) - activesupport (= 8.1.2) + actionpack (8.1.3.1) + actionview (= 8.1.3.1) + activesupport (= 8.1.3.1) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -20,13 +20,13 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actionview (8.1.2) - activesupport (= 8.1.2) + actionview (8.1.3.1) + activesupport (= 8.1.3.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activesupport (8.1.2) + activesupport (8.1.3.1) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -43,14 +43,14 @@ GEM base64 (0.3.0) bigdecimal (4.0.1) builder (3.3.0) - concurrent-ruby (1.3.6) + concurrent-ruby (1.3.8) connection_pool (3.0.2) - crass (1.0.6) + crass (1.0.7) date (3.5.1) diff-lcs (1.6.2) diffy (3.4.4) drb (2.2.3) - erb (6.0.1) + erb (6.0.7) erubi (1.13.1) i18n (1.14.8) concurrent-ruby (~> 1.0) @@ -59,20 +59,20 @@ GEM pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - json (2.18.0) + json (2.21.2) language_server-protocol (3.17.0.5) lint_roller (1.1.0) logger (1.7.0) - loofah (2.25.0) + loofah (2.25.2) crass (~> 1.0.2) nokogiri (>= 1.12.0) minitest (6.0.1) prism (~> 1.5) - nokogiri (1.19.0-arm64-darwin) + nokogiri (1.19.4-arm64-darwin) racc (~> 1.4) - nokogiri (1.19.0-x86_64-darwin) + nokogiri (1.19.4-x86_64-darwin) racc (~> 1.4) - nokogiri (1.19.0-x86_64-linux-gnu) + nokogiri (1.19.4-x86_64-linux-gnu) racc (~> 1.4) parallel (1.27.0) parser (3.3.10.1) @@ -86,8 +86,8 @@ GEM date stringio racc (1.8.1) - rack (3.2.4) - rack-session (2.1.1) + rack (3.2.7) + rack-session (2.1.2) base64 (>= 0.1.0) rack (>= 3.0.0) rack-test (2.2.0) @@ -98,12 +98,12 @@ GEM activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.6.2) - loofah (~> 2.21) + rails-html-sanitizer (1.7.1) + loofah (~> 2.25, >= 2.25.2) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) + railties (8.1.3.1) + actionpack (= 8.1.3.1) + activesupport (= 8.1.3.1) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) From 48278d90153bb13c5c01cccb6068b77fa7660abe Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:08:37 +0200 Subject: [PATCH 08/13] Deps: refresh remaining locked gems to current minor versions --- Gemfile.lock | 68 +++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e1b3f95..f3c185d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -41,32 +41,33 @@ GEM uri (>= 0.13.1) ast (2.4.3) base64 (0.3.0) - bigdecimal (4.0.1) + bigdecimal (4.1.2) builder (3.3.0) concurrent-ruby (1.3.8) connection_pool (3.0.2) crass (1.0.7) - date (3.5.1) diff-lcs (1.6.2) diffy (3.4.4) drb (2.2.3) erb (6.0.7) erubi (1.13.1) - i18n (1.14.8) + i18n (1.15.2) concurrent-ruby (~> 1.0) - io-console (0.8.2) - irb (1.16.0) + io-console (0.9.2) + irb (1.18.0) pp (>= 0.6.0) + prism (>= 1.3.0) rdoc (>= 4.0.0) reline (>= 0.4.2) json (2.21.2) - language_server-protocol (3.17.0.5) + language_server-protocol (3.17.0.6) lint_roller (1.1.0) logger (1.7.0) loofah (2.25.2) crass (~> 1.0.2) nokogiri (>= 1.12.0) - minitest (6.0.1) + minitest (6.0.6) + drb (~> 2.0) prism (~> 1.5) nokogiri (1.19.4-arm64-darwin) racc (~> 1.4) @@ -74,17 +75,14 @@ GEM racc (~> 1.4) nokogiri (1.19.4-x86_64-linux-gnu) racc (~> 1.4) - parallel (1.27.0) - parser (3.3.10.1) + parallel (2.1.0) + parser (3.3.12.0) ast (~> 2.4.1) racc - pp (0.6.3) + pp (0.6.4) prettyprint prettyprint (0.2.0) - prism (1.8.0) - psych (5.3.1) - date - stringio + prism (1.9.0) racc (1.8.1) rack (3.2.7) rack-session (2.1.2) @@ -111,48 +109,52 @@ GEM tsort (>= 0.2) zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.3.1) - rdoc (7.1.0) + rake (13.4.2) + rbs (4.2.0) + logger + prism (>= 1.6.0) + tsort + rdoc (8.0.0) erb - psych (>= 4.0.0) + prism (>= 1.6.0) + rbs (>= 4.0.0) tsort - regexp_parser (2.11.3) - reline (0.6.3) + regexp_parser (2.12.0) + reline (0.7.0) io-console (~> 0.5) rspec-core (3.13.6) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-mocks (3.13.7) + rspec-mocks (3.13.8) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-rails (8.0.2) + rspec-rails (8.0.4) actionpack (>= 7.2) activesupport (>= 7.2) railties (>= 7.2) - rspec-core (~> 3.13) - rspec-expectations (~> 3.13) - rspec-mocks (~> 3.13) - rspec-support (~> 3.13) - rspec-support (3.13.6) - rubocop (1.82.1) - json (~> 2.3) + rspec-core (>= 3.13.0, < 5.0.0) + rspec-expectations (>= 3.13.0, < 5.0.0) + rspec-mocks (>= 3.13.0, < 5.0.0) + rspec-support (>= 3.13.0, < 5.0.0) + rspec-support (3.13.7) + rubocop (1.90.0) + json (>= 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) - parallel (~> 1.10) + parallel (>= 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.48.0, < 2.0) + rubocop-ast (>= 1.49.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.49.0) + rubocop-ast (1.50.0) parser (>= 3.3.7.2) prism (~> 1.7) ruby-progressbar (1.13.0) securerandom (0.4.1) - stringio (3.2.0) thor (1.5.0) tsort (0.2.0) tzinfo (2.0.6) @@ -162,7 +164,7 @@ GEM unicode-emoji (4.2.0) uri (1.1.1) useragent (0.16.11) - zeitwerk (2.7.4) + zeitwerk (2.8.3) PLATFORMS arm64-darwin-22 From 7576fb8672030d955c695e50e7553e7c08ecdc46 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:09:51 +0200 Subject: [PATCH 09/13] Chore: package only the library and reference docs in the gem --- rspec-json_api.gemspec | 13 ++++++----- spec/rspec/json_api/gemspec_spec.rb | 34 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 spec/rspec/json_api/gemspec_spec.rb diff --git a/rspec-json_api.gemspec b/rspec-json_api.gemspec index ecd42cb..6e1cbfb 100644 --- a/rspec-json_api.gemspec +++ b/rspec-json_api.gemspec @@ -18,13 +18,16 @@ Gem::Specification.new do |spec| spec.metadata["changelog_uri"] = "https://github.com/nomtek/rspec-json_api/blob/master/CHANGELOG.md" spec.metadata["rubygems_mfa_required"] = "true" - # Specify which files should be added to the gem when it is released. - # The `git ls-files -z` loads the files in the RubyGem that have been added into git. + # Ship what a consumer loads and nothing else: the library, the generators + # (templates included, so the dotfile markers that keep the empty interface + # and type directories must be globbed too), plus the licence and reference + # documents. Listing every tracked file, as `git ls-files` did, packaged the + # repository's own tooling inside the released gem: the CI workflow, the + # RuboCop config, the Gemfile and lockfile, the Rakefile, bin/ and gemfiles/. spec.files = Dir.chdir(File.expand_path(__dir__)) do - `git ls-files -z`.split("\x0").reject { |f| f.match(%r{\A(?:test|spec|features)/}) } + Dir.glob("lib/**/*", File::FNM_DOTMATCH).select { |f| File.file?(f) }.sort + + %w[CHANGELOG.md LICENSE.txt README.md] end - spec.bindir = "exe" - spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] # Runtime dependencies. The gem only needs ActiveSupport's blank?/present? diff --git a/spec/rspec/json_api/gemspec_spec.rb b/spec/rspec/json_api/gemspec_spec.rb new file mode 100644 index 0000000..0a7d05c --- /dev/null +++ b/spec/rspec/json_api/gemspec_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +RSpec.describe "rspec-json_api.gemspec" do + subject(:gemspec) do + Gem::Specification.load(File.expand_path("../../../rspec-json_api.gemspec", __dir__)) + end + + it "packages the library" do + expect(gemspec.files).to include("lib/rspec/json_api.rb", "lib/rspec/json_api/version.rb") + end + + it "packages the generator templates, including the empty-directory markers" do + expect(gemspec.files).to include( + "lib/generators/rspec/json_api/install/install_generator.rb", + "lib/generators/rspec/json_api/install/templates/rspec/json_api/types/.empty_directory", + "lib/generators/rspec/json_api/install/templates/rspec/json_api/interfaces/.empty_directory" + ) + end + + it "packages the licence and the reference documents" do + expect(gemspec.files).to include("LICENSE.txt", "README.md", "CHANGELOG.md") + end + + it "does not package repository tooling" do + expect(gemspec.files).not_to include( + ".github/workflows/main.yml", ".rubocop.yml", ".gitignore", "Gemfile", "Gemfile.lock", "Rakefile" + ) + end + + it "does not package the spec suite, the appraisals or the roadmap" do + expect(gemspec.files.grep(%r{\A(spec|gemfiles|bin)/})).to be_empty + expect(gemspec.files).not_to include("ROADMAP.md") + end +end From e5482e06202851ed5fc6562fb8d9e66dd7447f8e Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:10:27 +0200 Subject: [PATCH 10/13] CI: add bundler-audit job, dependabot config and least-privilege permissions --- .github/dependabot.yml | 15 +++++++++++++++ .github/workflows/main.yml | 22 +++++++++++++++++++--- Gemfile | 1 + Gemfile.lock | 4 ++++ 4 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..a74d667 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,15 @@ +version: 2 + +updates: + # The lockfile is a development artifact (consumers resolve their own), but + # keeping it current is what stops the advisory backlog rebuilding quietly. + - package-ecosystem: bundler + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 5 + + - package-ecosystem: github-actions + directory: "/" + schedule: + interval: weekly diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6eebee5..9815f8d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -2,10 +2,13 @@ name: Ruby on: [push, pull_request] +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest - name: "rspec — Ruby ${{ matrix.ruby }} / ${{ matrix.gemfile }}" + name: "rspec - Ruby ${{ matrix.ruby }} / ${{ matrix.gemfile }}" strategy: fail-fast: false matrix: @@ -22,7 +25,7 @@ jobs: env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: @@ -35,7 +38,7 @@ jobs: runs-on: ubuntu-latest name: rubocop steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: @@ -43,3 +46,16 @@ jobs: bundler-cache: true - name: Run rubocop run: bundle exec rubocop + + audit: + runs-on: ubuntu-latest + name: bundler-audit + steps: + - uses: actions/checkout@v5 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.2" + bundler-cache: true + - name: Check the lockfile against the advisory database + run: bundle exec bundle-audit check --update diff --git a/Gemfile b/Gemfile index e0fef3d..8c52f61 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } gemspec gem "activesupport", ">= 6.1.4.1" +gem "bundler-audit", "~> 0.9" gem "diffy", "~> 3.4" gem "rake", "~> 13.2" gem "rspec-rails", ">= 5.0.2" diff --git a/Gemfile.lock b/Gemfile.lock index f3c185d..ac68f46 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -43,6 +43,9 @@ GEM base64 (0.3.0) bigdecimal (4.1.2) builder (3.3.0) + bundler-audit (0.9.3) + bundler (>= 1.2.0) + thor (~> 1.0) concurrent-ruby (1.3.8) connection_pool (3.0.2) crass (1.0.7) @@ -174,6 +177,7 @@ PLATFORMS DEPENDENCIES activesupport (>= 6.1.4.1) + bundler-audit (~> 0.9) diffy (~> 3.4) rake (~> 13.2) rspec-json_api! From 1bacc54e3ad6e8430d95f5ea25bd6802e6ceadce Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:11:36 +0200 Subject: [PATCH 11/13] Release: bump version to 1.5.1 and update CHANGELOG --- CHANGELOG.md | 19 ++++++++++++++++++- Gemfile.lock | 2 +- ROADMAP.md | 24 ++++++++++++++---------- lib/rspec/json_api/version.rb | 2 +- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6a7210..23130b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,26 @@ ## [Unreleased] +## [1.5.1] - 2026-09-03 + +### Fixed +- A list schema (`[String]`, `[INTERFACE]`) fails the match instead of raising `NoMethodError` when the response holds `null` or a scalar where an array was expected. This affected nested lists too, so an interface element with a scalar in place of a list crashed the example. +- `Types::URI` is anchored with `\A...\z`. It previously accepted any value that merely contained a URI, so `"see https://example.com for details"` matched. `EMAIL` and `UUID` were already anchored. Suites that relied on the substring behaviour will start failing. +- `match_json_schema` fails with `expected a JSON String to match against the schema, got NilClass` instead of raising `TypeError` when handed `nil`, an already-parsed Hash, or any other non-String. +- A schema `Proc` that returns something other than an options Hash, or that expects the value as an argument, raises a descriptive `ArgumentError` naming the mistake. Both previously surfaced as a bare `NoMethodError` or `wrong number of arguments` from inside the matcher. +- The `have_no_content` specs gave every body its own context. A repeated `let(:actual)` in one context meant the `"{}"` case never ran. + +### Changed +- The released gem contains `lib/`, the licence, the README and the CHANGELOG, and nothing else. It previously packaged the repository's own tooling: the CI workflow, the RuboCop config, the Gemfile and lockfile, the Rakefile, `bin/` and `gemfiles/`. +- Updated the locked development dependencies past every advisory `bundler-audit` reported: rack, nokogiri, railties, activesupport, concurrent-ruby, loofah, crass, erb, json, rails-html-sanitizer and rack-session. + +### Added +- A `bundler-audit` job in CI, a Dependabot config for bundler and github-actions, and `permissions: contents: read` on the workflow. +- `ROADMAP.md`, the prioritised findings from a full review of the codebase. + ## [1.5.0] - 2026-06-05 ### Added -- CI compatibility matrix across Ruby 3.2–3.4 and Rails 6.1, 7.1, 7.2, 8.0 and 8.1 (`gemfiles/` + GitHub Actions matrix), so the advertised version support is actually tested. +- CI compatibility matrix across Ruby 3.2-3.4 and Rails 6.1, 7.1, 7.2, 8.0 and 8.1 (`gemfiles/` + GitHub Actions matrix), so the advertised version support is actually tested. - `RSpec::JsonApi::Constraints` module encapsulating the schema `Proc` options DSL. - `RSpec::JsonApi::SchemaMatch` as the single comparison entry point, and `RSpec::JsonApi::Traversal` for the internal structural helpers. diff --git a/Gemfile.lock b/Gemfile.lock index ac68f46..9246681 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rspec-json_api (1.5.0) + rspec-json_api (1.5.1) activesupport (>= 6.1.4.1) diffy (>= 3.4.2) railties (>= 6.1.4.1) diff --git a/ROADMAP.md b/ROADMAP.md index 679a31b..19bb911 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -26,9 +26,13 @@ Each item is marked either **Confirmed** (reproduced or directly visible in the | 4.1 | Optional keys and nullable values in the schema DSL | High | M | Suggestion | | 5.2 | CI matrix exercises almost no Rails code | Medium | M | Confirmed | +## Progress + +Phase 1 is implemented on branch `fix/roadmap-phase-1` and versioned as 1.5.1. Done: 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5. Every code fix was written test-first, and the item headings below carry a "Done in 1.5.1" marker. Nothing has been published to RubyGems. + ## 1. Bugs to fix -### 1.1 List schemas crash when the actual value is not an array +### 1.1 List schemas crash when the actual value is not an array (done in 1.5.1) Priority: Critical. Category: Correctness. Effort: S. Status: Confirmed. @@ -72,7 +76,7 @@ match_json_schema({ n: -> { { type: String, allow_blank: true } } }).matches?('{ **Dependencies and risks.** Behaviour change: suites that relied on the lax check will start failing, correctly. Note it under "Fixed" in the CHANGELOG. Pairs naturally with 3.2, which touches the same method. -### 1.3 `Types::URI` is unanchored +### 1.3 `Types::URI` is unanchored (done in 1.5.1) Priority: High. Category: Correctness. Effort: S. Status: Confirmed. @@ -91,7 +95,7 @@ match_json_schema({ u: RSpec::JsonApi::Types::URI }) **Dependencies and risks.** Strings with surrounding whitespace start failing; that is the intended behaviour. -### 1.4 Matcher raises `TypeError` for `nil` or already-parsed input +### 1.4 Matcher raises `TypeError` for `nil` or already-parsed input (done in 1.5.1) Priority: High. Category: Robustness. Effort: S. Status: Confirmed. @@ -125,7 +129,7 @@ match_json_schema({ code: /.*/ }).matches?('{"code":null}') # => true (nil. **Dependencies and risks.** Breaking for suites that regex-match numbers. Ship with 2.1 in a version that already carries a CHANGELOG "Changed" section; consider 2.0.0 for the combined set (see phasing). -### 1.6 Misused Proc schemas raise raw Ruby errors +### 1.6 Misused Proc schemas raise raw Ruby errors (done in 1.5.1) Priority: Medium. Category: Robustness. Effort: S. Status: Confirmed. @@ -142,7 +146,7 @@ match_json_schema({ a: ->(v) { v > 1 } }).matches?('{"a":2}') # ArgumentError **Dependencies and risks.** If arity-1 lambdas become predicates, document it and add specs; it overlaps with 4.2. -### 1.7 `have_no_content_spec.rb` never tests the `"{}"` case +### 1.7 `have_no_content_spec.rb` never tests the `"{}"` case (done in 1.5.1) Priority: Low. Category: Test correctness. Effort: S. Status: Confirmed. @@ -196,7 +200,7 @@ Priority: Medium. Category: Dependencies. Effort: S. Status: Confirmed (usage), **Dependencies and risks.** Requires 2.1 to be worthwhile. `blank?` on unusual objects (e.g. `BigDecimal`) is not relevant because input always comes from `JSON.parse`. -### 2.3 Move the development lockfile past open security advisories +### 2.3 Move the development lockfile past open security advisories (done in 1.5.1) Priority: High. Category: Security / dependencies. Effort: S. Status: Confirmed. @@ -224,7 +228,7 @@ Priority: High. Category: Security / dependencies. Effort: S. Status: Confirmed. **Dependencies and risks.** Low. All are patch-level within the ranges the Gemfile allows. -### 2.4 Routine minor updates +### 2.4 Routine minor updates (done in 1.5.1) Priority: Low. Category: Dependencies. Effort: S. Status: Confirmed. @@ -450,7 +454,7 @@ Priority: Medium. Category: CI. Effort: M. Status: Confirmed. **Recommended solution.** After 2.1, the runtime matrix only needs Ruby versions (3.2, 3.3, 3.4, 4.0) with the plain Gemfile. Keep one or two Rails appraisals that actually load `rspec-rails` and run the generator specs from 5.1 against a minimal dummy app. Add `permissions: contents: read` and a `concurrency` group while touching the file. -### 5.3 Supply-chain checks are not automated +### 5.3 Supply-chain checks are not automated (done in 1.5.1) Priority: Medium. Category: Security / DevOps. Effort: S. Status: Confirmed. @@ -466,7 +470,7 @@ Priority: Medium. Category: DevOps. Effort: M. Status: Confirmed. **Recommended solution.** A release workflow using RubyGems Trusted Publishing triggered by a `v*` tag, so publishing requires a tag and a green build. Backfill the missing tags from the version-bump commits and write short CHANGELOG entries from `git log` for 1.1 to 1.3.1. -### 5.5 The gem packages repository tooling +### 5.5 The gem packages repository tooling (done in 1.5.1) Priority: Low. Category: Packaging. Effort: S. Status: Confirmed. @@ -509,7 +513,7 @@ Priority: Low. Category: Developer experience. Effort: S. Status: Confirmed. ## Phased plan -**Phase 1, patch release 1.5.1 (about a day).** 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5. All additive or pure fixes, each with a spec. +**Phase 1, patch release 1.5.1. Done.** 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5, all additive or pure fixes with a spec each. The suite went from 63 examples to 83, `bundler-audit` reports no vulnerabilities, and the packaged gem dropped from 39 files to 20. Not yet released to RubyGems. **Phase 2, 2.0.0 (one to two weeks).** 1.2, 1.5, 3.2, 3.4, 3.9 (behaviour changes bundled in one CHANGELOG), 2.1 and 2.2 (dependency cut), 2.5, 3.7, 5.1 unit specs, 5.2 matrix rework, 5.6 documentation. Bump the major because 1.2, 1.5 and 2.1 can each break an existing suite. diff --git a/lib/rspec/json_api/version.rb b/lib/rspec/json_api/version.rb index 363b347..84ef763 100644 --- a/lib/rspec/json_api/version.rb +++ b/lib/rspec/json_api/version.rb @@ -2,6 +2,6 @@ module RSpec module JsonApi - VERSION = "1.5.0" + VERSION = "1.5.1" end end From d01fdf56cd6e24a641e20aa5cee536f4e4409892 Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:12:12 +0200 Subject: [PATCH 12/13] Refactor: name the proc-argument helper for what it checks --- lib/rspec/json_api/schema_match.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/rspec/json_api/schema_match.rb b/lib/rspec/json_api/schema_match.rb index a980350..fa618c9 100644 --- a/lib/rspec/json_api/schema_match.rb +++ b/lib/rspec/json_api/schema_match.rb @@ -84,7 +84,7 @@ def compare_regexp(actual_value, expected_value) # as an argument is a common misreading of the DSL, and calling it here # would raise a bare "wrong number of arguments" from deep in the matcher. def compare_proc(actual_value, expected_value) - unless zero_arity?(expected_value) + unless callable_without_arguments?(expected_value) raise ArgumentError, "schema Proc must take no arguments; " \ "write -> { { lambda: ->(value) { ... } } } to test the value itself" @@ -93,7 +93,10 @@ def compare_proc(actual_value, expected_value) Constraints.match(actual_value, expected_value.call) end - def zero_arity?(callable) + # True when the Proc can be called with no arguments at all. Splat and + # optional-argument forms qualify; a required positional or keyword + # argument does not. + def callable_without_arguments?(callable) callable.parameters.none? { |type, _name| %i[req keyreq].include?(type) } end From 87927d1900eee2545b479d9243c098f4050ebb8c Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 3 Sep 2026 22:38:30 +0200 Subject: [PATCH 13/13] Review: harden proc and object-path guards, dispatch exact-array elements --- .github/workflows/main.yml | 7 ++- .rubocop.yml | 3 + CHANGELOG.md | 9 ++- Gemfile.lock | 2 +- README.md | 7 ++- ROADMAP.md | 28 ++++----- lib/rspec/json_api/constraints.rb | 2 +- .../json_api/matchers/match_json_schema.rb | 35 +++++------ lib/rspec/json_api/schema_match.rb | 20 ++++--- lib/rspec/json_api/version.rb | 2 +- rspec-json_api.gemspec | 12 ++-- spec/rspec/json_api/constraints_spec.rb | 2 +- spec/rspec/json_api/gemspec_spec.rb | 17 +++--- .../matchers/match_json_schema_spec.rb | 58 ++++++++++++++++++- 14 files changed, 135 insertions(+), 69 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9815f8d..5752b77 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,7 +42,9 @@ jobs: - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: "3.2" + # The root lockfile pins dev tooling that needs Ruby >= 3.3; the + # gem's own 3.2 floor is covered by the test matrix. + ruby-version: "3.4" bundler-cache: true - name: Run rubocop run: bundle exec rubocop @@ -55,7 +57,8 @@ jobs: - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: "3.2" + # As in the lint job: the root lockfile needs Ruby >= 3.3. + ruby-version: "3.4" bundler-cache: true - name: Check the lockfile against the advisory database run: bundle exec bundle-audit check --update diff --git a/.rubocop.yml b/.rubocop.yml index 4532e5d..63247e2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,9 +32,12 @@ Metrics/BlockLength: Style/Documentation: Enabled: false +# RSpec fixes both names: `have_no_content` is the matcher DSL entry point and +# `does_not_match?` is part of the matcher protocol. Naming/PredicatePrefix: Exclude: - "lib/rspec/json_api/matchers.rb" + - "lib/rspec/json_api/matchers/match_json_schema.rb" Naming/PredicateMethod: Exclude: diff --git a/CHANGELOG.md b/CHANGELOG.md index 23130b8..c8935b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,19 +1,24 @@ ## [Unreleased] -## [1.5.1] - 2026-09-03 +## [1.6.0] - 2026-09-03 ### Fixed - A list schema (`[String]`, `[INTERFACE]`) fails the match instead of raising `NoMethodError` when the response holds `null` or a scalar where an array was expected. This affected nested lists too, so an interface element with a scalar in place of a list crashed the example. - `Types::URI` is anchored with `\A...\z`. It previously accepted any value that merely contained a URI, so `"see https://example.com for details"` matched. `EMAIL` and `UUID` were already anchored. Suites that relied on the substring behaviour will start failing. - `match_json_schema` fails with `expected a JSON String to match against the schema, got NilClass` instead of raising `TypeError` when handed `nil`, an already-parsed Hash, or any other non-String. - A schema `Proc` that returns something other than an options Hash, or that expects the value as an argument, raises a descriptive `ArgumentError` naming the mistake. Both previously surfaced as a bare `NoMethodError` or `wrong number of arguments` from inside the matcher. +- An object schema compared against array elements of another shape (`[{ id: Integer }]` against `["a", "b"]`) fails instead of raising `NoMethodError`. This is the same class of bug as the list-schema crash above, on the object comparison path. +- `expect(body).not_to match_json_schema(schema)` fails when the body is not a JSON String, rather than passing by default. A type error in the spec is now a failure whichever way the expectation is written. +- A non-lambda `proc { |value| ... }` used as a schema Proc is rejected alongside the lambda form. Ruby reports a non-lambda block parameter as optional, so the check reads the parameter list rather than the arity; a Proc declaring an optional parameter is rejected for the same reason. - The `have_no_content` specs gave every body its own context. A repeated `let(:actual)` in one context meant the `"{}"` case never ran. ### Changed -- The released gem contains `lib/`, the licence, the README and the CHANGELOG, and nothing else. It previously packaged the repository's own tooling: the CI workflow, the RuboCop config, the Gemfile and lockfile, the Rakefile, `bin/` and `gemfiles/`. +- The released gem contains the tracked files under `lib/` plus the licence, the README and the CHANGELOG, and nothing else. It previously packaged the repository's own tooling: the CI workflow, the RuboCop config, the Gemfile and lockfile, the Rakefile, `bin/` and `gemfiles/`. Scoping the file list to tracked paths also keeps an untracked local file in `lib/` out of a release. +- README regex examples anchor with `\A` and `\z` instead of `^` and `$`, with a note explaining that the line-boundary anchors let a multi-line value satisfy a schema. - Updated the locked development dependencies past every advisory `bundler-audit` reported: rack, nokogiri, railties, activesupport, concurrent-ruby, loofah, crass, erb, json, rails-html-sanitizer and rack-session. ### Added +- Exact-array schemas dispatch each element the same way any other schema value is dispatched, so `[Integer, Integer]` is a fixed-length tuple and `[RSpec::JsonApi::Types::URI]` type-checks its single element. Elements were previously compared with `==`, so a Class, Regexp or Proc in that position could never match. - A `bundler-audit` job in CI, a Dependabot config for bundler and github-actions, and `permissions: contents: read` on the workflow. - `ROADMAP.md`, the prioritised findings from a full review of the codebase. diff --git a/Gemfile.lock b/Gemfile.lock index 9246681..0202cb4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rspec-json_api (1.5.1) + rspec-json_api (1.6.0) activesupport (>= 6.1.4.1) diffy (>= 3.4.2) railties (>= 6.1.4.1) diff --git a/README.md b/README.md index f211e06..c963508 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ RSpec.describe UsersController, type: :controller do id: RSpec::JsonApi::Types::UUID, name: String, age: Integer, - favoriteColorHex: /^\#([a-fA-F]|[0-9]){3,6}$/, + favoriteColorHex: /\A\#([a-fA-F]|[0-9]){3,6}\z/, number: -> { { type: Integer, min: 10, max: 20, lambda: lambda(&:even?) } } }] end @@ -133,7 +133,7 @@ Custom type example: module RSpec module JsonApi module Types - COLOR_HEX = /^#(?:[0-9a-fA-F]{3}){1,2}$/ + COLOR_HEX = /\A#(?:[0-9a-fA-F]{3}){1,2}\z/ end end end @@ -223,10 +223,11 @@ end ```ruby let(:expected_schema) do { - color: /^\#([a-fA-F]|[0-9]){3,6}$/ + color: /\A\#([a-fA-F]|[0-9]){3,6}\z/ } end ``` +_Note: anchor with `\A` and `\z`, not `^` and `$`. `^` and `$` match at line boundaries, so `/^\#[0-9a-fA-F]{3}$/` also accepts `"not a color\n#FFF"` and the value only has to contain a matching line for the schema to pass. The built-in `EMAIL`, `URI` and `UUID` types are anchored this way._ ### Interface match ```ruby diff --git a/ROADMAP.md b/ROADMAP.md index 19bb911..8e868f7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -28,11 +28,13 @@ Each item is marked either **Confirmed** (reproduced or directly visible in the ## Progress -Phase 1 is implemented on branch `fix/roadmap-phase-1` and versioned as 1.5.1. Done: 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5. Every code fix was written test-first, and the item headings below carry a "Done in 1.5.1" marker. Nothing has been published to RubyGems. +Phase 1 is implemented on branch `fix/roadmap-phase-1`, released as 1.6.0 rather than 1.5.1 because code review pulled item 3.2 forward and that adds a capability. Done: 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 3.2, 5.3, 5.5, each marked on its heading below. Nothing has been published to RubyGems. + +Review of that work also turned up two crashes the original audit missed, both now fixed and specced. `SchemaMatch.compare` raised `NoMethodError` when an object schema met array elements of another shape, which is item 1.1's bug on the object path rather than the array path. And the guard added for item 1.6 read a Proc's arity, which does not catch `proc { |value| ... }`, because Ruby reports a non-lambda block parameter as optional; the check now reads the parameter list. ## 1. Bugs to fix -### 1.1 List schemas crash when the actual value is not an array (done in 1.5.1) +### 1.1 List schemas crash when the actual value is not an array (done in 1.6.0) Priority: Critical. Category: Correctness. Effort: S. Status: Confirmed. @@ -76,7 +78,7 @@ match_json_schema({ n: -> { { type: String, allow_blank: true } } }).matches?('{ **Dependencies and risks.** Behaviour change: suites that relied on the lax check will start failing, correctly. Note it under "Fixed" in the CHANGELOG. Pairs naturally with 3.2, which touches the same method. -### 1.3 `Types::URI` is unanchored (done in 1.5.1) +### 1.3 `Types::URI` is unanchored (done in 1.6.0) Priority: High. Category: Correctness. Effort: S. Status: Confirmed. @@ -95,7 +97,7 @@ match_json_schema({ u: RSpec::JsonApi::Types::URI }) **Dependencies and risks.** Strings with surrounding whitespace start failing; that is the intended behaviour. -### 1.4 Matcher raises `TypeError` for `nil` or already-parsed input (done in 1.5.1) +### 1.4 Matcher raises `TypeError` for `nil` or already-parsed input (done in 1.6.0) Priority: High. Category: Robustness. Effort: S. Status: Confirmed. @@ -129,7 +131,7 @@ match_json_schema({ code: /.*/ }).matches?('{"code":null}') # => true (nil. **Dependencies and risks.** Breaking for suites that regex-match numbers. Ship with 2.1 in a version that already carries a CHANGELOG "Changed" section; consider 2.0.0 for the combined set (see phasing). -### 1.6 Misused Proc schemas raise raw Ruby errors (done in 1.5.1) +### 1.6 Misused Proc schemas raise raw Ruby errors (done in 1.6.0) Priority: Medium. Category: Robustness. Effort: S. Status: Confirmed. @@ -146,7 +148,7 @@ match_json_schema({ a: ->(v) { v > 1 } }).matches?('{"a":2}') # ArgumentError **Dependencies and risks.** If arity-1 lambdas become predicates, document it and add specs; it overlaps with 4.2. -### 1.7 `have_no_content_spec.rb` never tests the `"{}"` case (done in 1.5.1) +### 1.7 `have_no_content_spec.rb` never tests the `"{}"` case (done in 1.6.0) Priority: Low. Category: Test correctness. Effort: S. Status: Confirmed. @@ -200,7 +202,7 @@ Priority: Medium. Category: Dependencies. Effort: S. Status: Confirmed (usage), **Dependencies and risks.** Requires 2.1 to be worthwhile. `blank?` on unusual objects (e.g. `BigDecimal`) is not relevant because input always comes from `JSON.parse`. -### 2.3 Move the development lockfile past open security advisories (done in 1.5.1) +### 2.3 Move the development lockfile past open security advisories (done in 1.6.0) Priority: High. Category: Security / dependencies. Effort: S. Status: Confirmed. @@ -228,7 +230,7 @@ Priority: High. Category: Security / dependencies. Effort: S. Status: Confirmed. **Dependencies and risks.** Low. All are patch-level within the ranges the Gemfile allows. -### 2.4 Routine minor updates (done in 1.5.1) +### 2.4 Routine minor updates (done in 1.6.0) Priority: Low. Category: Dependencies. Effort: S. Status: Confirmed. @@ -271,7 +273,7 @@ Diff: **Dependencies and risks.** Largest item in the roadmap. Do it after 1.1, 1.2 and 3.2 have specs, so the rewrite has a safety net. It changes the failure text; that is not a public API, but mention it. -### 3.2 `compare_exact_array` does not dispatch element schemas +### 3.2 `compare_exact_array` does not dispatch element schemas (done in 1.6.0) Priority: Medium. Category: Correctness / consistency. Effort: S. Status: Confirmed. @@ -454,7 +456,7 @@ Priority: Medium. Category: CI. Effort: M. Status: Confirmed. **Recommended solution.** After 2.1, the runtime matrix only needs Ruby versions (3.2, 3.3, 3.4, 4.0) with the plain Gemfile. Keep one or two Rails appraisals that actually load `rspec-rails` and run the generator specs from 5.1 against a minimal dummy app. Add `permissions: contents: read` and a `concurrency` group while touching the file. -### 5.3 Supply-chain checks are not automated (done in 1.5.1) +### 5.3 Supply-chain checks are not automated (done in 1.6.0) Priority: Medium. Category: Security / DevOps. Effort: S. Status: Confirmed. @@ -470,7 +472,7 @@ Priority: Medium. Category: DevOps. Effort: M. Status: Confirmed. **Recommended solution.** A release workflow using RubyGems Trusted Publishing triggered by a `v*` tag, so publishing requires a tag and a green build. Backfill the missing tags from the version-bump commits and write short CHANGELOG entries from `git log` for 1.1 to 1.3.1. -### 5.5 The gem packages repository tooling (done in 1.5.1) +### 5.5 The gem packages repository tooling (done in 1.6.0) Priority: Low. Category: Packaging. Effort: S. Status: Confirmed. @@ -513,8 +515,8 @@ Priority: Low. Category: Developer experience. Effort: S. Status: Confirmed. ## Phased plan -**Phase 1, patch release 1.5.1. Done.** 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 5.3, 5.5, all additive or pure fixes with a spec each. The suite went from 63 examples to 83, `bundler-audit` reports no vulnerabilities, and the packaged gem dropped from 39 files to 20. Not yet released to RubyGems. +**Phase 1, released as 1.6.0. Done.** 1.1, 1.3, 1.4, 1.6, 1.7, 2.3, 2.4, 3.2, 5.3, 5.5, with a spec each. The suite went from 63 examples to 89, `bundler-audit` reports no vulnerabilities, and the packaged gem dropped from 39 files to 20. Not yet published to RubyGems. -**Phase 2, 2.0.0 (one to two weeks).** 1.2, 1.5, 3.2, 3.4, 3.9 (behaviour changes bundled in one CHANGELOG), 2.1 and 2.2 (dependency cut), 2.5, 3.7, 5.1 unit specs, 5.2 matrix rework, 5.6 documentation. Bump the major because 1.2, 1.5 and 2.1 can each break an existing suite. +**Phase 2, 2.0.0 (one to two weeks).** 1.2, 1.5, 3.4, 3.9 (behaviour changes bundled in one CHANGELOG), 2.1 and 2.2 (dependency cut), 2.5, 3.7, 5.1 unit specs, 5.2 matrix rework, 5.6 documentation. Bump the major because 1.2, 1.5 and 2.1 can each break an existing suite. **Phase 3, 2.x (ongoing).** 3.3 DSL helpers, then 4.1, 4.2, 4.8 on top of them; 3.1 mismatch reporting once the DSL is settled; 4.3, 4.4, 4.5, 4.6, 4.7; 5.4 release automation; 1.8 and 3.6 disappear as part of 3.1. diff --git a/lib/rspec/json_api/constraints.rb b/lib/rspec/json_api/constraints.rb index 89634b8..7c048d2 100644 --- a/lib/rspec/json_api/constraints.rb +++ b/lib/rspec/json_api/constraints.rb @@ -29,7 +29,7 @@ def match(value, options) end def validate!(options) - raise ArgumentError, "schema Proc must return an options Hash, got #{options.class}" unless options.is_a?(Hash) + raise ArgumentError, "options must be a Hash, got #{options.class}" unless options.is_a?(Hash) unknown = options.keys - SUPPORTED_OPTIONS return if unknown.empty? diff --git a/lib/rspec/json_api/matchers/match_json_schema.rb b/lib/rspec/json_api/matchers/match_json_schema.rb index a7b30af..d1ba310 100644 --- a/lib/rspec/json_api/matchers/match_json_schema.rb +++ b/lib/rspec/json_api/matchers/match_json_schema.rb @@ -24,12 +24,25 @@ def initialize(expected) # @return [Boolean] true if the actual JSON matches the expected schema, false otherwise. def matches?(actual) @diff = nil - @type_error = false @actual = actual + @type_error = !actual.is_a?(String) - return false unless parse + return false if @type_error + + @actual = JSON.parse(actual, symbolize_names: true) RSpec::JsonApi::SchemaMatch.match(@actual, expected) + rescue JSON::ParserError + @actual = actual + false + end + + # A non-String actual is a mistake in the spec rather than a fact about the + # response, so the negated form has to fail rather than pass by default. + # @param actual [String] The JSON string to test against the expected schema. + # @return [Boolean] true if the actual JSON does not match the expected schema. + def does_not_match?(actual) + !matches?(actual) && !@type_error end # Provides a failure message for when the JSON data does not match the expected schema. @@ -57,24 +70,6 @@ def failure_message_when_negated private - # Replaces @actual with its parsed form. Returns false when the input - # could not be parsed, leaving @actual as the raw value so the failure - # message can still show what came back. - # - # JSON.parse raises TypeError rather than JSON::ParserError when handed - # something that is not a String at all, such as nil or an already-parsed - # Hash. Both are easy mistakes to make in a request spec, so they fail - # the match instead of erroring the example out. - def parse - @actual = JSON.parse(@actual, symbolize_names: true) - true - rescue JSON::ParserError - false - rescue TypeError - @type_error = true - false - end - # The diff is only needed to render a failure message, so it is built # lazily and memoized rather than on every matches? call. def diff diff --git a/lib/rspec/json_api/schema_match.rb b/lib/rspec/json_api/schema_match.rb index fa618c9..49dd198 100644 --- a/lib/rspec/json_api/schema_match.rb +++ b/lib/rspec/json_api/schema_match.rb @@ -33,6 +33,7 @@ def same_key_structure?(actual, expected) end def compare(actual, expected) + return false unless actual.is_a?(Hash) return false if actual.blank? && expected.present? keys = Traversal.deep_key_paths(expected) | Traversal.deep_key_paths(actual) @@ -84,20 +85,23 @@ def compare_regexp(actual_value, expected_value) # as an argument is a common misreading of the DSL, and calling it here # would raise a bare "wrong number of arguments" from deep in the matcher. def compare_proc(actual_value, expected_value) - unless callable_without_arguments?(expected_value) + if declares_value_parameter?(expected_value) raise ArgumentError, "schema Proc must take no arguments; " \ "write -> { { lambda: ->(value) { ... } } } to test the value itself" end - Constraints.match(actual_value, expected_value.call) + options = expected_value.call + raise ArgumentError, "schema Proc must return an options Hash, got #{options.class}" unless options.is_a?(Hash) + + Constraints.match(actual_value, options) end - # True when the Proc can be called with no arguments at all. Splat and - # optional-argument forms qualify; a required positional or keyword - # argument does not. - def callable_without_arguments?(callable) - callable.parameters.none? { |type, _name| %i[req keyreq].include?(type) } + # A non-lambda Proc reports its block parameters as optional, so + # `proc { |value| ... }` has to be caught on the parameter list rather + # than on arity. A bare splat states no expectation and is left alone. + def declares_value_parameter?(callable) + callable.parameters.any? { |type, _name| %i[req opt keyreq].include?(type) } end # A list schema only ever matches an actual Array. Without this guard the @@ -138,7 +142,7 @@ def compare_exact_array(actual_value, expected_value) return false if actual_value.size != expected_value.size expected_value.each_with_index.all? do |elem, index| - elem.is_a?(Hash) ? compare(actual_value[index], elem) : compare_simple_value(actual_value[index], elem) + elem.is_a?(Hash) ? compare(actual_value[index], elem) : compare_values(actual_value[index], elem) end end diff --git a/lib/rspec/json_api/version.rb b/lib/rspec/json_api/version.rb index 84ef763..551928b 100644 --- a/lib/rspec/json_api/version.rb +++ b/lib/rspec/json_api/version.rb @@ -2,6 +2,6 @@ module RSpec module JsonApi - VERSION = "1.5.1" + VERSION = "1.6.0" end end diff --git a/rspec-json_api.gemspec b/rspec-json_api.gemspec index 6e1cbfb..60a3501 100644 --- a/rspec-json_api.gemspec +++ b/rspec-json_api.gemspec @@ -18,14 +18,12 @@ Gem::Specification.new do |spec| spec.metadata["changelog_uri"] = "https://github.com/nomtek/rspec-json_api/blob/master/CHANGELOG.md" spec.metadata["rubygems_mfa_required"] = "true" - # Ship what a consumer loads and nothing else: the library, the generators - # (templates included, so the dotfile markers that keep the empty interface - # and type directories must be globbed too), plus the licence and reference - # documents. Listing every tracked file, as `git ls-files` did, packaged the - # repository's own tooling inside the released gem: the CI workflow, the - # RuboCop config, the Gemfile and lockfile, the Rakefile, bin/ and gemfiles/. + # Ship what a consumer loads and nothing else: the library and generators + # (dotfile markers included, so the empty template directories survive), plus + # the licence and reference documents. Scoping `git ls-files` to lib/ keeps the + # repository's own tooling out without letting untracked artefacts in. spec.files = Dir.chdir(File.expand_path(__dir__)) do - Dir.glob("lib/**/*", File::FNM_DOTMATCH).select { |f| File.file?(f) }.sort + + `git ls-files -z lib`.split("\x0").select { |f| File.file?(f) }.sort + %w[CHANGELOG.md LICENSE.txt README.md] end spec.require_paths = ["lib"] diff --git a/spec/rspec/json_api/constraints_spec.rb b/spec/rspec/json_api/constraints_spec.rb index 7f7f4a2..055b36c 100644 --- a/spec/rspec/json_api/constraints_spec.rb +++ b/spec/rspec/json_api/constraints_spec.rb @@ -9,7 +9,7 @@ it "raises ArgumentError when the options are not a Hash" do expect { described_class.match("value", true) } - .to raise_error(ArgumentError, /must return an options Hash/) + .to raise_error(ArgumentError, /options must be a Hash/) end it "accepts a blank value when allow_blank is true" do diff --git a/spec/rspec/json_api/gemspec_spec.rb b/spec/rspec/json_api/gemspec_spec.rb index 0a7d05c..13e3eb7 100644 --- a/spec/rspec/json_api/gemspec_spec.rb +++ b/spec/rspec/json_api/gemspec_spec.rb @@ -2,9 +2,11 @@ RSpec.describe "rspec-json_api.gemspec" do subject(:gemspec) do - Gem::Specification.load(File.expand_path("../../../rspec-json_api.gemspec", __dir__)) + Gem::Specification.load(File.join(repo_root, "rspec-json_api.gemspec")) end + let(:repo_root) { File.expand_path("../../..", __dir__) } + it "packages the library" do expect(gemspec.files).to include("lib/rspec/json_api.rb", "lib/rspec/json_api/version.rb") end @@ -21,14 +23,13 @@ expect(gemspec.files).to include("LICENSE.txt", "README.md", "CHANGELOG.md") end - it "does not package repository tooling" do - expect(gemspec.files).not_to include( - ".github/workflows/main.yml", ".rubocop.yml", ".gitignore", "Gemfile", "Gemfile.lock", "Rakefile" - ) + it "packages every tracked file under lib and nothing else from there" do + tracked_lib = Dir.chdir(repo_root) { `git ls-files -z lib`.split("\x0").sort } + + expect(gemspec.files.grep(%r{\Alib/})).to eq(tracked_lib) end - it "does not package the spec suite, the appraisals or the roadmap" do - expect(gemspec.files.grep(%r{\A(spec|gemfiles|bin)/})).to be_empty - expect(gemspec.files).not_to include("ROADMAP.md") + it "packages nothing outside lib but the licence and the reference documents" do + expect(gemspec.files.grep_v(%r{\Alib/})).to contain_exactly("CHANGELOG.md", "LICENSE.txt", "README.md") end end diff --git a/spec/rspec/json_api/matchers/match_json_schema_spec.rb b/spec/rspec/json_api/matchers/match_json_schema_spec.rb index 9c57016..366acec 100644 --- a/spec/rspec/json_api/matchers/match_json_schema_spec.rb +++ b/spec/rspec/json_api/matchers/match_json_schema_spec.rb @@ -31,14 +31,12 @@ it "fails instead of raising for nil" do matcher = match_json_schema({ id: String }) - expect { matcher.matches?(nil) }.not_to raise_error expect(matcher.matches?(nil)).to be(false) end it "fails instead of raising for an already-parsed Hash" do matcher = match_json_schema({ id: String }) - expect { matcher.matches?({ id: "x" }) }.not_to raise_error expect(matcher.matches?({ id: "x" })).to be(false) end @@ -48,6 +46,13 @@ expect(matcher.failure_message).to include("JSON String", "NilClass") end + + it "fails the negated form too, rather than passing by default" do + matcher = match_json_schema({ id: String }) + + expect(matcher.does_not_match?(nil)).to be(false) + expect(matcher.failure_message_when_negated).to include("JSON String", "NilClass") + end end context "when a schema Proc is misused" do @@ -60,6 +65,11 @@ expect { match_json_schema({ a: ->(value) { value > 1 } }).matches?({ a: 2 }.to_json) } .to raise_error(ArgumentError, /must take no arguments/) end + + it "raises ArgumentError when a non-lambda Proc expects an argument" do + expect { match_json_schema({ a: proc { |value| value > 1 } }).matches?({ a: 2 }.to_json) } + .to raise_error(ArgumentError, /must take no arguments/) + end end context "when the schema matches" do @@ -627,6 +637,50 @@ end end + context "when an exact-array schema of objects meets elements of another shape" do + let(:expected) do + { tags: [{ id: Integer }, { id: Integer }] } + end + + context "when the elements are scalars" do + let(:actual) do + { tags: %w[a b] }.to_json + end + + include_examples "incorrect-match" + end + + context "when the elements are null" do + let(:actual) do + { tags: [nil, nil] }.to_json + end + + include_examples "incorrect-match" + end + end + + context "when an exact-array schema holds a type" do + let(:expected) do + { uris: [RSpec::JsonApi::Types::URI] } + end + + context "when correct match" do + let(:actual) do + { uris: ["https://example.com/a"] }.to_json + end + + include_examples "correct-match" + end + + context "when incorrect match" do + let(:actual) do + { uris: ["not a uri"] }.to_json + end + + include_examples "incorrect-match" + end + end + context "when proc given" do context "when type comparison" do let(:expected) do