From 3cbccd376cb0b9f623cc125b7b6c9c93a9b35cc3 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 30 Jan 2019 11:03:37 -0500 Subject: [PATCH] Adding configuration for resource_klass resolver Prior to this commit, there was an assumption that the constant used as the source for sending to the persistence layer is the same as the target used for reifying from the persistence layer. With this commit, we have a crease for which we can, in upstream implementations, create a different resolver strategy. From revgum: > It seems there's room for documenting the dependency and raising an > appropriate exception for a circumstance with a dynamically generating > a class. > > See https://github.com/samvera/hyrax/blob/5a2906ad69266245bd80c8528e35060a21aefde5/lib/hyrax/valkyrie/resource_factory.rb#L39 for the dynamic generation of the class. > > `internal_resource.constantize` fails, expectedly at : https://github.com/samvera-labs/valkyrie/blob/master/lib/valkyrie/persistence/postgres/orm_converter.rb#L58 > > Related to our valkyrie-hyrax work. @jeremyf @no-reply > > ``` > SQL (16.3ms) INSERT INTO "orm_resources" ("metadata", "created_at", "updated_at", "internal_resource") VALUES ($1, $2, $3, $4) RETURNING "id" [["metadata", "{\"new_record\":[true],\"title\":[\"Test\"]}"], ["created_at", "2019-01-29 20:32:04.948751"], ["updated_at", "2019-01-29 20:32:04.948751"], ["internal_resource", "#"]] > (2.1ms) COMMIT > NameError: wrong constant name # > from /usr/local/bundle/gems/activesupport-5.1.6.1/lib/active_support/inflector/methods.rb:269:in `const_get' > ``` From jeremyf: > For Hyrax, we are looking at options that will: create a named dynamic > class (e.g. defined Class.new but one that could work with > String#constantize. My suspicion, is that we'll need to inject a > lambda with a default behavior of internal_resource.constantize, > however, a configuration would allow us to register a lambda for > looking up the class associated with an internal resource. > > Considerations: > > The internal_resource appears to be assigned in Valkyrie::Resource > We fetch the class resource_class via the #constantize in both > Valkyrie::Persistence::Postgres::ORMConverter and > Valkyrie::Persistence::Solr::ORMConverter Closes #647 Questions: * Should the resolver use named parameters? * Should the resolver factory also pass the attributes? I can imagine a case when the name and the attributes could be used in a lookup table; I don't believe it likly to use attributes but it would be a more durable interface declaration --- README.md | 11 ++++++++ lib/valkyrie.rb | 28 ++++++++++++++++++- .../persistence/postgres/orm_converter.rb | 2 +- .../persistence/solr/orm_converter.rb | 2 +- lib/valkyrie/types.rb | 3 +- spec/valkyrie_spec.rb | 25 +++++++++++++++++ 6 files changed, 67 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ef31b848a..fc53c5735 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,17 @@ The initializer also registers three `Valkyrie::StorageAdapter` instances for st * `:memory` which stores files in an in-memory cache (again, not persistent, so this is only appropriate for testing) +### Sample configuration with custom `Valkyrie.config.resource_class_resolver`: + +``` +require 'valkyrie' +Rails.application.config.to_prepare do + Valkyrie.config.resource_class_resolver = lambda do |resource_klass_name| + # Do complicated lookup based on the string + end +end +``` + ### Sample configuration: `config/valkyrie.yml`: A sample configuration file that configures your application to use different adapters: diff --git a/lib/valkyrie.rb b/lib/valkyrie.rb index c3c7352e7..9ef02b1c3 100644 --- a/lib/valkyrie.rb +++ b/lib/valkyrie.rb @@ -86,13 +86,39 @@ def storage_adapter Valkyrie::StorageAdapter.find(super.to_sym) end + # @api public + # + # The returned anonymous method (e.g. responds to #call) has a signature of + # an unamed parameter that is a string. Calling the anonymous method should + # return a Valkyrie::Resource from which Valkyrie will map the persisted + # data into. + # + # @return [#call] with method signature of 1 + # + # @see #default_resource_class_resolver for full interface + def resource_class_resolver + super + end + + # @!attribute [w] resource_class_resolver= + # The setter for #resource_class_resolver; see it's implementation + private def defaults { - standardize_query_result: false + standardize_query_result: false, + resource_class_resolver: method(:default_resource_class_resolver) } end + + # String constantize is a "by convention" factory. This works, but assumes + # the ruby class once used to persist is the model used to now reify. + # + # @param [String] class_name + def default_resource_class_resolver(class_name) + class_name.constantize + end end module_function :config, :logger, :logger=, :config_root_path, :environment, :warn_about_standard_queries!, :config_file, :config_hash diff --git a/lib/valkyrie/persistence/postgres/orm_converter.rb b/lib/valkyrie/persistence/postgres/orm_converter.rb index bcdedd982..d9cf439be 100644 --- a/lib/valkyrie/persistence/postgres/orm_converter.rb +++ b/lib/valkyrie/persistence/postgres/orm_converter.rb @@ -56,7 +56,7 @@ def lock_token_warning # Retrieve the Class used to construct the Valkyrie Resource # @return [Class] def resource_klass - internal_resource.constantize + Valkyrie.config.resource_class_resolver.call(internal_resource) end # Access the String for the Valkyrie Resource type within the attributes diff --git a/lib/valkyrie/persistence/solr/orm_converter.rb b/lib/valkyrie/persistence/solr/orm_converter.rb index 95fcbdce9..10d2a25fa 100644 --- a/lib/valkyrie/persistence/solr/orm_converter.rb +++ b/lib/valkyrie/persistence/solr/orm_converter.rb @@ -26,7 +26,7 @@ def resource # Access the Class for the Valkyrie Resource # @return [Class] def resource_klass - internal_resource.constantize + Valkyrie.config.resource_class_resolver.call(internal_resource) end # Access the String specifying the Valkyrie Resource type in the Solr Document diff --git a/lib/valkyrie/types.rb b/lib/valkyrie/types.rb index 02ecf1a2a..682a8da46 100644 --- a/lib/valkyrie/types.rb +++ b/lib/valkyrie/types.rb @@ -61,7 +61,8 @@ module Params # Used for casting {Valkyrie::Resources} if possible. Anything = Valkyrie::Types::Any.constructor do |value| if value.respond_to?(:fetch) && value.fetch(:internal_resource, nil) - value.fetch(:internal_resource).constantize.new(value) + resource_klass = Valkyrie.config.resource_class_resolver.call(value.fetch(:internal_resource)) + resource_klass.new(value) else value end diff --git a/spec/valkyrie_spec.rb b/spec/valkyrie_spec.rb index 9901d51a9..9cb1f2e64 100644 --- a/spec/valkyrie_spec.rb +++ b/spec/valkyrie_spec.rb @@ -43,4 +43,29 @@ expect(Rails).to have_received(:root).exactly(8).times end end + describe ".config" do + describe '.resource_class_resolver' do + subject(:resolver) { described_class.config.resource_class_resolver } + it { is_expected.to respond_to(:call).with(1).argument } + context 'when called' do + it 'will by default constantize the given string' do + expect(resolver.call('Valkyrie')).to eq(described_class) + end + end + context 'when configured' do + around do |example| + original = described_class.config.resource_class_resolver + example.run + described_class.config.resource_class_resolver = original + end + it 'will use the configured lambda' do + # Yes. This does not conform to the expected output, but + # I'm looking to demonstrate how this works + new_resolver = ->(string) { string.to_sym } + described_class.config.resource_class_resolver = new_resolver + expect(described_class.config.resource_class_resolver.call('hello')).to eq(:hello) + end + end + end + end end