Merge pull request #6621 from avalonmediasystem/faster_unique

Use a solr query tailored for speed instead of Model.where.first when validating uniqueness

Author: cjcolvar

app/validators/uniqueness_validator.rb

@@ -22,13 +22,12 @@ def initialize(options)
   end
   def validate_each(record, attribute, value)
     klass = record.class
-    # existing_doc = find_doc(klass, value)
-    existing_doc = find_doc(klass, record.to_solr[@solr_field])
-    if ! existing_doc.nil? && existing_doc.id != record.id
+    solr_value = record.to_solr[@solr_field]
+    query = "has_model_ssim:\"#{klass.name}\""
+    query += " AND #{@solr_field}:#{solr_value}" if solr_value.present? 
+    existing_doc_id = ActiveFedora::SolrService.query(query, fl: [:id], rows: 1).first&.dig('id')
+    if existing_doc_id.present? && existing_doc_id != record.id
       record.errors.add(attribute, :taken, value: value)
     end
   end
-  def find_doc(klass, value)
-    klass.where(@solr_field => value).first
-  end
 end

spec/validators/uniqueness_validator_spec.rb

@@ -19,61 +19,40 @@
   before(:all) do
     class Foo < ActiveFedora::Base
       property :title, predicate: ::RDF::Vocab::DC.title, multiple: false
+
+      def to_solr
+        super.tap do |solr_doc|
+          solr_doc["title_uniq_si"] = title.downcase.gsub(/\s+/,'') if title.present?
+        end
+      end
     end
   end
   after(:all) { Object.send(:remove_const, :Foo) }
 
-  let(:solr_field) {"title_uniq_si"}
-  let(:validator) {UniquenessValidator.new({:attributes => [:title], :solr_name => solr_field})}
-  let(:record) {Foo.new}
+  let(:field) { 'title' }
+  let(:solr_field) { "title_uniq_si" }
+  let(:validator) { UniquenessValidator.new({ :attributes => [field], :solr_name => solr_field }) }
+  let(:record) { Foo.new(title: title) }
+  let(:title) { 'new_title' }
 
   it "should raise an exception if solr_name option is missing" do
-     expect{UniquenessValidator.new({attributes: [:title]})}.to raise_error ArgumentError
+     expect { UniquenessValidator.new({ attributes: [:title] }) }.to raise_error ArgumentError
   end
 
   it "should not return errors when field is unique" do
-    allow(validator).to receive("find_doc").and_return(nil)
     expect(record).not_to receive('errors')
-    validator.validate_each(record, "title", "new_title")
+    validator.validate_each(record, field, record.attributes[field])
   end
 
   it "should not return errors when field is unique but record is the same" do
-    doc = double(id: record.id)
-    allow(validator).to receive("find_doc").and_return(doc)
+    record.save!
     expect(record.errors).not_to receive('add')
-    validator.validate_each(record, "title", "new_title")
+    validator.validate_each(record, field, record.attributes[field])
   end
 
   it "should return errors when field is not unique" do
-    doc = double(id: 'different-id')
-    allow(validator).to receive("find_doc").and_return(doc)
-    # expect(record.errors).to receive('add')
-    validator.validate_each(record, "title", "old_title")
+    Foo.create(id: 'different-id', title: 'new_title')
+    validator.validate_each(record, field, record.attributes[field])
     expect(record.errors).to_not be_empty
   end
-
-  describe "#find_doc" do
-    let (:klass) {record.class}
-    let (:value) {"old_title"}
-
-    it "should use the solr field name and supplied values" do
-      relation = double()
-      allow(relation).to receive("first")
-      expect(klass).to receive(:where).once.with(solr_field => value).and_return(relation)
-      validator.find_doc(klass, value)
-    end
-    it "should return one record when present" do
-      doc = Foo.new
-      relation = double()
-      allow(relation).to receive("first").and_return(doc)
-      allow(klass).to receive("where").and_return(relation)
-      expect(validator.find_doc(klass, value)).to be_an_instance_of klass
-    end
-    it "should return nil when not present" do
-      relation = double()
-      allow(relation).to receive("first").and_return(nil)
-      allow(klass).to receive("where").and_return(relation)
-      expect(validator.find_doc(klass, value)).to be_nil
-    end
-  end
 end