Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit Order Column value checker #211

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ ForeignKeyTypeChecker fail User something association (something) of class (User
ThreeStateBooleanChecker fail Company active boolean column should have NOT NULL constraint
MissingAssociationClassChecker fail Company anything refers to undefined model "Anything"
MissingTableChecker fail LegacyModel should have a table in the database
ImplicitOrderingChecker fail Secondary::User id implicit_order_column is recommended when using uuid column type for primary key
```

## Funding
Expand Down
2 changes: 2 additions & 0 deletions lib/database_consistency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
require 'database_consistency/writers/simple/three_state_boolean'
require 'database_consistency/writers/simple/missing_association_class'
require 'database_consistency/writers/simple/missing_table'
require 'database_consistency/writers/simple/implicit_order_column_missing'
require 'database_consistency/writers/simple_writer'

require 'database_consistency/writers/autofix/helpers/migration'
Expand Down Expand Up @@ -80,6 +81,7 @@
require 'database_consistency/checkers/column_checkers/primary_key_type_checker'
require 'database_consistency/checkers/column_checkers/enum_value_checker'
require 'database_consistency/checkers/column_checkers/three_state_boolean_checker'
require 'database_consistency/checkers/column_checkers/implicit_ordering_checker.rb'

require 'database_consistency/checkers/validator_checkers/validator_checker'
require 'database_consistency/checkers/validator_checkers/missing_unique_index_checker'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

module DatabaseConsistency
module Checkers
# This class checks that primary column type is "uuid" and the model class defines `self.implicit_order_column`
class ImplicitOrderingChecker < ColumnChecker
Report = ReportBuilder.define(DatabaseConsistency::Report)

private

TARGET_COLUMN_TYPE = 'uuid'

# We skip check when:
# - adapter is not PostgreSQL
# - column is not a primary key
# - column type is not "uuid"
def preconditions
ActiveRecord::VERSION::MAJOR >= 6 &&
Helper.postgresql? &&
primary_field? &&
column.sql_type.to_s.match(TARGET_COLUMN_TYPE)
end

# Table of possible statuses
# | defined `self.implicit_order_column` | status |
# | ----------------------------------- | ------ |
# | yes | ok |
# | no | fail |
def check
if implicit_order_column_defined?
report_template(:ok)
else
report_template(:fail, error_slug: :implicit_order_column_missing)
end
end

# @return [Boolean]
def primary_field?
column.name.to_s == model.primary_key.to_s
end

# @return [Boolean]
def implicit_order_column_defined?
model.implicit_order_column.present?
end
end
end
end
3 changes: 2 additions & 1 deletion lib/database_consistency/processors/columns_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ class ColumnsProcessor < BaseProcessor
Checkers::LengthConstraintChecker,
Checkers::PrimaryKeyTypeChecker,
Checkers::EnumValueChecker,
Checkers::ThreeStateBooleanChecker
Checkers::ThreeStateBooleanChecker,
Checkers::ImplicitOrderingChecker
].freeze

private
Expand Down
2 changes: 1 addition & 1 deletion lib/database_consistency/report_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def self.define(klass, *attrs)
attr_reader(*attrs)

class_eval(<<~DEF, __FILE__, __LINE__ + 1)
def initialize(#{attrs.map { |attr| "#{attr}:" }.join(', ')}, **opts)
def initialize(#{attrs.map { |attr| "#{attr}:" }.join(', ')}#{"," if attrs.any?} **opts)
super(**opts) if opts.any?
#{attrs.map { |attr| "@#{attr} = #{attr}" }.join("\n")}
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

module DatabaseConsistency
module Writers
module Simple
class ImplicitOrderColumnMissing < Base # :nodoc:
private

def template
'implicit_order_column is recommended when using uuid column type for primary key'
end

def unique_attributes
{
table_or_model_name: report.table_or_model_name,
column_or_attribute_name: report.column_or_attribute_name
}
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/database_consistency/writers/simple_writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class SimpleWriter < BaseWriter
enum_values_inconsistent_with_ar_enum: Simple::EnumValuesInconsistentWithArEnum,
enum_values_inconsistent_with_inclusion: Simple::EnumValuesInconsistentWithInclusion,
has_one_missing_unique_index: Simple::HasOneMissingUniqueIndex,
implicit_order_column_missing: Simple::ImplicitOrderColumnMissing,
inconsistent_enum_type: Simple::InconsistentEnumType,
inconsistent_types: Simple::InconsistentTypes,
length_validator_greater_limit: Simple::LengthValidatorGreaterLimit,
Expand Down
2 changes: 2 additions & 0 deletions rails7-example/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ ruby "3.2.0"

gem 'database_consistency', path: '../', group: :development, require: false

gem "pg"

# Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main"
gem "rails", "~> 7.0.6"

Expand Down
2 changes: 2 additions & 0 deletions rails7-example/app/models/secondary/user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class Secondary::User < SecondaryRecord
end
16 changes: 16 additions & 0 deletions rails7-example/config/.database.postgresql.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
default: &default
adapter: postgresql
pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
timeout: 5000
username: test
password: password
host: localhost

development:
primary:
<<: *default
database: development
secondary:
<<: *default
database: development_secondary
migrations_paths: db/migrate_secondary
30 changes: 30 additions & 0 deletions rails7-example/config/.database.sqlite.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# SQLite. Versions 3.8.0 and up are supported.
# gem install sqlite3
#
# Ensure the SQLite 3 gem is defined in your Gemfile
# gem "sqlite3"
#
default: &default
adapter: sqlite3
pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
timeout: 5000

development:
primary:
<<: *default
database: db/development.sqlite3
secondary:
<<: *default
database: db/development_secondary.sqlite3
migrations_paths: db/migrate_secondary

# Warning: The database defined as "test" will be erased and
# re-generated from your development database when you run "rake".
# Do not set this db to the same as development or production.
test:
<<: *default
database: db/test.sqlite3

production:
<<: *default
database: db/production.sqlite3
26 changes: 6 additions & 20 deletions rails7-example/config/database.yml
Original file line number Diff line number Diff line change
@@ -1,30 +1,16 @@
# SQLite. Versions 3.8.0 and up are supported.
# gem install sqlite3
#
# Ensure the SQLite 3 gem is defined in your Gemfile
# gem "sqlite3"
#
default: &default
adapter: sqlite3
adapter: postgresql
pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
timeout: 5000
username: test
password: password
host: localhost

development:
primary:
<<: *default
database: db/development.sqlite3
database: development
secondary:
<<: *default
database: db/development_secondary.sqlite3
database: development_secondary
migrations_paths: db/migrate_secondary

# Warning: The database defined as "test" will be erased and
# re-generated from your development database when you run "rake".
# Do not set this db to the same as development or production.
test:
<<: *default
database: db/test.sqlite3

production:
<<: *default
database: db/production.sqlite3
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class CreateUsers < ActiveRecord::Migration[7.0]
def change
create_table :users, id: :uuid do |t|
t.timestamps
end
end
end
11 changes: 8 additions & 3 deletions rails7-example/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2021_12_29_101039) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

create_table "companies", force: :cascade do |t|
t.boolean "available", default: false, null: false
t.boolean "active"
Expand All @@ -26,8 +29,10 @@
t.datetime "updated_at", null: false
end

# Could not dump table "organizations" because of following StandardError
# Unknown type 'bigserial' for column 'id'
create_table "organizations", force: :cascade do |t|
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "users", force: :cascade do |t|
t.string "email", null: false
Expand All @@ -36,7 +41,7 @@
t.string "address"
t.string "code", null: false
t.string "slug", null: false
t.integer "company_id", limit: 8, null: false
t.bigint "company_id", null: false
t.integer "country_id"
t.integer "organization_id", null: false
t.integer "invitable_id", null: false
Expand Down
10 changes: 9 additions & 1 deletion rails7-example/db/secondary_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,17 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2019_10_12_163944) do
ActiveRecord::Schema[7.0].define(version: 2023_07_22_090559) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

create_table "companies", force: :cascade do |t|
t.string "name", null: false
end

create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

end
67 changes: 67 additions & 0 deletions spec/checkers/implicit_ordering_checker_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# frozen_string_literal: true

RSpec.describe DatabaseConsistency::Checkers::ImplicitOrderingChecker, :postgresql do
subject(:checker) { described_class.new(model, column) }

let(:klass) { define_class { |klass| klass.primary_key = :id } }
let(:model) { klass }
let(:column) { klass.columns.first }

before do
skip if ActiveRecord::VERSION::MAJOR < 6
end

context 'when primary key type is uuid and model defines self.implicit_order_column' do
before do
define_database do
create_table :entities, id: :uuid
end

klass.class_eval do
self.implicit_order_column = :created_at
end
end

specify do
expect(checker.report).to have_attributes(
checker_name: 'ImplicitOrderingChecker',
table_or_model_name: klass.name,
column_or_attribute_name: 'id',
status: :ok,
error_slug: nil,
error_message: nil
)
end
end

context 'when primary key type is uuid and model does not define self.implicit_order_column' do
before do
define_database do
create_table :entities, id: :uuid
end
end

specify do
expect(checker.report).to have_attributes(
checker_name: 'ImplicitOrderingChecker',
table_or_model_name: klass.name,
column_or_attribute_name: 'id',
status: :fail,
error_slug: :implicit_order_column_missing,
error_message: nil
)
end
end

context 'when primary key type is not uuid' do
before do
define_database do
create_table :entities, id: :bigint
end
end

specify do
expect(checker.report).to be_nil
end
end
end