From 3657278da28672220e8fbc66445440db180ae352 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Tue, 27 Oct 2020 10:37:21 +0100 Subject: [PATCH] Fixes #3257 - Data in DB and elasticsearch (e.g. ticket overview) out of sync. --- .../cop/zammad/exists_date_time_precision.rb | 65 +++++++++++++++++++ .rubocop/rubocop_zammad.rb | 1 + db/migrate/20120101000001_create_base.rb | 26 ++++---- ...20161101131409_create_doorkeeper_tables.rb | 10 +-- db/migrate/20170419000001_ldap_support.rb | 6 +- ...0190531180304_initialize_knowledge_base.rb | 16 ++--- .../20191001090809_create_active_job_locks.rb | 2 +- db/migrate/20200121000001_smime_support.rb | 4 +- .../20201002094932_datetime_precision.rb | 45 +++++++++++++ 9 files changed, 143 insertions(+), 32 deletions(-) create mode 100644 .rubocop/cop/zammad/exists_date_time_precision.rb create mode 100644 db/migrate/20201002094932_datetime_precision.rb diff --git a/.rubocop/cop/zammad/exists_date_time_precision.rb b/.rubocop/cop/zammad/exists_date_time_precision.rb new file mode 100644 index 000000000..0f67c58fc --- /dev/null +++ b/.rubocop/cop/zammad/exists_date_time_precision.rb @@ -0,0 +1,65 @@ +module RuboCop + module Cop + module Zammad + class ExistsDateTimePrecision < Cop + def_node_matcher :column?, <<-PATTERN + $(send _ {:create_column :change_column} (sym _) (sym _) (sym :datetime) ... ) + PATTERN + + def_node_matcher :column_limit?, <<-PATTERN + (send ... (hash <(pair (sym :limit) $(int 3)) ...> ) ) + PATTERN + + def_node_matcher :table?, <<-PATTERN + $(block + (send _ {:create_table :alter_table} ... ) + ... + ) + PATTERN + + def_node_matcher :table_column?, <<-PATTERN + $(send (:lvar _) {:timestamps :datetime} ... ) + PATTERN + + MSG = 'Columns of type :timestamps and :datetime needs to have limit: 3.'.freeze + +=begin + +This rubocop will match all change_column/create_column/create_table/alter_table statements +and check if there are :datetime or :timestamps column which do not have the limit: 3 setting. + + good: + + change_column :smime_certificates, :not_after_at, :datetime, limit: 3 + + create_table :sessions do |t| + t.timestamps limit: 3, null: false + end + + bad: + + change_column :smime_certificates, :not_after_at, :datetime, limit: 4 + + create_table :sessions do |t| + t.timestamps null: false + end + +=end + + def on_send(node) + return add_offense(node) if invalid_column?(node) + return add_offense(node) if invalid_table?(node) + end + + def invalid_table?(node) + table?(node&.parent&.parent) && table_column?(node) && !column_limit?(node) + end + + def invalid_column?(node) + column?(node) && !column_limit?(node) + end + + end + end + end +end diff --git a/.rubocop/rubocop_zammad.rb b/.rubocop/rubocop_zammad.rb index 630678c27..9b4753811 100644 --- a/.rubocop/rubocop_zammad.rb +++ b/.rubocop/rubocop_zammad.rb @@ -1,4 +1,5 @@ require_relative 'cop/zammad/exists_condition' +require_relative 'cop/zammad/exists_date_time_precision' require_relative 'cop/zammad/have_no_over_not_to' require_relative 'cop/zammad/no_to_sym_on_string' require_relative 'cop/zammad/prefer_negated_if_over_unless' diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 41b51b3dc..1cffcac92 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -8,7 +8,7 @@ class CreateBase < ActiveRecord::Migration[4.2] t.string :session_id, null: false t.boolean :persistent, null: true t.text :data - t.timestamps null: false + t.timestamps limit: 3, null: false end add_index :sessions, :session_id add_index :sessions, :updated_at @@ -287,7 +287,7 @@ class CreateBase < ActiveRecord::Migration[4.2] create_table :taskbars do |t| t.references :user, null: false - t.datetime :last_contact, null: false + t.datetime :last_contact, null: false, limit: 3 t.string :client_id, null: false t.string :key, limit: 100, null: false t.string :callback, limit: 100, null: false @@ -601,13 +601,13 @@ class CreateBase < ActiveRecord::Migration[4.2] add_foreign_key :object_manager_attributes, :users, column: :updated_by_id create_table :delayed_jobs, force: true do |t| - t.integer :priority, default: 0 # Allows some jobs to jump to the front of the queue - t.integer :attempts, default: 0 # Provides for retries, but still fail eventually. + t.integer :priority, default: 0 # Allows some jobs to jump to the front of the queue + t.integer :attempts, default: 0 # Provides for retries, but still fail eventually. t.text :handler # YAML-encoded string of the object that will do work t.text :last_error # reason for last failure (See Note below) - t.datetime :run_at # When to run. Could be Time.zone.now for immediately, or sometime in the future. - t.datetime :locked_at # Set when a client is working on this object - t.datetime :failed_at # Set when all retries have failed (actually, by default, the record is deleted instead) + t.datetime :run_at, limit: 3 # When to run. Could be Time.zone.now for immediately, or sometime in the future. + t.datetime :locked_at, limit: 3 # Set when a client is working on this object + t.datetime :failed_at, limit: 3 # Set when all retries have failed (actually, by default, the record is deleted instead) t.string :locked_by # Who is working on this object (if locked) t.string :queue # The name of the queue this job is in t.timestamps limit: 3, null: false @@ -635,10 +635,10 @@ class CreateBase < ActiveRecord::Migration[4.2] t.text :payload, limit: 80_000 t.text :result, limit: 80_000 - t.datetime :started_at - t.datetime :finished_at + t.datetime :started_at, limit: 3 + t.datetime :finished_at, limit: 3 - t.timestamps null: false + t.timestamps limit: 3, null: false end create_table :cti_logs do |t| @@ -716,7 +716,7 @@ class CreateBase < ActiveRecord::Migration[4.2] t.string :lock_key t.string :active_job_id - t.timestamps + t.timestamps limit: 3 end add_index :active_job_locks, :lock_key, unique: true add_index :active_job_locks, :active_job_id, unique: true @@ -726,8 +726,8 @@ class CreateBase < ActiveRecord::Migration[4.2] t.string :doc_hash, limit: 250, null: false t.string :fingerprint, limit: 250, null: false t.string :modulus, limit: 1024, null: false - t.datetime :not_before_at, null: true - t.datetime :not_after_at, null: true + t.datetime :not_before_at, null: true, limit: 3 + t.datetime :not_after_at, null: true, limit: 3 t.binary :raw, limit: 10.megabytes, null: false t.binary :private_key, limit: 10.megabytes, null: true t.string :private_key_secret, limit: 500, null: true diff --git a/db/migrate/20161101131409_create_doorkeeper_tables.rb b/db/migrate/20161101131409_create_doorkeeper_tables.rb index bfdcc37d7..30fe14268 100644 --- a/db/migrate/20161101131409_create_doorkeeper_tables.rb +++ b/db/migrate/20161101131409_create_doorkeeper_tables.rb @@ -6,7 +6,7 @@ class CreateDoorkeeperTables < ActiveRecord::Migration[4.2] t.string :secret, null: false t.text :redirect_uri, null: false t.string :scopes, null: false, default: '' - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end add_index :oauth_applications, :uid, unique: true @@ -17,8 +17,8 @@ class CreateDoorkeeperTables < ActiveRecord::Migration[4.2] t.string :token, null: false t.integer :expires_in, null: false t.text :redirect_uri, null: false - t.datetime :created_at, null: false - t.datetime :revoked_at + t.datetime :created_at, null: false # rubocop:disable Zammad/ExistsDateTimePrecision + t.datetime :revoked_at # rubocop:disable Zammad/ExistsDateTimePrecision t.string :scopes end @@ -43,8 +43,8 @@ class CreateDoorkeeperTables < ActiveRecord::Migration[4.2] t.string :refresh_token t.integer :expires_in - t.datetime :revoked_at - t.datetime :created_at, null: false + t.datetime :revoked_at # rubocop:disable Zammad/ExistsDateTimePrecision + t.datetime :created_at, null: false # rubocop:disable Zammad/ExistsDateTimePrecision t.string :scopes # If there is a previous_refresh_token column, diff --git a/db/migrate/20170419000001_ldap_support.rb b/db/migrate/20170419000001_ldap_support.rb index 97d3f69f8..3f2a066db 100644 --- a/db/migrate/20170419000001_ldap_support.rb +++ b/db/migrate/20170419000001_ldap_support.rb @@ -13,10 +13,10 @@ class LdapSupport < ActiveRecord::Migration[4.2] t.text :payload, limit: 80_000 t.text :result, limit: 80_000 - t.datetime :started_at - t.datetime :finished_at + t.datetime :started_at # rubocop:disable Zammad/ExistsDateTimePrecision + t.datetime :finished_at # rubocop:disable Zammad/ExistsDateTimePrecision - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end end diff --git a/db/migrate/20190531180304_initialize_knowledge_base.rb b/db/migrate/20190531180304_initialize_knowledge_base.rb index e4bacc129..96170ca6d 100644 --- a/db/migrate/20190531180304_initialize_knowledge_base.rb +++ b/db/migrate/20190531180304_initialize_knowledge_base.rb @@ -16,7 +16,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.string :custom_address - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_locales do |t| @@ -24,7 +24,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.belongs_to :system_locale, null: false, foreign_key: { to_table: :locales } t.boolean :primary, null: false, default: false - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_translations do |t| @@ -34,7 +34,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.references :kb_locale, null: false, foreign_key: { to_table: :knowledge_base_locales } t.references :knowledge_base, null: false, foreign_key: { to_table: :knowledge_bases, on_delete: :cascade } - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_categories do |t| @@ -44,7 +44,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.string :category_icon, null: false, limit: 30 t.integer :position, null: false, index: true - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_category_translations do |t| @@ -53,7 +53,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.references :kb_locale, null: false, foreign_key: { to_table: :knowledge_base_locales } t.references :category, null: false, foreign_key: { to_table: :knowledge_base_categories, on_delete: :cascade } - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_answers do |t| @@ -70,7 +70,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.timestamp :published_at, limit: 3, null: true t.references :published_by, foreign_key: { to_table: :users } - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_answer_translation_contents do |t| # rubocop:disable Rails/CreateTableWithTimestamps @@ -87,7 +87,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.references :created_by, null: false, foreign_key: { to_table: :users } t.references :updated_by, null: false, foreign_key: { to_table: :users } - t.timestamps null: false + t.timestamps null: false # rubocop:disable Zammad/ExistsDateTimePrecision end create_table :knowledge_base_menu_items do |t| @@ -98,7 +98,7 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] t.string :url, null: false, limit: 500 t.boolean :new_tab, null: false, default: false - t.timestamps + t.timestamps # rubocop:disable Zammad/ExistsDateTimePrecision end Setting.create_if_not_exists( diff --git a/db/migrate/20191001090809_create_active_job_locks.rb b/db/migrate/20191001090809_create_active_job_locks.rb index 9dd0f12bb..386cd959c 100644 --- a/db/migrate/20191001090809_create_active_job_locks.rb +++ b/db/migrate/20191001090809_create_active_job_locks.rb @@ -7,7 +7,7 @@ class CreateActiveJobLocks < ActiveRecord::Migration[5.2] t.string :lock_key t.string :active_job_id - t.timestamps + t.timestamps # rubocop:disable Zammad/ExistsDateTimePrecision end add_index :active_job_locks, :lock_key, unique: true add_index :active_job_locks, :active_job_id, unique: true diff --git a/db/migrate/20200121000001_smime_support.rb b/db/migrate/20200121000001_smime_support.rb index c7c385565..212bd9138 100644 --- a/db/migrate/20200121000001_smime_support.rb +++ b/db/migrate/20200121000001_smime_support.rb @@ -59,8 +59,8 @@ class SMIMESupport < ActiveRecord::Migration[5.2] t.string :doc_hash, limit: 250, null: false t.string :fingerprint, limit: 250, null: false t.string :modulus, limit: 1024, null: false - t.datetime :not_before_at, null: true - t.datetime :not_after_at, null: true + t.datetime :not_before_at, null: true # rubocop:disable Zammad/ExistsDateTimePrecision + t.datetime :not_after_at, null: true # rubocop:disable Zammad/ExistsDateTimePrecision t.binary :raw, limit: 10.megabytes, null: false t.binary :private_key, limit: 10.megabytes, null: true t.string :private_key_secret, limit: 500, null: true diff --git a/db/migrate/20201002094932_datetime_precision.rb b/db/migrate/20201002094932_datetime_precision.rb new file mode 100644 index 000000000..f7d33c1a4 --- /dev/null +++ b/db/migrate/20201002094932_datetime_precision.rb @@ -0,0 +1,45 @@ +class DatetimePrecision < ActiveRecord::Migration[5.2] + + # rubocop:disable Metrics/AbcSize + def change + + # return if it's a new setup + return if !Setting.exists?(name: 'system_init_done') + + change_column :active_job_locks, :updated_at, :datetime, limit: 3 + change_column :active_job_locks, :created_at, :datetime, limit: 3 + change_column :taskbars, :last_contact, :datetime, limit: 3, null: false + change_column :delayed_jobs, :run_at, :datetime, limit: 3 + change_column :delayed_jobs, :locked_at, :datetime, limit: 3 + change_column :delayed_jobs, :failed_at, :datetime, limit: 3 + change_column :import_jobs, :started_at, :datetime, limit: 3 + change_column :import_jobs, :finished_at, :datetime, limit: 3 + change_column :sessions, :updated_at, :datetime, limit: 3, null: false + change_column :sessions, :created_at, :datetime, limit: 3, null: false + change_column :smime_certificates, :not_before_at, :datetime, limit: 3 + change_column :smime_certificates, :not_after_at, :datetime, limit: 3 + change_column :knowledge_bases, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_bases, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_locales, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_locales, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_translations, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_translations, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_categories, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_categories, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_category_translations, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_category_translations, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_answers, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_answers, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_answer_translations, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_answer_translations, :updated_at, :datetime, limit: 3, null: false + change_column :knowledge_base_menu_items, :created_at, :datetime, limit: 3, null: false + change_column :knowledge_base_menu_items, :updated_at, :datetime, limit: 3, null: false + change_column :oauth_access_grants, :created_at, :datetime, limit: 3, null: false + change_column :oauth_access_grants, :revoked_at, :datetime, limit: 3 + change_column :oauth_access_tokens, :created_at, :datetime, limit: 3, null: false + change_column :oauth_access_tokens, :revoked_at, :datetime, limit: 3 + change_column :oauth_applications, :created_at, :datetime, limit: 3, null: false + change_column :oauth_applications, :updated_at, :datetime, limit: 3, null: false + end + # rubocop:enable Metrics/AbcSize +end