Skip to content

Commit

Permalink
Harmonize time zone spelling in InflateColumn::DateTime
Browse files Browse the repository at this point in the history
The rest of the DateTime ecosystem consistently uses "time zone" and
"time_zone", so use that in InflateColumn::DateTime too

"timezone" is still accepted for backwards compatibility
  • Loading branch information
ilmari authored and ribasushi committed May 19, 2016
1 parent 59d017a commit eef9b48
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 30 deletions.
2 changes: 2 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Revision history for DBIx::Class
- When using non-scalars (e.g. arrays) as literal bind values it is no
longer necessary to explicitly specify a bindtype (this turned out
to be a mostly useless overprotection)
- InflateColumn::DateTime now accepts the ecosystem-standard option
'time_zone', in addition to the DBIC-only 'timezone' (GH#28)
- DBIx::Class::Optional::Dependencies now properly understands
combinations of requirements and does the right thing with e.g.
->req_list_for([qw( rdbms_oracle ic_dt )]) bringing in the Oracle
Expand Down
40 changes: 25 additions & 15 deletions lib/DBIx/Class/InflateColumn/DateTime.pm
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@ Then you can treat the specified column as a L<DateTime> object.
print "This event starts the month of ".
$event->starts_when->month_name();
If you want to set a specific timezone and locale for that field, use:
If you want to set a specific time zone and locale for that field, use:
__PACKAGE__->add_columns(
starts_when => { data_type => 'datetime', timezone => "America/Chicago", locale => "de_DE" }
starts_when => { data_type => 'datetime', time_zone => "America/Chicago", locale => "de_DE" }
);
Note: DBIC before 0.082900 only accepted C<timezone>, and silently discarded
any C<time_zone> arguments. For backwards compatibility, C<timezone> will
continue being accepted as a synonym for C<time_zone>.
If you want to inflate no matter what data_type your column is,
use inflate_datetime or inflate_date:
Expand Down Expand Up @@ -73,7 +77,7 @@ that this feature is new as of 0.07, so it may not be perfect yet - bug
reports to the list very much welcome).
If the data_type of a field is C<date>, C<datetime> or C<timestamp> (or
a derivative of these datatypes, e.g. C<timestamp with timezone>), this
a derivative of these datatypes, e.g. C<timestamp with time zone>), this
module will automatically call the appropriate parse/format method for
deflation/inflation as defined in the storage class. For instance, for
a C<datetime> field the methods C<parse_datetime> and C<format_datetime>
Expand Down Expand Up @@ -152,7 +156,7 @@ sub register_column {
}

if ($info->{extra}) {
for my $slot (qw/timezone locale floating_tz_ok/) {
for my $slot (qw/time_zone timezone locale floating_tz_ok/) {
if ( defined $info->{extra}{$slot} ) {
carp "Putting $slot into extra => { $slot => '...' } has been deprecated, ".
"please put it directly into the '$column' column definition.";
Expand All @@ -161,6 +165,12 @@ sub register_column {
}
}

if ( defined $info->{timezone} ) {
$self->throw_exception("Cannot specify both 'timezone' and 'time_zone' in '$column' column defintion.")
if defined $info->{time_zone};
$info->{time_zone} = delete $info->{timezone};

This comment has been minimized.

Copy link
@ribasushi

ribasushi Jun 9, 2016

Collaborator

@ilmari I am afraid we will need to partially revert this

The fault is entirely mine I didn't think this through.

The column_info RV is semi-public API. As such there are things relying on the shitty spelling and likely many many more like that on DarkPan.

So while we could and should accept the new name, we need to keep it under the old one in the publicly accessible info-hash.

Let me know if you can get to this in the next several days.

This comment has been minimized.

Copy link
@ilmari

ilmari Jun 9, 2016

Author Contributor

I should be able to get to it over the weekend.

}

# shallow copy to avoid unfounded(?) Devel::Cycle complaints
my $infcopy = {%$info};

Expand Down Expand Up @@ -225,7 +235,7 @@ sub _datetime_parser {
sub _post_inflate_datetime {
my( $self, $dt, $info ) = @_;

$dt->set_time_zone($info->{timezone}) if defined $info->{timezone};
$dt->set_time_zone($info->{time_zone}) if defined $info->{time_zone};
$dt->set_locale($info->{locale}) if defined $info->{locale};

return $dt;
Expand All @@ -234,14 +244,14 @@ sub _post_inflate_datetime {
sub _pre_deflate_datetime {
my( $self, $dt, $info ) = @_;

if (defined $info->{timezone}) {
carp "You're using a floating timezone, please see the documentation of"
if (defined $info->{time_zone}) {
carp "You're using a floating time zone, please see the documentation of"
. " DBIx::Class::InflateColumn::DateTime for an explanation"
if ref( $dt->time_zone ) eq 'DateTime::TimeZone::Floating'
and not $info->{floating_tz_ok}
and not $ENV{DBIC_FLOATING_TZ_OK};

$dt->set_time_zone($info->{timezone});
$dt->set_time_zone($info->{time_zone});
}

$dt->set_locale($info->{locale}) if defined $info->{locale};
Expand All @@ -254,13 +264,13 @@ __END__
=head1 USAGE NOTES
If you have a datetime column with an associated C<timezone>, and subsequently
If you have a datetime column with an associated C<time_zone>, and subsequently
create/update this column with a DateTime object in the L<DateTime::TimeZone::Floating>
timezone, you will get a warning (as there is a very good chance this will not have the
time zone, you will get a warning (as there is a very good chance this will not have the
result you expect). For example:
__PACKAGE__->add_columns(
starts_when => { data_type => 'datetime', timezone => "America/Chicago" }
starts_when => { data_type => 'datetime', time_zone => "America/Chicago" }
);
my $event = $schema->resultset('EventTZ')->create({
Expand All @@ -273,7 +283,7 @@ The warning can be avoided in several ways:
=item Fix your broken code
When calling C<set_time_zone> on a Floating DateTime object, the timezone is simply
When calling C<set_time_zone> on a Floating DateTime object, the time zone is simply
set to the requested value, and B<no time conversion takes place>. It is always a good idea
to be supply explicit times to the database:
Expand All @@ -284,7 +294,7 @@ to be supply explicit times to the database:
=item Suppress the check on per-column basis
__PACKAGE__->add_columns(
starts_when => { data_type => 'datetime', timezone => "America/Chicago", floating_tz_ok => 1 }
starts_when => { data_type => 'datetime', time_zone => "America/Chicago", floating_tz_ok => 1 }
);
=item Suppress the check globally
Expand All @@ -293,7 +303,7 @@ Set the environment variable DBIC_FLOATING_TZ_OK to some true value.
=back
Putting extra attributes like timezone, locale or floating_tz_ok into extra => {} has been
Putting extra attributes like time_zone, locale or floating_tz_ok into extra => {} has been
B<DEPRECATED> because this gets you into trouble using L<DBIx::Class::Schema::Versioned>.
Instead put it directly into the columns definition like in the examples above. If you still
use the old way you'll see a warning - please fix your code then!
Expand All @@ -305,7 +315,7 @@ use the old way you'll see a warning - please fix your code then!
=item More information about the add_columns method, and column metadata,
can be found in the documentation for L<DBIx::Class::ResultSource>.
=item Further discussion of problems inherent to the Floating timezone:
=item Further discussion of problems inherent to the Floating time zone:
L<Floating DateTimes|DateTime/Floating DateTimes>
and L<< $dt->set_time_zone|DateTime/"Set" Methods >>
Expand Down
22 changes: 11 additions & 11 deletions t/icdt/offline_mysql.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use DBIx::Class::_Util 'sigwarn_silencer';

my $schema = DBICTest->init_schema();

# Test "timezone" parameter
# Test "time_zone" parameter
foreach my $tbl (qw/EventTZ EventTZDeprecated/) {
my $event_tz = $schema->resultset($tbl)->create({
starts_at => DateTime->new(year=>2007, month=>12, day=>31, time_zone => "America/Chicago" ),
Expand All @@ -34,25 +34,25 @@ foreach my $tbl (qw/EventTZ EventTZDeprecated/) {
is ($event_tz->created_on->month_name, "January", 'Default locale loaded: month_name');

my $starts_at = $event_tz->starts_at;
is("$starts_at", '2007-12-31T00:00:00', 'Correct date/time using timezone');
is("$starts_at", '2007-12-31T00:00:00', 'Correct date/time using time zone');

my $created_on = $event_tz->created_on;
is("$created_on", '2006-01-31T12:34:56', 'Correct timestamp using timezone');
is($event_tz->created_on->time_zone->name, "America/Chicago", "Correct timezone");
is("$created_on", '2006-01-31T12:34:56', 'Correct timestamp using time zone');
is($event_tz->created_on->time_zone->name, "America/Chicago", "Correct time zone");

my $loaded_event = $schema->resultset($tbl)->find( $event_tz->id );

isa_ok($loaded_event->starts_at, 'DateTime', 'DateTime returned');
$starts_at = $loaded_event->starts_at;
is("$starts_at", '2007-12-31T00:00:00', 'Loaded correct date/time using timezone');
is($starts_at->time_zone->name, 'America/Chicago', 'Correct timezone');
is("$starts_at", '2007-12-31T00:00:00', 'Loaded correct date/time using time zone');
is($starts_at->time_zone->name, 'America/Chicago', 'Correct time zone');

isa_ok($loaded_event->created_on, 'DateTime', 'DateTime returned');
$created_on = $loaded_event->created_on;
is("$created_on", '2006-01-31T12:34:56', 'Loaded correct timestamp using timezone');
is($created_on->time_zone->name, 'America/Chicago', 'Correct timezone');
is("$created_on", '2006-01-31T12:34:56', 'Loaded correct timestamp using time zone');
is($created_on->time_zone->name, 'America/Chicago', 'Correct time zone');

# Test floating timezone warning
# Test floating time zone warning
# We expect one warning
SKIP: {
skip "ENV{DBIC_FLOATING_TZ_OK} was set, skipping", 1 if $ENV{DBIC_FLOATING_TZ_OK};
Expand All @@ -63,8 +63,8 @@ foreach my $tbl (qw/EventTZ EventTZDeprecated/) {
created_on => DateTime->new(year=>2006, month=>1, day=>31, hour => 13, minute => 34, second => 56 ),
});
},
qr/You're using a floating timezone, please see the documentation of DBIx::Class::InflateColumn::DateTime for an explanation/,
'Floating timezone warning'
qr/You're using a floating time zone, please see the documentation of DBIx::Class::InflateColumn::DateTime for an explanation/,
'Floating time zone warning'
);
};

Expand Down
4 changes: 3 additions & 1 deletion t/lib/DBICTest/Schema/EventTZ.pm
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ __PACKAGE__->table('event');

__PACKAGE__->add_columns(
id => { data_type => 'integer', is_auto_increment => 1 },
starts_at => { data_type => 'datetime', timezone => "America/Chicago", locale => 'de_DE', datetime_undef_if_invalid => 1 },
starts_at => { data_type => 'datetime', time_zone => "America/Chicago", locale => 'de_DE', datetime_undef_if_invalid => 1 },

# DO NOT change 'timezone' - there to test the legacy syntax
created_on => { data_type => 'timestamp', timezone => "America/Chicago", floating_tz_ok => 1 },
);

Expand Down
4 changes: 3 additions & 1 deletion t/lib/DBICTest/Schema/EventTZDeprecated.pm
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ __PACKAGE__->table('event');

__PACKAGE__->add_columns(
id => { data_type => 'integer', is_auto_increment => 1 },
starts_at => { data_type => 'datetime', extra => { timezone => "America/Chicago", locale => 'de_DE' } },
starts_at => { data_type => 'datetime', extra => { time_zone => "America/Chicago", locale => 'de_DE' } },

# DO NOT change 'timezone' - there to test the legacy syntax
created_on => { data_type => 'timestamp', extra => { timezone => "America/Chicago", floating_tz_ok => 1 } },
);

Expand Down
4 changes: 2 additions & 2 deletions t/lib/DBICTest/Schema/EventTZPg.pm
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ __PACKAGE__->table('event');

__PACKAGE__->add_columns(
id => { data_type => 'integer', is_auto_increment => 1 },
starts_at => { data_type => 'datetime', timezone => "America/Chicago", locale => 'de_DE' },
created_on => { data_type => 'timestamp with time zone', timezone => "America/Chicago" },
starts_at => { data_type => 'datetime', time_zone => "America/Chicago", locale => 'de_DE' },
created_on => { data_type => 'timestamp with time zone', time_zone => "America/Chicago" },
ts_without_tz => { data_type => 'timestamp without time zone' },
);

Expand Down

0 comments on commit eef9b48

Please sign in to comment.