refactor the attribute handlers out of the attribute type, and into a…

…ttribute #1648

it was mostly being delegated, so simplifies things and makes the now necessary link to attribute when validating cleaner.

app/models/sample.rb

@@ -77,7 +77,7 @@ def related_sample_ids
   def referenced_resources
     sample_type.sample_attributes.select(&:seek_resource?).map do |sa|
       value = get_attribute_value(sa)
-      type = sa.sample_attribute_type.base_type_handler(sa).type
+      type = sa.base_type_handler.type
       return [] unless type
       Array.wrap(value).map { |v| type.find_by_id(v['id']) if v }
     end.flatten.compact

app/models/sample_attribute_type.rb

@@ -30,14 +30,6 @@ def default?
     self == self.class.default
   end
 
-  def test_blank?(value)
-    base_type_handler({}).test_blank?(value)
-  end
-
-  def validate_value?(value, attribute = nil)
-    check_value_against_base_type(value, attribute) && check_value_against_regular_expression(value)
-  end
-
   def as_json(_options = nil)
     { title: title, base_type: base_type, regexp: regexp }
   end
@@ -56,23 +48,10 @@ def regular_expression
     /#{regexp}/m
   end
 
-  def check_value_against_regular_expression(value)
-    match = regular_expression.match(value.to_s)
-    match && (match.to_s == value.to_s)
-  end
-
   def validate_resolution
     !resolution.present? || (resolution.include? '\\')
   end
 
-  def check_value_against_base_type(value, attribute)
-    base_type_handler(attribute).validate_value?(value)
-  end
-
-  def pre_process_value(value, attribute)
-    base_type_handler(attribute).convert(value)
-  end
-
   def controlled_vocab?
     [Seek::Samples::BaseType::CV, Seek::Samples::BaseType::CV_LIST].include?(base_type)
   end
@@ -81,10 +60,6 @@ def seek_cv_list?
     base_type == Seek::Samples::BaseType::CV_LIST
   end
 
-  def seek_resource?
-    base_type_handler(nil).is_a?(Seek::Samples::AttributeTypeHandlers::SeekResourceAttributeTypeHandler)
-  end
-
   def linked_extended_metadata?
     base_type == Seek::Samples::BaseType::LINKED_EXTENDED_METADATA
   end
@@ -109,7 +84,5 @@ def seek_data_file?
     base_type == Seek::Samples::BaseType::SEEK_DATA_FILE
   end
 
-  def base_type_handler(attribute)
-    Seek::Samples::AttributeTypeHandlers::AttributeTypeHandlerFactory.instance.for_base_type(base_type, attribute)
-  end
+
 end

lib/seek/json_metadata/attribute.rb

@@ -16,20 +16,23 @@ module Attribute
         # validates that the attribute type is SeekSample if linked_sample_type is set, and vice-versa
         validate :linked_sample_type_and_attribute_type_consistency
 
-        delegate :controlled_vocab?, :seek_cv_list?, :seek_sample?, :seek_sample_multi?, :seek_strain?, :seek_resource?, :linked_extended_metadata?,:linked_extended_metadata_multi?, to: :sample_attribute_type, allow_nil: true
+        delegate :controlled_vocab?, :seek_cv_list?, :seek_sample?, :seek_sample_multi?, :seek_strain?, :linked_extended_metadata?,:linked_extended_metadata_multi?, to: :sample_attribute_type, allow_nil: true
       end
 
       # checks whether the value is blank against the attribute type and base type
       def test_blank?(value)
-        sample_attribute_type.test_blank?(value)
+        base_type_handler.test_blank?(value)
       end
 
       def validate_value?(value)
         return false if required? && test_blank?(value)
         return true if test_blank?(value) && !required?
 
-        sample_attribute_type.validate_value?(value, self)
+        check_value_against_base_type(value) && check_value_against_regular_expression(value)
+      end
 
+      def pre_process_value(value)
+        base_type_handler.convert(value)
       end
 
       def accessor_name
@@ -43,8 +46,12 @@ def resolve(value)
         resolution
       end
 
-      def pre_process_value(value)
-        sample_attribute_type.pre_process_value(value, self)
+      def seek_resource?
+        base_type_handler.is_a?(Seek::Samples::AttributeTypeHandlers::SeekResourceAttributeTypeHandler)
+      end
+
+      def base_type_handler
+        Seek::Samples::AttributeTypeHandlers::AttributeTypeHandlerFactory.instance.for_base_type(sample_attribute_type.base_type, self)
       end
 
       private
@@ -77,6 +84,16 @@ def linked_sample_type_and_attribute_type_consistency
           errors.add(:seek_sample_multi, 'Linked Sample Type must be set if attribute type is Registered Sample (multiple)')
         end
       end
+
+      def check_value_against_regular_expression(value)
+        match = sample_attribute_type.regular_expression.match(value.to_s)
+        match && (match.to_s == value.to_s)
+      end
+
+      def check_value_against_base_type(value)
+        base_type_handler.validate_value?(value)
+      end
+
     end
   end
 end

lib/seek/samples/attribute_type_handlers/base_attribute_handler.rb

@@ -4,7 +4,7 @@ module AttributeTypeHandlers
       class AttributeHandlerException < RuntimeError; end
 
       class BaseAttributeHandler
-        def initialize(attribute=nil)
+        def initialize(attribute)
           @attribute = attribute
         end
 

test/unit/sample_attribute_test.rb

@@ -353,6 +353,166 @@ class SampleAttributeTest < ActiveSupport::TestCase
     assert attribute.ontology_based?
   end
 
+  test 'boolean' do
+    bool_type = SampleAttributeType.new title: 'bool', base_type: Seek::Samples::BaseType::BOOLEAN
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: bool_type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?(true)
+    assert attribute.validate_value?(false)
+    refute attribute.validate_value?('fish')
+  end
+
+  test 'chebi atribute' do
+    type = SampleAttributeType.new title: 'CHEBI ID', regexp: 'CHEBI:[0-9]+', base_type: Seek::Samples::BaseType::STRING
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?('CHEBI:1111')
+    assert attribute.validate_value?('CHEBI:1121')
+    refute attribute.validate_value?('fish')
+    refute attribute.validate_value?('fish:22')
+    refute attribute.validate_value?('CHEBI:1121a')
+    refute attribute.validate_value?('chebi:222')
+  end
+
+  test 'regular expression match' do
+    # whole string must match
+    type = SampleAttributeType.new(title: 'first name', base_type: Seek::Samples::BaseType::STRING, regexp: '[A-Z][a-z]+')
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?('Fred')
+    refute attribute.validate_value?(' Fred')
+    refute attribute.validate_value?('FRed')
+    refute attribute.validate_value?('Fred2')
+    refute attribute.validate_value?('Fred ')
+  end
+
+  test 'uri attribute type' do
+    type = SampleAttributeType.new title: 'URI', base_type: Seek::Samples::BaseType::STRING, regexp: URI.regexp.to_s
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?('zzz:222')
+    assert attribute.validate_value?('http://ontology.org#term')
+    refute attribute.validate_value?('fish')
+    refute attribute.validate_value?('fish;cow')
+  end
+
+  test 'validate text with newlines' do
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::TEXT)
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?('fish\\n\\rsoup')
+    assert attribute.validate_value?('fish\n\rsoup')
+    assert attribute.validate_value?('fish\r\nsoup')
+    assert attribute.validate_value?('   fish\n\rsoup ')
+    str = %(with
+  a
+  new
+  line%)
+    assert attribute.validate_value?(str)
+  end
+
+  test 'validate_value' do
+    type = SampleAttributeType.new(title: 'x-type', base_type: Seek::Samples::BaseType::STRING, regexp: 'xxx')
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert attribute.validate_value?('xxx')
+    refute attribute.validate_value?('fish')
+    refute attribute.validate_value?(nil)
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::INTEGER)
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?(1)
+    assert attribute.validate_value?('1')
+    assert attribute.validate_value?('01')
+    refute attribute.validate_value?('frog')
+    refute attribute.validate_value?('1.1')
+    refute attribute.validate_value?(1.1)
+    refute attribute.validate_value?(nil)
+    refute attribute.validate_value?('')
+
+    # contriversial, but after much argument decided to allow these values as integers
+    assert attribute.validate_value?(1.0)
+    assert attribute.validate_value?('1.0')
+    assert attribute.validate_value?(1.00)
+    assert attribute.validate_value?('1.00')
+    assert attribute.validate_value?(1.000)
+    assert attribute.validate_value?('1.000')
+    assert attribute.validate_value?(2.0)
+    assert attribute.validate_value?('2.0')
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::STRING, regexp: '.*yyy')
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?('yyy')
+    assert attribute.validate_value?('happpp - yyy')
+    refute attribute.validate_value?('')
+    refute attribute.validate_value?(nil)
+    refute attribute.validate_value?(1)
+    refute attribute.validate_value?('xxx')
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::TEXT, regexp: '.*yyy')
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?('yyy')
+    assert attribute.validate_value?('happpp - yyy')
+
+    refute attribute.validate_value?('')
+    refute attribute.validate_value?(nil)
+    refute attribute.validate_value?(1)
+    refute attribute.validate_value?('xxx')
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::FLOAT)
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?(1.0)
+    assert attribute.validate_value?(1.2)
+    assert attribute.validate_value?(0.78)
+    assert attribute.validate_value?('0.78')
+    assert attribute.validate_value?(12.70)
+    assert attribute.validate_value?('12.70')
+    refute attribute.validate_value?('fish')
+    refute attribute.validate_value?('2 Feb 2015')
+    refute attribute.validate_value?(nil)
+
+    assert attribute.validate_value?(1.0)
+    assert attribute.validate_value?(1)
+    assert attribute.validate_value?('1.0')
+    assert attribute.validate_value?('012')
+    assert attribute.validate_value?('012.3')
+    assert attribute.validate_value?('12.30')
+    assert attribute.validate_value?('1')
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::DATE_TIME)
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?('2 Feb 2015')
+    assert attribute.validate_value?('Thu, 11 Feb 2016 15:39:55 +0000')
+    assert attribute.validate_value?('2016-02-11T15:40:14+00:00')
+    assert attribute.validate_value?(DateTime.parse('2 Feb 2015'))
+    assert attribute.validate_value?(DateTime.now)
+    refute attribute.validate_value?(1)
+    refute attribute.validate_value?(1.2)
+    refute attribute.validate_value?(nil)
+    refute attribute.validate_value?('30 Feb 2015')
+
+    type = SampleAttributeType.new(title: 'fish', base_type: Seek::Samples::BaseType::DATE)
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?('2 Feb 2015')
+    assert attribute.validate_value?('Thu, 11 Feb 2016 15:39:55 +0000')
+    assert attribute.validate_value?('2016-02-11T15:40:14+00:00')
+    assert attribute.validate_value?(Date.parse('2 Feb 2015'))
+    assert attribute.validate_value?(Date.today)
+    refute attribute.validate_value?(1)
+    refute attribute.validate_value?(1.2)
+    refute attribute.validate_value?(nil)
+    refute attribute.validate_value?('30 Feb 2015')
+  end
+
+  test 'web and email regexp' do
+    type = SampleAttributeType.new title: 'Email address', base_type: Seek::Samples::BaseType::STRING, regexp: RFC822::EMAIL.to_s
+    attribute = FactoryBot.create(:sample_attribute, is_title: true, sample_attribute_type: type, sample_type: FactoryBot.create(:simple_sample_type))
+    assert_equal RFC822::EMAIL.to_s, type.regexp
+
+    assert attribute.validate_value?('[email protected]')
+    refute attribute.validate_value?('moonbeam')
+
+    type = SampleAttributeType.new title: 'Web link', base_type: Seek::Samples::BaseType::STRING, regexp: URI.regexp(%w(http https)).to_s
+    attribute.sample_attribute_type = type
+    assert attribute.validate_value?('http://google.com')
+    assert attribute.validate_value?('https://google.com')
+    refute attribute.validate_value?('moonbeam')
+  end
+
   private
 
   def valid_value?(attribute, value)