Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ defmodule CouchDBTest.Mixfile do
# Run "mix help deps" to learn about dependencies.
defp deps() do
[
{:httpotion, "~> 3.0", only: [:dev, :test, :integration], runtime: false},
{:httpotion, ">= 3.1.3", only: [:dev, :test, :integration], runtime: false},
{:jiffy, path: Path.expand("src/jiffy", __DIR__)},
{:ibrowse,
path: Path.expand("src/ibrowse", __DIR__), override: true, compile: false},
Expand Down
2 changes: 1 addition & 1 deletion mix.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
%{
"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
"credo": {:hex, :credo, "1.0.5", "fdea745579f8845315fe6a3b43e2f9f8866839cfbc8562bb72778e9fdaa94214", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"},
"httpotion": {:hex, :httpotion, "3.1.2", "50e3e559c2ffe8c8908c97e4ffb01efc1c18e8547cc7ce5dd173c9cf0a573a3b", [:mix], [{:ibrowse, "== 4.4.0", [hex: :ibrowse, repo: "hexpm", optional: false]}], "hexpm"},
"httpotion": {:hex, :httpotion, "3.1.3", "fdaf1e16b9318dcb722de57e75ac368c93d4c6e3c9125f93e960f953a750fb77", [:mix], [{:ibrowse, "== 4.4.0", [hex: :ibrowse, repo: "hexpm", optional: false]}], "hexpm"},
"ibrowse": {:hex, :ibrowse, "4.4.0", "2d923325efe0d2cb09b9c6a047b2835a5eda69d8a47ed6ff8bc03628b764e991", [:rebar3], [], "hexpm"},
"jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"},
"jiffy": {:hex, :jiffy, "0.15.2", "de266c390111fd4ea28b9302f0bc3d7472468f3b8e0aceabfbefa26d08cd73b7", [:rebar3], [], "hexpm"},
Expand Down
2 changes: 1 addition & 1 deletion src/chttpd/src/chttpd.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1302,7 +1302,7 @@ get_trace_headers(MochiReq) ->
parse_span_id(MochiReq:get_header_value("X-B3-ParentSpanId"))
];
Value ->
case binary:split(Value, <<"-">>, [global]) of
case string:split(Value, "-", all) of
[TraceIdStr, SpanIdStr, _SampledStr, ParentSpanIdStr] ->
[
parse_trace_id(TraceIdStr),
Expand Down
2 changes: 2 additions & 0 deletions src/chttpd/test/exunit/test_helper.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ExUnit.configure(formatters: [JUnitFormatter, ExUnit.CLIFormatter])
ExUnit.start()
101 changes: 101 additions & 0 deletions src/chttpd/test/exunit/tracing_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
defmodule Couch.Test.OpenTracing do
use Couch.Test.ExUnit.Case
alias Couch.Test.Setup
alias Couch.Test.Setup.Step
alias Couch.Test.Utils
import Couch.DBTest, only: [retry_until: 1]

defp create_admin(user_name, password) do
hashed = String.to_charlist(:couch_passwords.hash_admin_password(password))
:config.set('admins', String.to_charlist(user_name), hashed, false)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like the create_admin function along with some of the other function below would be useful to have in the shared Couch library. Any particular reason to not put these functions into the couch.ex module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Few concerns:

  • I tried that before [1] and [2]
    • the complain was: we are developing our own framework which people need to learn.
  • Adding create_admin to Couch exibit a chicken and egg problem. Couch implements HTTP interface. We cannot use HTTP interface without adding admin first. In context of eunit tests we always cheated and created admin via config manipulation.
  • We could consider create_admin an exception and still add it to Couch inspite of it not being HTTP based interface. However it will introduce a confusion. People use Couch module from elixir integration tests and the function wouldn't work in the context of an integration test.

When I was writing this test I considered using Step.User.new(:admin, roles: [:server_admin]). However since pluguble adapters [1] didn't make it to the framework and [2] doesn't have fabric2 support, so I had to do it differently. I followed the advise of the committers who believe that DRY is bad for tests and they should be as self contained as possible.


[1]: #2036 - plugable adaptors based approach
[2]: #2039 - fabric only interface

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhhh ok, thanks for the detailed explanation @iilyak 👍. It's outside the scope of this PR, but I wonder if it would make sense to add a tag like @tag :with_admin like we've done with dbs and what not [1], to inject an admin user context into the test and then we can abstract away whether the admin user was created through config or the http interface. As for framework vs anti DRY, initially with the Elixir suite we tried to avoid making a heavy framework, which I think is worthwhile, but at the same time there's common activities which should be easy to take care of, like creating dbs/docs/users/ddocs/etc.

[1] https://github.com/apache/couchdb/blob/master/test/elixir/test/security_validation_test.exs#L151-L153


defp base_url() do
addr = :config.get('chttpd', 'bind_address', '127.0.0.1')
port = :mochiweb_socket_server.get(:chttpd, :port)
"http://#{addr}:#{port}"
end

setup_all context do
test_ctx = :test_util.start_couch([:chttpd])
:ok = create_admin("adm", "pass")

Map.merge(context, %{
base_url: base_url(),
user: "adm",
pass: "pass"
})
end

setup context do
db_name = Utils.random_name("db")
session = Couch.login(context.base_url, context.user, context.pass)

on_exit(fn ->
delete_db(session, db_name)
end)

create_db(session, db_name)

Map.merge(context, %{
db_name: db_name,
session: session
})
end

def create_db(session, db_name, opts \\ []) do
retry_until(fn ->
resp = Couch.Session.put(session, "/#{db_name}", opts)
assert resp.status_code in [201, 202]
assert resp.body == %{"ok" => true}
{:ok, resp}
end)
end

def delete_db(session, db_name) do
retry_until(fn ->
resp = Couch.Session.delete(session, "/#{db_name}")
assert resp.status_code in [200, 202, 404]
{:ok, resp}
end)
end

def create_doc(session, db_name, body) do
retry_until(fn ->
resp = Couch.Session.post(session, "/#{db_name}", body: body)
assert resp.status_code in [201, 202]
assert resp.body["ok"]
{:ok, resp}
end)
end

defp trace_id() do
:couch_util.to_hex(:crypto.strong_rand_bytes(16))
end

defp span_id() do
:couch_util.to_hex(:crypto.strong_rand_bytes(8))
end

describe "Open Tracing" do
test "should return success with combined b3 header", ctx do
%{session: session, db_name: db_name} = ctx
doc = '{"mr": "rockoartischocko"}'
{:ok, _} = create_doc(session, db_name, doc)

resp =
retry_until(fn ->
b3 = "#{trace_id()}-#{span_id()}-#{span_id()}"

response =
Couch.Session.get(session, "/#{db_name}/_all_docs", headers: [b3: b3])

assert %HTTPotion.Response{} = response
response
end)

assert resp.status_code == 200, "Expected 200, got: #{resp.status_code}"
assert length(resp.body["rows"]) == 1
end
end
end
124 changes: 15 additions & 109 deletions test/elixir/lib/couch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Couch.Session do
"""

@enforce_keys [:cookie]
defstruct [:cookie]
defstruct [:cookie, :base_url]

def new(cookie) do
%Couch.Session{cookie: cookie}
Expand Down Expand Up @@ -34,6 +34,7 @@ defmodule Couch.Session do

def go(%Couch.Session{} = sess, method, url, opts) do
opts = Keyword.merge(opts, cookie: sess.cookie)
opts = Keyword.merge(opts, base_url: sess.base_url)
Couch.request(method, url, opts)
end

Expand All @@ -54,12 +55,17 @@ defmodule Couch do
url
end

def process_url(url) do
base_url = System.get_env("EX_COUCH_URL") || "http://127.0.0.1:15984"
def process_url(url, options) do
base_url = case Keyword.get(options, :base_url) do
nil ->
System.get_env("EX_COUCH_URL") || "http://127.0.0.1:15984"
base_url ->
base_url
end
base_url <> url
end

def process_request_headers(headers, options) do
def process_request_headers(headers, _body, options) do
headers = Keyword.put(headers, :"User-Agent", "couch-potion")

headers =
Expand Down Expand Up @@ -114,116 +120,16 @@ defmodule Couch do

def login(userinfo) do
[user, pass] = String.split(userinfo, ":", parts: 2)
login(user, pass)
login(nil, user, pass)
end

def login(user, pass) do
resp = Couch.post("/_session", body: %{:username => user, :password => pass})
def login(base_url, user, pass) do
resp = Couch.post("/_session",
body: %{:username => user, :password => pass}, base_url: base_url)
true = resp.body["ok"]
cookie = resp.headers[:"set-cookie"]
[token | _] = String.split(cookie, ";")
%Couch.Session{cookie: token}
end

# HACK: this is here until this commit lands in a release
# https://github.com/myfreeweb/httpotion/commit/f3fa2f0bc3b9b400573942b3ba4628b48bc3c614
def handle_response(response) do
case response do
{:ok, status_code, headers, body, _} ->
processed_headers = process_response_headers(headers)

%HTTPotion.Response{
status_code: process_status_code(status_code),
headers: processed_headers,
body: process_response_body(processed_headers, body)
}

{:ok, status_code, headers, body} ->
processed_headers = process_response_headers(headers)

%HTTPotion.Response{
status_code: process_status_code(status_code),
headers: processed_headers,
body: process_response_body(processed_headers, body)
}

{:ibrowse_req_id, id} ->
%HTTPotion.AsyncResponse{id: id}

{:error, {:conn_failed, {:error, reason}}} ->
%HTTPotion.ErrorResponse{message: error_to_string(reason)}

{:error, :conn_failed} ->
%HTTPotion.ErrorResponse{message: "conn_failed"}

{:error, reason} ->
%HTTPotion.ErrorResponse{message: error_to_string(reason)}
end
%Couch.Session{cookie: token, base_url: base_url}
end

# Anther HACK: Until we can get process_request_headers/2 merged
# upstream.
@spec process_arguments(atom, String.t(), [{atom(), any()}]) :: %{}
defp process_arguments(method, url, options) do
options = process_options(options)

body = Keyword.get(options, :body, "")

headers =
Keyword.merge(
Application.get_env(:httpotion, :default_headers, []),
Keyword.get(options, :headers, [])
)

timeout =
Keyword.get(
options,
:timeout,
Application.get_env(:httpotion, :default_timeout, 5000)
)

ib_options =
Keyword.merge(
Application.get_env(:httpotion, :default_ibrowse, []),
Keyword.get(options, :ibrowse, [])
)

follow_redirects =
Keyword.get(
options,
:follow_redirects,
Application.get_env(:httpotion, :default_follow_redirects, false)
)

ib_options =
if stream_to = Keyword.get(options, :stream_to),
do:
Keyword.put(
ib_options,
:stream_to,
spawn(__MODULE__, :transformer, [stream_to, method, url, options])
),
else: ib_options

ib_options =
if user_password = Keyword.get(options, :basic_auth) do
{user, password} = user_password
Keyword.put(ib_options, :basic_auth, {to_charlist(user), to_charlist(password)})
else
ib_options
end

%{
method: method,
url: url |> to_string |> process_url(options) |> to_charlist,
body: body |> process_request_body,
headers:
headers
|> process_request_headers(options)
|> Enum.map(fn {k, v} -> {to_charlist(k), to_charlist(v)} end),
timeout: timeout,
ib_options: ib_options,
follow_redirects: follow_redirects
}
end
end