Avoid creating multiple streams

lib/ndr_import/file/base.rb

@@ -34,10 +34,10 @@ def files
         yield @filename
       end
 
-      # This method iterates over the tables in the given file and yields with two arguments:
-      # a tablename and a row enumerator (for that table). For a spreadsheet it may yield for
-      # every worksheet in the file and for a CSV file it will only yield once (the entire
-      # file is one table).
+      # This method iterates over the tables in the given file and yields with three arguments:
+      # a tablename, a row enumerator (for that table) and any file metdata.
+      # For a spreadsheet it may yield for  every worksheet in the file and for a CSV file it
+      # will only yield once (the entire file is one table).
       #
       # As single table files are in the majority, the Base implementation is defined for
       # single table handlers and you will only need to implement the rows iterator. If your

lib/ndr_import/file/xml.rb

@@ -35,27 +35,32 @@ def slurp_metadata_values
       def stream_metadata_values
         return unless @xml_file_metadata.is_a?(Hash)
 
-        self.file_metadata = @xml_file_metadata.transform_values { |xpath| metadata_at(xpath) }
+        with_encoding_check(@filename) do |stream, encoding|
+          self.file_metadata = @xml_file_metadata.transform_values.with_index do |xpath, index|
+            # Ensure we're at the start of the stream each time
+            stream.rewind unless index.zero?
+
+            metadata_from_stream(xpath, stream, encoding)
+          end
+        end
       end
 
-      def metadata_at(xpath)
-        with_encoding_check(@filename) do |stream, encoding|
-          cursor = Cursor.new(xpath, false)
-
-          # If markup isn't well-formed, try to work around it:
-          options = Nokogiri::XML::ParseOptions::RECOVER
-          reader  = Nokogiri::XML::Reader(stream, nil, encoding, options)
-
-          reader.each do |node|
-            case node.node_type
-            when Nokogiri::XML::Reader::TYPE_ELEMENT # "opening tag"
-              raise NestingError, node if cursor.in?(node)
-
-              cursor.enter(node)
-              return cursor.inner_text if cursor.send(:current_stack_match?)
-            when Nokogiri::XML::Reader::TYPE_END_ELEMENT # "closing tag"
-              cursor.leave(node)
-            end
+      def metadata_from_stream(xpath, stream, encoding)
+        cursor = Cursor.new(xpath, false)
+
+        # If markup isn't well-formed, try to work around it:
+        options = Nokogiri::XML::ParseOptions::RECOVER
+        reader  = Nokogiri::XML::Reader(stream, nil, encoding, options)
+
+        reader.each do |node|
+          case node.node_type
+          when Nokogiri::XML::Reader::TYPE_ELEMENT # "opening tag"
+            raise NestingError, node if cursor.in?(node)
+
+            cursor.enter(node)
+            return cursor.inner_text if cursor.send(:current_stack_match?)
+          when Nokogiri::XML::Reader::TYPE_END_ELEMENT # "closing tag"
+            cursor.leave(node)
           end
         end
       end

test/file/xml_test.rb

@@ -95,8 +95,7 @@ def setup
             'submitting_providercode' => '//OrganisationIdentifierCodeOfSubmittingOrganisation/@extension'
           }
         }
-        handler = NdrImport::File::Xml.new(file_path, nil, options)
-        handler.expects(:each_node).never
+        handler           = NdrImport::File::Xml.new(file_path, nil, options)
         expected_metadata = { 'submitting_providercode' => 'LT4' }
         assert_equal expected_metadata, handler.file_metadata
 
@@ -114,8 +113,7 @@ def setup
             'record_count' => '//COSD:RecordCount/@value'
           }
         }
-        handler = NdrImport::File::Xml.new(file_path, nil, options)
-        handler.expects(:read_xml_file).never
+        handler           = NdrImport::File::Xml.new(file_path, nil, options)
         expected_metadata = { 'submitting_providercode' => 'LT4', 'record_count' => '6349923' }
         assert_equal expected_metadata, handler.file_metadata