Refactor JWT::JWK::Set#initialize - #758
Merged
Merged
Conversation
anakinj
force-pushed
the
refactor/jwk-set-initializer
branch
from
September 5, 2026 18:45
e15cba0 to
250f3e0
Compare
The two construction paths that build keys from hashes had drifted: the JWKS document form skips a key whose kty is unsupported, the array form raises. That is easy to miss when the two are nested blocks in the middle of a case expression, and it is how the gap in jwt#744 came about. Give each branch a named method, so what differs between them is visible in the branch itself. initialize now fits the metrics limits without the Metrics/CyclomaticComplexity exemption it has carried until now. No behaviour change.
anakinj
force-pushed
the
refactor/jwk-set-initializer
branch
from
September 5, 2026 18:51
250f3e0 to
f7219a8
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
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.
Description
Pure refactor of
JWT::JWK::Set#initialize, no behaviour change. Groundwork for #744.The two branches that build keys from hashes had drifted apart: the JWKS document form (
Hash) skips a key whosektyis unsupported, per RFC 7517 §5, while the array form raises. Both wereeach_with_object/mapblocks nested in the middle of acase, which makes the difference between them easy to miss — and that is how the gap #744 reports came about.Each branch now calls a named method, so the difference is stated rather than buried:
Side effects of the extraction:
initializefits the metrics limits, so the# rubocop:disable Metrics/CyclomaticComplexityit has carried is gone.jwks = jwks.transform_keys(&:to_sym)no longer reassigns the subject of thecasefrom inside one of its own branches;jwks ||= {}becomes an explicitwhen nil then [].On #744
#744 currently fails RuboCop, because copying the rescue block into the array branch pushes
initializetoMetrics/MethodLength [22/18]andMetrics/PerceivedComplexity [9/8]. On top of this refactor that PR becomes a one-line change — point theArraybranch atbuild_supported_keysand deletebuild_keys— with its spec unchanged.Verification
mainacross 22 input shapes (nil / no args / empty hash, symbol and stringkeys, unsupportedktyin first, last and only position,keys: nil,keys:given a bare hash, a bare JWK hash, arrays of hashes and of JWK objects, a single JWK object, construction from aSet, copy independence, bad types, options pass-through,merge). Output is identical, including the array-with-unsupported-ktycases still raisingJWT::UnsupportedKeyType.Checklist
Before the PR can be merged be sure the following are checked: