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

Define URI::Generic#as_json #25350

Merged
merged 1 commit into from
Jun 25, 2016
Merged

Conversation

tricknotes
Copy link
Contributor

Summary

When the URI object is converted as JSON,
it is expected that it is a string that means its URI.

Expected:

>> URI.parse('http://example.com').as_json
"http://example.com"

Actual:

>> URI.parse('http://example.com').as_json
{"scheme"=>"http",
 "user"=>nil,
 "password"=>nil,
 "host"=>"example.com",
 "port"=>80,
 "path"=>"",
 "query"=>nil,
 "opaque"=>nil,
 "fragment"=>nil,
 "parser"=>
  {"regexp"=>
    {"SCHEME"=>"(?-mix:\\A[A-Za-z][A-Za-z0-9+\\-.]*\\z)",
     "USERINFO"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=A-Z_a-z~])*\\z)",
     "HOST"=>
      "(?-mix:\\A(?:(?<IP-literal>\\[(?:(?<IPv6address>(?:\\h{1,4}:){6}(?<ls32>\\h{1,4}:\\h{1,4}|(?<IPv4address>(?<dec-octet>[1-9]\\d|1\\d{2}|2[0-4]\\d|25[0-5]|\\d)\\.\\g<dec-octet>\\.\\g<dec-octet>\\.\\g<dec-octet>))|::(?:\\h{1,4}:){5}\\g<ls32>|\\h{,4}::(?:\\h{1,4}:){4}\\g<ls32>|(?:(?:\\h{1,4}:)?\\h{1,4})?::(?:\\h{1,4}:){3}\\g<ls32>|(?:(?:\\h{1,4}:){,2}\\h{1,4})?::(?:\\h{1,4}:){2}\\g<ls32>|(?:(?:\\h{1,4}:){,3}\\h{1,4})?::\\h{1,4}:\\g<ls32>|(?:(?:\\h{1,4}:){,4}\\h{1,4})?::\\g<ls32>|(?:(?:\\h{1,4}:){,5}\\h{1,4})?::\\h{1,4}|(?:(?:\\h{1,4}:){,6}\\h{1,4})?::)|(?<IPvFuture>v\\h+\\.[!$&-.0-;=A-Z_a-z~]+))\\])|\\g<IPv4address>|(?<reg-name>(?:%\\h\\h|[!$&-.0-9;=A-Z_a-z~])*))\\z)",
     "ABS_PATH"=>
      "(?-mix:\\A\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*(?:\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*)*\\z)",
     "REL_PATH"=>
      "(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])+(?:\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*)*\\z)",
     "QUERY"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~\\/?])*\\z)",
     "FRAGMENT"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~\\/?])*\\z)",
     "OPAQUE"=>"(?-mix:\\A(?:[^\\/].*)?\\z)",
     "PORT"=>
      "(?-mix:\\A[\\x09\\x0a\\x0c\\x0d ]*\\d*[\\x09\\x0a\\x0c\\x0d ]*\\z)"}}}

@rails-bot
Copy link

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@rafaelfranca
Copy link
Member

@chancancode @jeremy WDYT?

@tricknotes tricknotes force-pushed the uri-generic-as-json branch 2 times, most recently from 15e1c5e to 0d2e2a2 Compare June 16, 2016 01:00
@@ -192,6 +193,12 @@ def as_json(options = nil) #:nodoc:
end
end

class URI::Generic #:nodoc:
def as_json(options = nil)
to_s.as_json(options)
Copy link
Member

Choose a reason for hiding this comment

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

Just to_s should be fine here

@chancancode
Copy link
Member

Looks good to me!

When the URI object is converted as JSON,
it is expected that it is a string that means its URI.

Expected:
```
>> URI.parse('http://example.com').as_json
"http://example.com"
```

Actual:
```
>> URI.parse('http://example.com').as_json
{"scheme"=>"http",
 "user"=>nil,
 "password"=>nil,
 "host"=>"example.com",
 "port"=>80,
 "path"=>"",
 "query"=>nil,
 "opaque"=>nil,
 "fragment"=>nil,
 "parser"=>
  {"regexp"=>
    {"SCHEME"=>"(?-mix:\\A[A-Za-z][A-Za-z0-9+\\-.]*\\z)",
     "USERINFO"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=A-Z_a-z~])*\\z)",
     "HOST"=>
      "(?-mix:\\A(?:(?<IP-literal>\\[(?:(?<IPv6address>(?:\\h{1,4}:){6}(?<ls32>\\h{1,4}:\\h{1,4}|(?<IPv4address>(?<dec-octet>[1-9]\\d|1\\d{2}|2[0-4]\\d|25[0-5]|\\d)\\.\\g<dec-octet>\\.\\g<dec-octet>\\.\\g<dec-octet>))|::(?:\\h{1,4}:){5}\\g<ls32>|\\h{,4}::(?:\\h{1,4}:){4}\\g<ls32>|(?:(?:\\h{1,4}:)?\\h{1,4})?::(?:\\h{1,4}:){3}\\g<ls32>|(?:(?:\\h{1,4}:){,2}\\h{1,4})?::(?:\\h{1,4}:){2}\\g<ls32>|(?:(?:\\h{1,4}:){,3}\\h{1,4})?::\\h{1,4}:\\g<ls32>|(?:(?:\\h{1,4}:){,4}\\h{1,4})?::\\g<ls32>|(?:(?:\\h{1,4}:){,5}\\h{1,4})?::\\h{1,4}|(?:(?:\\h{1,4}:){,6}\\h{1,4})?::)|(?<IPvFuture>v\\h+\\.[!$&-.0-;=A-Z_a-z~]+))\\])|\\g<IPv4address>|(?<reg-name>(?:%\\h\\h|[!$&-.0-9;=A-Z_a-z~])*))\\z)",
     "ABS_PATH"=>
      "(?-mix:\\A\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*(?:\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*)*\\z)",
     "REL_PATH"=>
      "(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])+(?:\\/(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~])*)*\\z)",
     "QUERY"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~\\/?])*\\z)",
     "FRAGMENT"=>"(?-mix:\\A(?:%\\h\\h|[!$&-.0-;=@-Z_a-z~\\/?])*\\z)",
     "OPAQUE"=>"(?-mix:\\A(?:[^\\/].*)?\\z)",
     "PORT"=>
      "(?-mix:\\A[\\x09\\x0a\\x0c\\x0d ]*\\d*[\\x09\\x0a\\x0c\\x0d ]*\\z)"}}}
```
@tricknotes
Copy link
Contributor Author

Thanks for your review @chancancode .
I agreed with your all comments and rebased my commit.

@chancancode chancancode merged commit 6da5f6b into rails:master Jun 25, 2016
@tricknotes tricknotes deleted the uri-generic-as-json branch June 25, 2016 10:09
@chancancode
Copy link
Member

Thank you!!!

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

Successfully merging this pull request may close these issues.

None yet

6 participants