grape icon indicating copy to clipboard operation
grape copied to clipboard

Unable to use File Parameters with Grape::Extensions::Hash::ParamBuilder

Open Senchatay opened this issue 10 months ago • 5 comments

params do
  build_with Grape::Extensions::Hash::ParamBuilder
  requires :avatar, type: File
end
post '/' do
  ...
end

Returns an error #<Grape::Exceptions::ValidationErrors: avatar is invalid> because:

  • This line calls deep_dup of hash values ( including tempfile) https://github.com/ruby-grape/grape/blob/0f57e01dc79df9903949c3a1cf42ffc73dabb670/lib/grape/extensions/hash.rb#L14
  • Tempfile#deep_dup returns instance of File
  • https://github.com/ruby-grape/grape/blob/0f57e01dc79df9903949c3a1cf42ffc73dabb670/lib/grape/validations/types/file.rb#L25

Senchatay avatar Feb 27 '25 14:02 Senchatay

@Senchatay The following test passes. Anything special on your side ?

describe 'build with File' do
  subject { last_response }

  let(:app) do
    Class.new(described_class) do
      params do
        build_with :hash
        requires :avatar, type: File
      end
      post '/' do
        return_no_content
      end
    end
  end

  before do
    post '/', { avatar: Rack::Test::UploadedFile.new(__FILE__, 'text/plain') }
  end

  it { is_expected.to be_no_content }
end

ericproulx avatar Mar 23 '25 16:03 ericproulx

@ericproulx Hi. I think it will reproduce only with Grape::Extensions::Hash::ParamBuilder because of deep_dup call.

After you deprecate this one (https://github.com/ruby-grape/grape/pull/2540), this issue is no longer relevant for master branch

Senchatay avatar Mar 24 '25 08:03 Senchatay

We are facing the same problem after upgrading from grape 1.7.1 to 1.8.0

Switching from Grape::Extensions::Hash::ParamBuilder to Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder solves this. It's not great as this would require us to change quite a few test expectations and we'd prefer to use plain ruby Hashes over HashWithIndifferentAccess.

Is there any chance that there will be a 1.8.1 release fixing this?

If I understand correctly from the linked PR above, the upcoming 2.4.0 release would fix this? Then we could try to make the jump from 1.7.1 to 2.4.0, which might be a big one though...

dsager avatar Jun 17 '25 15:06 dsager

@dsager I think in your case the best option would be to define your own builder and use it regardless of the Grape version.

module ParamBuilder
  extend ::ActiveSupport::Concern

  included do
    namespace_inheritable(:build_params_with, ParamBuilder)
  end

  def build_params
    rack_params.deep_symbolize_keys.tap do |params|
      params.deep_merge!(grape_routing_args.deep_symbolize_keys!) if env.key?(Grape::Env::GRAPE_ROUTING_ARGS)
    end
  end
end

Senchatay avatar Jun 19 '25 13:06 Senchatay

Thank you @Senchatay , that's indeed a good option. I'll give it a spin!

dsager avatar Jun 19 '25 14:06 dsager