Update both versions of get_phi, add specs for both

.gitignore

@@ -13,11 +13,12 @@
 
 /spec/dummy/tmp/
 /spec/dummy/db/schema.rb
-*.sqlite
-*.sqlite3
+*.sqlite*
+*.sqlite3*
 *.log
 .rspec_status
 gemfiles/*.gemfile.lock
+gemfiles/.bundle
 
 # Macs. Ugh.
 .DS_Store

Dockerfile

@@ -14,7 +14,6 @@ WORKDIR $APP_HOME
 COPY . $APP_HOME/
 
 RUN gem update --system
-RUN bundle config set force_ruby_platform true
 
 EXPOSE 3000
 

lib/phi_attrs/phi_record.rb

@@ -149,7 +149,7 @@ def get_phi(user_id = nil, reason = nil, allow_only: nil)
             allow_only.each { |t| t.allow_phi!(user_id, reason) }
           end
 
-          return yield if block_given?
+          return yield
         ensure
           __instances_with_extended_phi.each do |obj|
             if frozen_instances.include?(obj)
@@ -385,15 +385,15 @@ def get_phi(user_id = nil, reason = nil)
       raise ArgumentError, 'block required' unless block_given?
 
       extended_instances = @__phi_relations_extended.clone
-      allow_phi!(user_id, reason)
-
-      result = yield if block_given?
-
-      new_extensions = @__phi_relations_extended - extended_instances
-      disallow_last_phi!(preserve_extensions: true)
-      revoke_extended_phi!(new_extensions) if new_extensions.any?
-
-      result
+      begin
+        allow_phi!(user_id, reason)
+
+        return yield
+      ensure
+        new_extensions = @__phi_relations_extended - extended_instances
+        disallow_last_phi!(preserve_extensions: true)
+        revoke_extended_phi!(new_extensions) if new_extensions.any?
+      end
     end
 
     # Revoke all PHI access for a single instance of this class.

spec/dummy/db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2017_02_14_100255) do
+ActiveRecord::Schema[7.2].define(version: 2017_02_14_100255) do
   create_table "addresses", force: :cascade do |t|
     t.integer "patient_info_id"
     t.string "address"
@@ -23,16 +23,6 @@
     t.index ["patient_info_id"], name: "index_health_records_on_patient_info_id"
   end
 
-  create_table "missing_attribute_model", force: :cascade  do |t|
-    t.datetime "created_at", precision: nil, null: false
-    t.datetime "updated_at", precision: nil, null: false
-  end
-
-  create_table "missing_extend_model", force: :cascade  do |t|
-    t.datetime "created_at", precision: nil, null: false
-    t.datetime "updated_at", precision: nil, null: false
-  end
-
   create_table "patient_details", force: :cascade do |t|
     t.integer "patient_info_id"
     t.string "detail"
@@ -48,5 +38,4 @@
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
   end
-
 end

spec/phi_attrs/phi_record/class__allow_phi_spec.rb

@@ -111,6 +111,17 @@
     it 'get_phi with block returns value' do |t|
       expect(PatientInfo.get_phi(file_name, t.full_description) { patient_jane.first_name }).to eq('Jane')
     end
+
+    it 'does not leak phi allowance if get_phi returns', :aggregate_failures do |t|
+      def name_getter(reason, description)
+        PatientInfo.get_phi(reason, description) { return patient_jane.first_name }
+      end
+
+      expect(patient_jane.phi_allowed?).to be false
+      first_name = name_getter(file_name, t.full_description)
+      expect(first_name).to eq('Jane')
+      expect(patient_jane.phi_allowed?).to be false
+    end
   end
 
   context 'extended authorization' do

spec/phi_attrs/phi_record/instance__allow_phi_spec.rb

@@ -125,7 +125,7 @@
         expect(patient_jane.get_phi(file_name, t.full_description) { patient_jane.first_name }).to eq('Jane')
       end
 
-      it 'does not leak phi allowance if get_phi returns' do |t|
+      it 'does not leak phi allowance if get_phi returns', :aggregate_failures do |t|
         def name_getter(reason, description)
           patient_jane.get_phi(reason, description) { return patient_jane.first_name }
         end