From 134f3f814c7cc9a61e8dfa109c21c680bba9b63b Mon Sep 17 00:00:00 2001 From: Marica Odagaki Date: Wed, 13 Dec 2017 23:22:11 -0800 Subject: [PATCH] Make the Reference object stateless and its #resolve method side-effect free --- lib/open_api_parser/document.rb | 7 +-- lib/open_api_parser/reference.rb | 39 ++++++--------- spec/open_api_parser/reference_spec.rb | 68 +++++++++++++------------- 3 files changed, 53 insertions(+), 61 deletions(-) diff --git a/lib/open_api_parser/document.rb b/lib/open_api_parser/document.rb index 1ba7d3a..6620044 100644 --- a/lib/open_api_parser/document.rb +++ b/lib/open_api_parser/document.rb @@ -37,11 +37,12 @@ def expand_refs(fragment, current_pointer) if fragment.is_a?(Hash) && fragment.key?("$ref") raw_uri = fragment["$ref"] ref = OpenApiParser::Reference.new(raw_uri) - fully_resolved = ref.resolve(@path, current_pointer, @content, @file_cache) + fully_resolved, referrent_document, referrent_pointer = + ref.resolve(@path, current_pointer, @content, @file_cache) unless fully_resolved - expand_refs(ref.referrent_document, ref.referrent_pointer) + expand_refs(referrent_document, referrent_pointer) else - [ref.referrent_document, ref.referrent_pointer] + [referrent_document, referrent_pointer] end else [fragment, current_pointer] diff --git a/lib/open_api_parser/reference.rb b/lib/open_api_parser/reference.rb index 6a7df52..5d620b7 100644 --- a/lib/open_api_parser/reference.rb +++ b/lib/open_api_parser/reference.rb @@ -1,32 +1,24 @@ module OpenApiParser class Reference - # The resolved document. This gets set only after calling `#resolve`. - attr_reader :referrent_document - - # Pointer of the referrent_document if it's embedded in a larger document. - # This gets set only after calling `#resolve`. - # Empty string means the whole document. - attr_reader :referrent_pointer - def initialize(raw_uri) @raw_uri = raw_uri - @resolved = false end - # Sets referrent_document and referrent_pointer to the resolved - # raw specification and pointer, respectively. + # Resolve this reference in the given context. + # + # Returns a three-element array of: + # + # - Whether the referrent has been fully expanded + # - The resolved document + # - Pointer of the document if it's embedded in a larger document. + # Empty string means the whole document. # # @param base_path [String] Location of the document where the $ref originates. # @param base_pointer [String] Location of the $ref within the document. # @param current_document [Hash] Document where the $ref originates. # @param file_cache [OpenApiParser::FileCache] File cache instance. - # @return [Boolean] Whether the referrent has been fully expanded. + # @return [Array] def resolve(base_path, base_pointer, current_document, file_cache) - if @resolved - fail 'Do not try to resolve an already resolved reference.' - end - @resolved = true - # ref_uri needs to be normalized before being joined, as normalization # affects absolute/relativeness. ref_uri = normalize_file_uri(Addressable::URI.parse(@raw_uri)) @@ -45,14 +37,11 @@ def resolve(base_path, base_pointer, current_document, file_cache) fail "$ref with scheme #{ref_uri.scheme} is not supported" end - fully_expanded, @referrent_document, @referrent_pointer = - if !ref_uri.fragment.nil? && ref_uri.fragment != '' - resolve_pointer(ref_uri.fragment, base_pointer, referenced_document, fully_expanded) - else - [fully_expanded, referenced_document, ''] - end - - fully_expanded + if !ref_uri.fragment.nil? && ref_uri.fragment != '' + resolve_pointer(ref_uri.fragment, base_pointer, referenced_document, fully_expanded) + else + [fully_expanded, referenced_document, ''] + end end private diff --git a/spec/open_api_parser/reference_spec.rb b/spec/open_api_parser/reference_spec.rb index 998203d..b3ed51f 100644 --- a/spec/open_api_parser/reference_spec.rb +++ b/spec/open_api_parser/reference_spec.rb @@ -23,32 +23,31 @@ def project_root include PathHelpers describe "#resolve" do - it "cannot be called twice" do + it "can be called repeatedly" do ref = OpenApiParser::Reference.new('') - resolve = -> { ref.resolve('', '', {}, file_cache) } - resolve.call expect do - resolve.call - end.to raise_error(/already resolved/) + ref.resolve("http:", '', {}, file_cache) + end.to raise_error(Addressable::URI::InvalidURIError) + expect(ref.resolve('', '', {}, file_cache)).to eq [false, {}, ""] end describe "supported schemes" do it "supports the file scheme with relative path" do ref = OpenApiParser::Reference.new('file:nested/person.yaml') - ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) - expect(ref.referrent_document).to eq({"name" => "Drew"}) + _, referrent_doc, _ = ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) + expect(referrent_doc).to eq({"name" => "Drew"}) end it "supports the file scheme with absolute path" do ref = OpenApiParser::Reference.new('file:' + absolute('spec/resources/nested/person.yaml')) - ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) - expect(ref.referrent_document).to eq({"name" => "Drew"}) + _, referrent_doc, _ = ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) + expect(referrent_doc).to eq({"name" => "Drew"}) end it "interprets an empty scheme as a file path" do ref = OpenApiParser::Reference.new('nested/person.yaml') - ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) - expect(ref.referrent_document).to eq({"name" => "Drew"}) + _, referrent_doc, _ = ref.resolve(cwd_relative("spec/resources/valid_spec.yaml"), '', {}, file_cache) + expect(referrent_doc).to eq({"name" => "Drew"}) end it "does not support URI schemes other than file" do @@ -78,8 +77,8 @@ def project_root it "does not check for base uri's existence" do ref = OpenApiParser::Reference.new('nested/person.yaml') bad_base_path = cwd_relative("spec/resources/this-should-never-exist.lmay") - ref.resolve(bad_base_path, '', {}, file_cache) - expect(ref.referrent_document).to eq({"name" => "Drew"}) + _, referrent_doc, _ = ref.resolve(bad_base_path, '', {}, file_cache) + expect(referrent_doc).to eq({"name" => "Drew"}) end end @@ -103,8 +102,8 @@ def project_root context "given $ref #{ref_uri} and base_uri #{base_uri}" do it "resolves successfully" do ref = OpenApiParser::Reference.new(ref_uri) - ref.resolve(base_uri, '', {}, file_cache) - expect(ref.referrent_document).to eq({"name" => "Drew"}) + _, referrent_doc, _ = ref.resolve(base_uri, '', {}, file_cache) + expect(referrent_doc).to eq({"name" => "Drew"}) end end end @@ -114,8 +113,8 @@ def project_root expect(YAML).to_not receive(:load) document = {"current" => true} ref = OpenApiParser::Reference.new('person.yaml') - ref.resolve('person.yaml', '', document, file_cache) - expect(ref.referrent_document).to eq(document) + _, referrent_doc, _ = ref.resolve('person.yaml', '', document, file_cache) + expect(referrent_doc).to eq(document) end end end @@ -146,10 +145,10 @@ def project_root it "resolves '#{ref_pointer}' as expected when base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - ref.resolve(base_path, base_pointer, document, file_cache) + _, referrent_doc, referrent_pointer = ref.resolve(base_path, base_pointer, document, file_cache) - expect(ref.referrent_document).to eq(expected_doc) - expect(ref.referrent_pointer).to eq(expected_pointer) + expect(referrent_doc).to eq(expected_doc) + expect(referrent_pointer).to eq(expected_pointer) end end end @@ -170,10 +169,10 @@ def project_root it "resolves '#{ref_pointer}' as expected when base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - ref.resolve(base_path, base_pointer, document, file_cache) + _, referrent_doc, referrent_pointer = ref.resolve(base_path, base_pointer, document, file_cache) - expect(ref.referrent_document).to eq(expected_doc) - expect(ref.referrent_pointer).to eq(expected_pointer) + expect(referrent_doc).to eq(expected_doc) + expect(referrent_pointer).to eq(expected_pointer) end end @@ -207,10 +206,10 @@ def project_root it "resolves '#{ref_pointer}' as expected when base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - ref.resolve(base_path, base_pointer, document, file_cache) + _, referrent_doc, referrent_pointer = ref.resolve(base_path, base_pointer, document, file_cache) - expect(ref.referrent_document).to eq(expected_doc) - expect(ref.referrent_pointer).to eq(expected_pointer) + expect(referrent_doc).to eq(expected_doc) + expect(referrent_pointer).to eq(expected_pointer) end end end @@ -236,16 +235,16 @@ def project_root it "resolves '#{ref_pointer}' as expected when base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - ref.resolve(base_path, base_pointer, document, file_cache) + _, referrent_doc, referrent_pointer = ref.resolve(base_path, base_pointer, document, file_cache) - expect(ref.referrent_document).to eq(expected_doc) - expect(ref.referrent_pointer).to eq(expected_pointer) + expect(referrent_doc).to eq(expected_doc) + expect(referrent_pointer).to eq(expected_pointer) end end end end - describe 'its return value' do + describe 'return value for fully_expanded' do let(:document) { STANDARD_DOCUMENT } context "given an empty ref path" do @@ -263,7 +262,8 @@ def project_root it "is #{expected} when $ref pointer is '#{ref_pointer}' and base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - expect(ref.resolve(base_path, base_pointer, document, file_cache)).to be expected + fully_expanded, *_rest = ref.resolve(base_path, base_pointer, document, file_cache) + expect(fully_expanded).to be expected end end end @@ -283,7 +283,8 @@ def project_root it "is #{expected} when $ref pointer is '#{ref_pointer}' and base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - expect(ref.resolve(base_path, base_pointer, document, file_cache)).to be expected + fully_expanded, *_rest = ref.resolve(base_path, base_pointer, document, file_cache) + expect(fully_expanded).to be expected end end end @@ -307,7 +308,8 @@ def project_root it "is #{expected} when $ref pointer is '#{ref_pointer}' and base pointer is '#{base_pointer}'" do ref_uri = ref_path + ref_pointer ref = OpenApiParser::Reference.new(ref_uri) - expect(ref.resolve(base_path, base_pointer, document, file_cache)).to be expected + fully_expanded, *_rest = ref.resolve(base_path, base_pointer, document, file_cache) + expect(fully_expanded).to be expected end end end