coerce_*_params modifies the parameter types passed to the Controller

[makdad]

I am using Committee in a Rails 5 stack on an application, and I am documenting old API routes (and validating them).
One API accepts a date parameter as part of a form submission (content type application/x-www-form-urlencoded), that is defined in OAS3 like this schema:

    DateTime:
      type: string
      format: date-time
      example: '2020-04-21T17:54:31+09:00'
      description: 'A Ruby timestamp with timezone'

Suddenly, a test began to fail where the date was being sent as "2020-12-11" (to be fair, that would be format: date).
Committee is coercing the parameter to a DateTime object (due to coerce_form_params being true).

Imagine a Rails controller implementation like this (not exact, but you will get the point):

def create
  # This used to be a string (e.g. params[:date].class == String)
  date = params[:date]
  amount = params[:amount]
  TransactionService.add_new_transaction(date, amount)
  render :created
end

After implementing the OAS3 specification for this API route, the behavior is:

def create
  # Now, params[:date].class == DateTime
  date = params[:date]
  ...

My question/bug? is to ask, is modifying the request chain intended behavior for the Committee gem (as a direction for the tool)? My understanding (or, at least how we are using it) is for request/response validation but we want to assume that it will never directly alter the behavior of the API itself, which in this case, it did.

I can create a bug test case and attempt to fix this issue if the maintainers believe it to be a bug (as I do). I have not yet dug into where the bug is happening.

[ota42y]

It's not bug but you can avoid this problem (Please read the last part)

---

The committee have two features.

1. request/response validation
2. convert request parameter using schema (I will talk more about it next)

i.e.

OpenAPI 3 support integer type in query parameter and query parameters are always set as String by rack,
so we can't validation query parameter (always failed...)

The committee support coerce option to this problem
When the request parameter is string, we convert it using schema before validation.
So we can validate query parameter.

And the committee overwrite request parameter by converted value.

When we dose not overwrite the value, we'll get error.

def index
    page = params[:page] # type: integer in OpenAPI 3 schema
    posts = Post.page(page)
    # we'll get String can't be coerced into Integer error
    remaining_count = Post.count - (page * PAGE_NUM)
    render :index
end

The request are passed committee's validation so 'page' parameter should be Integer, but it is String class...
And when we send integer data by JSON to POST endpoint, we get it as Integer.

I think when the parameters pass validation it must be equal to the definition.
If there is difference between the definition and the validated parameter, we can't trust the definition and validation.
So the committee overwrite parameter by converted value to ensure that it is as defined.

I explain query parameter with integer type, but we are also converting 'format: date-time' for the same reason

---

But I know some people don't want this.
So we provide other options.

The committee overwrite parameter using 'params_key' option.
https://github.com/interagent/committee#use-ruby-on-rails-with-coerce-option

When we set committee.params to 'params_key', the rails doesn't use it so we can get string value.

Or, we can stop converting to 'Datetime' class using coerce_date_times=false

[makdad]

@ota42y Thank you for taking your time to explain how committee is intended to function, I appreciate the support. I believe I understand now, so I will close this issue for now.

[makdad]

@ota42y one question.

When we set committee.params to 'params_key', the rails doesn't use it so we can get string value.

Yes, but the default value of params_key is committee.params. This would imply that the default behavior is not to modify the Rails request, but that seems to be what is happening to me.

For example, this test:

committee/test/middleware/request_validation_open_api_3_test.rb

Line 37 in f3df347

assert_equal DateTime, env['rack.request.query_hash']["datetime_string"].class

Seems to confirm that coerce_date_time is overwriting not the value request.env["committee.params"]["datetime_string"], but actually request.query_hash, which seems to be what Rails is using for controller params?

I would expect that behavior if the params_key option was set to query_hash, etc.

Do you have any comment on that?

[ota42y]

oh... sorry , it's bug, we overwrite query_hash by default 🙇
We don't overwrite request body and header, so I'll fix this bug.

[makdad]

@ota42y This bug is preventing us from moving forward with some refactoring -- so I am happy to help if you can point me to the right direction on where, or how, you would fix this bug.

[ota42y]

I'm sorry, the fix is ​​complete this PR(#310) .

[makdad]

@ota42y wow, thank you so much. Again, please let me know if there is any new feature or anything I can help with. We are using committee a lot so I want to help with its development 👍

[ota42y]

Thank you very much for your help!!!

[EllaBellaStella]

Hi I am a little bit confused about the issue still being open despite the work done above

[makdad]

@StochasticSpring you're correct, from my perspective, we don't have this problem anymore, so I am happy to close it!