From e3fb12b4e8debe300c7ae4ffed6b4d14de388eb2 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Tue, 11 Nov 2014 14:29:35 -0600 Subject: [PATCH 01/17] allow owner to be overridden when creating a database from a template --- lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm | 4 +++- t/commands.t | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index d12e057..1ad0200 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -6,6 +6,7 @@ use TestDbServer::Exceptions; use Moose; use namespace::autoclean; +has owner => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); has host => ( isa => 'Str', is => 'ro', required => 1 ); has port => ( isa => 'Int', is => 'ro', required => 1 ); has template_id => ( isa => 'Str', is => 'ro', required => 1 ); @@ -20,10 +21,11 @@ sub execute { Exception::TemplateNotFound->throw(template_id => $self->template_id); } + my $owner = $self->owner || $template->owner; my $pg = TestDbServer::PostgresInstance->new( host => $self->host, port => $self->port, - owner => $template->owner, + owner => $owner, superuser => $self->superuser, ); diff --git a/t/commands.t b/t/commands.t index 5909f17..0b0a0dc 100644 --- a/t/commands.t +++ b/t/commands.t @@ -156,6 +156,7 @@ subtest 'create database from template' => sub { } my $cmd = TestDbServer::Command::CreateDatabaseFromTemplate->new( + owner => undef, host => $config->db_host, port => $config->db_port, template_id => $template->template_id, From 876fbb6514bafcd312f031b88f006995faf01f8b Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Tue, 11 Nov 2014 14:30:21 -0600 Subject: [PATCH 02/17] always create database from template (with owner override allowed) --- bin/test-db-database-create | 5 ----- lib/TestDbServer/DatabaseRoutes.pm | 20 ++++++++------------ 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/bin/test-db-database-create b/bin/test-db-database-create index 8a29d0e..a3deb81 100755 --- a/bin/test-db-database-create +++ b/bin/test-db-database-create @@ -11,11 +11,6 @@ GetOptions($opts, 'owner=s', 'based-on=s','short-help','help'); print_short_help() if ($opts->{'short-help'}); print_help() if ($opts->{help}); -unless ($opts->{owner} xor $opts->{'based-on'}) { - print STDERR "You must use one of --owner or --based-on, but not both\n"; - exit 1; -} - my $ua = get_user_agent(); my @options = make_post_options($opts); my $req = HTTP::Request->new(POST => url_for('databases', \@options)); diff --git a/lib/TestDbServer/DatabaseRoutes.pm b/lib/TestDbServer/DatabaseRoutes.pm index 40dd42f..543c211 100644 --- a/lib/TestDbServer/DatabaseRoutes.pm +++ b/lib/TestDbServer/DatabaseRoutes.pm @@ -118,18 +118,13 @@ sub _hashref_for_database_obj { sub create { my $self = shift; - if (my $template_id = $self->req->param('based_on')) { - $self->app->log->info("create database from template $template_id"); - $self->_create_database_from_template($template_id); - - } elsif (my $owner = $self->req->param('owner')) { - $self->app->log->info("create database with owner $owner"); - $self->_create_new_database($owner); + my $schema = $self->app->db_storage(); + my $default_template_id = $schema->search_template(name => 'template1')->next->id; + my $template_id = $self->req->param('based_on') || $default_template_id; + my $owner = $self->req->param('owner'); - } else { - $self->app->log->error('create databse with bad params'); - $self->render_not_found; - } + $self->app->log->info("create database from template $template_id"); + $self->_create_database_from_template($owner, $template_id); } sub _create_new_database { @@ -149,11 +144,12 @@ sub _create_new_database { } sub _create_database_from_template { - my($self, $template_id) = @_; + my($self, $owner, $template_id) = @_; $self->_create_database_common(sub { my($host, $port) = $self->app->host_and_port_for_created_database(); TestDbServer::Command::CreateDatabaseFromTemplate->new( + owner => $owner, template_id => $template_id, host => $host, port => $port, From 644de6e9c795547aa6b9bdc76b5bf82275f706ba Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Tue, 11 Nov 2014 14:31:34 -0600 Subject: [PATCH 03/17] remove no-longer-used function, _create_new_database --- lib/TestDbServer/DatabaseRoutes.pm | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lib/TestDbServer/DatabaseRoutes.pm b/lib/TestDbServer/DatabaseRoutes.pm index 543c211..93d042c 100644 --- a/lib/TestDbServer/DatabaseRoutes.pm +++ b/lib/TestDbServer/DatabaseRoutes.pm @@ -127,22 +127,6 @@ sub create { $self->_create_database_from_template($owner, $template_id); } -sub _create_new_database { - my($self, $owner) = @_; - - $self->_create_database_common(sub { - my($host, $port) = $self->app->host_and_port_for_created_database(); - my $cmd = TestDbServer::Command::CreateDatabase->new( - owner => $owner, - template_id => undef, - host => $host, - port => $port, - superuser => $self->app->configuration->db_user, - schema => $self->app->db_storage, - ); - }); -} - sub _create_database_from_template { my($self, $owner, $template_id) = @_; From 1079150e7aca220510528ee2efc10111520f44c2 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Tue, 11 Nov 2014 16:43:23 -0600 Subject: [PATCH 04/17] add real_owner to database --- lib/TestDbServer/Schema/Result/Database.pm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/TestDbServer/Schema/Result/Database.pm b/lib/TestDbServer/Schema/Result/Database.pm index ae3ec47..9737d66 100644 --- a/lib/TestDbServer/Schema/Result/Database.pm +++ b/lib/TestDbServer/Schema/Result/Database.pm @@ -6,4 +6,12 @@ __PACKAGE__->add_columns(qw(database_id host port name owner create_time expire_ __PACKAGE__->set_primary_key('database_id'); __PACKAGE__->belongs_to(template => 'TestDbServer::Schema::Result::Template', 'template_id'); +sub real_owner { + my $self = shift; + my $dbh = $self->result_source->storage->dbh(); + my $statement = "SELECT rolname FROM pg_database JOIN pg_authid ON pg_database.datdba = pg_authid.oid WHERE pg_database.datname = ?;"; + my $row = $dbh->selectrow_arrayref($statement, undef, $self->name); + return $row->[0]; +} + 1; From 07a853e48945b9ca575c7849cc7ece50f20f82dd Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Tue, 11 Nov 2014 16:43:44 -0600 Subject: [PATCH 05/17] add test to create database with owner --- t/commands.t | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/t/commands.t b/t/commands.t index 0b0a0dc..d6b6fc0 100644 --- a/t/commands.t +++ b/t/commands.t @@ -26,7 +26,7 @@ my $config = TestDbServer::Configuration->new_from_path(); my $schema = create_new_schema($config); my $uuid_gen = Data::UUID->new(); -plan tests => 6; +plan tests => 7; subtest 'create template from database' => sub { plan tests => 5; @@ -136,6 +136,61 @@ subtest 'create database' => sub { ok($db_pg->dropdb, 'drop db'); }; +subtest 'create database with owner' => sub { + plan tests => 6; + + my $pg = new_pg_instance(); + + note('original template named '.$pg->name); + my $template = $schema->create_template( map { $_ => $pg->$_ } qw( name owner ) ); + # Make a table in the template + my $table_name = "test_table_$$"; + { + my $dbi = DBI->connect(sprintf('dbi:Pg:dbname=%s;host=%s;port=%s', + $pg->name, $pg->host, $pg->port), + $pg->owner, + ''); + ok($dbi->do("CREATE TABLE $table_name (foo integer NOT NULL PRIMARY KEY)"), + 'Create table in base template'); + $dbi->disconnect; + } + + my $new_owner = 'genome'; + my $cmd = TestDbServer::Command::CreateDatabaseFromTemplate->new( + owner => $new_owner, + host => $config->db_host, + port => $config->db_port, + template_id => $template->template_id, + schema => $schema, + superuser => $config->db_user, + ); + ok($cmd, 'new'); + my $database = $cmd->execute(); + ok($database, 'execute'); + + # connect to the template database + my $dbi = DBI->connect(sprintf('dbi:Pg:dbname=%s;host=%s;port=%s', + $database->name, $database->host, $database->port), + $pg->owner, ''); + ok($dbi->do("SELECT foo FROM $table_name WHERE FALSE"), 'table exists in template database'); + $dbi->disconnect; + + isnt($new_owner, $template->owner, 'new_owner is not the same as template owner'); + is($database->real_owner, $new_owner, 'database has new_owner not template owner'); + + # remove the original template + $pg->dropdb; + + # remove the created database + TestDbServer::PostgresInstance->new( + host => $database->host, + port => $database->port, + owner => $database->owner, + superuser => $config->db_user, + name => $database->name + )->dropdb; +}; + subtest 'create database from template' => sub { plan tests => 4; From 2ab3c7f8543020d2080a08b5414dffcdb22a72a0 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:07:08 -0600 Subject: [PATCH 06/17] join to pg_roles instead of pg_authid "Since this catalog (pg_authid) contains passwords, it must not be publicly readable. pg_roles is a publicly readable view on pg_authid that blanks out the password field." --- lib/TestDbServer/Schema/Result/Database.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/TestDbServer/Schema/Result/Database.pm b/lib/TestDbServer/Schema/Result/Database.pm index 9737d66..214f565 100644 --- a/lib/TestDbServer/Schema/Result/Database.pm +++ b/lib/TestDbServer/Schema/Result/Database.pm @@ -9,7 +9,7 @@ __PACKAGE__->belongs_to(template => 'TestDbServer::Schema::Result::Template', 't sub real_owner { my $self = shift; my $dbh = $self->result_source->storage->dbh(); - my $statement = "SELECT rolname FROM pg_database JOIN pg_authid ON pg_database.datdba = pg_authid.oid WHERE pg_database.datname = ?;"; + my $statement = "SELECT pg_roles.rolname FROM pg_database JOIN pg_roles ON pg_database.datdba = pg_roles.oid WHERE pg_database.datname = ?;"; my $row = $dbh->selectrow_arrayref($statement, undef, $self->name); return $row->[0]; } From 5d54308f144953cc57929ebc96753b3385842d9f Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:13:29 -0600 Subject: [PATCH 07/17] automatically grant role to the superuser Automatically grant roles to the superuser so that it can create new databases with that owner. --- lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm | 10 ++++++++++ lib/TestDbServer/Exceptions.pm | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index 1ad0200..37b7b1c 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -29,6 +29,11 @@ sub execute { superuser => $self->superuser, ); + if ($owner ne $self->superuser) { + my $dbh = $self->schema->storage->dbh(); + grant_role($dbh, $owner, $self->superuser); + } + $pg->createdb_from_template($template->name); my $database = $self->schema->create_database( @@ -45,6 +50,11 @@ sub execute { return $database; } +sub grant_role { + my ($dbh, $source, $target) = @_; + $dbh->do(sprintf('GRANT %s to %s', $source, $target)); +} + __PACKAGE__->meta->make_immutable; 1; diff --git a/lib/TestDbServer/Exceptions.pm b/lib/TestDbServer/Exceptions.pm index bec5e0c..9608400 100644 --- a/lib/TestDbServer/Exceptions.pm +++ b/lib/TestDbServer/Exceptions.pm @@ -58,6 +58,11 @@ use Exception::Class ( isa => 'Exception::BaseException', fields => [qw(template_id)], }, + + Exception::RoleNotFound => { + isa => 'Exception::BaseException', + fields => [qw(role_name)], + }, ); package Exception::BaseException; From ce049a3a303cab4a4834cdd65c68d8ed9415ec9a Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:20:33 -0600 Subject: [PATCH 08/17] validate roles to avoid SQL injection --- .../Command/CreateDatabaseFromTemplate.pm | 10 +++++ t/commands.t | 39 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index 37b7b1c..8296aa1 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -52,6 +52,16 @@ sub execute { sub grant_role { my ($dbh, $source, $target) = @_; + + my $is_valid_role = sub { + my $row = $dbh->selectrow_arrayref(q(SELECT 1 FROM pg_roles WHERE rolname=?), undef, @_); + return $row->[0]; + }; + for my $role_name ($source, $target) { + unless ($is_valid_role->($role_name)) { + Exception::RoleNotFound->throw(role_name => $role_name); + } + } $dbh->do(sprintf('GRANT %s to %s', $source, $target)); } diff --git a/t/commands.t b/t/commands.t index d6b6fc0..286dc58 100644 --- a/t/commands.t +++ b/t/commands.t @@ -26,7 +26,7 @@ my $config = TestDbServer::Configuration->new_from_path(); my $schema = create_new_schema($config); my $uuid_gen = Data::UUID->new(); -plan tests => 7; +plan tests => 8; subtest 'create template from database' => sub { plan tests => 5; @@ -191,6 +191,43 @@ subtest 'create database with owner' => sub { )->dropdb; }; +subtest 'create database with invalid owner' => sub { + plan tests => 3; + + my $pg = new_pg_instance(); + + note('original template named '.$pg->name); + my $template = $schema->create_template( map { $_ => $pg->$_ } qw( name owner ) ); + # Make a table in the template + my $table_name = "test_table_$$"; + { + my $dbi = DBI->connect(sprintf('dbi:Pg:dbname=%s;host=%s;port=%s', + $pg->name, $pg->host, $pg->port), + $pg->owner, + ''); + ok($dbi->do("CREATE TABLE $table_name (foo integer NOT NULL PRIMARY KEY)"), + 'Create table in base template'); + $dbi->disconnect; + } + + my $invalid_owner = 'xxx'; + my $cmd = TestDbServer::Command::CreateDatabaseFromTemplate->new( + owner => $invalid_owner, + host => $config->db_host, + port => $config->db_port, + template_id => $template->template_id, + schema => $schema, + superuser => $config->db_user, + ); + ok($cmd, 'new'); + throws_ok { $cmd->execute() } + 'Exception::RoleNotFound', + 'Cannot create database with unknown owner'; + + # remove the original template + $pg->dropdb; +}; + subtest 'create database from template' => sub { plan tests => 4; From 5a5dda1c9b642082a9d7f5ea93314ebb851b0088 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:23:58 -0600 Subject: [PATCH 09/17] update rest-api doc --- docs/rest-api.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/rest-api.txt b/docs/rest-api.txt index 1ae9d37..960ce29 100644 --- a/docs/rest-api.txt +++ b/docs/rest-api.txt @@ -131,8 +131,11 @@ The Location header will be the URL for this entity ----------------------------------------------------- POST /databases?based_on= +POST /databases?based_on=&owner= -Create a new database and populate it with the given database template +Create a new database and populate it with the given database template. +Optionally specifying an owner name to use instead of the owner of the +template. Returns 201 and JSON in the body { From 3573d3a2edd8544e336f83c2bdb30c840566a965 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:35:22 -0600 Subject: [PATCH 10/17] move default template to command --- .../Command/CreateDatabaseFromTemplate.pm | 13 ++++++++++--- lib/TestDbServer/DatabaseRoutes.pm | 11 +++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index 8296aa1..a0f141d 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -9,16 +9,23 @@ use namespace::autoclean; has owner => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); has host => ( isa => 'Str', is => 'ro', required => 1 ); has port => ( isa => 'Int', is => 'ro', required => 1 ); -has template_id => ( isa => 'Str', is => 'ro', required => 1 ); +has template_id => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); has schema => ( isa => 'TestDbServer::Schema', is => 'ro', required => 1 ); has superuser => ( isa => 'Str', is => 'ro', required => 1 ); sub execute { my $self = shift; - my $template = $self->schema->find_template($self->template_id); + my $default_template_id = $self->schema + ->search_template(name => 'template1') + ->next->id; + my $template_id = defined $self->template_id + ? $self->template_id + : $default_template_id; + + my $template = $self->schema->find_template($template_id); unless ($template) { - Exception::TemplateNotFound->throw(template_id => $self->template_id); + Exception::TemplateNotFound->throw(template_id => $template_id); } my $owner = $self->owner || $template->owner; diff --git a/lib/TestDbServer/DatabaseRoutes.pm b/lib/TestDbServer/DatabaseRoutes.pm index 93d042c..249232f 100644 --- a/lib/TestDbServer/DatabaseRoutes.pm +++ b/lib/TestDbServer/DatabaseRoutes.pm @@ -119,11 +119,14 @@ sub create { my $self = shift; my $schema = $self->app->db_storage(); - my $default_template_id = $schema->search_template(name => 'template1')->next->id; - my $template_id = $self->req->param('based_on') || $default_template_id; + my $template_id = $self->req->param('based_on'); my $owner = $self->req->param('owner'); - - $self->app->log->info("create database from template $template_id"); + if ($template_id) { + $self->app->log->info("create database from template $template_id"); + } + else { + $self->app->log->info("create database from default template"); + } $self->_create_database_from_template($owner, $template_id); } From 4371c364ae28179e2ac187ef1fffcb76b752450f Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:51:33 -0600 Subject: [PATCH 11/17] remove test --- t/commands.t | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/t/commands.t b/t/commands.t index 286dc58..3a104f2 100644 --- a/t/commands.t +++ b/t/commands.t @@ -84,7 +84,7 @@ subtest 'create template from database' => sub { }; subtest 'create database' => sub { - plan tests => 6; + plan tests => 3; # blank database my $create_blank_db_cmd = TestDbServer::Command::CreateDatabase->new( @@ -107,33 +107,6 @@ subtest 'create database' => sub { superuser => $config->db_user, ); ok($blank_pg->dropdb, 'drop blank db'); - - - # with a template ID - my $template = $schema->create_template( - name => $uuid_gen->create_str, - owner => $config->test_db_owner, - ); - my $create_db_cmd = TestDbServer::Command::CreateDatabase->new( - host => $config->db_host, - port => $config->db_port, - owner => $config->test_db_owner, - superuser => $config->db_user, - template_id => $template->template_id, - schema => $schema, - ); - ok($create_db_cmd, 'new - create with template'); - my $db = $create_db_cmd->execute(); - ok($db, 'execute - with template'); - - my $db_pg = TestDbServer::PostgresInstance->new( - host => $db->host, - port => $db->port, - name => $db->name, - owner => $db->owner, - superuser => $config->db_user, - ); - ok($db_pg->dropdb, 'drop db'); }; subtest 'create database with owner' => sub { From c980a51128fcac1932a88c3bf4641785e778fd7e Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:56:28 -0600 Subject: [PATCH 12/17] complete transition to CreateDatabaseFromTemplate --- lib/TestDbServer/DatabaseRoutes.pm | 1 - t/commands.t | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/TestDbServer/DatabaseRoutes.pm b/lib/TestDbServer/DatabaseRoutes.pm index 249232f..345ccca 100644 --- a/lib/TestDbServer/DatabaseRoutes.pm +++ b/lib/TestDbServer/DatabaseRoutes.pm @@ -4,7 +4,6 @@ use Mojo::Base 'Mojolicious::Controller'; use Try::Tiny; use TestDbServer::Utils; -use TestDbServer::Command::CreateDatabase; use TestDbServer::Command::CreateDatabaseFromTemplate; use TestDbServer::Command::DeleteDatabase; diff --git a/t/commands.t b/t/commands.t index 3a104f2..a14ad5d 100644 --- a/t/commands.t +++ b/t/commands.t @@ -17,7 +17,6 @@ use Data::UUID; use TestDbServer::Command::SaveTemplateFile; use TestDbServer::Command::CreateTemplateFromDatabase; -use TestDbServer::Command::CreateDatabase; use TestDbServer::Command::CreateDatabaseFromTemplate; use TestDbServer::Command::DeleteTemplate; use TestDbServer::Command::DeleteDatabase; @@ -87,7 +86,7 @@ subtest 'create database' => sub { plan tests => 3; # blank database - my $create_blank_db_cmd = TestDbServer::Command::CreateDatabase->new( + my $create_blank_db_cmd = TestDbServer::Command::CreateDatabaseFromTemplate->new( host => $config->db_host, port => $config->db_port, owner => $config->test_db_owner, @@ -275,7 +274,7 @@ subtest 'delete template' => sub { subtest 'delete database' => sub { plan tests => 5; - my $database = TestDbServer::Command::CreateDatabase->new( + my $database = TestDbServer::Command::CreateDatabaseFromTemplate->new( host => $config->db_host, port => $config->db_port, owner => $config->test_db_owner, @@ -308,7 +307,7 @@ subtest 'delete database' => sub { subtest 'delete with connections' => sub { plan tests => 5; - my $database = TestDbServer::Command::CreateDatabase->new( + my $database = TestDbServer::Command::CreateDatabaseFromTemplate->new( host => $config->db_host, port => $config->db_port, owner => $config->test_db_owner, From d4d35369aa3a1780fdf178da6d24063fafb8ec72 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 14:56:52 -0600 Subject: [PATCH 13/17] remove no-longer-used CreateDatabase --- lib/TestDbServer/Command/CreateDatabase.pm | 40 ---------------------- 1 file changed, 40 deletions(-) delete mode 100644 lib/TestDbServer/Command/CreateDatabase.pm diff --git a/lib/TestDbServer/Command/CreateDatabase.pm b/lib/TestDbServer/Command/CreateDatabase.pm deleted file mode 100644 index fd0dcb5..0000000 --- a/lib/TestDbServer/Command/CreateDatabase.pm +++ /dev/null @@ -1,40 +0,0 @@ -package TestDbServer::Command::CreateDatabase; - -use TestDbServer::PostgresInstance; - -use Moose; -use namespace::autoclean; - -has host => ( isa => 'Str', is => 'ro', required => 1 ); -has port => ( isa => 'Int', is => 'ro', required => 1 ); -has owner => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); -has template_id => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); -has superuser => ( isa => 'Str', is => 'ro', required => 1 ); -has schema => (isa => 'TestDbServer::Schema', is => 'ro', required => 1 ); - -sub execute { - my $self = shift; - - my $pg = TestDbServer::PostgresInstance->new( - host => $self->host, - port => $self->port, - owner => $self->owner, - superuser => $self->superuser, - ); - - my $database = $self->schema->create_database( - host => $self->host, - port => $self->port, - name => $pg->name, - owner => $self->owner, - template_id => $self->template_id, - ); - - $pg->createdb(); - - return $database; -} - -__PACKAGE__->meta->make_immutable; - -1; From eb38ecd127c47a2445676256ddc7b7d4a93f225f Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 16:55:23 -0600 Subject: [PATCH 14/17] fix incorrect comments --- t/commands.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/commands.t b/t/commands.t index a14ad5d..ed42d36 100644 --- a/t/commands.t +++ b/t/commands.t @@ -140,7 +140,7 @@ subtest 'create database with owner' => sub { my $database = $cmd->execute(); ok($database, 'execute'); - # connect to the template database + # connect to the newly created database my $dbi = DBI->connect(sprintf('dbi:Pg:dbname=%s;host=%s;port=%s', $database->name, $database->host, $database->port), $pg->owner, ''); @@ -231,7 +231,7 @@ subtest 'create database from template' => sub { my $database = $cmd->execute(); ok($database, 'execute'); - # connect to the template database + # connect to the newly created database my $dbi = DBI->connect(sprintf('dbi:Pg:dbname=%s;host=%s;port=%s', $database->name, $database->host, $database->port), $database->owner, ''); From 36a92520c64daf9a5d605368413bdb5c569724bf Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 16:57:12 -0600 Subject: [PATCH 15/17] move magic string to constant --- lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index a0f141d..4be837d 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -6,6 +6,8 @@ use TestDbServer::Exceptions; use Moose; use namespace::autoclean; +use constant DEFAULT_TEMPLATE_NAME => 'template1'; + has owner => ( isa => 'Maybe[Str]', is => 'ro', required => 1 ); has host => ( isa => 'Str', is => 'ro', required => 1 ); has port => ( isa => 'Int', is => 'ro', required => 1 ); @@ -17,7 +19,7 @@ sub execute { my $self = shift; my $default_template_id = $self->schema - ->search_template(name => 'template1') + ->search_template(name => DEFAULT_TEMPLATE_NAME) ->next->id; my $template_id = defined $self->template_id ? $self->template_id From 0fec6d5e1c3f9afbea7bf56dd1a306e8283803b6 Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 17:06:39 -0600 Subject: [PATCH 16/17] convert grant_role from a function to a method --- .../Command/CreateDatabaseFromTemplate.pm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm index 4be837d..7522471 100644 --- a/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm +++ b/lib/TestDbServer/Command/CreateDatabaseFromTemplate.pm @@ -39,8 +39,7 @@ sub execute { ); if ($owner ne $self->superuser) { - my $dbh = $self->schema->storage->dbh(); - grant_role($dbh, $owner, $self->superuser); + $self->grant_role_to_superuser($owner); } $pg->createdb_from_template($template->name); @@ -59,19 +58,20 @@ sub execute { return $database; } -sub grant_role { - my ($dbh, $source, $target) = @_; +sub grant_role_to_superuser { + my ($self, $source) = @_; + my $dbh = $self->schema->storage->dbh(); my $is_valid_role = sub { my $row = $dbh->selectrow_arrayref(q(SELECT 1 FROM pg_roles WHERE rolname=?), undef, @_); return $row->[0]; }; - for my $role_name ($source, $target) { + for my $role_name ($source, $self->superuser) { unless ($is_valid_role->($role_name)) { Exception::RoleNotFound->throw(role_name => $role_name); } } - $dbh->do(sprintf('GRANT %s to %s', $source, $target)); + $dbh->do(sprintf('GRANT %s to %s', $source, $self->superuser)); } __PACKAGE__->meta->make_immutable; From d0cd24d3715c02a9d4d03654f8ced20bed19f1ee Mon Sep 17 00:00:00 2001 From: Nathaniel Nutter Date: Wed, 12 Nov 2014 17:17:39 -0600 Subject: [PATCH 17/17] remove hard-coded role name For this test to pass it is required that `test_db_user` and `db_user` are never the same. `new_pg_instance()` used `test_db_user` as the default owner so this will try to override that owner with `db_user`. --- t/commands.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/commands.t b/t/commands.t index ed42d36..dec07c8 100644 --- a/t/commands.t +++ b/t/commands.t @@ -127,7 +127,7 @@ subtest 'create database with owner' => sub { $dbi->disconnect; } - my $new_owner = 'genome'; + my $new_owner = $config->db_user; my $cmd = TestDbServer::Command::CreateDatabaseFromTemplate->new( owner => $new_owner, host => $config->db_host,