fix: correctly handle reading locked pact file on windows

The yielded file from Filelock must be read, rather than using File.read, otherwise "Permission denied @ io_fread" is raised.

Fixes: pact-foundation/pact-js#150

Author: bethesque

lib/pact/consumer_contract/consumer_contract_writer.rb

@@ -60,10 +60,15 @@ def update_pactfile_if_needed
     def update_pactfile
       logger.info log_message
       FileUtils.mkdir_p File.dirname(pactfile_path)
-      Filelock pactfile_path do | file |
+      Filelock(pactfile_path) do | pact_file |
+        # must be read after obtaining the lock, and must be read from the yielded file object, otherwise windows freaks out
+        @existing_contents = pact_file.read
         new_contents = pact_json
-        file.truncate 0
-        file.write new_contents
+        pact_file.rewind
+        pact_file.truncate 0
+        pact_file.write new_contents
+        pact_file.flush
+        pact_file.truncate(pact_file.pos)
       end
     end
 
@@ -109,9 +114,12 @@ def pactfile_has_contents?
       File.size(pactfile_path) != 0
     end
 
+    def existing_contents
+      @existing_contents
+    end
+
     def existing_consumer_contract
-      # This must only be read after the file has been locked
-      Pact::ConsumerContract.from_uri(pactfile_path)
+      @existing_consumer_contract ||= Pact::ConsumerContract.from_json(existing_contents)
     end
 
     def warn_and_stderr msg

spec/lib/pact/consumer_contract/consumer_contract_writer_spec.rb

@@ -36,31 +36,36 @@ module Pact
 
     let(:consumer_contract_writer) { ConsumerContractWriter.new(consumer_contract_details, logger) }
 
-    describe "consumer_contract" do
-
-      let(:subject) { consumer_contract_writer.consumer_contract }
+    describe "#write" do
+      it "writes the pact file to the pact_dir" do
+        FileUtils.rm_rf target_pact_file_location
+        consumer_contract_writer.write
+        expect(File.exist?(target_pact_file_location)).to be true
+      end
 
       context "when overwriting pact" do
 
         it "it uses only the interactions from the current test run" do
-          expect(consumer_contract_writer.consumer_contract.interactions).to eq new_interactions
+          consumer_contract_writer.write
+          expect(Pact::ConsumerContract.from_json(File.read(target_pact_file_location)).interactions.collect(&:to_hash)).to eq new_interactions.collect(&:to_hash)
         end
 
       end
 
       context "when updating pact" do
 
-        let(:pactfile_write_mode) {:update}
+        let(:pactfile_write_mode) { :update }
 
         it "merges the interactions from the current test run with the interactions from the existing file" do
           allow_any_instance_of(ConsumerContractWriter).to receive(:info_and_puts)
-          expect(consumer_contract_writer.consumer_contract.interactions).to eq  existing_interactions + new_interactions
+          consumer_contract_writer.write
+          expect(Pact::ConsumerContract.from_uri(target_pact_file_location).interactions.size).to eq 3
         end
 
-        let(:line0) { /\*/ }
-        let(:line1) { /Updating existing file/ }
-        let(:line2) { /Only interactions defined in this test run will be updated/ }
-        let(:line3) { /As interactions are identified by description and provider state/ }
+        let(:line0) { "*" * 77 }
+        let(:line1) { %r{Updating existing file} }
+        let(:line2) { %r{Only interactions defined in this test run will be updated} }
+        let(:line3) { %r{As interactions are identified by description and provider state} }
 
         it "logs a description message" do
           expect($stdout).to receive(:puts).with(line0).twice
@@ -71,40 +76,37 @@ module Pact
           expect(logger).to receive(:info).with(line1)
           expect(logger).to receive(:info).with(line2)
           expect(logger).to receive(:info).with(line3)
-          consumer_contract_writer.consumer_contract
+          consumer_contract_writer.write
         end
       end
 
       context "when an error occurs deserializing the existing pactfile" do
 
-        let(:pactfile_write_mode) {:update}
+        let(:pactfile_write_mode) { :update }
         let(:error) { RuntimeError.new('some error')}
-        let(:line1) { /Could not load existing consumer contract from .* due to some error/ }
+        let(:line1) { %r{Could not load existing consumer contract from .* due to} }
 
         before do
-          allow(ConsumerContract).to receive(:from_json).and_raise(error)
+          File.open(target_pact_file_location, "w") { |file| file << "wiffle"  }
+
           allow($stderr).to receive(:puts)
           allow(logger).to receive(:puts)
         end
 
+        after do
+          FileUtils.rm(target_pact_file_location)
+        end
+
         it "logs the error" do
           expect($stderr).to receive(:puts).with(line1)
           expect(logger).to receive(:warn).with(line1)
-          consumer_contract_writer.consumer_contract
+          consumer_contract_writer.write
         end
 
         it "uses the new interactions" do
           expect(consumer_contract_writer.consumer_contract.interactions).to eq new_interactions
         end
       end
-    end
-
-    describe "#write" do
-      it "writes the pact file to the pact_dir" do
-        FileUtils.rm_rf target_pact_file_location
-        consumer_contract_writer.write
-        expect(File.exist?(target_pact_file_location)).to be true
-      end
 
       context "when pactfile_write_mode is update" do
         let(:pactfile_write_mode) { :update }

spec/support/factories.rb

@@ -37,7 +37,7 @@ def self.create hash = {}
         },
         'response' => {
             'status' => 200,
-            'body' => {a: 'response body'}
+            'body' => {'a' => 'response body'}
         }
     }
     Pact::Interaction.from_hash(stringify_keys(deep_merge(defaults, stringify_keys(hash))))