Allow for optimistic locking to be turned on post-production.

Data should be persisted in such a way that it's okay if optimistic
locking is turned on later, and shouldn't require a data migration.

This will require a migration for postgres, but will give a warning if
it hasn't been run. Existing data will need to be migrated - maybe delay
this for 2.0?

Author: tpendragon

db/migrate/20190415200643_add_default_to_lock_version.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+class AddDefaultToLockVersion < ActiveRecord::Migration[5.1]
+  def change
+    change_column_default :orm_resources, :lock_version, from: nil, to: 0
+  end
+end

lib/valkyrie/persistence/fedora/persister/model_converter.rb

@@ -36,7 +36,15 @@ def properties
         resource_attributes.keys - [:new_record]
       end
 
-      delegate :attributes, to: :resource, prefix: true
+      # Always generate a token, in case it's enabled later.
+      def resource_attributes
+        @resource_attributes ||=
+          begin
+            attributes = resource.attributes.dup
+            attributes[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK] ||= [Valkyrie::Persistence::OptimisticLockToken.new(adapter_id: adapter.id, token: Time.now.to_r)]
+            attributes
+          end
+      end
 
       # Construct the LDP Basic Container modeling the Valkyrie Resource in Fedora
       # @see https://www.w3.org/TR/ldp/#ldpc

lib/valkyrie/persistence/fedora/persister/orm_converter.rb

@@ -42,7 +42,7 @@ def populate_native_lock(resource)
         return resource unless lastmod
 
         token = Valkyrie::Persistence::OptimisticLockToken.new(adapter_id: "native-#{adapter.id}", token: DateTime.parse(lastmod.to_s).httpdate)
-        resource.send(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK) << token
+        resource.set_value(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK, resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK] + [token])
         resource
       end
 

lib/valkyrie/persistence/memory/metadata_adapter.rb

@@ -27,7 +27,7 @@ def cache
 
     # @return [Valkyrie::ID] Identifier for this metadata adapter.
     def id
-      @id ||= Valkyrie::ID.new(Digest::MD5.hexdigest(self.class.to_s))
+      @id ||= Valkyrie::ID.new(Digest::MD5.hexdigest(self.class.to_s + SecureRandom.uuid))
     end
   end
 end

lib/valkyrie/persistence/memory/persister.rb

@@ -84,8 +84,10 @@ def normalize_date_value(value)
 
       # Create a new lock token based on the current timestamp.
       def generate_lock_token(resource)
-        return unless resource.optimistic_locking_enabled?
         token = Valkyrie::Persistence::OptimisticLockToken.new(adapter_id: adapter.id, token: Time.now.to_r)
+        cache[:versions] ||= {}
+        cache[:versions][resource.id] = token
+        return unless resource.optimistic_locking_enabled?
         resource.set_value(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK, token)
       end
 
@@ -96,11 +98,12 @@ def valid_lock?(resource)
         cached_resource = cache[resource.id]
         return true if cached_resource.blank?
 
-        resource_lock_tokens = resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK]
+        resource_lock_tokens = resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK] || []
         resource_value = resource_lock_tokens.find { |lock_token| lock_token.adapter_id == adapter.id }
-        return true if resource_value.blank?
+        cached_token = cached_resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK] || []
+        cached_value = cached_token.find { |lock_token| lock_token.adapter_id == adapter.id }
+        return true if resource_value.nil?
 
-        cached_value = cached_resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK].first
         cached_value == resource_value
       end
   end

lib/valkyrie/persistence/memory/query_service.rb

@@ -24,7 +24,14 @@ def initialize(adapter:)
     def find_by(id:)
       id = Valkyrie::ID.new(id.to_s) if id.is_a?(String)
       validate_id(id)
-      cache[id] || raise(::Valkyrie::Persistence::ObjectNotFoundError)
+      fetch_from_cache(id) || raise(::Valkyrie::Persistence::ObjectNotFoundError)
+    end
+
+    def fetch_from_cache(id)
+      result = cache[id]
+      return result if result.nil? || !result.optimistic_locking_enabled? || result[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK].present?
+      result.set_value(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK, cache[:versions][id])
+      result
     end
 
     # Get a single resource by `alternate_identifier`. Alternate identifiers are identifiers (like NOIDs,
@@ -62,15 +69,17 @@ def find_many_by_ids(ids:)
     # Get all objects.
     # @return [Array<Valkyrie::Resource>] All objects in the persistence backend.
     def find_all
-      cache.values
+      cache.except(:versions).values.map do |resource|
+        fetch_from_cache(resource.id)
+      end
     end
 
     # Get all objects of a given model.
     # @param model [Class] Class to query for.
     # @return [Array<Valkyrie::Resource>] All objects in the persistence backend
     #   with the given class.
     def find_all_of_model(model:)
-      cache.values.select do |obj|
+      find_all.select do |obj|
         obj.is_a?(model)
       end
     end

lib/valkyrie/persistence/postgres/orm_converter.rb

@@ -35,7 +35,7 @@ def resource
       # Construct the optimistic lock token using the adapter and lock version for the Resource
       # @return [Valkyrie::Persistence::OptimisticLockToken]
       def lock_token
-        return lock_token_warning unless orm_object.class.column_names.include?("lock_version")
+        return lock_token_warning unless orm_object.class.column_names.include?("lock_version") && orm_object.class.columns.find { |x| x.name == "lock_version" }.default == "0"
         @lock_token ||=
           Valkyrie::Persistence::OptimisticLockToken.new(
             adapter_id: resource_factory.adapter_id,

lib/valkyrie/specs/shared_specs/persister.rb

@@ -313,6 +313,33 @@ class CustomResource < Valkyrie::Resource
     expect(query_service.find_all.to_a.length).to eq 0
   end
 
+  context "optimistic locking enabled later" do
+    before do
+      class MyLockingResource < Valkyrie::Resource
+        attribute :title
+      end
+    end
+    describe "#save" do
+      it "will error appropriately if optimistic locking is enabled later" do
+        resource = MyLockingResource.new
+        saved_resource = persister.save(resource: resource)
+
+        MyLockingResource.enable_optimistic_locking
+
+        saved_resource = query_service.find_by(id: saved_resource.id)
+        reloaded = query_service.find_by(id: saved_resource.id)
+        saved_resource.title = "Don't Save Me"
+        reloaded.title = "Save Me"
+
+        persister.save(resource: reloaded)
+        expect { persister.save(resource: saved_resource) }.to raise_error Valkyrie::Persistence::StaleObjectError
+      end
+    end
+    after do
+      Object.send(:remove_const, :MyLockingResource)
+    end
+  end
+
   context "optimistic locking" do
     before do
       class MyLockingResource < Valkyrie::Resource

spec/spec_helper.rb

@@ -24,6 +24,7 @@
 WebMock.disable_net_connect!(allow_localhost: true)
 
 RSpec.configure do |config|
+  config.example_status_persistence_file_path = "tmp/rspec_examples.txt"
   config.order = :random
   Kernel.srand config.seed
 

spec/valkyrie/persistence/memory/metadata_adapter_spec.rb

@@ -7,9 +7,8 @@
   it_behaves_like "a Valkyrie::MetadataAdapter"
 
   describe "#id" do
-    it "creates an md5 hash from the class name" do
-      expected = Digest::MD5.hexdigest described_class.to_s
-      expect(adapter.id.to_s).to eq expected
+    it "creates a unique identifier for each instance" do
+      expect(adapter.id.to_s).not_to eq described_class.new.id.to_s
     end
   end
 end