Skip to content

Commit 051baf6

Browse files
author
rinpatch
authored
Postgres/MyXQL adapters: Do not fail storage_up if the user has access to an already-created database (#297)
Closes #296
1 parent fc6c7a5 commit 051baf6

4 files changed

Lines changed: 83 additions & 28 deletions

File tree

integration_test/myxql/storage_test.exs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ defmodule Ecto.Integration.StorageTest do
2626
run_mysql("DROP DATABASE #{params()[:database]};")
2727
end
2828

29-
def create_database do
29+
def create_database(grant_privileges_to \\ nil) do
3030
run_mysql("CREATE DATABASE #{params()[:database]};")
31+
if grant_privileges_to do
32+
run_mysql("GRANT ALL PRIVILEGES ON #{params()[:database]}.* to #{grant_privileges_to}")
33+
end
3134
end
3235

3336
def create_posts do
@@ -73,6 +76,20 @@ defmodule Ecto.Integration.StorageTest do
7376
drop_database()
7477
end
7578

79+
test "storage up with unprivileged user with access to the database" do
80+
unprivileged_params = Keyword.merge(params(),
81+
username: "unprivileged",
82+
password: "pass"
83+
)
84+
run_mysql("CREATE USER unprivileged IDENTIFIED BY 'pass'")
85+
refute Ecto.Adapters.MyXQL.storage_up(unprivileged_params) == :ok
86+
create_database("unprivileged")
87+
assert Ecto.Adapters.MyXQL.storage_up(unprivileged_params) == {:error, :already_up}
88+
after
89+
run_mysql("DROP USER unprivileged")
90+
drop_database()
91+
end
92+
7693
test "structure dump and load" do
7794
create_database()
7895
create_posts()

integration_test/pg/storage_test.exs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,14 @@ defmodule Ecto.Integration.StorageTest do
2727
run_psql("DROP DATABASE #{params()[:database]};")
2828
end
2929

30-
def create_database do
31-
run_psql("CREATE DATABASE #{params()[:database]};")
30+
def create_database(owner \\ nil) do
31+
query = "CREATE DATABASE #{params()[:database]}"
32+
query = if owner do
33+
query <> " OWNER #{owner};"
34+
else
35+
query <> ";"
36+
end
37+
run_psql(query)
3238
end
3339

3440
def create_posts do
@@ -74,6 +80,20 @@ defmodule Ecto.Integration.StorageTest do
7480
drop_database()
7581
end
7682

83+
test "storage up with unprivileged user with access to the database" do
84+
unprivileged_params = Keyword.merge(params(),
85+
username: "unprivileged",
86+
password: "pass"
87+
)
88+
run_psql("CREATE USER unprivileged WITH NOCREATEDB PASSWORD 'pass'")
89+
refute Postgres.storage_up(unprivileged_params) == :ok
90+
create_database("unprivileged")
91+
assert Postgres.storage_up(unprivileged_params) == {:error, :already_up}
92+
after
93+
run_psql("DROP USER unprivileged")
94+
drop_database()
95+
end
96+
7797
test "structure dump and load" do
7898
create_database()
7999
create_posts()

lib/ecto/adapters/myxql.ex

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,19 +159,25 @@ defmodule Ecto.Adapters.MyXQL do
159159
opts = Keyword.delete(opts, :database)
160160
charset = opts[:charset] || "utf8mb4"
161161

162-
command =
163-
~s(CREATE DATABASE `#{database}` DEFAULT CHARACTER SET = #{charset})
164-
|> concat_if(opts[:collation], &"DEFAULT COLLATE = #{&1}")
165-
166-
case run_query(command, opts) do
167-
{:ok, _} ->
168-
:ok
169-
{:error, %{mysql: %{name: :ER_DB_CREATE_EXISTS}}} ->
162+
check_existence_command = "SELECT TRUE FROM information_schema.schemata WHERE schema_name = '#{database}'"
163+
case run_query(check_existence_command, opts) do
164+
{:ok, %{num_rows: 1}} ->
170165
{:error, :already_up}
171-
{:error, error} ->
172-
{:error, Exception.message(error)}
173-
{:exit, exit} ->
174-
{:error, exit_to_exception(exit)}
166+
_ ->
167+
create_command =
168+
~s(CREATE DATABASE `#{database}` DEFAULT CHARACTER SET = #{charset})
169+
|> concat_if(opts[:collation], &"DEFAULT COLLATE = #{&1}")
170+
171+
case run_query(create_command, opts) do
172+
{:ok, _} ->
173+
:ok
174+
{:error, %{mysql: %{name: :ER_DB_CREATE_EXISTS}}} ->
175+
{:error, :already_up}
176+
{:error, error} ->
177+
{:error, Exception.message(error)}
178+
{:exit, exit} ->
179+
{:error, exit_to_exception(exit)}
180+
end
175181
end
176182
end
177183

lib/ecto/adapters/postgres.ex

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,25 +127,37 @@ defmodule Ecto.Adapters.Postgres do
127127

128128
@impl true
129129
def storage_up(opts) do
130-
database = Keyword.fetch!(opts, :database) || raise ":database is nil in repository configuration"
130+
database =
131+
Keyword.fetch!(opts, :database) || raise ":database is nil in repository configuration"
132+
131133
encoding = if opts[:encoding] == :unspecified, do: nil, else: opts[:encoding] || "UTF8"
132134
maintenance_database = Keyword.get(opts, :maintenance_database, @default_maintenance_database)
133135
opts = Keyword.put(opts, :database, maintenance_database)
134136

135-
command =
136-
~s(CREATE DATABASE "#{database}")
137-
|> concat_if(encoding, &"ENCODING '#{&1}'")
138-
|> concat_if(opts[:template], &"TEMPLATE=#{&1}")
139-
|> concat_if(opts[:lc_ctype], &"LC_CTYPE='#{&1}'")
140-
|> concat_if(opts[:lc_collate], &"LC_COLLATE='#{&1}'")
137+
check_existence_command = "SELECT FROM pg_database WHERE datname = '#{database}'"
141138

142-
case run_query(command, opts) do
143-
{:ok, _} ->
144-
:ok
145-
{:error, %{postgres: %{code: :duplicate_database}}} ->
139+
case run_query(check_existence_command, opts) do
140+
{:ok, %{num_rows: 1}} ->
146141
{:error, :already_up}
147-
{:error, error} ->
148-
{:error, Exception.message(error)}
142+
143+
_ ->
144+
create_command =
145+
~s(CREATE DATABASE "#{database}")
146+
|> concat_if(encoding, &"ENCODING '#{&1}'")
147+
|> concat_if(opts[:template], &"TEMPLATE=#{&1}")
148+
|> concat_if(opts[:lc_ctype], &"LC_CTYPE='#{&1}'")
149+
|> concat_if(opts[:lc_collate], &"LC_COLLATE='#{&1}'")
150+
151+
case run_query(create_command, opts) do
152+
{:ok, _} ->
153+
:ok
154+
155+
{:error, %{postgres: %{code: :duplicate_database}}} ->
156+
{:error, :already_up}
157+
158+
{:error, error} ->
159+
{:error, Exception.message(error)}
160+
end
149161
end
150162
end
151163

0 commit comments

Comments
 (0)