Commit bc00806a authored by blackst0ne's avatar blackst0ne

Add database helpers 'add_timestamps_with_timezone' and 'timestamps_with_timezone'

parent de20057c
---
title: Add database helpers 'add_timestamps_with_timezone' and 'timestamps_with_timezone'
merge_request: 11229
author: @blackst0ne
# ActiveRecord custom data type for storing datetimes with timezone information.
# See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11229
if Gitlab::Database.postgresql?
require 'active_record/connection_adapters/postgresql_adapter'
module ActiveRecord
module ConnectionAdapters
class PostgreSQLAdapter
NATIVE_DATABASE_TYPES.merge!(datetime_with_timezone: { name: 'timestamptz' })
end
end
end
elsif Gitlab::Database.mysql?
require 'active_record/connection_adapters/mysql2_adapter'
module ActiveRecord
module ConnectionAdapters
class AbstractMysqlAdapter
NATIVE_DATABASE_TYPES.merge!(datetime_with_timezone: { name: 'timestamp' })
end
end
end
end
# ActiveRecord custom method definitions with timezone information.
# See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11229
require 'active_record/connection_adapters/abstract/schema_definitions'
# Appends columns `created_at` and `updated_at` to a table.
#
# It is used in table creation like:
# create_table 'users' do |t|
# t.timestamps_with_timezone
# end
module ActiveRecord
module ConnectionAdapters
class TableDefinition
def timestamps_with_timezone(**options)
options[:null] = false if options[:null].nil?
[:created_at, :updated_at].each do |column_name|
column(column_name, :datetime_with_timezone, options)
end
end
end
end
end
# rubocop:disable Migration/Datetime
class AddRequestedAtToMembers < ActiveRecord::Migration
def change
add_column :members, :requested_at, :datetime
......
# rubocop:disable Migration/Datetime
# rubocop:disable Migration/Timestamps
class CreatePersonalAccessTokens < ActiveRecord::Migration
def change
create_table :personal_access_tokens do |t|
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Datetime
class AddDeployments < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Datetime
class AddEnvironments < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Timestamps
class AddProtectedBranchesPushAccess < ActiveRecord::Migration
DOWNTIME = false
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Timestamps
class AddProtectedBranchesMergeAccess < ActiveRecord::Migration
DOWNTIME = false
......
# rubocop:disable Migration/Datetime
class AddResolvedToNotes < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateUserAgentDetails < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateBoards < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateLists < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
# rubocop:disable RemoveIndex
class AddDeletedAtToNamespaces < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Datetime
# rubocop:disable Migration/Timestamps
class AddTableIssueMetrics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Datetime
# rubocop:disable Migration/Timestamps
class AddTableMergeRequestMetrics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateProjectFeatures < ActiveRecord::Migration
DOWNTIME = false
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Timestamps
class CreateMergeRequestsClosingIssues < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateLabelPriorities < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
# rubocop:disable Migration/Timestamps
class CreateUserChatNamesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Timestamps
class AddRoutesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class AddLastUsedAtToKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateTimelogsCe < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
# rubocop:disable Migration/Datetime
class ChangeExpiresAtToDateInPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateChatTeams < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class CreateUploads < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class DropCiProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class RemoveUnusedCiTablesAndColumns < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateProtectedTags < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateSystemNoteMetadata < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class AddClosedAtToIssues < ActiveRecord::Migration
DOWNTIME = false
......
# rubocop:disable Migration/Timestamps
class CreateContainerRepository < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class CreateCiTriggerSchedules < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
# rubocop:disable Migration/Timestamps
class CreatePipelineSchedulesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Timestamps
class CreateRedirectRoutes < ActiveRecord::Migration
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
......
# rubocop:disable Migration/Datetime
class AddLastRepositoryUpdatedAtToProjects < ActiveRecord::Migration
DOWNTIME = false
......
# rubocop:disable Migration/Timestamps
class CreateConversationalDevelopmentIndexMetrics < ActiveRecord::Migration
DOWNTIME = false
......
# rubocop:disable Migration/Timestamps
class CreatePipelineStages < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
# rubocop:disable Migration/Datetime
class DropCiTriggerSchedulesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
......
......@@ -122,7 +122,7 @@ limit can vary from installation to installation. As a result it's recommended
you do not use more than 32 threads in a single migration. Usually 4-8 threads
should be more than enough.
## Removing indices
## Removing indexes
When removing an index make sure to use the method `remove_concurrent_index` instead
of the regular `remove_index` method. The `remove_concurrent_index` method
......@@ -142,7 +142,7 @@ class MyMigration < ActiveRecord::Migration
end
```
## Adding indices
## Adding indexes
If you need to add a unique index please keep in mind there is the possibility
of existing duplicates being present in the database. This means that should
......@@ -222,6 +222,41 @@ add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8)
add_column(:projects, :foo, :integer, default: 10, limit: 8)
```
## Timestamp column type
By default, Rails uses the `timestamp` data type that stores timestamp data without timezone information.
The `timestamp` data type is used by calling either the `add_timestamps` or the `timestamps` method.
Also Rails converts the `:datetime` data type to the `timestamp` one.
Example:
```ruby
# timestamps
create_table :users do |t|
t.timestamps
end
# add_timestamps
def up
add_timestamps :users
end
# :datetime
def up
add_column :users, :last_sign_in, :datetime
end
```
Instead of using these methods one should use the following methods to store timestamps with timezones:
* `add_timestamps_with_timezone`
* `timestamps_with_timezone`
This ensures all timestamps have a time zone specified. This in turn means existing timestamps won't
suddenly use a different timezone when the system's timezone changes. It also makes it very clear which
timezone was used in the first place.
## Testing
Make sure that your migration works with MySQL and PostgreSQL with data. An
......
module Gitlab
module Database
module MigrationHelpers
# Adds `created_at` and `updated_at` columns with timezone information.
#
# This method is an improved version of Rails' built-in method `add_timestamps`.
#
# Available options are:
# default - The default value for the column.
# null - When set to `true` the column will allow NULL values.
# The default is to not allow NULL values.
def add_timestamps_with_timezone(table_name, options = {})
options[:null] = false if options[:null].nil?
[:created_at, :updated_at].each do |column_name|
if options[:default] && transaction_open?
raise '`add_timestamps_with_timezone` with default value cannot be run inside a transaction. ' \
'You can disable transactions by calling `disable_ddl_transaction!` ' \
'in the body of your migration class'
end
# If default value is presented, use `add_column_with_default` method instead.
if options[:default]
add_column_with_default(
table_name,
column_name,
:datetime_with_timezone,
default: options[:default],
allow_null: options[:null]
)
else
add_column(table_name, column_name, :datetime_with_timezone, options)
end
end
end
# Creates a new index, concurrently when supported
#
# On PostgreSQL this method creates an index concurrently, on MySQL this
......
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that checks if 'add_timestamps' method is called with timezone information.
class AddTimestamps < RuboCop::Cop::Cop
include MigrationHelpers
MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead'.freeze
# Check methods.
def on_send(node)
return unless in_migration?(node)
add_offense(node, :selector) if method_name(node) == :add_timestamps
end
def method_name(node)
node.children[1]
end
end
end
end
end
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that checks if datetime data type is added with timezone information.
class Datetime < RuboCop::Cop::Cop
include MigrationHelpers
MSG = 'Do not use the `datetime` data type, use `datetime_with_timezone` instead'.freeze
# Check methods in table creation.
def on_def(node)
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
add_offense(send_node, :selector) if method_name(send_node) == :datetime
end
end
# Check methods.
def on_send(node)
return unless in_migration?(node)
node.each_descendant do |descendant|
add_offense(node, :expression) if descendant.type == :sym && descendant.children.last == :datetime
end
end
def method_name(node)
node.children[1]
end
end
end
end
end
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that checks if 'timestamps' method is called with timezone information.
class Timestamps < RuboCop::Cop::Cop
include MigrationHelpers
MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead'.freeze
# Check methods in table creation.
def on_def(node)
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
add_offense(send_node, :selector) if method_name(send_node) == :timestamps
end
end
def method_name(node)
node.children[1]
end
end
end
end
end
......@@ -8,7 +8,10 @@ require_relative 'cop/migration/add_column_with_default_to_large_table'
require_relative 'cop/migration/add_concurrent_foreign_key'
require_relative 'cop/migration/add_concurrent_index'
require_relative 'cop/migration/add_index'
require_relative 'cop/migration/add_timestamps'
require_relative 'cop/migration/datetime'
require_relative 'cop/migration/remove_concurrent_index'
require_relative 'cop/migration/remove_index'
require_relative 'cop/migration/reversible_add_column_with_default'
require_relative 'cop/migration/timestamps'
require_relative 'cop/migration/update_column_in_batches'
......@@ -7,7 +7,42 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
)
end
before { allow(model).to receive(:puts) }
before do
allow(model).to receive(:puts)
end
describe '#add_timestamps_with_timezone' do
before do
allow(model).to receive(:transaction_open?).and_return(false)
end
context 'using PostgreSQL' do
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
allow(model).to receive(:disable_statement_timeout)
end
it 'adds "created_at" and "updated_at" fields with the "datetime_with_timezone" data type' do
expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false })
expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false })
model.add_timestamps_with_timezone(:foo)
end
end
context 'using MySQL' do
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
end
it 'adds "created_at" and "updated_at" fields with "datetime_with_timezone" data type' do
expect(model).to receive(:add_column).with(:foo, :created_at, :datetime_with_timezone, { null: false })
expect(model).to receive(:add_column).with(:foo, :updated_at, :datetime_with_timezone, { null: false })
model.add_timestamps_with_timezone(:foo)
end
end
end
describe '#add_concurrent_index' do
context 'outside a transaction' do
......
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/add_timestamps'
describe RuboCop::Cop::Migration::AddTimestamps do
include CopHelper
subject(:cop) { described_class.new }
let(:migration_with_add_timestamps) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_timestamps(:users)
end
end
)
end
let(:migration_without_add_timestamps) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
end
end
)
end
let(:migration_with_add_timestamps_with_timezone) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_timestamps_with_timezone(:users)
end
end
)
end
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
it 'registers an offense when the "add_timestamps" method is used' do
inspect_source(cop, migration_with_add_timestamps)
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([7])
end
end
it 'does not register an offense when the "add_timestamps" method is not used' do
inspect_source(cop, migration_without_add_timestamps)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end
it 'does not register an offense when the "add_timestamps_with_timezone" method is used' do
inspect_source(cop, migration_with_add_timestamps_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'outside of migration' do
it 'registers no offense' do
inspect_source(cop, migration_with_add_timestamps)
inspect_source(cop, migration_without_add_timestamps)
inspect_source(cop, migration_with_add_timestamps_with_timezone)
expect(cop.offenses.size).to eq(0)
end
end
end
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/datetime'
describe RuboCop::Cop::Migration::Datetime do
include CopHelper
subject(:cop) { described_class.new }
let(:migration_with_datetime) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :datetime)
end
end
)
end
let(:migration_without_datetime) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
end
end
)
end
let(:migration_with_datetime_with_timezone) do
%q(
class Users < ActiveRecord::Migration
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :datetime_with_timezone)
end
end
)
end
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
it 'registers an offense when the ":datetime" data type is used' do
inspect_source(cop, migration_with_datetime)
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([7])
end
end
it 'does not register an offense when the ":datetime" data type is not used' do
inspect_source(cop, migration_without_datetime)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end
it 'does not register an offense when the ":datetime_with_timezone" data type is used' do
inspect_source(cop, migration_with_datetime_with_timezone)
aggregate_failures do
expect(cop.offenses.size).to eq(0)
end
end
end
context 'outside of migration' do
it 'registers no offense' do