Route Grape::Json through the non-deprecated MultiJSON API#2764
Merged
Conversation
Danger ReportNo issues found. |
ericproulx
force-pushed
the
fix/2763-multijson-deprecations
branch
from
d965e84 to
80b6d08
Compare
ericproulx
force-pushed
the
fix/2763-multijson-deprecations
branch
2 times, most recently
from
9a594bc to
ad360c6
Compare
ericproulx
force-pushed
the
fix/2763-multijson-deprecations
branch
3 times, most recently
from
9edefc8 to
31e27b9
Compare
multi_json 1.21.0 deprecated the `MultiJson` constant (in favor of `MultiJSON`) and `MultiJSON.dump` (in favor of `generate`), removed in 2.0. Grape detected the backend via `::MultiJson` and called `dump`, so users on multi_json >= 1.21 saw deprecation warnings (#2763). Detect `::MultiJSON` and give `Grape::Json` a small facade whose `dump` delegates to `MultiJSON.generate` (byte-identical output), re-exposing `parse` since the facade no longer inherits MultiJSON's methods. Behavior is unchanged; the stdlib fallback is untouched. Following #2762 (Grape standardizes on `parse`, never `load`, for JSON), the two remaining `Grape::Json.load` spec helpers move to `parse`, so `Grape::Json.load` no longer appears anywhere and the facade needs no `load`. Also give the legacy multi_json (< 1.21) branch the same facade treatment mapping dump/parse onto the dump/load it exposes — it has no generate/parse, so `Grape::Json.parse` would otherwise raise NoMethodError there. Cover both backends in CI: pin gemfiles/multi_json.gemfile to >= 1.21 and add gemfiles/multi_json_1_20.gemfile (< 1.21), each running the spec/integration/multi_json suite. The integration spec keys off the `MultiJson` constant (defined on both lines) and drives a real Grape API through the parser and JSON formatter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ericproulx
force-pushed
the
fix/2763-multijson-deprecations
branch
from
31e27b9 to
39422f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2763.
Background
multi_json 1.21.0 introduced three deprecations at once (all slated for removal in 2.0):
MultiJsonconstantMultiJSONMultiJSON.loadMultiJSON.parseMultiJSON.dumpMultiJSON.generatelib/grape/json.rbdetected the backend via::MultiJsonandGrape::Json.dumpresolved toMultiJSON.dump, so anyone on multi_json >= 1.21 saw the deprecation warning reported in #2763:Simply swapping the constant (
::MultiJson→::MultiJSON) is not enough:Grape::Json.dump/.loadwould then resolve to the deprecated method names and warn instead.generate/parseare byte-identical replacements fordump/load(verified against both a pre-1.21 and a 1.21 build — only the names changed), so the fix is to route Grape's calls to the non-deprecated names.What this PR does
1.
Grape::Jsonbecomes a small engine-agnostic facadeGrape keeps its own stable
dump/parsesurface; each backend maps it to its non-deprecated methods:Grape::Jsondump→parse→MultiJSON)MultiJSON.generateMultiJSON.parseMultiJson)MultiJson.dumpMultiJson.load= ::JSONJSON.dumpJSON.parseCall sites are unchanged and output is identical on every path, so this is behavior-preserving.
2.
Grape::Json.loadis removed entirelyFollowing #2762 (Grape parses request bodies with
JSON.parse, neverJSON.load, to avoid thejson_class/create_additionsinstantiation vector),loadshould not appear in the codebase. The two remainingGrape::Json.loadtest helpers now useparse, and the facade no longer definesload.3. Legacy multi_json (< 1.21) no longer raises
multi_json < 1.21 has no
parsemethod (onlydump/load), so after #2762Grape::Json.parsewould raiseNoMethodErroron those versions. The legacy branch now mapsparse→MultiJson.load, fixing that while preserving the user's adapter (oj/yajl).4. CI covers both multi_json lines
gemfiles/multi_json.gemfileis pinned to>= 1.21and a newgemfiles/multi_json_1_20.gemfilepins< 1.21; both run thespec/integration/multi_jsonsuite, so the MultiJSON and legacy-MultiJson facades are each exercised. The integration spec keys off theMultiJsonconstant (defined on both lines) and drives a real Grape API end to end — a JSON request body through the parser intoparams, and a Hash response through the JSON formatter.Testing
BUNDLE_GEMFILE=gemfiles/multi_json.gemfile bundle exec rspec(>= 1.21, reproduces Deprecation warning forMultiJsonwhen using multi_json 1.21.0 or later #2763): 2323 examples, 0 failuresBUNDLE_GEMFILE=gemfiles/multi_json_1_20.gemfile bundle exec rspec spec/integration/multi_json(< 1.21, resolves to 1.20.1): 1 example, 0 failuresbundle exec rspec(stdlibJSONfallback): 2322 examples, 0 failures🤖 Generated with Claude Code