bigpresh / Dancer-Plugin-Database

Dancer::Plugin::Database - easy database support for Dancer applications
http://search.cpan.org/dist/Dancer-Plugin-Database
37 stars 36 forks source link

database() requires manual setting of UTF-8 mode even with charset: "UTF-8" in place #73

Open drmuey opened 8 years ago

drmuey commented 8 years ago

Greetings!

The POD says:

if your application is configured to use UTF-8 (you've defined the charset setting in your app config as UTF-8) then support for UTF-8 for the database connection will be enabled, if we know how to do so for the database driver in use.

however (in GWT format):

Given config.yml has charset: "UTF-8" and Given an SQLite database with proper utf-8 data When I use database( { driver => 'SQLite', database => $path_to_db }) Then the data is garbled until I $dbh->{sqlite_unicode} = 1; w/ the handle it returns.

May be SQLite specific or not, not sure ATM.

ambs commented 8 years ago

Thanks for the heads up. Will look at it. I changed some minor details that might be making it misbehave.

Can you please clarify if you are using Dancer1 or Dancer2?

drmuey commented 8 years ago

Sure, sorry about that, its Dancer2.

ambs commented 8 years ago

Thought so, but better be sure before starting debugging a working module O:-) will look into it later today, hopefully.

ambs commented 8 years ago

Did you try to use config.yml with your database details, and use just database->something?

I am trying to debug your problem meanwhile.

ambs commented 8 years ago

As far as I could check, when using the config.yml, your logs will say something like:

Adding sqlite_unicode to DBI connection params to enable UTF-8 support in

Will try to understand if we can do the same when you supply the config from the database command.

ambs commented 8 years ago

Hey Can you please install http://eremita.di.uminho.pt/~ambs/Dancer2-Plugin-Database-2.13.tar.gz and see if it works now?

drmuey commented 8 years ago

Sorry for the delay, I'll look this over and get back to you ASAP over the nest day or so

ambs commented 8 years ago

No prob :)

bigpresh commented 8 years ago

Sorry this waited so long!

Looking at it, I think it's a bug introduced by refactoring (and apparently not tested enough to show up - that's not good).

The code that sets the UTF-8 support is: https://metacpan.org/source/Dancer::Plugin::Database::Core#L248-271

However - it's looking at $settings->{charset}, but $settings passed in to _get_connection is actually just the settings for that particular connection. It should be renamed to $conn_details or similar, to avoid this confusion, and it will need to be able to get the charset from the app-wide settings; I imagine this means that the D1/D2-specific code which passes the plugin's settings to the Core stuff will need to read the charset setting for us and inject it into the plugin settings passed to D::P::Core::database().

I'll try to find a bit of time to work on this in the near future if I can; things have been hectic lately with DIY and an impending baby any day now, though, so I can't promise an ETA.

drmuey commented 8 years ago

Congratulations ;) @bigpresh , @ambs that version did not do it. I just added a wrapper function to use instead of database():

my $dbh = database( { driver => 'SQLite', database => $path_to_db } );
$dbh->{sqlite_unicode} = 1;
return $dbh;