grape icon indicating copy to clipboard operation
grape copied to clipboard

Param with multiple acceptable Hash Types

Open mia-n opened this issue 2 years ago • 4 comments

How would I define that a given param can have multiple possible hash values?

As a very simplified example, say that either of these hashes are valid params:

"value": {
    "time_unit": "hour",
    "rate": 100.0
}
"value": {
    "fixed_price": 100.0,
}

but that value can take only one or the other of those forms. It's also possible I could add more acceptable formats in the future, but that there will always be a finite number of acceptable formats to the "value" key.

I attempted something like this but it does not seem to be possible:

params do
  requires :value, desc: "TODO", types: [
    Hash {
      requires :fixed_price, type: Float
    },
    Hash {
      requires :time_unit, type: String
      requires :rate, type: Float
    },
  ]
end

I can sort of achieve what I want right now by delcaring its type as hash, declaring all of the possible fields as optional, and setting some complicated rules with mutually_exclusive and given, but this is messy and it is difficult to parse at a glance what the expected possible schemas are.

Is there a better way to achieve this?

mia-n avatar Dec 11 '23 20:12 mia-n

I can't believe we don't support this today! I think it's a new feature request.

One other idea of how we could express this is by allowing duplicate keys, but it's probably too weird.

requires :value, type: Hash do
  requires :fixed_price, type: Float
end
requires :value, type: Hash do
  requires :time_unit, type: String
  requires :rate, type: Float
end

Another could use :as.

requires :value_with_fixed_price, as: :value, type: Hash do
  requires :fixed_price, type: Float
end
requires :value_with_time_and_rate, as: :value, type: Hash do
  requires :time_unit, type: String
  requires :rate, type: Float
end

I think extending types as you suggest is the way to go.

dblock avatar Dec 12 '23 01:12 dblock

@dblock Thanks for the fast response!

Using :as I don't think will work as the whole point is that it is expected as :value on the request contract regardless. I ran a quick test with:

optional :value_fixed, as: :value, type: Hash do
  requires :fixed_price, type: Float
end
optional :value_labor_rate, as: :value, type: Hash do
  requires :rate, type: Float
  requires :time_unit, type: String
end
exactly_one_of :value_fixed, :value_labor_rate

and tried sending a request with

"value": {
        "time_unit": "hour",
        "rate": 10000
    }

but it just gives "error": "value_fixed, value_labor_rate are missing, exactly one parameter must be provided so that won't do.

For now I'll just deal with how it is but I'm happy this can be a feature request.

mia-n avatar Dec 12 '23 19:12 mia-n

Hey 👋

Is this similar to #2151 ?

I know they're not exactly the same, as #2151 is talking about different types using allowed values depending on the type. But I think having different Hashes containing different properties is like having different types too, right?

In #2151 , @dblock commented about the preference on allowing re-declaration that would be cumulative by default. He already did the same comment for this issue, so maybe is a solution for taking into account? For me is also weird to have multiple times the same property, because from my understanding the last one will win :) so I also prefer an approach for defining multiple types inside the array.

However, should this be solved by just passing custom value objects instead of Hash? Like explained here? https://github.com/ruby-grape/grape?tab=readme-ov-file#custom-types-and-coercions

require_relative 'fixed_price'
require_relative 'labor_rate'

params do
  requires :value, desc: "TODO", types: [FixedPrice, LaborRate]
end

Where each value object is just a Ruby class that includes the parsed? and parse methods:

class FixedPrice
  attr_accessor :fixed_price
  
  def initialize(fixed_price)
    @fixed_price = fixed_price
  end

  def to_h
    {
      fixed_price: @fixed_price
    }
  end

  def self.parsed?(value)
    keys = value.keys.sort
    [:fixed_price].sort == keys
  end

  def self.parse(value)
    if value["fixed_price"]
      new(value["fixed_price"]).to_h
    else
      {}
    end
  end
end
class LaborRate
  attr_accessor :rate, :time_unit
  
  def initialize(rate, time_unit)
    @rate = rate
    @time_unit = time_unit
  end

  def to_h
    {
      rate: @rate,
      time_unit: @time_unit
    }
  end

  def self.parsed?(value)
    keys = value.keys.sort
    [:rate, :time_unit].sort == keys
  end

  def self.parse(value)
    if value["rate"] && value["time_unit"]
      new(value["rate"], value["time_unit"]).to_h
    else
      {}
    end
  end
end

Or do we want to avoid having custom types and you want to be able to define Hash with specific properties directly when defining params? I think it overcomplicates the solution and it could be difficult to read.

jcagarcia avatar Dec 12 '23 21:12 jcagarcia

Or do we want to avoid having custom types and you want to be able to define Hash with specific properties directly when defining params? I think it overcomplicates the solution and it could be difficult to read.

This is the feature request: one shouldn't have to declare an entire custom type, just use a Hash

dblock avatar Dec 13 '23 15:12 dblock