Skip to content

Commit

Permalink
Allow configuration overriding on connect()
Browse files Browse the repository at this point in the history
* When username, password, or extra arguments are set in
  connect(), have those arguments replace the ones loaded
  from the configuration file.

* Remove Test::MockObject requirement and do it the old fashioned
  way.

* Documentation and tests updated.
  • Loading branch information
symkat committed Sep 5, 2014
1 parent 46ea897 commit 60d6346
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 35 deletions.
1 change: 0 additions & 1 deletion Makefile.PL
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ requires 'File::HomeDir' => '0';

test_requires 'Test::More' => '0.42';
test_requires 'DBD::SQLite' => '0';
test_requires 'Test::MockObject' => '1.09';
test_requires 'Config::Any' => '0.23';

WriteAll;
50 changes: 38 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ This module will load the files in the following order if they exist:
- ~/.dbic.\*
- /etc/dbic.\*

The files should have an extension that [Config::Any](http://search.cpan.org/perldoc?Config::Any) recognizes,
for example /etc/dbic.__yaml__.
The files should have an extension that [Config::Any](https://metacpan.org/pod/Config::Any) recognizes,
for example /etc/dbic.**yaml**.

NOTE: The first available credential will be used. Therefore _DATABASE_
in ~/.dbic.yaml will only be looked at if it was not found in ./dbic.yaml.
Expand Down Expand Up @@ -93,9 +93,9 @@ instead.
This will check the files, `/var/www/secret/dbic.yaml`,
and `/opt/database.yaml` in the same way as `config_paths`,
however it will only check the specific files, instead of checking
for each extension that [Config::Any](http://search.cpan.org/perldoc?Config::Any) supports. You MUST use the
for each extension that [Config::Any](https://metacpan.org/pod/Config::Any) supports. You MUST use the
extension that corresponds to the file type you are loading.
See [Config::Any](http://search.cpan.org/perldoc?Config::Any) for information on supported file types and
See [Config::Any](https://metacpan.org/pod/Config::Any) for information on supported file types and
extension mapping.

# ACCESSING THE CONFIG FILE
Expand All @@ -114,13 +114,39 @@ The above code would have _/var/www/secret/dbic.\*_ and _/opt/database.\*_
searched, in that order. As above, the first credentials found would be used.
This will replace the files origionally searched for, not add to them.



# OVERRIDING

The API has been designed to be simple to override if you have additional
needs in loading DBIC configurations.

## Overriding Connection Configuration

Simple cases where one wants to replace specific configuration tokens can be
given as extra parameters in the ->connect call.

For example, suppose we have the database MY\_DATABASE from above:

MY_DATABASE:
dsn: "dbi:Pg:host=localhost;database=blog"
user: "TheDoctor"
password: "dnoPydoleM"
TraceLevel: 1

If you’d like to replace the username with “Eccleston” and we’d like to turn
PrintError off.

The following connect line would achieve this:

$Schema->connect(“MY_DATABASE”, “Eccleston”, undef, { PrintError => 0 } );

The name of the connection to load from the configuration file is still given
as the first argument, while the username and password follow and finally any
extra attributes you’d like to override.

Please note that the username and password field must be set to undef if you
are not overriding them and wish to use extra attributes to override or add
additional configuration for the connection.

## filter\_loaded\_credentials

Override this function if you want to change the loaded credentials before
Expand All @@ -136,14 +162,14 @@ using it.

`$loaded_credentials` is the structure after it has been loaded from the
configuration file. In this case, `$loaded_credentials->{user}` eq
__WalterWhite__ and `$loaded_credentials->{dsn}` eq
__DBI:mysql:database=students;host=%s;port=3306__.
**WalterWhite** and `$loaded_credentials->{dsn}` eq
**DBI:mysql:database=students;host=%s;port=3306**.

`$connect_args` is the structure originally passed on `->connect()`
after it has been turned into a hash. For instance,
`->connect('DATABASE', 'USERNAME')` will result in
`$connect_args->{dsn}` eq __DATABASE__ and `$connect_args->{user}`
eq __USERNAME__.
`$connect_args->{dsn}` eq **DATABASE** and `$connect_args->{user}`
eq **USERNAME**.

Additional parameters can be added by appending a hashref,
to the connection call, as an example, `->connect( 'CONFIG',
Expand Down Expand Up @@ -181,12 +207,12 @@ In your Schema class, you could include the following:
Then the connection could be done with
`$Schema->connect('DATABASE', { hostname =` 'my.hostname.com' });>

See ["load\_credentials"](#load\_credentials) for more complex changes that require changing
See ["load\_credentials"](#load_credentials) for more complex changes that require changing
how the configuration itself is loaded.

## load\_credentials

Override this function to change the way that [DBIx::Class::Schema::Config](http://search.cpan.org/perldoc?DBIx::Class::Schema::Config)
Override this function to change the way that [DBIx::Class::Schema::Config](https://metacpan.org/pod/DBIx::Class::Schema::Config)
loads credentials. The function takes the class name, as well as a hashref.

If you take the route of having `->connect('DATABASE')` used as a key for
Expand Down
71 changes: 65 additions & 6 deletions lib/DBIx/Class/Schema/Config.pm
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use warnings;
use strict;
use base 'DBIx::Class::Schema';
use File::HomeDir;
use Storable qw( dclone );


our $VERSION = '0.001010'; # 0.1.10
$VERSION = eval $VERSION;
Expand Down Expand Up @@ -32,10 +34,10 @@ sub _make_connect_attrs {

return {
dsn => $dsn,
%{ref $user eq 'HASH' ? $user : { user => $user }},
%{ref $pass eq 'HASH' ? $pass : { password => $pass }},
%{$dbi_attr || {} },
%{ $extra_attr || {} }
%{ ref $user eq 'HASH' ? $user : { user => $user } },
%{ ref $pass eq 'HASH' ? $pass : { password => $pass } },
%{ $dbi_attr || {} },
%{ $extra_attr || {} },
};
}

Expand Down Expand Up @@ -99,15 +101,44 @@ sub get_env_vars {
return ();
}

sub _merge {
my ( $lhs, $rhs ) = ( dclone($_[0]), dclone($_[1]) );

if ( ref $lhs eq 'HASH' ) {
for my $key ( keys %$lhs ) {
if ( ref $lhs->{$key} eq 'HASH' ) {
$lhs->{$key} = _merge($lhs->{$key}, $rhs->{$key});
delete $rhs->{$key};
} else {
$lhs->{$key} = delete $rhs->{$key} if exists $rhs->{$key};
}
}
# Unhandled keys (simply do injection on uneven rhs structure)
for my $key ( keys %$rhs ) {
$lhs->{$key} = delete $rhs->{$key};
}
}

return $lhs;
}

# Intended to be sub-classed, we'll just return the
# credentials we used in the first place.
sub filter_loaded_credentials { $_[1] };
sub filter_loaded_credentials {
my ( $class, $new, $old ) = @_;

local $old->{password}, delete $old->{password} unless $old->{password};
local $old->{user}, delete $old->{user} unless $old->{user};
local $old->{dsn}, delete $old->{dsn};

return _merge( $new, $old );
};

__PACKAGE__->mk_classaccessor('config_paths');
__PACKAGE__->mk_classaccessor('config_files');
__PACKAGE__->mk_classaccessor('_config');
__PACKAGE__->config_paths([( get_env_vars(), './dbic', File::HomeDir->my_home . '/.dbic', '/etc/dbic')]);
__PACKAGE__->config_files([ ] );
__PACKAGE__->config_files([ ]);

1;

Expand Down Expand Up @@ -241,6 +272,34 @@ This will replace the files origionally searched for, not add to them.
The API has been designed to be simple to override if you have additional
needs in loading DBIC configurations.
=head2 Overriding Connection Configuration
Simple cases where one wants to replace specific configuration tokens can be
given as extra parameters in the ->connect call.
For example, suppose we have the database MY_DATABASE from above:
MY_DATABASE:
dsn: "dbi:Pg:host=localhost;database=blog"
user: "TheDoctor"
password: "dnoPydoleM"
TraceLevel: 1
If you’d like to replace the username with “Eccleston” and we’d like to turn
PrintError off.
The following connect line would achieve this:
$Schema->connect(“MY_DATABASE”, “Eccleston”, undef, { PrintError => 0 } );
The name of the connection to load from the configuration file is still given
as the first argument, while the username and password follow and finally any
extra attributes you’d like to override.
Please note that the username and password field must be set to undef if you
are not overriding them and wish to use extra attributes to override or add
additional configuration for the connection.
=head2 filter_loaded_credentials
Override this function if you want to change the loaded credentials before
Expand Down
1 change: 0 additions & 1 deletion t/00_load.t
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use Test::More;
my @want_modules = qw/
DBI
DBIx::Class
Test::MockObject
DBIx::Class::Schema
DBIx::Class::Schema::Config
/;
Expand Down
84 changes: 69 additions & 15 deletions t/02_load_credentials.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,31 @@
use warnings;
use strict;
use Test::More;
use Test::MockObject;
use DBIx::Class::Schema::Config;

Test::MockObject->fake_module(
'Config::Any',
'load_stems' => sub {
{
package Config::Any;

$INC{"Config/Any.pm"} = __FILE__;

sub load_stems {
return [
{
'some_file' => {
SOME_DATABASE => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'MyUser',
pass => 'MyPass',
password => 'MyPass',
},
AWESOME_DB => {
dsn => 'dbi:mysql:dbname=epsilon',
user => 'Bravo',
pass => 'ShiJulIanDav',
password => 'ShiJulIanDav',
},
OPTIONS => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Happy',
pass => 'User',
password => 'User',
TRACE_LEVEL => 5,
}
},
Expand All @@ -34,21 +36,21 @@ Test::MockObject->fake_module(
SOME_DATABASE => {
dsn => 'dbi:mysql:dbname=acronym',
user => 'YawnyPants',
pass => 'WhyDoYouHateUs?',
password => 'WhyDoYouHateUs?',
},
},
}
]
}
);
}

my $tests = [
{
put => { dsn => 'SOME_DATABASE', user => '', password => '' },
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'MyUser',
pass => 'MyPass',
password => 'MyPass',
},
title => "Get DB info from hashref.",
},
Expand All @@ -57,7 +59,7 @@ my $tests = [
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'MyUser',
pass => 'MyPass',
password => 'MyPass',
},
title => "Get DB info from array.",
},
Expand All @@ -66,7 +68,7 @@ my $tests = [
get => {
dsn => 'dbi:mysql:dbname=epsilon',
user => 'Bravo',
pass => 'ShiJulIanDav',
password => 'ShiJulIanDav',
},
title => "Get DB from hashref without user and pass.",
},
Expand Down Expand Up @@ -97,10 +99,62 @@ my $tests = [
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Happy',
pass => 'User',
password => 'User',
TRACE_LEVEL => 5,
}
}
},
title => "Default loading",
},
{
put => [ 'OPTIONS', undef, undef, { TRACE_LEVEL => 10 } ],
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Happy',
password => 'User',
TRACE_LEVEL => 10,
},
title => "Override of replaced key works.",
},
{
put => [ 'OPTIONS', undef, undef, { TRACE_LEVEL => 10, MAGIC => 1 } ],
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Happy',
password => 'User',
TRACE_LEVEL => 10,
MAGIC => 1,
},
title => "Override for non-replaced key works.",
},
{
put => [ 'OPTIONS', "Foobar", undef, { TRACE_LEVEL => 10 } ],
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Foobar',
password => 'User',
TRACE_LEVEL => 10,
},
title => "Overriding the username works.",
},
{
put => [ 'OPTIONS', undef, "Foobar", { TRACE_LEVEL => 10 } ],
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'Happy',
password => 'Foobar',
TRACE_LEVEL => 10,
},
title => "Overriding the password works.",
},
{
put => [ 'OPTIONS', "BleeBaz", "Foobar", { TRACE_LEVEL => 10 } ],
get => {
dsn => 'dbi:SQLite:dbfile=:memory:',
user => 'BleeBaz',
password => 'Foobar',
TRACE_LEVEL => 10,
},
title => "Overriding the user and password works.",
},


];
Expand Down

1 comment on commit 60d6346

@ribasushi
Copy link

Choose a reason for hiding this comment

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

Generally it looks right, but the commit is too large with too many unrelated changes to properly review. If you redo it into the ~5 commits it actually contains I will be able to offer more detailed review points.

Please sign in to comment.