feature(#2385): Param with multiple acceptable Hash Types#2661
feature(#2385): Param with multiple acceptable Hash Types#2661jcagarcia wants to merge 4 commits intoruby-grape:masterfrom
Conversation
Danger ReportNo issues found. |
|
I can see some failed tests using Ruby 4. I'll take a look 👀 Also, rubocop failing. |
| params do | ||
| requires :value, types: [ | ||
| hash_schema { requires :fixed_price, type: Float }, | ||
| hash_schema { requires :time_unit, type: String; requires :rate, type: Float } |
There was a problem hiding this comment.
Would omitting hash_schema be equivalent and clearer and apply to all parameters? Isn't it just "one of the types"?
requires :value, types: [
{ requires :fixed_price, type: Float },
{ requires :time_unit, type: String; requires :rate, type: Float }
]There was a problem hiding this comment.
Hey @dblock thanks for taking a look!
While that would be ideal, I think it's unfortunately not technically possible. The requires/optional keywords inside schema definitions are DSL methods and plain Hash literals are just data structures - they can't contain executable DSL code.
At the very beginning I was just defining the HashSchema class so you need to do something like
requires :value, types: [
HashSchema.new { requires :fixed_price, type: Float }
]However, I decided to create a new DSL parameter called hash_schema to improve the readability.
Do you see any other approaches that could make the syntax cleaner within these technical constraints, or does the current hash_schema approach seem reasonable to you?
There was a problem hiding this comment.
I mean requires could parse types and implicitly transform hashes into HashSchema.new, at load time, no?
There was a problem hiding this comment.
Hey @dblock thanks for reviewing it again,
I would say that { requires :fixed_price, type: Float } is not valid Ruby syntax, because a plan Hash can't contain method calls like that.
Is true that requires could parse the types array at load time and implicitly transform plain Hash literals into HashSchema instances. That part is doable.
The limitation I see is about expressiveness: a plain Hash can only represent a flat mapping of key → type, with no way to distinguish required vs optional keys, nested schemas, or any other per-key options.
I think that with the proposed solution of hash_schema we can have much more customisation and is keeping the same structure.
Make sense?
There was a problem hiding this comment.
I am confused why we are able to handle requires: :value, types: at the parent level, but not in the inner level in your example? Can we omit the {} and make an array of requires and optional (arrays)? But maybe that's worse ....
There was a problem hiding this comment.
I think the difference is that you are talking about requires: (key inside a hash) but this is the dsl method requires, that cannot be used inside a plain hash.
There was a problem hiding this comment.
Another crazy idea.
requires :value, types: [
either { requires :fixed_price, type: Float },
or { requires :time_unit, type: String; requires :rate, type: Float }
]
I also don't know if it's better.
@ericproulx Do you have naming preferences or other ideas? You can break the tie, I'll side with your opinion.
There was a problem hiding this comment.
Hey @ericproulx , any suggestion or feedback about this? 😄 thanks!
There was a problem hiding this comment.
I like the either, or style. Intention is clear. I need to think about the whole thing.
There was a problem hiding this comment.
So what about more than 2 types, something like this? Multiple ors?
requires :value, types: [
either { requires :fixed_price, type: Float },
or { requires :time_unit, type: String; requires :rate, type: Float },
or { requires :another_param, type: String; requires :rate, type: Float },
or { requires :another_param, type: String; requires :rate, type: Float },
or { requires :another_param, type: String; requires :rate, type: Float },
or { requires :another_param, type: String; requires :rate, type: Float }
]|
Hello @jcagarcia I was wondering if this would already help. |
|
Hey @ericproulx , I think that the aim of this solution (and the requested feature) was to have an array of different hash schemas without the need of defining multiple optional properties (polymorphism). I would say that dry-schemas can't offer that polymorphism that we are trying to achieve in this feature. |
| end | ||
|
|
||
| # Helper class to parse schema definition blocks | ||
| class SchemaParser |
There was a problem hiding this comment.
I think there's room for optimization here. requires and optional are doing the same thing but write in a different key.
Also, the SchemaParser usage is always
schema = { required: {}, optional: {} }
parser = SchemaParser.new(schema)
parser.instance_eval(&block)
@schema_structure ...I think we could encapsulate the instance_eval call in a method and not create an instance .new every time. I'll think about it this weekend.
Tries to resolve #2385 😄
Summary
This PR implements support for defining parameters that accept multiple different hash structures (polymorphic hashes) using the new
hash_schemahelper with the types option. This addresses the feature request in #2385, which asked for a cleaner way to validate parameters that can have multiple valid schemas.Previously, developers had to use a combination of optional fields with complex mutually_exclusive and given constraints, which was difficult to read and maintain. This new approach provides a clear, declarative syntax for defining multiple acceptable hash structures.
What's New
Usage Examples
Basic Usage - Multiple Hash Schemas
Nested Hash Schemas
Error Reporting
When validation fails, the error messages are clear and specific:
All tests pass and the implementation maintains backward compatibility with existing Grape functionality.
cc @mia-n @dblock - This implements the feature you requested in #2385. Would love to hear your feedback!