Add validation for Query Name #947
Conversation
lib/graphql/name_validator.rb
Outdated
| end | ||
|
|
||
| def self.valid?(name) | ||
| name =~ /^[_a-zA-Z][_a-zA-Z0-9]*$/ |
There was a problem hiding this comment.
You can make this a const and then use it in the error message.
irb(main):004:0> regexp = /^[_a-zA-Z][_a-zA-Z0-9]*$/
=> /^[_a-zA-Z][_a-zA-Z0-9]*$/
irb(main):005:0> "Names must match #{regexp.inspect} but 'Root Query' does not"
=> "Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but 'Root Query' does not"
spec/graphql/object_type_spec.rb
Outdated
| # Force evaluation | ||
| InvalidNameObject.name | ||
| } | ||
| assert_equal("Names must match (?-mix:^[_a-zA-Z][_a-zA-Z0-9]*$) but 'Three Word Query' does not", exception.message) |
There was a problem hiding this comment.
If you call #inspect on the regexp and interpolate that instead then you won’t get the (?-mix:…) part.
There was a problem hiding this comment.
Good point, I thought maybe its useful to have it there, but i agree it's cleaner with inspect! thanks.
|
👍 This looks great, thanks for pushing a fix! I totally agree that we should enforce spec-compliant behavior. Here are my thoughts:
|
|
thanks @rmosolgo! both of your points make total sense, I've already done the Going to replace |
|
Very nice, thanks again for making it happen! |
Problem
We should validate
nameofQuery. Having space and other invalid characters causes issues when using schema with GraphiQL. Doing this we'll also follow GraphQL reference implementation here.Solution
Added new
GraphQL::NameVlidatorclass which canvalidate!a name. UpdatedObjectTypeto overridename=method which first validates usingNameValidatorand then sets@name.Switched
EnumTypeto also use the same validator, if you think it's ok forEnumTypeto raise newly addedGraphQL::InvalidNameErrorthen we can switch it to useNameValidator.validate!instead ofNameValidator.valid?.