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.

Sign up for GitHub

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

feat: Add plain text responses #25

Merged
merged 3 commits into from
Apr 9, 2024
Merged

feat: Add plain text responses #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5420ab0
spike: Add pain text responses
mattbearman Mar 22, 2024
2cb79e6
feat: Tidy up code and add tests
mattbearman Mar 26, 2024
20e422a
feat: PR Feedback - add plain_text_body helper method and only make b…
mattbearman Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions lib/apia/rack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,41 @@ def triplet_for_exception(exception)

class << self

# Return a plain text triplet for the given body.
#
# @param body [String]
# @param status [Integer]
# @param headers [Hash]
# @return [Array]
def plain_triplet(body, status: 200, headers: {})
response_triplet(body, content_type: 'text/plain', status: status, headers: headers)
end
Comment on lines +155 to +163
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good @mattbearman !

Would you be able to have a go at adding a plain text endpoint to the example api used in the tests for our apia-openapi gem.

https://github.com/apiaframework/apia-openapi/blob/main/examples/core_api/base.rb

It would be good to confirm we can successfully add this change into the openapi spec that is generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea 😄


# Return a JSON-ready triplet for the given body.
#
# @param body [Hash, Array]
# @param status [Integer]
# @param headers [Hash]
# @return [Array]
def json_triplet(body, status: 200, headers: {})
body_as_json = body.to_json
response_triplet(body.to_json, content_type: 'application/json', status: status, headers: headers)
end

# Return a triplet for the given body.
#
# @param body [Hash, Array]
# @param content_type [String]
# @param status [Integer]
# @param headers [Hash]
# @return [Array]
def response_triplet(body, content_type:, status: 200, headers: {})
content_length = body.bytesize.to_s
Copy link

Choose a reason for hiding this comment

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

I believe that bytesize is only available on String objects. We should probably do a responds to check for it, and simply not set Content-Length if it does not respond to the method.

However, we don't directly expose a means of sending a custom response. While the Response#plain_text_body method does pass the body value through as is, I would considering giving it anything other than a String object as incorrect usage.

Hence, I'd say this is not a blocker for merging this PR, but something that needs to be dealt with in the future should we want to expose fully custom responses.

body = [body] unless body.respond_to?(:each)

[
status,
headers.merge('content-type' => 'application/json', 'content-length' => body_as_json.bytesize.to_s),
[body_as_json]
headers.merge('content-type' => content_type, 'content-length' => content_length),
body
]
end

Expand Down
21 changes: 20 additions & 1 deletion lib/apia/response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
module Apia
class Response

TYPES = [
JSON = :json,
PLAIN = :plain
].freeze

attr_accessor :status
attr_reader :fields
attr_reader :headers
Expand All @@ -20,6 +25,11 @@ def initialize(request, endpoint)
@headers = {}
end

def plain_text_body(body)
@type = PLAIN
@body = body
end

# Add a field value for this endpoint
#
# @param name [Symbol]
Expand Down Expand Up @@ -53,11 +63,20 @@ def body
@body || hash
end

def type
@type || JSON
end

# Return the rack triplet for this response
#
# @return [Array]
def rack_triplet
Rack.json_triplet(body, headers: @headers, status: @status)
case type
when JSON
Rack.json_triplet(body, headers: headers, status: status)
when PLAIN
Rack.plain_triplet(body, headers: headers, status: status)
end
Comment on lines +74 to +79
Copy link

Choose a reason for hiding this comment

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

Nitpick: Since we're in the area, and we have the Rack.response_triplet helper method now, do you think it might be worth expanding the response object slightly to also allow fully custom response body objects and content types?

This might also have even further implications with the OpenAPI schema though. Basically, if this wouldn't be a quick win, let's ignore it until we need it :)

end

end
Expand Down
43 changes: 43 additions & 0 deletions spec/specs/apia/rack_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,49 @@ def call(_env)
end
end

context '.plain_triplet' do
it 'should return json encoded data' do
data = 'hello world'
triplet = Apia::Rack.plain_triplet(data)
expect(triplet).to be_a Array
expect(triplet[0]).to eq 200
expect(triplet[1]).to be_a Hash
expect(triplet[2]).to be_a Array
expect(triplet[2][0]).to eq 'hello world'
end

it 'should set the content type' do
data = 'hello world'
triplet = Apia::Rack.plain_triplet(data)
expect(triplet).to be_a Array
expect(triplet[1]['content-type']).to eq 'text/plain'
end

it 'should set the content length' do
data = 'hello world'
triplet = Apia::Rack.plain_triplet(data)
expect(triplet).to be_a Array
expect(triplet[1]['content-length']).to eq '11'
end

it 'should set the status' do
data = 'hello world'
triplet = Apia::Rack.plain_triplet(data, status: 400)
expect(triplet).to be_a Array
expect(triplet[0]).to eq 400
end

it 'should merge additional headers' do
data = 'hello world'
triplet = Apia::Rack.plain_triplet(data, headers: { 'x-something' => 'hello' })
expect(triplet).to be_a Array
expect(triplet[1]).to be_a Hash
expect(triplet[1]['x-something']).to eq 'hello'
expect(triplet[1]['content-length']).to eq '11'
expect(triplet[1]['content-type']).to eq 'text/plain'
end
end

context '.error_triplet' do
it 'should format the JSON appropriately' do
triplet = Apia::Rack.error_triplet('example_error', description: 'Some example', detail: { hello: 'world' })
Expand Down
136 changes: 97 additions & 39 deletions spec/specs/apia/response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,52 +34,110 @@
end

context '#rack_triplet' do
it 'should return 200 by default' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[0]).to eq 200
end
context 'with a JSON response' do
it 'should return 200 by default' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[0]).to eq 200
end

it 'should return whatever the status is set to' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.status = 403
expect(response.rack_triplet[0]).to eq 403
end
it 'should return whatever the status is set to' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.status = 403
expect(response.rack_triplet[0]).to eq 403
end

it 'should return the status from the endpoint' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
endpoint.http_status :created
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[0]).to eq 201
end
it 'should return the status from the endpoint' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
endpoint.http_status :created
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[0]).to eq 201
end

it 'should return the headers' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.add_header 'x-example', 'hello world'
expect(response.rack_triplet[1]['x-example']).to eq 'hello world'
end
it 'should return the headers' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.add_header 'x-example', 'hello world'
expect(response.rack_triplet[1]['x-example']).to eq 'hello world'
end

it 'should always provide the content-type as json' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[1]['content-type']).to eq 'application/json'
end
it 'should always provide the content-type as json' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[1]['content-type']).to eq 'application/json'
end

it 'should always set a content-length' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[2][0]).to eq '{}'
expect(response.rack_triplet[1]['content-length']).to eq '2'
it 'should always set a content-length' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
expect(response.rack_triplet[2][0]).to eq '{}'
expect(response.rack_triplet[1]['content-length']).to eq '2'
end

it 'should return the body if one has been set' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.body = { hello: 'world' }
expect(response.rack_triplet[2][0]).to eq '{"hello":"world"}'
expect(response.rack_triplet[1]['content-length']).to eq '17'
end
end

it 'should return the body if one has been set' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.body = { hello: 'world' }
expect(response.rack_triplet[2][0]).to eq '{"hello":"world"}'
expect(response.rack_triplet[1]['content-length']).to eq '17'
context 'with a plain text response' do
it 'should return 200 by default' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
expect(response.rack_triplet[0]).to eq 200
end

it 'should return whatever the status is set to' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
response.status = 403
expect(response.rack_triplet[0]).to eq 403
end

it 'should return the status from the endpoint' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
endpoint.http_status :created
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
expect(response.rack_triplet[0]).to eq 201
end

it 'should return the headers' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
response.add_header 'x-example', 'hi!'
expect(response.rack_triplet[1]['x-example']).to eq 'hi!'
end

it 'should always provide the content-type as plain' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
expect(response.rack_triplet[1]['content-type']).to eq 'text/plain'
end

it 'should always set a content-length' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('')
expect(response.rack_triplet[2][0]).to eq ''
expect(response.rack_triplet[1]['content-length']).to eq '0'
end

it 'should return the body if one has been set' do
endpoint = Apia::Endpoint.create('ExampleEndpoint')
response = Apia::Response.new(request, endpoint)
response.plain_text_body('hello world')
expect(response.rack_triplet[2][0]).to eq 'hello world'
expect(response.rack_triplet[1]['content-length']).to eq '11'
end
end
end
end
Loading