Improve base class IdentityValidator based on code review feedback

lib/identity_validations/identity_validator.rb

@@ -3,7 +3,7 @@ class IdentityValidator < ActiveModel::Validator
     attr_writer :attribute
 
     def validate(record)
-      raise NotImplementedError, "IdentityValidator#validator is an abstract method and needs to be overridden"
+      raise NotImplementedError, "IdentityValidator#validate is an abstract method that needs to be overridden"
     end
 
     def attribute
@@ -13,7 +13,9 @@ def attribute
     # I was tempted to make `record` an instance variable with an accessor, but that could
     # cause problems with how Rails instantiates and reuses validator instances
     def get_attribute(record)
-      return unless attribute.present?
+      if attribute.blank?
+        raise ArgumentError, "IdentityValidator: can't validate #{record.class}, no attribute specified"
+      end
       if !record.respond_to?(attribute.to_sym)
         raise ArgumentError, "IdentityValidator: attribute '#{attribute}' not found in class #{record.class}"
       end

spec/identity_validations/identity_validator_spec.rb

@@ -10,8 +10,8 @@ class TestModel
   let(:model) { TestModel.new }
 
   it 'throws an error due to being abstract' do
-    expect {model.validates_with IdentityValidations::IdentityValidator}.
-      to raise_error(NotImplementedError, 'IdentityValidator#validator is an abstract method and needs to be overridden')
+    expect { model.validates_with IdentityValidations::IdentityValidator }.
+      to raise_error(NotImplementedError, 'IdentityValidator#validate is an abstract method that needs to be overridden')
   end
 
   describe 'a simple inheriting validator' do
@@ -21,17 +21,39 @@ def validate(record)
       end
     end
 
-    it 'throws an error when an invalid attribute is specified' do
-      model.an_attribute = "something"
-      expect {model.validates_with SimpleValidator, attribute: :invalid_attribute}.
-        to raise_error(ArgumentError, "IdentityValidator: attribute 'invalid_attribute' not found in class TestModel")
+    describe "invoked via ActiveModel's #validates_with" do
+      it 'throws an error when an invalid attribute is specified' do
+        model.an_attribute = "something"
+        expect {model.validates_with SimpleValidator, attribute: :invalid_attribute}.
+          to raise_error(ArgumentError, "IdentityValidator: attribute 'invalid_attribute' not found in class TestModel")
+      end
+
+      it 'will validate when passed an attribute' do
+        model.an_attribute = "something"
+        model.validates_with SimpleValidator, attribute: :an_attribute
+        expect(model.errors).to be_blank
+        expect(model.errors.messages).to be_blank
+      end
+
+      it 'throws an error when not given an attribute' do
+        model.an_attribute = "something"
+        expect { model.validates_with SimpleValidator }.
+          to raise_error(ArgumentError, "IdentityValidator: can't validate TestModel, no attribute specified")
+      end
     end
 
-    it 'will validate when passed an attribute' do
-      model.an_attribute = "something"
-      model.validates_with SimpleValidator, attribute: :an_attribute
-      expect(model.errors).to be_blank
-      expect(model.errors.messages).to be_blank
+    describe "#validate" do
+      it 'throws an error when not given an attribute' do
+        model.an_attribute = "something"
+
+        # In theory, we can call SimpleValidator.new with arguments to pass in an option.
+        # However, this is an ability inherited from the Rails parent class.
+        # This inherited option argument is more likely to be changed by Rails than the `validates_with`
+        # option argument, so ideally our usage and tests shouldn't rely on that Rails implementation detail
+
+        expect {SimpleValidator.new.validate(model)}.
+          to raise_error(ArgumentError, "IdentityValidator: can't validate TestModel, no attribute specified")
+      end
     end
   end
 end