Adding Type and Nullability Validation to exposed fields

[dhruvCW]

Hello 👋

I am creating this ticket to ask if it would be possible to introduce type validation to exposed fields.

Currently there is not checking if the exposed fields are of a specified type. We can add documentation to specify a type (that is then exposed as part of the swagger docs) but if there is a mismatch there is no way for the server to validate this break in contract.

I am thinking we can add type validation (using dry-types or dry-validation) to the exposed fields (maybe after formatting). This would of course be opt-in with the expose DSL method accepting two new options

type (this is the type the field should be or be coercible to) a dry type or standard ruby type like Numeric.
nullable a boolean field to validate if the value can be nil or not.
These options then can be used to populate the swagger documentation automatically without specifying them in the documents hash.

I am happy to start contribute this feature but first want to check if the maintainers @dblock & @LeFnord are okay with it 😅

[dblock]

Does params: API::Entities::Status.documentation not expose those parameters with validation (Grape has extensive support for that)?

And yes, validation would be definitely very useful if the above doesn’t work. When used with grape I’d want to rely on its type checking mechanisms for consistency.

[LeFnord]

mmh, grape-entity is a presenting layer, so the data should be valid before it will be presented,
but yeah, if want to have a validation in place it should be based on grape validations

[dblock]

@LeFnord i think the idea is that you can use entities to receive data in the same format as presented

[dhruvCW]

mmh, grape-entity is a presenting layer, so the data should be valid before it will be presented

While I tend to agree. In the case of grape-entity the entity is effectively a presentation as well as DTO entity. Thus I think having basic validation around the coerced type as well as nullability would be useful.

[dhruvCW]

Does params: API::Entities::Status.documentation not expose those parameters with validation (Grape has extensive support for that)?
And yes, validation would be definitely very useful if the above doesn’t work. When used with grape I’d want to rely on its type checking mechanisms for consistency.

Ah I see there is some confusion. I agree the parameters can indeed be modeled with grape-entity, but this is about validating the data that is produced by an API rather than what is received. This is particularly useful in situations where the data source is not typed (from another upstream API or redis, etc).

[dhruvCW]

When used with grape I’d want to rely on its type checking mechanisms for consistency.

could you elaborate how this would work 🤔

[bogdan]

I came up with this in one day:

# frozen_string_literal: true

require 'test_helper'

class ApplicationEntityTestCase < ActiveSupport::TestCase

  ACCEPTIBLE_CLASSES_MAP = {
    "DateTime" => [Time, DateTime, ActiveSupport::TimeWithZone, String],
    "Boolean" => [TrueClass, FalseClass],
    "Float" => [Float, BigDecimal],
    'object' => [Hash, ActiveSupport::HashWithIndifferentAccess],
  }

  protected

  def assert_entity_types(entity_class, object)
    entity = entity_class.represent(object)
    entity_class.root_exposures.each do |exposure|
      next unless exposure.is_a?(Grape::Entity::Exposure::DelegatorExposure) ||
        exposure.is_a?(Grape::Entity::Exposure::BlockExposure)

      doc = exposure.documentation || {}
      type = doc[:type]&.to_s
      assert !type.nil?, "Expected #{entity_class}##{exposure.key} to be specified"

      is_array = doc[:is_array] || false
      nullable = doc.fetch(:x, {})[:nullable] || false


      classes = ACCEPTIBLE_CLASSES_MAP.fetch(type, [type]).map(&:to_s)
      value = exposure.value(entity, {})

      expectation = "Expected #{entity_class}##{exposure.key} #{value.inspect}"

      unless nullable
        assert !value.nil?, "#{expectation} to not be nil"
      else
        next if value.nil?
      end
      actual = value.class.to_s

      if is_array
        assert(
          value.is_a?(Enumerable),
        "#{expectation} to be instance of Enumerable, but was #{actual}.",
        )
        value.each do |v|
          assert(
            classes.include?(v.class.to_s),
            "#{expectation} to be Array[#{classes.join("|")}], but included #{v.class.to_s}.",
          )
        end
      else
        assert(
          classes.include?(actual),
          "#{expectation} to be instance of #{classes.join(" or ")}, but was #{actual}.",
        )
      end
    end
  end
end

Really helped me to eliminate a few inconsistencies in documented types.