Make the Reference object stateless and its #resolve method side-effe…

…ct free

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]

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<Boolean,Hash,String>]
     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

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