fix for dirty changed attributes

lib/vault/encrypted_model.rb

@@ -35,10 +35,16 @@ module ClassMethods
       #   a proc to encode the value with
       # @option options [Proc] :decode
       #   a proc to decode the value with
-      def vault_attribute(attribute, options = {})
-        encrypted_column = options[:encrypted_column] || "#{attribute}_encrypted"
+      def vault_attribute(vault_attr, options = {})
+        # https://github.com/rails/rails/issues/33753#issuecomment-417503496
+        # You cannot call attribute_will_change! on something not registered
+        # with the attributes API -- There must be attribute :user_ids if you
+        # want that to be managed by Active Record.
+        attribute(vault_attr) if defined? attribute
+
+        encrypted_column = options[:encrypted_column] || "#{vault_attr}_encrypted"
         path = options[:path] || "transit"
-        key = options[:key] || "#{Vault::Rails.application}_#{table_name}_#{attribute}"
+        key = options[:key] || "#{Vault::Rails.application}_#{table_name}_#{vault_attr}"
 
         # Sanity check options!
         _vault_validate_options!(options)
@@ -60,53 +66,56 @@ def vault_attribute(attribute, options = {})
         end
 
         # Getter
-        define_method("#{attribute}") do
-          self.__vault_load_attributes! unless @__vault_loaded
-          instance_variable_get("@#{attribute}")
+        define_method(vault_attr.to_s) do
+          __vault_load_attributes! unless @__vault_loaded
+          instance_variable_get("@#{vault_attr}")
         end
 
         # Setter
-        define_method("#{attribute}=") do |value|
-          self.__vault_load_attributes! unless @__vault_loaded
+        define_method("#{vault_attr}=") do |value|
+          __vault_load_attributes! unless @__vault_loaded
 
           # We always set it as changed without comparing with the current value
           # because we allow our held values to be mutated, so we need to assume
           # that if you call attr=, you want it send back regardless.
+          attribute_will_change!(vault_attr.to_s)
+          instance_variable_set("@#{vault_attr}", value)
 
-          attribute_will_change!("#{attribute}")
-          instance_variable_set("@#{attribute}", value)
-
-          # Return the value to be consistent with other AR methods.
-          value
+          # Call super method or return value to be consistent with other AR methods.
+          if defined?(super)
+            super(value)
+          else
+            value
+          end
         end
 
         # Checker
-        define_method("#{attribute}?") do
+        define_method("#{vault_attr}?") do
           self.__vault_load_attributes! unless @__vault_loaded
-          instance_variable_get("@#{attribute}").present?
+          instance_variable_get("@#{vault_attr}").present?
         end
 
         # Dirty method
-        define_method("#{attribute}_change") do
-          changes["#{attribute}"]
+        define_method("#{vault_attr}_change") do
+          changes["#{vault_attr}"]
         end
 
         # Dirty method
-        define_method("#{attribute}_changed?") do
-          changed.include?("#{attribute}")
+        define_method("#{vault_attr}_changed?") do
+          changed.include?("#{vault_attr}")
         end
 
         # Dirty method
-        define_method("#{attribute}_was") do
-          if changes["#{attribute}"]
-            changes["#{attribute}"][0]
+        define_method("#{vault_attr}_was") do
+          if changes["#{vault_attr}"]
+            changes["#{vault_attr}"][0]
           else
-            public_send("#{attribute}")
+            public_send("#{vault_attr}")
           end
         end
 
         # Make a note of this attribute so we can use it in the future (maybe).
-        __vault_attributes[attribute.to_sym] = {
+        __vault_attributes[vault_attr.to_sym] = {
           key: key,
           path: path,
           serializer: serializer,
@@ -158,6 +167,9 @@ def vault_lazy_decrypt!
       # Vault and decrypt any attributes unless vault_lazy_decrypt is set.
       after_initialize :__vault_initialize_attributes!
 
+      # Before save we keep changed attributes to variable
+      before_save :__vault_keep_changed_attributes!
+
       # After we save the record, persist all the values to Vault and reload
       # them attributes from Vault to ensure we have the proper attributes set.
       # The reason we use `after_save` here is because a `before_save` could
@@ -221,20 +233,21 @@ def __vault_load_attribute!(attribute, options)
       def __vault_persist_attributes!
         changes = {}
 
-        self.class.__vault_attributes.each do |attribute, options|
-          if c = self.__vault_persist_attribute!(attribute, options)
-            changes.merge!(c)
-          end
+        changed_attributes = instance_variable_get('@vault_changed_attributes')
+        # Only persist changed attributes to minimize requests - this helps
+        # minimize the number of requests to Vault.
+        changed_attributes.each do |attribute|
+          options = self.class.__vault_attributes[attribute]
+          persist_attribute = self.__vault_persist_attribute!(attribute, options)
+          changes.merge!(persist_attribute) if persist_attribute
         end
 
         # If there are any changes to the model, update them all at once,
         # skipping any callbacks and validation. This is okay, because we are
         # already in a transaction due to the callback.
-        if !changes.empty?
-          self.update_columns(changes)
-        end
+        self.update_columns(changes) unless changes.empty?
 
-        return true
+        true
       end
 
       # Encrypt a single attribute using Vault and persist back onto the
@@ -245,19 +258,11 @@ def __vault_persist_attribute!(attribute, options)
         serializer = options[:serializer]
         column     = options[:encrypted_column]
 
-        # Only persist changed attributes to minimize requests - this helps
-        # minimize the number of requests to Vault.
-        if !changed.include?("#{attribute}")
-          return
-        end
-
         # Get the current value of the plaintext attribute
         plaintext = instance_variable_get("@#{attribute}")
 
         # Apply the serialize to the plaintext value, if one exists
-        if serializer
-          plaintext = serializer.encode(plaintext)
-        end
+        plaintext = serializer.encode(plaintext) if serializer
 
         # Generate the ciphertext and store it back as an attribute
         ciphertext = Vault::Rails.encrypt(path, key, plaintext)
@@ -270,6 +275,16 @@ def __vault_persist_attribute!(attribute, options)
         { column => ciphertext }
       end
 
+      # Keep changed attributes
+      def __vault_keep_changed_attributes!
+        instance_variable_set(
+          '@vault_changed_attributes',
+          self.class.__vault_attributes.keys & changed.map(&:to_sym)
+        )
+
+        true
+      end
+
       # Override the reload method to reload the Vault attributes. This will
       # ensure that we always have the most recent data from Vault when we
       # reload a record from the database.

spec/dummy/app/models/person.rb

@@ -1,4 +1,4 @@
-require "binary_serializer"
+require 'binary_serializer'
 
 class Person < ActiveRecord::Base
   include Vault::EncryptedModel
@@ -7,8 +7,8 @@ class Person < ActiveRecord::Base
 
   vault_attribute :credit_card,
     encrypted_column: :cc_encrypted,
-    path: "credit-secrets",
-    key: "people_credit_cards"
+    path: 'credit-secrets',
+    key: 'people_credit_cards'
 
   vault_attribute :details,
     serialize: :json
@@ -22,4 +22,3 @@ class Person < ActiveRecord::Base
 
   vault_attribute :non_ascii
 end
-

spec/integration/rails_spec.rb

@@ -216,25 +216,6 @@
       expect(person.credit_card_was).to eq("1234567890111213")
     end
 
-    it "tracks dirty attributes init with empty field" do
-      person = Person.new(credit_card: '1234')
-      person.save!
-      expect(person.cc_encrypted).to eql('12312321')
-
-      # expect(person.credit_card_changed?).to be(false)
-      # expect(person.credit_card_change).to eq(nil)
-      # expect(person.credit_card_was).to eq("1234567890111213")
-
-      # person.credit_card = "123456789010"
-
-      # expect(person.credit_card_changed?).to be(true)
-      # expect(person.credit_card_change).to eq(["1234567890111213", "123456789010"])
-      # expect(person.credit_card_was).to eq("1234567890111213")
-
-      # person.save!
-      # expect(person.credit_card).to eq("")
-    end
-
     it "allows attributes to be unset" do
       person = Person.create!(credit_card: "1234567890111213")
       person.update_attributes!(credit_card: nil)