Fix attribute translator setter

[Jell]

This was simply not working before. Doesn't seem like there are any unit tests for this class, so I haven't added new ones, but might be worth considering.

[dblock]

Thanks, and yes, please add a test / CHANGELOG.

[grape-bot]

	1 Warning
⚠️	There're library changes, but not tests. That's OK as long as you're refactoring existing code.

Generated by 🚫 Danger

[dblock]

Not sure if failed tests are related, but I suspect they are.

[ericproulx]

ends_with? on symbol comes in 2.7 but we're still >= 2.6.
https://blog.saeloun.com/2019/12/09/ruby-2-7-added-symbol-start_with-and-symbol-end_with-method
@dblock I think we should consider deprecating 2.6. After all, rubocop dropped it and rubygems-update also. Might be a good time.

[dblock]

I am not against it.

[ericproulx]

@Jell if you rebase your branch, all tests should pass.

[Jell]

@ericproulx @dblock I've rebased, added tests and a line in the changelog

[ericproulx]

@dblock I thought I messed up my refactoring in 2019 but it always been like this since #1351. What @Jell did makes total sense but it changes the setter behavior. For instance,

describe "#header=" do
  subject { instance.header = 'new_value' }
  let(:instance) { Grape::Router::AttributeTranslator.new(header: 'value') }
  it "sets value for header" do
    expect do
      subject
      puts "to_h = #{instance.to_h}"
    end.to change(instance, :header).from('value').to('new_value')
  end
end

Grape::Router::AttributeTranslator
  #header=
to_h = {:header=>"value", "header="=>["new_value"]}
    sets value for header (FAILED - 1)

Failures:

  1) Grape::Router::AttributeTranslator#header= sets value for header
     Failure/Error:
       expect do
         subject
         puts "to_h = #{instance.to_h}"
       end.to change(instance, :header).from('value').to('new_value')
     
       expected `Grape::Router::AttributeTranslator#header` to have changed from "value" to "new_value", but did not change
     # ./spec/grape/router/attribute_translator_spec.rb:32:in `block (3 levels) in <top (required)>'

Finished in 0.0123 seconds (files took 0.28833 seconds to load)
1 example, 1 failure

On the other hand, I feel the setter methods aren't really used and everything is done through the initialize. For instance, removing the setter part in the method_missing and respond_to_missing? like this doesn't break any specs.

def method_missing(method_name, *args)
  return attributes[method_name] unless setter?(method_name[-1])

  super
end

def respond_to_missing?(method_name, _include_private = false)
  @attributes.key?(method_name)
end

Maybe we should consider removing the setter part and let the initialize do its work. After all, these attributes doesn't change after the APIs are compiled.

[Jell]

@ericproulx FYI the reason for me doing the PR in the first place is because I wanted to set some default headers for Swagger documentation purposes after the route was compiled, something along the lines of:

routes.each do |route|
  add_default_headers(route)
  add_default_response_codes(route)
end

and I had to do some workaround like:

  route.attributes.attributes[:headers] ||= {}
  route.attributes.attributes[:http_codes] ||= []

But I think it would also be OK if headers and http_codes where initialized with an empty collection?

[ericproulx]

@dblock #1348 your ruby bug has been fixed 3 years ago. Its been fix in ruby > =3.0 and the issue remains in 2.7 unfortunately.

[dblock]

Good work @Jell!