From 4acfdca239f36ffccca025615b5b96f56bb81dd4 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 18 Jun 2021 10:49:46 -0500 Subject: [PATCH 01/10] Introduce AWS.Response --- src/AWS.jl | 25 +++++-- src/AWSMetadata.jl | 1 + src/utilities/request.jl | 74 +++------------------ src/utilities/response.jl | 75 +++++++++++++++++++++ src/utilities/utilities.jl | 3 - test/AWS.jl | 132 ++++++++++--------------------------- 6 files changed, 141 insertions(+), 169 deletions(-) create mode 100644 src/utilities/response.jl diff --git a/src/AWS.jl b/src/AWS.jl index fe4e8c3e18..883f626ba8 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -32,6 +32,7 @@ include("AWSConfig.jl") include("AWSMetadata.jl") include(joinpath("utilities", "request.jl")) +include(joinpath("utilities", "response.jl")) include(joinpath("utilities", "sign.jl")) include(joinpath("utilities", "downloads_backend.jl")) @@ -209,6 +210,9 @@ function (service::RestXMLService)( aws_config::AbstractAWSConfig=global_aws_config(), ) return_headers = _pop!(args, "return_headers", false) + return_stream = _pop!(args, "return_stream", false) + return_raw = _pop!(args, "return_raw", false) + response_stream = _pop!(args, "response_stream", nothing) request = Request(; _extract_common_kw_args(service, args)..., @@ -234,7 +238,8 @@ function (service::RestXMLService)( aws_config, service.endpoint_prefix, request.resource ) - return submit_request(aws_config, request; return_headers=return_headers) + response = submit_request(aws_config, request) + return legacy(response; return_headers, return_stream, return_raw, response_stream) end """ @@ -262,6 +267,9 @@ function (service::QueryService)( ) POST_RESOURCE = "/" return_headers = _pop!(args, "return_headers", false) + return_stream = _pop!(args, "return_stream", false) + return_raw = _pop!(args, "return_raw", false) + response_stream = _pop!(args, "response_stream", nothing) request = Request(; _extract_common_kw_args(service, args)..., @@ -276,7 +284,8 @@ function (service::QueryService)( args["Version"] = service.api_version request.content = HTTP.escapeuri(_flatten_query(service.signing_name, args)) - return submit_request(aws_config, request; return_headers=return_headers) + response = submit_request(aws_config, request) + return legacy(response; return_headers, return_stream, return_raw, response_stream) end """ @@ -304,6 +313,9 @@ function (service::JSONService)( ) POST_RESOURCE = "/" return_headers = _pop!(args, "return_headers", false) + return_stream = _pop!(args, "return_stream", false) + return_raw = _pop!(args, "return_raw", false) + response_stream = _pop!(args, "response_stream", nothing) request = Request(; _extract_common_kw_args(service, args)..., @@ -316,7 +328,8 @@ function (service::JSONService)( request.headers["Content-Type"] = "application/x-amz-json-$(service.json_version)" request.headers["X-Amz-Target"] = "$(service.target).$(operation)" - return submit_request(aws_config, request; return_headers=return_headers) + response = submit_request(aws_config, request) + return legacy(response; return_headers, return_stream, return_raw, response_stream) end """ @@ -345,6 +358,9 @@ function (service::RestJSONService)( aws_config::AbstractAWSConfig=global_aws_config(), ) return_headers = _pop!(args, "return_headers", false) + return_stream = _pop!(args, "return_stream", false) + return_raw = _pop!(args, "return_raw", false) + response_stream = _pop!(args, "response_stream", nothing) request = Request(; _extract_common_kw_args(service, args)..., @@ -363,7 +379,8 @@ function (service::RestJSONService)( request.headers["Content-Type"] = "application/json" request.content = json(args) - return submit_request(aws_config, request; return_headers=return_headers) + response = submit_request(aws_config, request) + return legacy(response; return_headers, return_stream, return_raw, response_stream) end function __init__() diff --git a/src/AWSMetadata.jl b/src/AWSMetadata.jl index d4bcda793e..50dddbf633 100644 --- a/src/AWSMetadata.jl +++ b/src/AWSMetadata.jl @@ -1,3 +1,4 @@ + module AWSMetadata using Base64 diff --git a/src/utilities/request.jl b/src/utilities/request.jl index b75bee0f9e..96139a3249 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -33,16 +33,13 @@ Base.@kwdef mutable struct Request resource::String = "" url::String = "" - return_stream::Bool = false - response_stream::Union{<:IO,Nothing} = nothing http_options::AbstractDict{Symbol,<:Any} = LittleDict{Symbol,String}() - return_raw::Bool = false response_dict_type::Type{<:AbstractDict} = LittleDict backend::AbstractBackend = DEFAULT_BACKEND[] end """ - submit_request(aws::AbstractAWSConfig, request::Request; return_headers::Bool=false) + submit_request(aws::AbstractAWSConfig, request::Request) Submit the request to AWS. @@ -50,15 +47,10 @@ Submit the request to AWS. - `aws::AbstractAWSConfig`: AWSConfig containing credentials and other information for fulfilling the request, default value is the global configuration - `request::Request`: All the information about making a request to AWS -# Keywords -- `return_headers::Bool=false`: True if you want the headers from the response returned back - # Returns -- `Tuple or Dict`: Tuple if returning_headers, otherwise just return a Dict of the response body +- `Response`: A struct containing the response """ -function submit_request( - aws::AbstractAWSConfig, request::Request; return_headers::Bool=false -) +function submit_request(aws::AbstractAWSConfig, request::Request) response = nothing TOO_MANY_REQUESTS = 429 EXPIRED_ERROR_CODES = ["ExpiredToken", "ExpiredTokenException", "RequestExpired"] @@ -132,55 +124,7 @@ function submit_request( end end - response_dict_type = request.response_dict_type - - # For HEAD request, return headers... - if request.request_method == "HEAD" - return response_dict_type(response.headers) - end - - # Return response stream if requested... - if request.return_stream - return request.response_stream - end - - # Return raw data if requested... - if request.return_raw - return (return_headers ? (response.body, response.headers) : response.body) - end - - # Parse response data according to mimetype... - mime = HTTP.header(response, "Content-Type", "") - - if isempty(mime) - if length(response.body) > 5 && response.body[1:5] == b" "text/xml"] response = Patches._response(; headers=expected_headers) apply(Patches._aws_http_request_patch(response)) do - @testset "body" begin - result = AWS.submit_request(aws, request) - - @test result isa expected_body_type - @test result == expected_body - end + response = AWS.submit_request(aws, request) - @testset "body and headers" begin - body, headers = AWS.submit_request( - aws, request; return_headers=true - ) - - @test body == expected_body - @test body isa expected_body_type - - @test headers == LittleDict(expected_headers) - @test headers isa expected_header_type - end + @test body isa expected_body_type + @test body == expected_body + @test response.headers == expected_headers + @test response.headers isa Vector end end end @@ -271,22 +236,13 @@ end response = Patches._response(; headers=expected_headers) apply(Patches._aws_http_request_patch(response)) do - @testset "body" begin - result = AWS.submit_request(aws, request) + response = AWS.submit_request(aws, request) + body = parse(expected_body_type, response) - @test result == expected_body - @test result isa expected_body_type - end - - @testset "body and headers" begin - body, headers = AWS.submit_request(aws, request; return_headers=true) - - @test body == expected_body - @test body isa expected_body_type - - @test headers == LittleDict(expected_headers) - @test headers isa LittleDict{SubString{String},SubString{String}} - end + @test body isa expected_body_type + @test body == expected_body + @test response.headers isa Vector + @test response.headers == expected_headers end end @@ -310,22 +266,13 @@ end response = Patches._response(; body=json_body, headers=json_headers) apply(Patches._aws_http_request_patch(response)) do - @testset "body" begin - result = AWS.submit_request(aws, request) - - @test result isa expected_body_type - @test result == expected_body - end - - @testset "body and headers" begin - body, headers = AWS.submit_request(aws, request; return_headers=true) - - @test body == expected_body - @test body isa expected_body_type + response = AWS.submit_request(aws, request) + body = parse(expected_body_type, response) - @test headers == LittleDict(json_headers) - @test headers isa LittleDict{SubString{String},SubString{String}} - end + @test body isa expected_body_type + @test body == expected_body + @test response.headers isa Vector + @test response.headers == expected_headers end end @@ -343,22 +290,13 @@ end response = Patches._response(; headers=expected_headers) apply(Patches._aws_http_request_patch(response)) do - @testset "body" begin - result = AWS.submit_request(aws, request) - - @test result isa String - @test result == expected_body - end + response = AWS.submit_request(aws, request) + body = String(response.body) - @testset "body and headers" begin - body, headers = AWS.submit_request(aws, request; return_headers=true) - - @test body == expected_body - @test body isa String - - @test headers == LittleDict(expected_headers) - @test headers isa expected_header_type - end + @test body isa String + @test body == expected_body + @test response.headers isa Vector + @test response.headers == expected_headers end end end From 9f514a6d3a8f794105422d4207ee24cfff3bc3c6 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 18 Jun 2021 12:07:31 -0500 Subject: [PATCH 02/10] Iterating --- src/AWS.jl | 3 ++- src/AWSMetadata.jl | 1 - src/utilities/request.jl | 2 +- src/utilities/response.jl | 6 +++--- test/AWS.jl | 2 +- test/patch.jl | 9 ++++++--- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/AWS.jl b/src/AWS.jl index 883f626ba8..7350fd299b 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -1,7 +1,8 @@ module AWS -using Compat: Compat, @something +using Base: BufferStream using Base64 +using Compat: Compat, @something using Dates using Downloads: Downloads, Downloader, Curl using HTTP diff --git a/src/AWSMetadata.jl b/src/AWSMetadata.jl index 50dddbf633..d4bcda793e 100644 --- a/src/AWSMetadata.jl +++ b/src/AWSMetadata.jl @@ -1,4 +1,3 @@ - module AWSMetadata using Base64 diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 96139a3249..b5e6bfab57 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -133,7 +133,7 @@ function _http_request(http_backend::HTTPBackend, request::Request) @repeat 4 try http_stack = HTTP.stack(; redirect=false, retry=false, aws_authorization=false) - response_stream = Base.BufferStream() + response_stream = BufferStream() r = @mock HTTP.request( http_stack, diff --git a/src/utilities/response.jl b/src/utilities/response.jl index bcb1d25705..2c429db883 100644 --- a/src/utilities/response.jl +++ b/src/utilities/response.jl @@ -5,8 +5,8 @@ struct Response end function Base.getproperty(r::Response, f::Symbol) - if f === :headers - r.response.headers + if f === :headers || f === :status + getfield(r.response, f) else getfield(r, f) end @@ -42,7 +42,7 @@ Base.parse(::Type{T}, r::Response) where T = parse(T, r, mime_type(r)) function Base.parse(::Type{T}, r::Response, ::MIME"application/xml") where T <: AbstractDict xml = parse_xml(String(r.body)) root = XMLDict.root(xml.x) # TODO: Why x? - return xml_dict(root, T)) + return xml_dict(root, T) end function Base.parse(::Type{T}, r::Response, ::MIME"application/json") where T <: AbstractDict diff --git a/test/AWS.jl b/test/AWS.jl index 89e2568a39..8ec25589a9 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -135,7 +135,7 @@ end response = AWS.submit_request(aws, request) @test response.headers == Patches.headers - @test response isa Vector + @test response.headers isa Vector end end diff --git a/test/patch.jl b/test/patch.jl index aca9d19c16..f79e2394e9 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -53,12 +53,15 @@ function _response(; response.version = version response.status = status response.headers = headers - response.body = Vector{UInt8}(body) + response.body = b"[Message Body was streamed]" - return response + b = Base.BufferStream() + write(b, body) + + return AWS.Response(response, b) end -function _aws_http_request_patch(response::HTTP.Messages.Response=_response()) +function _aws_http_request_patch(response::AWS.Response=_response()) return @patch AWS._http_request(::AWS.AbstractBackend, request::Request) = response end From 3ab3aaf3ef1061ddac2dbb645378ca42b6191675 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Thu, 2 Sep 2021 12:02:35 -0500 Subject: [PATCH 03/10] Remove redundant assignment --- test/AWS.jl | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/test/AWS.jl b/test/AWS.jl index 8ec25589a9..24247aeeed 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -223,13 +223,6 @@ end end @testset "xml" begin - request = Request(; - service="s3", - api_version="api_version", - request_method="GET", - url="https://s3.us-east-1.amazonaws.com/sample-bucket", - ) - expected_body_type = LittleDict{Union{Symbol,String},Any} expected_body = _expected_body(Patches.body, expected_body_type) expected_headers = Pair["Content-Type" => "application/xml"] @@ -277,13 +270,6 @@ end end @testset "Text" begin - request = Request(; - service="s3", - api_version="api_version", - request_method="GET", - url="https://s3.us-east-1.amazonaws.com/sample-bucket", - ) - expected_headers = ["Content-Type" => "text/html"] expected_body = Patches.body expected_header_type = LittleDict{SubString{String},SubString{String}} From 5c90a4adeaf6b35fef62ab29bd578f877781b540 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Thu, 2 Sep 2021 15:24:28 -0500 Subject: [PATCH 04/10] Major AWS.Response functionality in place --- src/AWS.jl | 40 +++---- src/AWSCredentials.jl | 4 +- src/AWSExceptions.jl | 31 ++++-- src/utilities/credentials.jl | 6 +- src/utilities/request.jl | 16 +-- src/utilities/response.jl | 71 ++++++------ src/utilities/utilities.jl | 10 +- test/AWS.jl | 204 ++++++++++++++++------------------- test/issues.jl | 16 +-- test/minio.jl | 6 +- test/patch.jl | 5 +- 11 files changed, 199 insertions(+), 210 deletions(-) diff --git a/src/AWS.jl b/src/AWS.jl index 7350fd299b..4bbc7649ed 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -202,7 +202,7 @@ Perform a RestXML request to AWS. - `aws::AbstractAWSConfig`: AWSConfig containing credentials and other information for fulfilling the request, default value is the global configuration # Returns -- `Tuple or Dict`: If `return_headers` is passed in through `args` a Tuple containing the Headers and Response will be returned, otherwise just a Dict +- `AWS.Response`: A struct containing the response details """ function (service::RestXMLService)( request_method::String, @@ -210,10 +210,7 @@ function (service::RestXMLService)( args::AbstractDict{String,<:Any}=Dict{String,Any}(); aws_config::AbstractAWSConfig=global_aws_config(), ) - return_headers = _pop!(args, "return_headers", false) - return_stream = _pop!(args, "return_stream", false) - return_raw = _pop!(args, "return_raw", false) - response_stream = _pop!(args, "response_stream", nothing) + _delete_outdated_kw_args!(args) request = Request(; _extract_common_kw_args(service, args)..., @@ -239,8 +236,7 @@ function (service::RestXMLService)( aws_config, service.endpoint_prefix, request.resource ) - response = submit_request(aws_config, request) - return legacy(response; return_headers, return_stream, return_raw, response_stream) + return submit_request(aws_config, request) end """ @@ -259,7 +255,7 @@ Perform a Query request to AWS. - `aws::AbstractAWSConfig`: AWSConfig containing credentials and other information for fulfilling the request, default value is the global configuration # Returns -- `Tuple or Dict`: If `return_headers` is passed in through `args` a Tuple containing the Headers and Response will be returned, otherwise just a Dict +- `AWS.Response`: A struct containing the response details """ function (service::QueryService)( operation::String, @@ -267,10 +263,7 @@ function (service::QueryService)( aws_config::AbstractAWSConfig=global_aws_config(), ) POST_RESOURCE = "/" - return_headers = _pop!(args, "return_headers", false) - return_stream = _pop!(args, "return_stream", false) - return_raw = _pop!(args, "return_raw", false) - response_stream = _pop!(args, "response_stream", nothing) + _delete_outdated_kw_args!(args) request = Request(; _extract_common_kw_args(service, args)..., @@ -285,8 +278,7 @@ function (service::QueryService)( args["Version"] = service.api_version request.content = HTTP.escapeuri(_flatten_query(service.signing_name, args)) - response = submit_request(aws_config, request) - return legacy(response; return_headers, return_stream, return_raw, response_stream) + return submit_request(aws_config, request) end """ @@ -305,7 +297,7 @@ Perform a JSON request to AWS. - `aws::AbstractAWSConfig`: AWSConfig containing credentials and other information for fulfilling the request, default value is the global configuration # Returns -- `Tuple or Dict`: If `return_headers` is passed in through `args` a Tuple containing the Headers and Response will be returned, otherwise just a Dict +- `AWS.Response`: A struct containing the response details """ function (service::JSONService)( operation::String, @@ -313,10 +305,7 @@ function (service::JSONService)( aws_config::AbstractAWSConfig=global_aws_config(), ) POST_RESOURCE = "/" - return_headers = _pop!(args, "return_headers", false) - return_stream = _pop!(args, "return_stream", false) - return_raw = _pop!(args, "return_raw", false) - response_stream = _pop!(args, "response_stream", nothing) + _delete_outdated_kw_args!(args) request = Request(; _extract_common_kw_args(service, args)..., @@ -329,8 +318,7 @@ function (service::JSONService)( request.headers["Content-Type"] = "application/x-amz-json-$(service.json_version)" request.headers["X-Amz-Target"] = "$(service.target).$(operation)" - response = submit_request(aws_config, request) - return legacy(response; return_headers, return_stream, return_raw, response_stream) + return submit_request(aws_config, request) end """ @@ -350,7 +338,7 @@ Perform a RestJSON request to AWS. - `aws::AbstractAWSConfig`: AWSConfig containing credentials and other information for fulfilling the request, default value is the global configuration # Returns -- `Tuple or Dict`: If `return_headers` is passed in through `args` a Tuple containing the Headers and Response will be returned, otherwise just a Dict +- `AWS.Response`: A struct containing the response details """ function (service::RestJSONService)( request_method::String, @@ -358,10 +346,7 @@ function (service::RestJSONService)( args::AbstractDict{String,<:Any}=Dict{String,String}(); aws_config::AbstractAWSConfig=global_aws_config(), ) - return_headers = _pop!(args, "return_headers", false) - return_stream = _pop!(args, "return_stream", false) - return_raw = _pop!(args, "return_raw", false) - response_stream = _pop!(args, "response_stream", nothing) + _delete_outdated_kw_args!(args) request = Request(; _extract_common_kw_args(service, args)..., @@ -380,8 +365,7 @@ function (service::RestJSONService)( request.headers["Content-Type"] = "application/json" request.content = json(args) - response = submit_request(aws_config, request) - return legacy(response; return_headers, return_stream, return_raw, response_stream) + return submit_request(aws_config, request) end function __init__() diff --git a/src/AWSCredentials.jl b/src/AWSCredentials.jl index e3eb6b20b6..2f97f66014 100644 --- a/src/AWSCredentials.jl +++ b/src/AWSCredentials.jl @@ -373,7 +373,7 @@ function dot_aws_credentials(profile=nothing) credential_file = @mock dot_aws_credentials_file() if isfile(credential_file) - ini = read(Inifile(), credential_file) + ini = Base.read(Inifile(), credential_file) p = @something profile _aws_get_profile() access_key, secret_key, token = _aws_get_credential_details(p, ini) @@ -404,7 +404,7 @@ function dot_aws_config(profile=nothing) config_file = @mock dot_aws_config_file() if isfile(config_file) - ini = read(Inifile(), config_file) + ini = Base.read(Inifile(), config_file) p = @something profile _aws_get_profile() access_key, secret_key, token = _aws_get_credential_details(p, ini) diff --git a/src/AWSExceptions.jl b/src/AWSExceptions.jl index 9566f3bce2..1fb16ef1cd 100644 --- a/src/AWSExceptions.jl +++ b/src/AWSExceptions.jl @@ -5,6 +5,8 @@ using JSON using XMLDict using XMLDict: XMLDictElement +const BODY_STREAMED_PLACEHOLDER = b"[Message Body was streamed]" + export AWSException, ProtocolNotDefined, InvalidFileName, NoCredentials struct ProtocolNotDefined <: Exception @@ -27,18 +29,29 @@ struct AWSException <: Exception message::String info::Union{XMLDictElement,Dict,String,Nothing} cause::HTTP.StatusError + streamed_body::Union{String,Nothing} end + function Base.show(io::IO, e::AWSException) - message = isempty(e.message) ? "" : (" -- " * e.message) - return println(io, "AWSException: ", string(e.code, message, "\n", e.cause)) + print(io, AWSException, ": ", e.code) + !isempty(e.message) && print(io, " -- ", e.message) + print(io, "\n\n", e.cause) + e.streamed_body !== nothing && print(io, "\n\n", e.streamed_body) + println(io) + return nothing end -http_message(e::HTTP.StatusError) = String(copy(e.response.body)) http_status(e::HTTP.StatusError) = e.status content_type(e::HTTP.StatusError) = HTTP.header(e.response, "Content-Type") is_valid_xml_string(str) = startswith(str, '<') -function AWSException(e::HTTP.StatusError) +AWSException(e::HTTP.StatusError) = AWSException(e, String(copy(e.response.body))) + +function AWSException(e::HTTP.StatusError, body::IO) + return AWSException(e, String(take!(body))) +end + +function AWSException(e::HTTP.StatusError, body::AbstractString) code = string(http_status(e)) message = "AWSException" info = Dict{String,Dict}() @@ -50,7 +63,7 @@ function AWSException(e::HTTP.StatusError) # Extract API error code from JSON error message... if occursin(r"^application/x-amz-json-1\.[01]$", content_type(e)) - info = JSON.parse(http_message(e)) + info = JSON.parse(body) if haskey(info, "__type") code = rsplit(info["__type"], '#'; limit=2)[end] end @@ -58,8 +71,8 @@ function AWSException(e::HTTP.StatusError) # Extract API error code from XML error message... error_content_types = ["", "application/xml", "text/xml"] - if content_type(e) in error_content_types && is_valid_xml_string(http_message(e)) - info = parse_xml(http_message(e)) + if content_type(e) in error_content_types && is_valid_xml_string(body) + info = parse_xml(body) end # There are times when Errors or Error are returned back @@ -72,7 +85,9 @@ function AWSException(e::HTTP.StatusError) message = get(info, "Message", message) message = get(info, "message", message) - return AWSException(code, message, info, e) + streamed_body = e.response.body == BODY_STREAMED_PLACEHOLDER ? body : nothing + + return AWSException(code, message, info, e, streamed_body) end end diff --git a/src/utilities/credentials.jl b/src/utilities/credentials.jl index f5a469c328..46a597d584 100644 --- a/src/utilities/credentials.jl +++ b/src/utilities/credentials.jl @@ -5,9 +5,9 @@ function _can_read_file(file_name::String) false end end -_begins_with_ec2(file_name::String) = return uppercase(String(read(file_name, 3))) == "EC2" +_begins_with_ec2(file_name::String) = uppercase(String(Base.read(file_name, 3))) == "EC2" function _ends_with_ec2(file_name::String) - return endswith(strip(uppercase(read(file_name, String))), "EC2") + return endswith(strip(uppercase(Base.read(file_name, String))), "EC2") end """ @@ -49,7 +49,7 @@ end function _aws_profile_config(config_file::AbstractString, profile) isfile(config_file) || return Dict() - return _aws_profile_config(read(Inifile(), config_file), profile) + return _aws_profile_config(Base.read(Inifile(), config_file), profile) end function _aws_profile_config(config_file::Nothing, profile) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index b5e6bfab57..0ba1a1fe75 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -34,7 +34,6 @@ Base.@kwdef mutable struct Request url::String = "" http_options::AbstractDict{Symbol,<:Any} = LittleDict{Symbol,String}() - response_dict_type::Type{<:AbstractDict} = LittleDict backend::AbstractBackend = DEFAULT_BACKEND[] end @@ -48,7 +47,7 @@ Submit the request to AWS. - `request::Request`: All the information about making a request to AWS # Returns -- `Response`: A struct containing the response +- `AWS.Response`: A struct containing the response details """ function submit_request(aws::AbstractAWSConfig, request::Request) response = nothing @@ -70,10 +69,13 @@ function submit_request(aws::AbstractAWSConfig, request::Request) request.headers["User-Agent"] = user_agent[] request.headers["Host"] = HTTP.URI(request.url).host + # TODO: Why did we originally use Base.BufferStream for streaming data? + stream = IOBuffer() + @repeat 3 try credentials(aws) === nothing || sign!(aws, request) - response = @mock _http_request(request.backend, request) + response = @mock _http_request(request.backend, request, stream) if response.status in REDIRECT_ERROR_CODES if HTTP.header(response, "Location") != "" @@ -85,7 +87,7 @@ function submit_request(aws::AbstractAWSConfig, request::Request) end catch e if e isa HTTP.StatusError - e = AWSException(e) + e = AWSException(e, stream) end @retry if :message in fieldnames(typeof(e)) && @@ -127,14 +129,12 @@ function submit_request(aws::AbstractAWSConfig, request::Request) return response end -function _http_request(http_backend::HTTPBackend, request::Request) +function _http_request(http_backend::HTTPBackend, request::Request, response_stream::IO) http_options = merge(http_backend.http_options, request.http_options) @repeat 4 try http_stack = HTTP.stack(; redirect=false, retry=false, aws_authorization=false) - response_stream = BufferStream() - r = @mock HTTP.request( http_stack, request.request_method, @@ -146,7 +146,7 @@ function _http_request(http_backend::HTTPBackend, request::Request) http_options..., ) - return Response(r, response_stream) + return Response(r, io) catch e # Base.IOError is needed because HTTP.jl can often have errors that aren't # caught and wrapped in an HTTP.IOError diff --git a/src/utilities/response.jl b/src/utilities/response.jl index 2c429db883..4ae013f1ab 100644 --- a/src/utilities/response.jl +++ b/src/utilities/response.jl @@ -1,7 +1,7 @@ # TODO: Include only the fields we care about -struct Response +struct Response{B <: IO} response::HTTP.Response - body::BufferStream + body::B # I/O type must support `take!` end function Base.getproperty(r::Response, f::Symbol) @@ -14,7 +14,7 @@ end function mime_type(r::Response) # Parse response data according to mimetype... - mime = HTTP.header(r.response, "Content-Type", "") + mime = HTTP.header(r.response, "Content-Type", "application/octet-stream") # https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types @@ -23,53 +23,58 @@ function mime_type(r::Response) # "xml" # e.g. "application/xml" or "text/xml" - if occursin(r"/xml", mime) - MIME(Symbol("application/xml")) + T = if occursin(r"/xml", mime) + MIME"application/xml" elseif occursin(r"/x-amz-json-1.[01]$", mime) || endswith(mime, "json") - MIME(Symbol("application/json")) + MIME"application/json" elseif startswith(mime, "text/") - MIME(Symbol("text/plain")) + MIME"text/plain" else - MIME(Symbol("text/plain")) + MIME{Symbol(mime)} # Unhandled MIME type end + + return T end # TODO: Interface isn't perfect. For example "text/plain" completely ignores `T` -Base.parse(::Type{T}, r::Response) where T = parse(T, r, mime_type(r)) +function read(r::Response) + M = mime_type(r) + return AWS.read(r.body, M()) +end -function Base.parse(::Type{T}, r::Response, ::MIME"application/xml") where T <: AbstractDict - xml = parse_xml(String(r.body)) - root = XMLDict.root(xml.x) # TODO: Why x? - return xml_dict(root, T) +function read(io::IO, ::MIME"application/xml") + xml = parse_xml(String(take!(io))) + root = XMLDict.root(xml.x) # Drop XML declaration + return xml_dict(root, OrderedDict{Union{String,Symbol},Any}) end -function Base.parse(::Type{T}, r::Response, ::MIME"application/json") where T <: AbstractDict - return JSON.parse(r.body, dicttype=T) +function read(io::IO, ::MIME"application/json") + # Note: Using JSON instead of JSON3 does not support OrderedDict + return JSON.parse(take!(io), dicttype=OrderedDict{String,Any}) end -function Base.parse(::Type{T}, r::Response, ::MIME"text/plain") where T <: AbstractDict - return String(r.body) +function read(io::IO, ::MIME"text/plain") + return String(take!(io)) end -function legacy(r::Response; response_dict_type::Type, return_headers=false, return_stream=false, return_raw=false, response_stream=nothing) - # For HEAD request, return headers... - if r.response.request.method == "HEAD" - return response_dict_type(r.response.headers) - end +# Dict-like access - # Return response stream if requested... - if return_stream - return write(response_stream, read(r.body)) - end +function _dict(r::Response) + d = AWS.read(r)::AbstractDict # TODO: Could be more user friendly + return d +end - # Return raw data if requested... - if return_raw - content = read(r.body) - return return_headers ? (content, r.response.headers) : content - end +Base.getindex(r::Response, key::Union{AbstractString, Symbol}) = getindex(_dict(r), key) +Base.haskey(r::Response, key::Union{AbstractString, Symbol}) = haskey(_dict(r), key) +Base.keys(r::Response) = keys(_dict(r)) +Base.values(r::Response) = values(_dict(r)) + +Base.String(r::Response) = String(take!(r.body)) - content = parse(response_dict_type, r) - return return_headers ? (content, r.response.headers) : content +function Base.show(io::IO, m::MIME"text/plain", r::Response) + println(io, "$(Response): $(mime_type(r)()) interpreted as:") + content = AWS.read(r) + show(io, m, content) end diff --git a/src/utilities/utilities.jl b/src/utilities/utilities.jl index 5e6f571d29..f85a6d9e27 100644 --- a/src/utilities/utilities.jl +++ b/src/utilities/utilities.jl @@ -68,11 +68,19 @@ function _extract_common_kw_args(service, args) api_version=service.api_version, headers=LittleDict{String,String}(_pop!(args, "headers", [])), http_options=_pop!(args, "http_options", LittleDict{Symbol,String}()), - response_dict_type=_pop!(args, "response_dict_type", LittleDict), backend=_pop!(args, "backend", DEFAULT_BACKEND[]), ) end +function _delete_outdated_kw_args!(args) + delete!(args, "return_headers") + delete!(args, "return_stream") + delete!(args, "return_raw") + delete!(args, "response_stream") + delete!(args, "response_dict_type") + return args +end + # Use this until the three arg pop! is available for LittleDict # https://github.com/JuliaCollections/OrderedCollections.jl/pull/59 function _pop!(dict::AbstractDict{String,<:Any}, kw, default) diff --git a/test/AWS.jl b/test/AWS.jl index 24247aeeed..aa5e61ffa9 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -116,29 +116,11 @@ end @testset "submit_request" begin aws = AWS.AWSConfig() - function _expected_body(body::AbstractString, dict_type::Type) + function _expected_xml(body::AbstractString, dict_type::Type) parsed = parse_xml(body) return xml_dict(XMLDict.root(parsed.x), dict_type) end - @testset "HEAD request method" begin - expected_result_type = LittleDict - - request = Request(; - service="s3", - api_version="api_version", - request_method="HEAD", - url="https://s3.us-east-1.amazonaws.com/sample-bucket", - ) - - apply(Patches._aws_http_request_patch()) do - response = AWS.submit_request(aws, request) - - @test response.headers == Patches.headers - @test response.headers isa Vector - end - end - @testset "301 redirect" begin request = Request(; service="s3", @@ -151,39 +133,53 @@ end end end - @testset "return stream" begin + @testset "HEAD response" begin request = Request(; service="s3", api_version="api_version", - request_method="GET", + request_method="HEAD", url="https://s3.us-east-1.amazonaws.com/sample-bucket", ) - apply(Patches._aws_http_request_patch()) do - response = AWS.submit_request(aws, request) - - @test response.body isa Base.BufferStream + response = apply(Patches._aws_http_request_patch()) do + AWS.submit_request(aws, request) end + + # Access to response headers + @test response.response.headers == Patches.headers + @test response.response.headers isa Vector + + # Access to streaming content + @test response.body isa IO + + # Content as a string + @test isempty(String(take!(response.body))) end - @testset "return raw" begin + @testset "GET response" begin request = Request(; service="s3", api_version="api_version", request_method="GET", url="https://s3.us-east-1.amazonaws.com/sample-bucket", - return_raw=true, ) - apply(Patches._aws_http_request_patch()) do - response = AWS.submit_request(aws, request) - - @test String(response.body) == Patches.body - @test response.headers == Patches.headers + response = apply(Patches._aws_http_request_patch()) do + AWS.submit_request(aws, request) end + + # Access to response headers + @test response.response.headers == Patches.headers + @test response.response.headers isa Vector + + # Access to streaming content + @test response.body isa IO + + # Content as a string + @test String(take!(response.body)) == Patches.body end - @testset "MIME" begin + @testset "read MIME-type" begin request = Request(; service="s3", api_version="api_version", @@ -191,99 +187,91 @@ end url="https://s3.us-east-1.amazonaws.com/sample-bucket", ) - @testset "empty" begin - @testset "default" begin - expected_body = "" - expected_headers = Pair["Content-Type" => ""] + @testset "invalid content type" begin + headers = Pair["Content-Type" => ""] + body = "" - response = Patches._response(; headers=expected_headers, body=expected_body) - apply(Patches._aws_http_request_patch(response)) do - response = AWS.submit_request(aws, request) - - @test String(response.body) == expected_body - @test response.headers == expected_headers - end + r = Patches._response(headers=headers, body=body) + response = apply(Patches._aws_http_request_patch(r)) do + AWS.submit_request(aws, request) end - @testset "text/xml" begin - expected_body_type = LittleDict{Union{Symbol,String},Any} - expected_body = _expected_body(Patches.body, expected_body_type) - expected_headers = Pair["Content-Type" => "text/xml"] + @test_throws MethodError AWS.read(response) + end - response = Patches._response(; headers=expected_headers) - apply(Patches._aws_http_request_patch(response)) do - response = AWS.submit_request(aws, request) + @testset "text/xml" begin + headers = Pair["Content-Type" => "text/xml"] + expected_body_type = OrderedDict{Union{String,Symbol},Any} + expected_body = _expected_xml(Patches.body, expected_body_type) - @test body isa expected_body_type - @test body == expected_body - @test response.headers == expected_headers - @test response.headers isa Vector - end + r = Patches._response(headers=headers) + response = apply(Patches._aws_http_request_patch(r)) do + AWS.submit_request(aws, request) end - end - @testset "xml" begin - expected_body_type = LittleDict{Union{Symbol,String},Any} - expected_body = _expected_body(Patches.body, expected_body_type) - expected_headers = Pair["Content-Type" => "application/xml"] + content = AWS.read(response) + @test content isa expected_body_type + @test content == expected_body + end - response = Patches._response(; headers=expected_headers) - apply(Patches._aws_http_request_patch(response)) do - response = AWS.submit_request(aws, request) - body = parse(expected_body_type, response) + @testset "application/xml" begin + headers = Pair["Content-Type" => "application/xml"] + expected_body_type = OrderedDict{Union{String,Symbol},Any} + expected_body = _expected_xml(Patches.body, expected_body_type) - @test body isa expected_body_type - @test body == expected_body - @test response.headers isa Vector - @test response.headers == expected_headers + r = Patches._response(headers=headers) + response = apply(Patches._aws_http_request_patch(r)) do + AWS.submit_request(aws, request) end + + content = AWS.read(response) + @test content isa expected_body_type + @test content == expected_body end - @testset "JSON" begin - json_headers = ["Content-Type" => "application/json"] - body = Dict{String,Any}( - "Marker" => nothing, - "VaultList" => Any[Dict{String,Any}( - "VaultName" => "test", - "SizeInBytes" => 0, - "NumberOfArchives" => 0, - "CreationDate" => "2020-06-22T03:14:41.754Z", - "VaultARN" => "arn:aws:glacier:us-east-1:000:vaults/test", - "LastInventoryDate" => nothing, - )], + @testset "application/json" begin + headers = ["Content-Type" => "application/json"] + body = JSON.json( + Dict{String,Any}( + "Marker" => nothing, + "VaultList" => Any[ + Dict{String,Any}( + "VaultName" => "test", + "SizeInBytes" => 0, + "NumberOfArchives" => 0, + "CreationDate" => "2020-06-22T03:14:41.754Z", + "VaultARN" => "arn:aws:glacier:us-east-1:000:vaults/test", + "LastInventoryDate" => nothing, + ), + ], + ), ) - json_body = JSON.json(body) - - expected_body_type = LittleDict{String,Any} - expected_body = JSON.parse(json_body; dicttype=LittleDict) - response = Patches._response(; body=json_body, headers=json_headers) - apply(Patches._aws_http_request_patch(response)) do - response = AWS.submit_request(aws, request) - body = parse(expected_body_type, response) + expected_body_type = OrderedDict{String,Any} + expected_body = JSON.parse(body, dicttype=expected_body_type) - @test body isa expected_body_type - @test body == expected_body - @test response.headers isa Vector - @test response.headers == expected_headers + r = Patches._response(body=body, headers=headers) + response = apply(Patches._aws_http_request_patch(r)) do + AWS.submit_request(aws, request) end + + content = AWS.read(response) + @test content isa expected_body_type + @test content == expected_body end - @testset "Text" begin - expected_headers = ["Content-Type" => "text/html"] + @testset "text/html" begin + headers = ["Content-Type" => "text/html"] expected_body = Patches.body - expected_header_type = LittleDict{SubString{String},SubString{String}} - - response = Patches._response(; headers=expected_headers) - apply(Patches._aws_http_request_patch(response)) do - response = AWS.submit_request(aws, request) - body = String(response.body) - @test body isa String - @test body == expected_body - @test response.headers isa Vector - @test response.headers == expected_headers + r = Patches._response(headers=headers) + response = apply(Patches._aws_http_request_patch(r)) do + AWS.submit_request(aws, request) end + + content = AWS.read(response) + @test content isa String + @test content == expected_body end end end @@ -763,10 +751,8 @@ end @test length([result["Contents"]]) == max_keys # GET with an IO target - mktemp() do f, io - S3.get_object(bucket_name, file_name, Dict("response_stream" => io)) - @test read(f, String) == body - end + response = S3.get_object(bucket_name, file_name) + @test read(response.body, String) == body finally # DELETE with parameters operation S3.delete_object(bucket_name, file_name) diff --git a/test/issues.jl b/test/issues.jl index 469e66be4c..002e00f4f9 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -54,6 +54,7 @@ end end end +# TODO: Evaluate usefulness of this test @testset "issue 324" begin body = "Hello World!" file_name = "streaming.bin" @@ -61,20 +62,13 @@ end try S3.create_bucket(bucket_name) S3.put_object(bucket_name, file_name, Dict("body" => body)) - resp = S3.get_object(bucket_name, file_name) - @test String(resp) == body + response = S3.get_object(bucket_name, file_name) + @test String(response) == body # ERROR: MethodError: no method matching iterate(::Base.BufferStream) # => BUG: header `response_stream` is pushed into the query... - io = Base.BufferStream() - S3.get_object( - bucket_name, file_name, Dict("response_stream" => io, "return_stream" => true) - ) - if bytesavailable(io) > 0 - @test String(readavailable(io)) == body - else - @test "no body data was available" == body - end + response = S3.get_object(bucket_name, file_name) + @test String(take!(response.body)) == body finally S3.delete_object(bucket_name, file_name) diff --git a/test/minio.jl b/test/minio.jl index 4194284ee5..12bcfb9a88 100644 --- a/test/minio.jl +++ b/test/minio.jl @@ -37,10 +37,8 @@ S3.put_object("anewbucket", "foo/baz", Dict("body" => "a secondnested object")) @test String(S3.get_object("anewbucket", "myobject")) == "Hi from Minio" # Test retrieving an object into a stream target -mktemp() do f, io - S3.get_object("anewbucket", "myobject", Dict("response_stream" => io)) - @test read(f, String) == "Hi from Minio" -end +response = S3.get_object("anewbucket", "myobject") +@test read(response.body, String) == "Hi from Minio" # Test listing objs = S3.list_objects_v2("anewbucket") diff --git a/test/patch.jl b/test/patch.jl index f79e2394e9..ba7679cc4a 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -55,14 +55,13 @@ function _response(; response.headers = headers response.body = b"[Message Body was streamed]" - b = Base.BufferStream() - write(b, body) + b = IOBuffer(body) return AWS.Response(response, b) end function _aws_http_request_patch(response::AWS.Response=_response()) - return @patch AWS._http_request(::AWS.AbstractBackend, request::Request) = response + return @patch AWS._http_request(::AWS.AbstractBackend, request::Request, ::IO) = response end _cred_file_patch = @patch function dot_aws_credentials_file() From bb045bfa64ce5b0e20d8236ec5c12bfc05835dff Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 08:50:29 -0500 Subject: [PATCH 05/10] Improve AWSException when streamed body is empty --- src/AWSExceptions.jl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/AWSExceptions.jl b/src/AWSExceptions.jl index 1fb16ef1cd..db76e0c710 100644 --- a/src/AWSExceptions.jl +++ b/src/AWSExceptions.jl @@ -36,7 +36,16 @@ function Base.show(io::IO, e::AWSException) print(io, AWSException, ": ", e.code) !isempty(e.message) && print(io, " -- ", e.message) print(io, "\n\n", e.cause) - e.streamed_body !== nothing && print(io, "\n\n", e.streamed_body) + + if e.streamed_body !== nothing + print(io, "\n\n") + if isempty(e.streamed_body) + printstyled(io, "(empty body)"; bold=true) + else + print(io, e.streamed_body) + end + end + println(io) return nothing end From 2857098ce92ce5cfb9888a4bbb31f9cd40863264 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 08:59:00 -0500 Subject: [PATCH 06/10] Formatter changes --- src/utilities/response.jl | 18 +++++++----------- test/AWS.jl | 30 ++++++++++++++---------------- test/patch.jl | 3 ++- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/utilities/response.jl b/src/utilities/response.jl index 4ae013f1ab..59c757f895 100644 --- a/src/utilities/response.jl +++ b/src/utilities/response.jl @@ -1,5 +1,5 @@ # TODO: Include only the fields we care about -struct Response{B <: IO} +struct Response{B<:IO} response::HTTP.Response body::B # I/O type must support `take!` end @@ -36,9 +36,6 @@ function mime_type(r::Response) return T end - -# TODO: Interface isn't perfect. For example "text/plain" completely ignores `T` - function read(r::Response) M = mime_type(r) return AWS.read(r.body, M()) @@ -52,7 +49,7 @@ end function read(io::IO, ::MIME"application/json") # Note: Using JSON instead of JSON3 does not support OrderedDict - return JSON.parse(take!(io), dicttype=OrderedDict{String,Any}) + return JSON.parse(take!(io); dicttype=OrderedDict{String,Any}) end function read(io::IO, ::MIME"text/plain") @@ -61,13 +58,11 @@ end # Dict-like access -function _dict(r::Response) - d = AWS.read(r)::AbstractDict # TODO: Could be more user friendly - return d -end +# TODO: Could be more user friendly +_dict(r::Response) = AWS.read(r)::AbstractDict -Base.getindex(r::Response, key::Union{AbstractString, Symbol}) = getindex(_dict(r), key) -Base.haskey(r::Response, key::Union{AbstractString, Symbol}) = haskey(_dict(r), key) +Base.getindex(r::Response, key::Union{AbstractString,Symbol}) = getindex(_dict(r), key) +Base.haskey(r::Response, key::Union{AbstractString,Symbol}) = haskey(_dict(r), key) Base.keys(r::Response) = keys(_dict(r)) Base.values(r::Response) = values(_dict(r)) @@ -77,4 +72,5 @@ function Base.show(io::IO, m::MIME"text/plain", r::Response) println(io, "$(Response): $(mime_type(r)()) interpreted as:") content = AWS.read(r) show(io, m, content) + return nothing end diff --git a/test/AWS.jl b/test/AWS.jl index aa5e61ffa9..9293c6a182 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -191,7 +191,7 @@ end headers = Pair["Content-Type" => ""] body = "" - r = Patches._response(headers=headers, body=body) + r = Patches._response(; headers=headers, body=body) response = apply(Patches._aws_http_request_patch(r)) do AWS.submit_request(aws, request) end @@ -204,7 +204,7 @@ end expected_body_type = OrderedDict{Union{String,Symbol},Any} expected_body = _expected_xml(Patches.body, expected_body_type) - r = Patches._response(headers=headers) + r = Patches._response(; headers=headers) response = apply(Patches._aws_http_request_patch(r)) do AWS.submit_request(aws, request) end @@ -219,7 +219,7 @@ end expected_body_type = OrderedDict{Union{String,Symbol},Any} expected_body = _expected_xml(Patches.body, expected_body_type) - r = Patches._response(headers=headers) + r = Patches._response(; headers=headers) response = apply(Patches._aws_http_request_patch(r)) do AWS.submit_request(aws, request) end @@ -234,23 +234,21 @@ end body = JSON.json( Dict{String,Any}( "Marker" => nothing, - "VaultList" => Any[ - Dict{String,Any}( - "VaultName" => "test", - "SizeInBytes" => 0, - "NumberOfArchives" => 0, - "CreationDate" => "2020-06-22T03:14:41.754Z", - "VaultARN" => "arn:aws:glacier:us-east-1:000:vaults/test", - "LastInventoryDate" => nothing, - ), - ], + "VaultList" => Any[Dict{String,Any}( + "VaultName" => "test", + "SizeInBytes" => 0, + "NumberOfArchives" => 0, + "CreationDate" => "2020-06-22T03:14:41.754Z", + "VaultARN" => "arn:aws:glacier:us-east-1:000:vaults/test", + "LastInventoryDate" => nothing, + )], ), ) expected_body_type = OrderedDict{String,Any} - expected_body = JSON.parse(body, dicttype=expected_body_type) + expected_body = JSON.parse(body; dicttype=expected_body_type) - r = Patches._response(body=body, headers=headers) + r = Patches._response(; body=body, headers=headers) response = apply(Patches._aws_http_request_patch(r)) do AWS.submit_request(aws, request) end @@ -264,7 +262,7 @@ end headers = ["Content-Type" => "text/html"] expected_body = Patches.body - r = Patches._response(headers=headers) + r = Patches._response(; headers=headers) response = apply(Patches._aws_http_request_patch(r)) do AWS.submit_request(aws, request) end diff --git a/test/patch.jl b/test/patch.jl index ba7679cc4a..ab105c4211 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -61,7 +61,8 @@ function _response(; end function _aws_http_request_patch(response::AWS.Response=_response()) - return @patch AWS._http_request(::AWS.AbstractBackend, request::Request, ::IO) = response + p = @patch AWS._http_request(::AWS.AbstractBackend, request::Request, ::IO) = response + return p end _cred_file_patch = @patch function dot_aws_credentials_file() From 50bd6aa5433c89677a564feea655c1f42e2bcdeb Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 10:16:30 -0500 Subject: [PATCH 07/10] Test fixes --- src/utilities/downloads_backend.jl | 18 ++++++------------ src/utilities/request.jl | 2 +- test/AWS.jl | 2 +- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 150d185815..2b841e3974 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -35,10 +35,10 @@ body_length(x::AbstractString) = sizeof(x) read_body(x::IOBuffer) = take!(x) function read_body(x::IO) close(x) - return read(x) + return Base.read(x) end -function _http_request(backend::DownloadsBackend, request) +function _http_request(backend::DownloadsBackend, request::Request, response_stream::IO) # If we pass `output`, older versions of Downloads.jl will # expect a message body in the response. Specifically, it sets # @@ -55,23 +55,16 @@ function _http_request(backend::DownloadsBackend, request) # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. # (Note: this is fixed on the latest Downloads.jl, but we include this workaround # for compatability). - if request.response_stream === nothing - request.response_stream = IOBuffer() - end output_arg = if request.request_method == "HEAD" NamedTuple() else - (; output=request.response_stream) + (; output=response_stream) end # If we're going to return the stream, we don't want to read the body into an # HTTP.Response we're never going to use. If we do that, the returned stream # will have no data available (and reading from it could hang forever). - body_arg = if request.request_method == "HEAD" || request.return_stream - () -> NamedTuple() - else - () -> (; body=read_body(request.response_stream)) - end + body_arg = () -> NamedTuple() # HTTP.jl sets this header automatically. request.headers["Content-Length"] = string(body_length(request.content)) @@ -122,7 +115,8 @@ function _http_request(backend::DownloadsBackend, request) ), ) end - return http_response + + return AWS.Response(http_response, response_stream) catch e @delay_retry if ( (isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500) || diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 0ba1a1fe75..7c25ca0db2 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -146,7 +146,7 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str http_options..., ) - return Response(r, io) + return Response(r, response_stream) catch e # Base.IOError is needed because HTTP.jl can often have errors that aren't # caught and wrapped in an HTTP.IOError diff --git a/test/AWS.jl b/test/AWS.jl index 9293c6a182..dbd306a463 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -278,7 +278,7 @@ struct TestBackend <: AWS.AbstractBackend param::Int end -function AWS._http_request(backend::TestBackend, request) +function AWS._http_request(backend::TestBackend, request::AWS.Request, response_stream::IO) return backend.param end From b96b5a94869c5ac266e2a0d0e43d0e62ab121246 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 10:16:40 -0500 Subject: [PATCH 08/10] HEAD returns content --- test/AWS.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/AWS.jl b/test/AWS.jl index dbd306a463..e2e47504b5 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -153,7 +153,7 @@ end @test response.body isa IO # Content as a string - @test isempty(String(take!(response.body))) + @test String(take!(response.body)) == Patches.body end @testset "GET response" begin From 0b6ad1e1675acaccdcec8e1d217a71c2f847cc52 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 11:34:17 -0500 Subject: [PATCH 09/10] Fix JSON processing --- src/utilities/response.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/response.jl b/src/utilities/response.jl index 59c757f895..00b66b273b 100644 --- a/src/utilities/response.jl +++ b/src/utilities/response.jl @@ -49,7 +49,7 @@ end function read(io::IO, ::MIME"application/json") # Note: Using JSON instead of JSON3 does not support OrderedDict - return JSON.parse(take!(io); dicttype=OrderedDict{String,Any}) + return JSON.parse(String(take!(io)); dicttype=OrderedDict{String,Any}) end function read(io::IO, ::MIME"text/plain") From d556cf6c37a65ed270681c9a1eaaad35c4fcfa71 Mon Sep 17 00:00:00 2001 From: Curtis Vogt Date: Fri, 3 Sep 2021 11:34:41 -0500 Subject: [PATCH 10/10] Update HTTPBackend tests --- src/utilities/request.jl | 2 +- test/AWS.jl | 22 ++++++++++++---------- test/patch.jl | 17 +++++++++-------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 7c25ca0db2..a3215ac04c 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -146,7 +146,7 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str http_options..., ) - return Response(r, response_stream) + return @mock Response(r, response_stream) catch e # Base.IOError is needed because HTTP.jl can often have errors that aren't # caught and wrapped in an HTTP.IOError diff --git a/test/AWS.jl b/test/AWS.jl index e2e47504b5..f8a1038203 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -278,7 +278,7 @@ struct TestBackend <: AWS.AbstractBackend param::Int end -function AWS._http_request(backend::TestBackend, request::AWS.Request, response_stream::IO) +function AWS._http_request(backend::TestBackend, ::AWS.Request, ::IO) return backend.param end @@ -290,28 +290,30 @@ end url="https://s3.us-east-1.amazonaws.com/sample-bucket", backend=AWS.HTTPBackend(), ) - apply(Patches._http_options_patch) do + io = IOBuffer() + + apply(Patches._http_options_patches) do # No default options - @test isempty(AWS._http_request(request.backend, request)) + @test isempty(AWS._http_request(request.backend, request, io)) # We can pass HTTP options via the backend custom_backend = AWS.HTTPBackend(Dict(:connection_limit => 5)) @test custom_backend isa AWS.AbstractBackend - @test AWS._http_request(custom_backend, request) == Dict(:connection_limit => 5) + @test AWS._http_request(custom_backend, request, io) == Dict(:connection_limit => 5) # We can pass options per-request request.http_options = Dict(:pipeline_limit => 20) - @test AWS._http_request(request.backend, request) == Dict(:pipeline_limit => 20) - @test AWS._http_request(custom_backend, request) == + @test AWS._http_request(request.backend, request, io) == Dict(:pipeline_limit => 20) + @test AWS._http_request(custom_backend, request, io) == Dict(:pipeline_limit => 20, :connection_limit => 5) # per-request options override backend options: custom_backend = AWS.HTTPBackend(Dict(:pipeline_limit => 5)) - @test AWS._http_request(custom_backend, request) == Dict(:pipeline_limit => 20) + @test AWS._http_request(custom_backend, request, io) == Dict(:pipeline_limit => 20) end request.backend = TestBackend(2) - @test AWS._http_request(request.backend, request) == 2 + @test AWS._http_request(request.backend, request, io) == 2 request = Request(; service="s3", @@ -320,7 +322,7 @@ end url="https://s3.us-east-1.amazonaws.com/sample-bucket", backend=TestBackend(4), ) - @test AWS._http_request(request.backend, request) == 4 + @test AWS._http_request(request.backend, request, io) == 4 # Let's test setting the default backend prev_backend = AWS.DEFAULT_BACKEND[] @@ -332,7 +334,7 @@ end request_method="GET", url="https://s3.us-east-1.amazonaws.com/sample-bucket", ) - @test AWS._http_request(request.backend, request) == 3 + @test AWS._http_request(request.backend, request, io) == 3 finally AWS.DEFAULT_BACKEND[] = prev_backend end diff --git a/test/patch.jl b/test/patch.jl index ab105c4211..3dfc94166e 100644 --- a/test/patch.jl +++ b/test/patch.jl @@ -118,11 +118,12 @@ end # except `require_ssl_verification` and `response_stream`. This is used to # test which other options are being passed to `HTTP.Request` inside of # `_http_request`. -_http_options_patch = @patch function HTTP.request(args...; kwargs...) - options = Dict(kwargs) - delete!(options, :require_ssl_verification) - delete!(options, :response_stream) - return options -end - -end +_http_options_patches = [ + @patch function HTTP.request(args...; kwargs...) + options = Dict(kwargs) + delete!(options, :require_ssl_verification) + delete!(options, :response_stream) + return options + end + @patch AWS.Response(options, args...) = options +]