Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Param with multiple acceptable Hash Types #2385

Open
mia-n opened this issue Dec 11, 2023 · 4 comments
Open

Param with multiple acceptable Hash Types #2385

mia-n opened this issue Dec 11, 2023 · 4 comments

Comments

@mia-n
Copy link

mia-n commented Dec 11, 2023

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?

@dblock
Copy link
Member

dblock commented Dec 12, 2023

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.

@mia-n
Copy link
Author

mia-n commented Dec 12, 2023

@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.

@jcagarcia
Copy link
Contributor

jcagarcia commented Dec 12, 2023

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.

@dblock
Copy link
Member

dblock commented Dec 13, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants