Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List databases #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
48 changes: 42 additions & 6 deletions lib/PGObject/Util/DBAdmin.pm
Original file line number Diff line number Diff line change
Expand Up @@ -559,26 +559,62 @@ sub server_version {
}


=head2 list_dbs([$dbname])
=head2 list_dbs([$dbname],[$excludes])
Copy link
Contributor

Choose a reason for hiding this comment

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

This implies my $excludes = shift with a list of excludes passed as an arrayref. The implementation below takes all remaining arguments as excludes. I like this definition better than the implementation, because the latter does not allow further extension of the API.


Returns a list of db names.

When a database name is specified, uses that database to connect to,
using the credentials specified in the instance.

If no database name is specified, 'template1' is used.
If an array of excluded databases is provided, it is used to limit the list
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. But what is the reason we want this? I mean Perl has a nice construct with grep { not $excludes{$_} } @all_databases where you can really simply limit the values in a list. Is there a reason this must happen in the SQL phase?

Copy link
Contributor

Choose a reason for hiding this comment

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

The latest commit does address the SQL injection, thanks. That leaves the above question (why is it necessary to do this at the query level) though.


=cut

sub list_dbs {
my $self = shift;
my $dbname = (shift @_) || 'template1';
my $dbname = shift || 'template1';
my @excludes = @_;

return map { $_->[0] }
@{ __PACKAGE__->new($self->export, (dbname => $dbname))
->connect->selectall_arrayref(
'SELECT datname from pg_database
WHERE datallowconn
AND datname NOT IN (?)
ORDER BY datname',
{},\@excludes)
};
}

=head2 list_dbs_this_user($user,[$dbname],[$excludes])

Returns a list of db names.

When a database name is specified, uses that database to connect to,
using the credentials specified in the instance.

If no database name is specified, 'template1' is used.
If an array of excluded databases is provided, it is used to limit the list

=cut

sub list_dbs_this_user {
my $self = shift;
my $user = shift;
my $dbname = shift || 'template1';
my @excludes = @_;

return map { $_->[0] }
@{ __PACKAGE__->new($self->export, (dbname => $dbname)
)->connect->selectall_arrayref(
'SELECT datname from pg_database order by datname'
) };
@{ __PACKAGE__->new($self->export, (dbname => $dbname))
->connect->selectall_arrayref(
"SELECT datname from pg_database
WHERE datdba=(SELECT usesysid FROM pg_user WHERE usename='$user')
AND datallowconn
AND datname NOT IN (?)
ORDER BY datname",
{},\@excludes)
};
}

=head2 create
Expand Down
9 changes: 6 additions & 3 deletions t/01.1-dbtests.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use PGObject::Util::DBAdmin;
use File::Temp;

plan skip_all => 'DB_TESTING not set' unless $ENV{DB_TESTING};
plan tests => 75;
plan tests => 78;

# Constructor

Expand All @@ -29,6 +29,9 @@ eval { $db->drop };
my @dblist;
ok(@dblist = $db->list_dbs, 'Got a db list');
ok (!grep {$_ eq 'pgobject_test_db'} @dblist, 'DB list does not contain pgobject_test_db');
ok(@dblist = $db->list_dbs('postgres',qw/template0 template1/), 'Got a db filtered list');
ok(@dblist = $db->list_dbs_this_user('postgres'), 'Got a db list for postgres user');
ok(0 == (@dblist = $db->list_dbs_this_user('unknown')), 'Got no db list for unknown user');

# Create db
$db->create;
Expand All @@ -43,7 +46,7 @@ my $stderr_log = File::Temp->new->filename;
ok($db->run_file(
file => 't/data/schema.sql',
stdout_log => $stdout_log,
errlog => $stderr_log,
errlog => $stderr_log,
), 'Loaded schema');
ok(-f $stdout_log, 'run_file stdout_log file written');
ok(-f $stderr_log, 'run_file errlog file written');
Expand Down Expand Up @@ -89,7 +92,7 @@ foreach my $format ((undef, 'p', 'c')) {
cmp_ok(-s $backup, '>', 0, "backup format $display_format output file has size > 0");

ok($db->drop, "dropped db, format $display_format");
ok (!(grep{$_ eq 'pgobject_test_db'} @dblist),
ok (!(grep{$_ eq 'pgobject_test_db'} @dblist),
'DB list does not contain pgobject_test_db');

dies_ok {
Expand Down
4 changes: 2 additions & 2 deletions t/01.2-backup_globals.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ ok($db = PGObject::Util::DBAdmin->new(
username => 'postgres',
password => undef,
dbname => 'pgobject_test_db',
host => 'localhost',
port => '5432'
host => $ENV{PGHOST} // 'localhost',
port => $ENV{PGPORT} // '5432'
), 'Created db admin object');

# Drop db if exists
Expand Down
14 changes: 7 additions & 7 deletions t/02-dbexceptions.t
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ plan tests => 24;

my $db = PGObject::Util::DBAdmin->new(
username => 'postgres' ,
host => 'localhost' ,
port => '5432' ,
dbname => 'pgobject_test_db',
host => $ENV{PGHOST} // 'localhost',
port => $ENV{PGPORT} // '5432'
);

eval { $db->drop };
Expand All @@ -30,7 +30,7 @@ dies_ok{
file => 't/data/bad.sql',
stdout_log => $stdout_log,
errlog => $stderr_log,
)
)
} 'run_file dies with bad sql';
ok(-f $stdout_log, 'run_file stdout_log file written');
ok(-f $stderr_log, 'run_file errlog file written');
Expand All @@ -51,7 +51,7 @@ dies_ok{
$db->restore(
file => 't/data/bad.sql',
format => 't',
)
)
} 'restore dies with bad input';


Expand All @@ -77,9 +77,9 @@ dies_ok { $db->restore(format => 'c', file => 't/data/backup.sqlc') } 'cannot re

$db = PGObject::Util::DBAdmin->new(
username => 'invalid',
host => 'localhost',
port => '5432',
dbname => 'pgobject_test_db',
host => $ENV{PGHOST} // 'localhost',
port => $ENV{PGPORT} // '5432'
);
$backup_file = File::Temp->new->filename;
dies_ok { $db->backup_globals(file => $backup_file) } 'backup_globals dies with bad username';
Expand All @@ -92,7 +92,7 @@ ok(! -e $backup_file, 'output file deleted after backup error');

$db = PGObject::Util::DBAdmin->new(
username => 'postgres' ,
host => 'localhost' ,
host => $ENV{PGHOST} // 'localhost',
port => '2' ,
dbname => 'pgobject_test_db',
);
Expand Down