From a29fc111dbeb318a14846dd38ccb826fbd7ca243 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Wed, 9 Mar 2016 14:29:56 -0500 Subject: [PATCH] Decompose ActiveFedora::Model into a generic model mapper and a classifier --- lib/active_fedora.rb | 5 +- lib/active_fedora/default_model_mapper.rb | 24 ++++++ lib/active_fedora/model.rb | 45 +----------- lib/active_fedora/model_classifier.rb | 77 ++++++++++++++++++++ lib/active_fedora/query_result_builder.rb | 4 +- lib/active_fedora/relation/finder_methods.rb | 3 +- spec/unit/default_model_mapper_spec.rb | 39 ++++++++++ spec/unit/model_classifier_spec.rb | 49 +++++++++++++ spec/unit/model_spec.rb | 35 --------- spec/unit/query_result_builder_spec.rb | 2 +- 10 files changed, 199 insertions(+), 84 deletions(-) create mode 100644 lib/active_fedora/default_model_mapper.rb create mode 100644 lib/active_fedora/model_classifier.rb create mode 100644 spec/unit/default_model_mapper_spec.rb create mode 100644 spec/unit/model_classifier_spec.rb diff --git a/lib/active_fedora.rb b/lib/active_fedora.rb index 184def15f..d3be39f2d 100644 --- a/lib/active_fedora.rb +++ b/lib/active_fedora.rb @@ -88,6 +88,7 @@ module ActiveFedora #:nodoc: autoload :LdpResourceService autoload :LoadableFromJson autoload :Model + autoload :ModelClassifier autoload :NestedAttributes autoload :NomDatastream autoload :NullRelation @@ -247,11 +248,11 @@ def version # ActiveFedora.class_from_string("TermProxy", ActiveFedora::RdfNode) # => ActiveFedora::RdfNode::TermProxy def class_from_string(*args) - model_mapper.class_from_string(*args) + ActiveFedora::ModelClassifier.class_from_string(*args) end def model_mapper - ActiveFedora::DefaultModelMapper + ActiveFedora::DefaultModelMapper.new end end diff --git a/lib/active_fedora/default_model_mapper.rb b/lib/active_fedora/default_model_mapper.rb new file mode 100644 index 000000000..05ccfc63a --- /dev/null +++ b/lib/active_fedora/default_model_mapper.rb @@ -0,0 +1,24 @@ +module ActiveFedora + # Create model classifiers for resources or solr documents + class DefaultModelMapper + attr_reader :classifier_class, :solr_field, :predicate + + def initialize(classifier_class: ActiveFedora::ModelClassifier, solr_field: ActiveFedora::QueryResultBuilder::HAS_MODEL_SOLR_FIELD, predicate: ActiveFedora::RDF::Fcrepo::Model.hasModel) + @classifier_class = classifier_class + @solr_field = solr_field + @predicate = predicate + end + + def classifier(resource) + models = if resource.respond_to? :graph + resource.graph.query([nil, predicate, nil]).map { |rg| rg.object.to_s } + elsif resource.respond_to? :[] + resource[solr_field] || [] + else + [] + end + + classifier_class.new(models) + end + end +end diff --git a/lib/active_fedora/model.rb b/lib/active_fedora/model.rb index 34c43cb51..e1aa13703 100644 --- a/lib/active_fedora/model.rb +++ b/lib/active_fedora/model.rb @@ -5,21 +5,6 @@ module ActiveFedora # This module mixes various methods into the including class, # much in the way ActiveRecord does. module Model - def self.best_class_from_uris(uris, default: nil) - best_model_match = default - - uris.each do |uri| - model_value = from_class_uri(uri) - next unless model_value - best_model_match ||= model_value - - # If there is an inheritance structure, use the most specific case. - best_model_match = model_value if best_model_match > model_value - end - - best_model_match || ActiveFedora::Base - end - # Convenience method for getting class constant based on a string # @example # ActiveFedora::Model.class_from_string("Om") @@ -29,19 +14,8 @@ def self.best_class_from_uris(uris, default: nil) # @example Search within ActiveFedora::RdfNode for a class called "TermProxy" # ActiveFedora::Model.class_from_string("TermProxy", ActiveFedora::RdfNode) # => ActiveFedora::RdfNode::TermProxy - def self.class_from_string(full_class_name, container_class = Kernel) - container_class = container_class.name if container_class.is_a? Module - container_parts = container_class.split('::') - (container_parts + full_class_name.split('::')).flatten.inject(Kernel) do |mod, class_name| - if mod == Kernel - Object.const_get(class_name) - elsif mod.const_defined? class_name.to_sym - mod.const_get(class_name) - else - container_parts.pop - class_from_string(class_name, container_parts.join('::')) - end - end + def self.class_from_string(*args) + ActiveFedora::ModelClassifier.class_from_string(*args) end # Takes a Fedora URI for a cModel, and returns a @@ -49,20 +23,7 @@ def self.class_from_string(full_class_name, container_class = Kernel) # This method should reverse ClassMethods#to_class_uri # @return [Class, False] the class of the model or false, if it does not exist def self.from_class_uri(model_value) - unless class_exists?(model_value) - ActiveFedora::Base.logger.warn "'#{model_value}' is not a real class" if ActiveFedora::Base.logger - return nil - end - ActiveFedora::Model.class_from_string(model_value) - end - - def self.class_exists?(class_name) - return false if class_name.empty? - klass = class_name.constantize - return klass.is_a?(Class) - rescue NameError - return false + ActiveFedora::ModelClassifier.new(Array(model_value)).best_model end - private_class_method :class_exists? end end diff --git a/lib/active_fedora/model_classifier.rb b/lib/active_fedora/model_classifier.rb new file mode 100644 index 000000000..e0d553b1e --- /dev/null +++ b/lib/active_fedora/model_classifier.rb @@ -0,0 +1,77 @@ +module ActiveFedora + # Translate model names to classes + class ModelClassifier + # Convenience method for getting class constant based on a string + # @example + # ActiveFedora::Model.class_from_string("Om") + # => Om + # ActiveFedora::Model.class_from_string("ActiveFedora::RdfNode::TermProxy") + # => ActiveFedora::RdfNode::TermProxy + # @example Search within ActiveFedora::RdfNode for a class called "TermProxy" + # ActiveFedora::Model.class_from_string("TermProxy", ActiveFedora::RdfNode) + # => ActiveFedora::RdfNode::TermProxy + def self.class_from_string(full_class_name, container_class = Kernel) + container_class = container_class.name if container_class.is_a? Module + container_parts = container_class.split('::') + (container_parts + full_class_name.split('::')).flatten.inject(Kernel) do |mod, class_name| + if mod == Kernel + Object.const_get(class_name) + elsif mod.const_defined? class_name.to_sym + mod.const_get(class_name) + else + container_parts.pop + class_from_string(class_name, container_parts.join('::')) + end + end + end + + attr_reader :class_names, :default + + def initialize(class_names, default: ActiveFedora::Base) + @class_names = Array(class_names) + @default = default + end + + ## + # Convert all the provided class names to class instances + def models + class_names.map do |uri| + classify(uri) + end.compact + end + + ## + # Select the "best" class from the list of class names. We define + # the "best" class as: + # - a subclass of the given default, base class + # - preferring subclasses over the parent class + def best_model(opts = {}) + best_model_match = opts.fetch(:default, default) + + models.each do |model_value| + # If there is an inheritance structure, use the most specific case. + best_model_match = model_value if best_model_match.nil? || best_model_match > model_value + end + + best_model_match + end + + private + + def classify(model_value) + unless class_exists?(model_value) + ActiveFedora::Base.logger.warn "'#{model_value}' is not a real class" if ActiveFedora::Base.logger + return nil + end + ActiveFedora::ModelClassifier.class_from_string(model_value) + end + + def class_exists?(class_name) + return false if class_name.empty? + klass = class_name.constantize + return klass.is_a?(Class) + rescue NameError + return false + end + end +end diff --git a/lib/active_fedora/query_result_builder.rb b/lib/active_fedora/query_result_builder.rb index 1e13796f3..1ee9f69d0 100644 --- a/lib/active_fedora/query_result_builder.rb +++ b/lib/active_fedora/query_result_builder.rb @@ -19,12 +19,12 @@ def self.reify_solr_result(hit, _opts = {}) # Returns all possible classes for the solr object def self.classes_from_solr_document(hit, _opts = {}) - ActiveFedora.model_mapper.from_solr_document(hit).models + ActiveFedora.model_mapper.classifier(hit).models end # Returns the best singular class for the solr object def self.class_from_solr_document(hit, opts = {}) - best_model_match = ActiveFedora.model_mapper.from_solr_document(hit).best_model(opts) + best_model_match = ActiveFedora.model_mapper.classifier(hit).best_model(opts) ActiveFedora::Base.logger.warn "Could not find a model for #{hit['id']}, defaulting to ActiveFedora::Base" if ActiveFedora::Base.logger && best_model_match == ActiveFedora::Base best_model_match end diff --git a/lib/active_fedora/relation/finder_methods.rb b/lib/active_fedora/relation/finder_methods.rb index a7b0df12f..30b594514 100644 --- a/lib/active_fedora/relation/finder_methods.rb +++ b/lib/active_fedora/relation/finder_methods.rb @@ -193,7 +193,6 @@ def class_to_load(resource, cast) if @klass == ActiveFedora::Base && cast == false ActiveFedora::Base else - # The true class may be a subclass of @klass, so always use from_class_uri resource_class = has_model_value(resource) unless equivalent_class?(resource_class) raise ActiveFedora::ActiveFedoraError, "Model mismatch. Expected #{@klass}. Got: #{resource_class}" @@ -203,7 +202,7 @@ def class_to_load(resource, cast) end def has_model_value(resource) - ActiveFedora.model_mapper.from_resource(resource).best_model + ActiveFedora.model_mapper.classifier(resource).best_model end def equivalent_class?(other_class) diff --git a/spec/unit/default_model_mapper_spec.rb b/spec/unit/default_model_mapper_spec.rb new file mode 100644 index 000000000..3fb05b608 --- /dev/null +++ b/spec/unit/default_model_mapper_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe ActiveFedora::DefaultModelMapper do + let(:classifier) { double } + let(:classifier_instance) { double } + let(:solr_field) { 'solr_field' } + let(:predicate) { 'info:predicate' } + subject { described_class.new classifier_class: classifier, solr_field: solr_field, predicate: predicate } + + describe '#classifier' do + context 'with a solr document' do + let(:solr_document) { { 'solr_field' => ['xyz'] } } + + before do + expect(classifier).to receive(:new).with(['xyz']).and_return(classifier_instance) + end + + it 'creates a classifier from the solr field data' do + expect(subject.classifier(solr_document)).to eq classifier_instance + end + end + + context 'with a resource' do + let(:graph) do + RDF::Graph.new << [:hello, predicate, 'xyz'] + end + + let(:resource) { double(graph: graph) } + + before do + expect(classifier).to receive(:new).with(['xyz']).and_return(classifier_instance) + end + + it 'creates a classifier from the resource model predicate' do + expect(subject.classifier(resource)).to eq classifier_instance + end + end + end +end diff --git a/spec/unit/model_classifier_spec.rb b/spec/unit/model_classifier_spec.rb new file mode 100644 index 000000000..67d97bfee --- /dev/null +++ b/spec/unit/model_classifier_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe ActiveFedora::ModelClassifier do + module ParentClass + class SiblingClass + end + class OtherSiblingClass + end + class SubclassClass < SiblingClass + end + end + + let(:class_names) { ["ParentClass::SiblingClass", "ParentClass::OtherSiblingClass", "ParentClass::SubclassClass", "ParentClass::NoSuchClass"] } + subject { described_class.new class_names } + + describe ".class_from_string" do + it "returns class constants based on strings" do + expect(described_class.class_from_string("Om")).to eq Om + expect(described_class.class_from_string("ActiveFedora::RDF::IndexingService")).to eq ActiveFedora::RDF::IndexingService + expect(described_class.class_from_string("IndexingService", ActiveFedora::RDF)).to eq ActiveFedora::RDF::IndexingService + end + + it "finds sibling classes" do + expect(described_class.class_from_string("SiblingClass", ParentClass::OtherSiblingClass)).to eq ParentClass::SiblingClass + end + + it "raises a NameError if the class isn't found" do + expect { + described_class.class_from_string("FooClass", ParentClass::OtherSiblingClass) + }.to raise_error NameError, /uninitialized constant (Object::)?FooClass/ + end + end + + describe '#models' do + it 'converts class names to classes' do + expect(subject.models).to match_array [ParentClass::SiblingClass, ParentClass::OtherSiblingClass, ParentClass::SubclassClass] + end + end + + describe '#best_model' do + it 'selects the most specific matching model' do + expect(subject.best_model(default: nil)).to eq ParentClass::SubclassClass + end + + it 'filters models to subclasses of the default' do + expect(subject.best_model(default: ActiveFedora::Base)).to eq ActiveFedora::Base + end + end +end diff --git a/spec/unit/model_spec.rb b/spec/unit/model_spec.rb index 9995b501e..6ab860649 100644 --- a/spec/unit/model_spec.rb +++ b/spec/unit/model_spec.rb @@ -27,39 +27,4 @@ class Basic < ActiveFedora::Base it { should eq 'search' } end end - - describe ".from_class_uri" do - subject { described_class.from_class_uri(uri) } - context "a blank string" do - before { expect(ActiveFedora::Base.logger).to receive(:warn) } - let(:uri) { '' } - it { should be_nil } - end - end - - describe ".class_from_string" do - before do - module ParentClass - class SiblingClass - end - class OtherSiblingClass - end - end - end - it "returns class constants based on strings" do - expect(described_class.class_from_string("Om")).to eq Om - expect(described_class.class_from_string("ActiveFedora::RDF::IndexingService")).to eq ActiveFedora::RDF::IndexingService - expect(described_class.class_from_string("IndexingService", ActiveFedora::RDF)).to eq ActiveFedora::RDF::IndexingService - end - - it "finds sibling classes" do - expect(described_class.class_from_string("SiblingClass", ParentClass::OtherSiblingClass)).to eq ParentClass::SiblingClass - end - - it "raises a NameError if the class isn't found" do - expect { - described_class.class_from_string("FooClass", ParentClass::OtherSiblingClass) - }.to raise_error NameError, /uninitialized constant (Object::)?FooClass/ - end - end end diff --git a/spec/unit/query_result_builder_spec.rb b/spec/unit/query_result_builder_spec.rb index 7b4068db3..550d68a08 100644 --- a/spec/unit/query_result_builder_spec.rb +++ b/spec/unit/query_result_builder_spec.rb @@ -3,7 +3,7 @@ describe ActiveFedora::QueryResultBuilder do describe "reify solr results" do before(:all) do - class AudioRecord + class AudioRecord < ActiveFedora::Base attr_accessor :id def self.connection_for_id(_id) end