Skip to content

Conversation

a-alhusaini
Copy link

Before:

class SomeController
  def index
    params.validation { # ... }
    
    params["x"] # => string "[1,2,3]"
end

After

class SomeController
  def index
    params.validation { # ... }
    
    params["x"] # => Array(JSON::Any)
end

@a-alhusaini a-alhusaini changed the title json arrays are now validated as Array(JSON::Any) Handle nested JSON and JSON arrays in params properly Aug 8, 2023
getter predicate : (String -> Bool)
getter field : String
getter value : String?
getter value : String | Array(JSON::Any) | JSON::Any | Nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON::Any::Type includes Array(JSON::Any), so is it redundant to include both, or is there something I'm missing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my thought as well, so I don't think we need the extra Array(JSON::Any)

@@ -1,4 +1,9 @@
module ValidationsHelper
def json_params_builder(body = "")
request = HTTP::Request.new("POST", "/", HTTP::Headers{"Content-Type" => "application/json"}, body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hard coded as a POST request. Does that mean that it can only be used with POST requests, or can it be used with PUT and PATCH requests too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is just a test, but we should be testing POST PATCH AND PUT

required("y")
end

validator.validate!["x"].should be_a Array(JSON::Any)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe returning a JSON::PullParser would be better here, so people could use JSON::Serializable to decode these parameters. 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When was that introduced in the language? I don't recall seeing this at the time I wrote this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a-alhusaini it's been part of the std library for a few years

https://crystal-lang.org/api/1.10.1/JSON/PullParser.html

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants