dry-schema icon indicating copy to clipboard operation
dry-schema copied to clipboard

Reusing schema definition and validating Hash

Open an0ther-koni opened this issue 5 years ago • 3 comments

Describe the bug

I'm having some schemas defined used in a contract for validating the HTTP params when creating an App, which can contain also an image as attachment (using nested attributes).

# frozen_string_literal: true

module Apps
  module Schemas
    class Base < Dry::Schema::Params
      define do
        required(:name).filled(:string, min_size?: 3)
        optional(:description).maybe(:string)
        optional(:public).filled(:bool)
        optional(:status).filled(:string, included_in?: %w[draft in_review approved published])
        optional(:contact_email).maybe(:string)

        optional(:image_attributes).maybe(:hash) do
          ::Attachments::Schemas::Base.new
        end
      end
    end
  end
end

Since the Attachments can be reused in other models as well, it was extracted into an own schema.

# frozen_string_literal: true

module Attachments
  module Schemas
    class Base < Dry::Schema::Params
      define do
        optional(:id).maybe(:integer)
        optional(:type).maybe(:string)
        optional(:file).maybe(:string)
        optional(:_destroy).maybe(:bool)
      end
    end
  end
end

And this would be the App contract.

# frozen_string_literal: true

module Apps
  module Contracts
    class Base < Dry::Validation::Contract
      params(::Apps::Schemas::Base.new)
    end
  end
end

The problem I'm seeing is that the contract will not extract the image_attributes from the params if I validate the image_attributes to be a hash.

To Reproduce

Set the params to be:

{
  "id": "some_id",
  "name": "test",
  "description": "test",
  "public": false,
  "status": "draft",
  "contact_email": nil,
  "image_attributes":
  {
    "file": "{some payload}}",
    "url": "https://some_url",
    "type": "ImageAttachment"
  }
}

and check the contract:

contract = ::Apps::Contracts::Base.new.call(params)

the result will not have image_attributes set:

 #<Dry::Validation::Result{:name=>"test", :description=>"test", :public=>false, :status=>"draft", :contact_email=>nil, :image_attributes=>{}} errors={}>

Expected behavior

To extract the image_attributes from the params.

IMPORTANT:

I noticed that validating the image_attributes to be a hash works also fine when the attachment attributes are directly defined in the schema (cannot reuse the Attachment::Schema::Base):

optional(:image_attributes).maybe(:hash) do
  optional(:id).maybe(:integer)
  optional(:type).maybe(:string)
  optional(:file).maybe(:string)
  optional(:_destroy).maybe(:bool)
end

Your environment

  • Ruby version: 2.6.6
  • OS: Linux
  • dry-validation: 1.5
  • dry-schema: 1.5

an0ther-koni avatar May 29 '20 16:05 an0ther-koni

🤔 Why do you think this is a bug? Is there any example of code working like this:

        optional(:image_attributes).maybe(:hash) do
          ::Attachments::Schemas::Base.new
        end

?

flash-gordon avatar May 29 '20 22:05 flash-gordon

@flash-gordon true! There were also couple of iterations on our side (from .value(:hash) , .maybe(:hash), ..), so somewhere along the way we deviated a bit.

As the following works just fine, we can close the bug report.

optional(:images_attributes).hash(::Attachments::Schemas::Base.new)

This would be the equivalent of optional(:images_attributes).filled(:hash), without validating the nested data right? If this is true, think it would be still good to have the possibility of a .maybe, where someone can still validated the nested data.

an0ther-koni avatar May 31 '20 05:05 an0ther-koni

@flash-gordon @landongrindheim I'm pretty sure there are no examples like that in the documentation but I think the actual problem here is that this code should explode. IIRC this should be possible to handle now, I'll look into this.

solnic avatar Jun 01 '20 08:06 solnic