Skip to content

Commit

Permalink
Removes utf-8 from JSON requests because it's redundant
Browse files Browse the repository at this point in the history
JSON requests should use UTF-8 by default according to
http://www.ietf.org/rfc/rfc4627.txt, so we will remove `charset=utf-8`
when we find it to avoid redundancy.

In the process, I moved code that was only used by APIB into the APIB
classes, such as exposing `content_type` to the templates. If I changed
the `content-type` for all templates it would break unrelated things.

Connects to #235.
  • Loading branch information
kurko committed Dec 13, 2016
1 parent 3f41da9 commit e601e5f
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 34 deletions.
10 changes: 5 additions & 5 deletions features/api_blueprint_documentation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Feature: Generate API Blueprint documentation from test examples
put 'Updates a single order' do
explanation "This is used to update orders."
header "Content-Type", "application/json"
header "Content-Type", "application/json; charset=utf-16"
context "with a valid id" do
let(:id) { 1 }
Expand Down Expand Up @@ -406,11 +406,11 @@ Feature: Generate API Blueprint documentation from test examples
### Updates a single order [PUT]
+ Request Invalid request (application/json)
+ Request Invalid request (application/json; charset=utf-16)
+ Headers
Content-Type: application/json
Content-Type: application/json; charset=utf-16
Host: example.org
+ Response 400 (application/json)
Expand All @@ -420,11 +420,11 @@ Feature: Generate API Blueprint documentation from test examples
Content-Type: application/json
Content-Length: 0
+ Request Update an order (application/json)
+ Request Update an order (application/json; charset=utf-16)
+ Headers
Content-Type: application/json
Content-Type: application/json; charset=utf-16
Host: example.org
+ Body
Expand Down
59 changes: 50 additions & 9 deletions lib/rspec_api_documentation/views/api_blueprint_example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,22 @@ def parameters

def requests
super.map do |request|
request[:request_body] = body_to_json(request, :request)
request[:response_body] = body_to_json(request, :response)
request[:request_headers_text] = remove_utf8_for_json(request[:request_headers_text])
request[:request_headers_text] = indent(request[:request_headers_text])
request[:request_content_type] = content_type(request[:request_headers])
request[:request_content_type] = remove_utf8_for_json(request[:request_content_type])
request[:request_body] = body_to_json(request, :request)
request[:request_body] = indent(request[:request_body])

request[:request_body] = indent(request[:request_body])
request[:request_headers_text] = indent(request[:request_headers_text])
request[:response_body] = indent(request[:response_body])
request[:response_headers_text] = remove_utf8_for_json(request[:response_headers_text])
request[:response_headers_text] = indent(request[:response_headers_text])
request[:response_content_type] = content_type(request[:response_headers])
request[:response_content_type] = remove_utf8_for_json(request[:response_content_type])
request[:response_body] = body_to_json(request, :response)
request[:response_body] = indent(request[:response_body])

request[:has_request?] = has_request?(request)
request[:has_response?] = has_response?(request)
request
end
end
Expand All @@ -37,11 +46,21 @@ def extension

private

def has_request?(metadata)
metadata.any? do |key, value|
[:request_body, :request_headers, :request_content_type].include?(key) && value
end
end

def has_response?(metadata)
metadata.any? do |key, value|
[:response_status, :response_body, :response_headers, :response_content_type].include?(key) && value
end
end

def indent(string)
string.tap do |str|
if str
str.gsub!(/\n/, "\n" + (" " * TOTAL_SPACES_INDENTATION))
end
str.gsub!(/\n/, "\n" + (" " * TOTAL_SPACES_INDENTATION)) if str
end
end

Expand All @@ -52,12 +71,34 @@ def body_to_json(http_call, message_direction)
content_type = http_call["#{message_direction}_content_type".to_sym]
body = http_call["#{message_direction}_body".to_sym] # e.g request_body

if content_type =~ /application\/.*json/ && body
if json?(content_type) && body
body = JSON.pretty_generate(JSON.parse(body))
end

body
end

# JSON requests should use UTF-8 by default according to
# http://www.ietf.org/rfc/rfc4627.txt, so we will remove `charset=utf-8`
# when we find it to remove noise.
def remove_utf8_for_json(headers)
return unless headers
headers
.split("\n")
.map { |header|
header.gsub!(/; *charset=utf-8/, "") if json?(header)
header
}
.join("\n")
end

def content_type(headers)
headers && headers.fetch("Content-Type", nil)
end

def json?(string)
string =~ /application\/.*json/
end
end
end
end
20 changes: 0 additions & 20 deletions lib/rspec_api_documentation/views/markup_example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,9 @@ def parameters

def requests
super.map do |hash|
hash[:request_content_type] = content_type(hash[:request_headers])
hash[:request_headers_text] = format_hash(hash[:request_headers])
hash[:request_query_parameters_text] = format_hash(hash[:request_query_parameters])
hash[:response_content_type] = content_type(hash[:response_headers])
hash[:response_headers_text] = format_hash(hash[:response_headers])
hash[:has_request?] = has_request?(hash)
hash[:has_response?] = has_response?(hash)
if @host
if hash[:curl].is_a? RspecApiDocumentation::Curl
hash[:curl] = hash[:curl].output(@host, @filter_headers)
Expand All @@ -69,28 +65,12 @@ def extension

private

def has_request?(metadata)
metadata.any? do |key, value|
[:request_body, :request_headers, :request_content_type].include?(key) && value
end
end

def has_response?(metadata)
metadata.any? do |key, value|
[:response_status, :response_body, :response_headers, :response_content_type].include?(key) && value
end
end

def format_hash(hash = {})
return nil unless hash.present?
hash.collect do |k, v|
"#{k}: #{v}"
end.join("\n")
end

def content_type(headers)
headers && headers.fetch("Content-Type", nil)
end
end
end
end
111 changes: 111 additions & 0 deletions spec/views/api_blueprint_example_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# -*- coding: utf-8 -*-
require 'spec_helper'

describe RspecApiDocumentation::Views::ApiBlueprintExample do
let(:metadata) { { :resource_name => "Orders" } }
let(:group) { RSpec::Core::ExampleGroup.describe("Orders", metadata) }
let(:rspec_example) { group.example("Ordering a cup of coffee") {} }
let(:rad_example) do
RspecApiDocumentation::Example.new(rspec_example, configuration)
end
let(:configuration) { RspecApiDocumentation::Configuration.new }
let(:html_example) { described_class.new(rad_example, configuration) }

let(:content_type) { "application/json; charset=utf-8" }
let(:requests) do
[{
request_body: "{}",
request_headers: {
"Content-Type" => content_type,
"Another" => "header; charset=utf-8"
},
request_content_type: "",
response_body: "{}",
response_headers: {
"Content-Type" => content_type,
"Another" => "header; charset=utf-8"
},
response_content_type: ""
}]
end

before do
rspec_example.metadata[:requests] = requests
end

subject(:view) { described_class.new(rad_example, configuration) }

describe '#requests' do
describe 'request_content_type' do
subject { view.requests[0][:request_content_type] }

context 'when charset=utf-8 is present' do
it "just strips that because it's the default for json" do
expect(subject).to eq "application/json"
end
end

context 'when charset=utf-16 is present' do
let(:content_type) { "application/json; charset=utf-16" }

it "keeps that because it's NOT the default for json" do
expect(subject).to eq "application/json; charset=utf-16"
end
end
end

describe 'request_headers_text' do
subject { view.requests[0][:request_headers_text] }

context 'when charset=utf-8 is present' do
it "just strips that because it's the default for json" do
expect(subject).to eq "Content-Type: application/json\n Another: header; charset=utf-8"
end
end

context 'when charset=utf-16 is present' do
let(:content_type) { "application/json; charset=utf-16" }

it "keeps that because it's NOT the default for json" do
expect(subject).to eq "Content-Type: application/json; charset=utf-16\n Another: header; charset=utf-8"
end
end
end

describe 'response_content_type' do
subject { view.requests[0][:response_content_type] }

context 'when charset=utf-8 is present' do
it "just strips that because it's the default for json" do
expect(subject).to eq "application/json"
end
end

context 'when charset=utf-16 is present' do
let(:content_type) { "application/json; charset=utf-16" }

it "keeps that because it's NOT the default for json" do
expect(subject).to eq "application/json; charset=utf-16"
end
end
end

describe 'response_headers_text' do
subject { view.requests[0][:response_headers_text] }

context 'when charset=utf-8 is present' do
it "just strips that because it's the default for json" do
expect(subject).to eq "Content-Type: application/json\n Another: header; charset=utf-8"
end
end

context 'when charset=utf-16 is present' do
let(:content_type) { "application/json; charset=utf-16" }

it "keeps that because it's NOT the default for json" do
expect(subject).to eq "Content-Type: application/json; charset=utf-16\n Another: header; charset=utf-8"
end
end
end
end
end

0 comments on commit e601e5f

Please sign in to comment.