Skip to content

Commit

Permalink
Optimistic locking for Postgres
Browse files Browse the repository at this point in the history
Closes #456.
  • Loading branch information
tpendragon committed Aug 3, 2018
1 parent aee424a commit 4cb500b
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 23 deletions.
@@ -0,0 +1,6 @@
# frozen_string_literal: true
class AddOptimisticLockingToOrmResources < ActiveRecord::Migration[5.1]
def change
add_column :orm_resources, :lock_version, :integer
end
end
2 changes: 1 addition & 1 deletion lib/valkyrie/persistence/postgres/metadata_adapter.rb
Expand Up @@ -23,7 +23,7 @@ def query_service

# @return [Class] {Valkyrie::Persistence::Postgres::ResourceFactory}
def resource_factory
Valkyrie::Persistence::Postgres::ResourceFactory
@resource_factory ||= Valkyrie::Persistence::Postgres::ResourceFactory.new(adapter: self)
end

def id
Expand Down
30 changes: 27 additions & 3 deletions lib/valkyrie/persistence/postgres/orm_converter.rb
Expand Up @@ -3,9 +3,10 @@ module Valkyrie::Persistence::Postgres
# Responsible for converting a
# {Valkyrie::Persistence::Postgres::ORM::Resource} to a {Valkyrie::Resource}
class ORMConverter
attr_reader :orm_object
def initialize(orm_object)
attr_reader :orm_object, :resource_factory
def initialize(orm_object, resource_factory:)
@orm_object = orm_object
@resource_factory = resource_factory
end

# Create a new instance of the class described in attributes[:internal_resource]
Expand All @@ -17,7 +18,30 @@ def convert!
private

def resource
resource_klass.new(attributes.merge(new_record: false))
resource_klass.new(
attributes.merge(
new_record: false,
Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK => lock_token
)
)
end

def lock_token
return lock_token_warning unless orm_object.class.column_names.include?("lock_version")
@lock_token ||=
Valkyrie::Persistence::OptimisticLockToken.new(
adapter_id: resource_factory.adapter_id,
token: orm_object.lock_version
)
end

def lock_token_warning
return nil unless resource_klass.optimistic_locking_enabled?
warn "[MIGRATION REQUIRED] You have loaded a resource from the Postgres adapter with " \
"optimistic locking enabled, but the necessary migrations have not been run. \n" \
"Please run `bin/rails valkyrie_engine:install:migrations && bin/rails db:migrate` " \
"to enable this feature for Postgres."
nil
end

def resource_klass
Expand Down
4 changes: 4 additions & 0 deletions lib/valkyrie/persistence/postgres/persister.rb
Expand Up @@ -15,6 +15,8 @@ def save(resource:)
orm_object = resource_factory.from_resource(resource: resource)
orm_object.save!
resource_factory.to_resource(object: orm_object)
rescue ActiveRecord::StaleObjectError
raise Valkyrie::Persistence::StaleObjectError, resource.id.to_s
end

# (see Valkyrie::Persistence::Memory::Persister#save_all)
Expand All @@ -24,6 +26,8 @@ def save_all(resources:)
save(resource: resource)
end
end
rescue Valkyrie::Persistence::StaleObjectError
raise Valkyrie::Persistence::StaleObjectError, resources.map(&:id).join(", ")
end

# (see Valkyrie::Persistence::Memory::Persister#delete)
Expand Down
10 changes: 10 additions & 0 deletions lib/valkyrie/persistence/postgres/resource_converter.rb
Expand Up @@ -13,10 +13,20 @@ def initialize(resource, resource_factory:)
def convert!
orm_class.find_or_initialize_by(id: resource.id && resource.id.to_s).tap do |orm_object|
orm_object.internal_resource = resource.internal_resource
process_lock_token(orm_object)
orm_object.metadata.merge!(attributes)
end
end

def process_lock_token(orm_object)
return unless resource.respond_to?(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK)
postgres_token = resource[Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK].find do |token|
token.adapter_id == resource_factory.adapter_id
end
return unless postgres_token
orm_object.lock_version = postgres_token.token
end

# Convert attributes to all be arrays to better enable querying and
# "changing of minds" later on.
def attributes
Expand Down
40 changes: 22 additions & 18 deletions lib/valkyrie/persistence/postgres/resource_factory.rb
Expand Up @@ -5,26 +5,30 @@ module Valkyrie::Persistence::Postgres
# Provides access to generic methods for converting to/from
# {Valkyrie::Resource} and {Valkyrie::Persistence::Postgres::ORM::Resource}.
class ResourceFactory
class << self
# @param object [Valkyrie::Persistence::Postgres::ORM::Resource] AR
# record to be converted.
# @return [Valkyrie::Resource] Model representation of the AR record.
def to_resource(object:)
::Valkyrie::Persistence::Postgres::ORMConverter.new(object).convert!
end
attr_reader :adapter
delegate :id, to: :adapter, prefix: true
def initialize(adapter:)
@adapter = adapter
end

# @param object [Valkyrie::Persistence::Postgres::ORM::Resource] AR
# record to be converted.
# @return [Valkyrie::Resource] Model representation of the AR record.
def to_resource(object:)
::Valkyrie::Persistence::Postgres::ORMConverter.new(object, resource_factory: self).convert!
end

# @param resource [Valkyrie::Resource] Model to be converted to ActiveRecord.
# @return [Valkyrie::Persistence::Postgres::ORM::Resource] ActiveRecord
# resource for the Valkyrie resource.
def from_resource(resource:)
::Valkyrie::Persistence::Postgres::ResourceConverter.new(resource, resource_factory: self).convert!
end
# @param resource [Valkyrie::Resource] Model to be converted to ActiveRecord.
# @return [Valkyrie::Persistence::Postgres::ORM::Resource] ActiveRecord
# resource for the Valkyrie resource.
def from_resource(resource:)
::Valkyrie::Persistence::Postgres::ResourceConverter.new(resource, resource_factory: self).convert!
end

# Accessor for the ActiveRecord class which all Postgres resources are an
# instance of.
def orm_class
::Valkyrie::Persistence::Postgres::ORM::Resource
end
# Accessor for the ActiveRecord class which all Postgres resources are an
# instance of.
def orm_class
::Valkyrie::Persistence::Postgres::ORM::Resource
end
end
end
6 changes: 5 additions & 1 deletion lib/valkyrie/resource.rb
Expand Up @@ -77,8 +77,12 @@ def self.enable_optimistic_locking
attribute(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK, Valkyrie::Types::Set.of(Valkyrie::Types::OptimisticLockToken))
end

def self.optimistic_locking_enabled?
schema.key?(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK)
end

def optimistic_locking_enabled?
respond_to?(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK)
self.class.optimistic_locking_enabled?
end

# @return [Hash] Hash of attributes
Expand Down
23 changes: 23 additions & 0 deletions spec/valkyrie/persistence/postgres/persister_spec.rb
@@ -1,13 +1,15 @@
# frozen_string_literal: true
require 'spec_helper'
require 'valkyrie/specs/shared_specs'
require 'valkyrie/specs/shared_specs/locking_persister'

RSpec.describe Valkyrie::Persistence::Postgres::Persister do
let(:query_service) { adapter.query_service }
let(:adapter) { Valkyrie::Persistence::Postgres::MetadataAdapter.new }

let(:persister) { adapter.persister }
it_behaves_like "a Valkyrie::Persister"
it_behaves_like "a Valkyrie locking persister"

context "single value behavior" do
before do
Expand Down Expand Up @@ -81,4 +83,25 @@ class CustomResource < Valkyrie::Resource
expect(query_service.find_by(id: resource1.id).author).to be_nil
end
end

context "when using an optimistically locked resource" do
before do
class MyLockingResource < Valkyrie::Resource
enable_optimistic_locking
end
end
after do
Object.send(:remove_const, :MyLockingResource)
end
context "and the migrations haven't been run" do
before do
allow(adapter.resource_factory.orm_class).to receive(:column_names)
.and_return(adapter.resource_factory.orm_class.column_names - ["lock_version"])
end
it "loads the object, but sends a warning with instructions" do
resource = MyLockingResource.new
expect { adapter.persister.save(resource: resource) }.to output(/\[MIGRATION REQUIRED\]/).to_stderr
end
end
end
end
2 changes: 2 additions & 0 deletions spec/valkyrie/persistence/postgres/query_service_spec.rb
@@ -1,8 +1,10 @@
# frozen_string_literal: true
require 'spec_helper'
require 'valkyrie/specs/shared_specs'
require 'valkyrie/specs/shared_specs/locking_query'

RSpec.describe Valkyrie::Persistence::Postgres::QueryService do
let(:adapter) { Valkyrie::Persistence::Postgres::MetadataAdapter.new }
it_behaves_like "a Valkyrie query provider"
it_behaves_like "a Valkyrie locking query provider"
end
2 changes: 2 additions & 0 deletions spec/valkyrie/resource_spec.rb
Expand Up @@ -130,6 +130,7 @@ class MyLockingResource < Valkyrie::Resource

expect(resource).to respond_to(Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK)
expect(resource.optimistic_locking_enabled?).to be true
expect(resource.class.optimistic_locking_enabled?).to be true
end

describe ".#{Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK}" do
Expand Down Expand Up @@ -165,6 +166,7 @@ class MyNonlockingResource < Valkyrie::Resource
it "does not have an optimistic_lock_token attribute" do
expect(MyNonlockingResource.new).not_to respond_to(:optimistic_lock_token)
expect(MyNonlockingResource.new.optimistic_locking_enabled?).to be false
expect(MyNonlockingResource.optimistic_locking_enabled?).to be false
end
end
end
Expand Down

0 comments on commit 4cb500b

Please sign in to comment.