From d3a34233b97c01da4e730ff0b145f23f2ce485a3 Mon Sep 17 00:00:00 2001 From: cryptogopher Date: Tue, 5 May 2026 20:33:21 +0200 Subject: [PATCH] Set constraints on Readouts.user_id foreign key Add Measurements table --- DESIGN.md | 8 +-- README.md | 4 ++ app/controllers/readouts_controller.rb | 2 +- app/models/quantity.rb | 1 + app/models/readout.rb | 2 + db/migrate/20230602185352_create_units.rb | 1 + .../20250104194343_create_quantities.rb | 1 + db/migrate/20250121230456_create_readouts.rb | 61 ++++++++++++++++--- db/schema.rb | 26 ++++++-- db/sqlite3_schema.rb | 23 +++++-- 10 files changed, 107 insertions(+), 22 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index d5d8cf1..d8abb33 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -34,10 +34,10 @@ whenever a change is considered, to avoid regressions. custom `BigDecimal` type ### Database layer vs application layer data model constraints -* database constraints are the final frontier against data corruption, - * they should safeguard against data _consistency_ loss under _all_ data - (not schema) manipulation scenarios, including application level logic - errors and direct data manipulation, +* database constraints are the final guard against data integrity corruption, + * they should safeguard against data referential integrity loss under _all_ + data (not schema) manipulation scenarios, including application level + logic errors and direct data manipulation (e.g. through `dbconsole`), * application constraints can be as restrictive as database constraints or more, but not less, as it doesn't serve any use case, * proper application level constraints should prevent unhandled database diff --git a/README.md b/README.md index ee82bb7..8df5a38 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,10 @@ below shows how to grant privileges to all databases which names start with mysql> grant all privileges on `fixinme-%`.* to `fixinme-dev`@localhost; mysql> flush privileges; +Dumping development data before database reset: + + mysqldump -h
-u -p --no-create-info --no-tablespaces --complete-insert > tmp/data.sql + ### Development environment Starting application server in development environment: diff --git a/app/controllers/readouts_controller.rb b/app/controllers/readouts_controller.rb index e47f271..b90c1f1 100644 --- a/app/controllers/readouts_controller.rb +++ b/app/controllers/readouts_controller.rb @@ -6,7 +6,7 @@ class ReadoutsController < ApplicationController def new @quantities -= @prev_quantities # TODO: raise ParameterInvalid if new_quantities.empty? - @readouts = current_user.readouts.build(@quantities.map { |q| {quantity: q} }) + @readouts = @quantities.map { |q| q.readouts.build } @user_units = current_user.units.ordered diff --git a/app/models/quantity.rb b/app/models/quantity.rb index ed81124..c1ad05e 100644 --- a/app/models/quantity.rb +++ b/app/models/quantity.rb @@ -6,6 +6,7 @@ class Quantity < ApplicationRecord belongs_to :parent, optional: true, class_name: "Quantity" has_many :subquantities, ->{ order(:name) }, class_name: "Quantity", inverse_of: :parent, dependent: :restrict_with_error + has_many :readouts, dependent: :restrict_with_error validate if: ->{ parent.present? } do errors.add(:parent, :user_mismatch) unless user_id == parent.user_id diff --git a/app/models/readout.rb b/app/models/readout.rb index f50037c..13bafcb 100644 --- a/app/models/readout.rb +++ b/app/models/readout.rb @@ -4,4 +4,6 @@ class Readout < ApplicationRecord belongs_to :user belongs_to :quantity belongs_to :unit + + # TODO: validate quantity.user_id == unit.user_id != NULL end diff --git a/db/migrate/20230602185352_create_units.rb b/db/migrate/20230602185352_create_units.rb index 2e7a9a6..5dcea1a 100644 --- a/db/migrate/20230602185352_create_units.rb +++ b/db/migrate/20230602185352_create_units.rb @@ -10,5 +10,6 @@ class CreateUnits < ActiveRecord::Migration[8.1] t.timestamps end add_index :units, [:user_id, :symbol] + add_index :units, [:id, :user_id] end end diff --git a/db/migrate/20250104194343_create_quantities.rb b/db/migrate/20250104194343_create_quantities.rb index 1a70d32..3816eb8 100644 --- a/db/migrate/20250104194343_create_quantities.rb +++ b/db/migrate/20250104194343_create_quantities.rb @@ -13,5 +13,6 @@ class CreateQuantities < ActiveRecord::Migration[8.1] t.string :pathname, null: false, limit: 511 end add_index :quantities, [:user_id, :parent_id, :name] + add_index :quantities, [:id, :user_id] end end diff --git a/db/migrate/20250121230456_create_readouts.rb b/db/migrate/20250121230456_create_readouts.rb index 208f6f4..996d879 100644 --- a/db/migrate/20250121230456_create_readouts.rb +++ b/db/migrate/20250121230456_create_readouts.rb @@ -1,16 +1,63 @@ class CreateReadouts < ActiveRecord::Migration[8.1] def change - create_table :readouts do |t| - t.references :user, null: false, foreign_key: {on_delete: :cascade} - t.references :quantity, null: false, foreign_key: true - # :category + :value + :unit as a separate table? (NumericValue, TextValue) - t.integer :category, null: false, default: 0 - t.float :value, null: false, limit: Float::MANT_DIG - t.references :unit, foreign_key: true + create_table :measurements do |t| + t.datetime :taken_at, null: false #t.references :collector, foreign_key: true #t.references :device, foreign_key: true + #t.references :note, foreign_key: true t.timestamps end + add_index :measurements, :taken_at + + # Defining Readouts as a super-/subclass polymorphic relations for different + # subclass data types (numeric, string, file) is not possible with proper + # referential integrity constraints. The required constraints are: + # * for every subclass record to have superclass record, + # * for every superclass record to have only one type of subclass record, + # * for every superclass record to have subclass record (unenforcable). + # * this can be partially remedied by making superlass an abstract class in + # Rails and disallow direct creation of records, but direct data + # manipulation can still break referential integrity. + # Defining separate {Numeric,Text,File}_Readouts tables would make the + # unique index constraint unenforcable. + create_table :readouts do |t| + t.references :user, null: false, foreign_key: {on_delete: :cascade} + t.references :measurement, foreign_key: {on_delete: :cascade} + t.references :quantity, null: false, foreign_key: {on_delete: :cascade} + t.integer :category, null: false, default: 0 + t.float :value, null: false, limit: Float::MANT_DIG + t.references :unit, null: false, foreign_key: {on_delete: :cascade} + # TODO: consider additional columns to allow wider range of value types + # t.text :text + # t.datetime :time + # t.references :file + # Possibly mutually exclusive with :unit or check constraint for: + # :unit is not null <=> :value is not null + + t.timestamps + end + add_index :readouts, [:measurement_id, :quantity_id, :category] + add_foreign_key :readouts, :quantities, column: [:quantity_id, :user_id], + primary_key: [:id, :user_id] + add_foreign_key :readouts, :units, column: [:unit_id, :user_id], + primary_key: [:id, :user_id] + + # TODO: remove below tables after current setup verified + #create_table :numeric_values do |t| + # t.references :readout, null: false, foreign_key: {on_delete: :cascade} + # t.float :value, null: false, limit: Float::MANT_DIG + # t.references :unit, null: false, foreign_key: {on_delete: :cascade} + # # + generated, not stored column :value_type + # # + foreign key constraint to readouts: [:readout_id, :value_id, :value_type] + # # or 2 constraints: [:readout_id, :value_id], [:value_id, :value_type] + # # if readouts.value_id needed, otherwise just one constraint: + # # [:readout_id, :value_type] + #end + + #create_table :string_values do |t| + # t.references :readout, null: false, foreign_key: {on_delete: :cascade} + # t.string :value, null: false, limit: 32 + #end end end diff --git a/db/schema.rb b/db/schema.rb index 9e833e9..184f80b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,14 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do +ActiveRecord::Schema[8.1].define(version: 2025_01_21_230456) do + create_table "measurements", charset: "utf8mb4", collation: "utf8mb4_0900_as_ci", force: :cascade do |t| + t.datetime "taken_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["taken_at"], name: "index_measurements_on_taken_at", unique: true + end + create_table "quantities", charset: "utf8mb4", collation: "utf8mb4_0900_as_ci", force: :cascade do |t| t.bigint "user_id" t.string "name", limit: 31, null: false @@ -20,6 +27,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do t.datetime "updated_at", null: false t.integer "depth", default: 0, null: false t.string "pathname", limit: 511, null: false + t.index ["id", "user_id"], name: "index_quantities_on_id_and_user_id", unique: true t.index ["parent_id"], name: "index_quantities_on_parent_id" t.index ["user_id", "parent_id", "name"], name: "index_quantities_on_user_id_and_parent_id_and_name", unique: true t.index ["user_id"], name: "index_quantities_on_user_id" @@ -27,14 +35,18 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do create_table "readouts", charset: "utf8mb4", collation: "utf8mb4_0900_as_ci", force: :cascade do |t| t.bigint "user_id", null: false + t.bigint "measurement_id" t.bigint "quantity_id", null: false t.integer "category", default: 0, null: false t.float "value", limit: 53, null: false - t.bigint "unit_id" + t.bigint "unit_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["quantity_id", "created_at"], name: "index_readouts_on_quantity_id_and_created_at", unique: true + t.index ["measurement_id", "quantity_id", "category"], name: "index_readouts_on_measurement_id_and_quantity_id_and_category", unique: true + t.index ["measurement_id"], name: "index_readouts_on_measurement_id" + t.index ["quantity_id", "user_id"], name: "fk_rails_9d92eaafc6" t.index ["quantity_id"], name: "index_readouts_on_quantity_id" + t.index ["unit_id", "user_id"], name: "fk_rails_348b0fb4c5" t.index ["unit_id"], name: "index_readouts_on_unit_id" t.index ["user_id"], name: "index_readouts_on_user_id" end @@ -48,6 +60,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["base_id"], name: "index_units_on_base_id" + t.index ["id", "user_id"], name: "index_units_on_id_and_user_id", unique: true t.index ["user_id", "symbol"], name: "index_units_on_user_id_and_symbol", unique: true t.index ["user_id"], name: "index_units_on_user_id" end @@ -72,8 +85,11 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do add_foreign_key "quantities", "quantities", column: "parent_id", on_delete: :cascade add_foreign_key "quantities", "users", on_delete: :cascade - add_foreign_key "readouts", "quantities" - add_foreign_key "readouts", "units" + add_foreign_key "readouts", "measurements", on_delete: :cascade + add_foreign_key "readouts", "quantities", column: ["quantity_id", "user_id"], primary_key: ["id", "user_id"] + add_foreign_key "readouts", "quantities", on_delete: :cascade + add_foreign_key "readouts", "units", column: ["unit_id", "user_id"], primary_key: ["id", "user_id"] + add_foreign_key "readouts", "units", on_delete: :cascade add_foreign_key "readouts", "users", on_delete: :cascade add_foreign_key "units", "units", column: "base_id", on_delete: :cascade add_foreign_key "units", "users", on_delete: :cascade diff --git a/db/sqlite3_schema.rb b/db/sqlite3_schema.rb index e2ea3fb..b717eeb 100644 --- a/db/sqlite3_schema.rb +++ b/db/sqlite3_schema.rb @@ -10,7 +10,13 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do +ActiveRecord::Schema[8.1].define(version: 2025_01_21_230456) do + create_table "measurements", force: :cascade do |t| + t.datetime "taken_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "quantities", force: :cascade do |t| t.integer "user_id" t.string "name", limit: 31, null: false @@ -20,6 +26,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do t.datetime "updated_at", null: false t.integer "depth", default: 0, null: false t.string "pathname", limit: 511, null: false + t.index ["id", "user_id"], name: "index_quantities_on_id_and_user_id", unique: true t.index ["parent_id"], name: "index_quantities_on_parent_id" t.index ["user_id", "parent_id", "name"], name: "index_quantities_on_user_id_and_parent_id_and_name", unique: true t.index ["user_id"], name: "index_quantities_on_user_id" @@ -27,13 +34,15 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do create_table "readouts", force: :cascade do |t| t.integer "user_id", null: false + t.integer "measurement_id" t.integer "quantity_id", null: false t.integer "category", default: 0, null: false t.float "value", limit: 53, null: false - t.integer "unit_id" + t.integer "unit_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["quantity_id", "created_at"], name: "index_readouts_on_quantity_id_and_created_at", unique: true + t.index ["measurement_id", "quantity_id", "category"], name: "index_readouts_on_measurement_id_and_quantity_id_and_category", unique: true + t.index ["measurement_id"], name: "index_readouts_on_measurement_id" t.index ["quantity_id"], name: "index_readouts_on_quantity_id" t.index ["unit_id"], name: "index_readouts_on_unit_id" t.index ["user_id"], name: "index_readouts_on_user_id" @@ -48,6 +57,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["base_id"], name: "index_units_on_base_id" + t.index ["id", "user_id"], name: "index_units_on_id_and_user_id", unique: true t.index ["user_id", "symbol"], name: "index_units_on_user_id_and_symbol", unique: true t.index ["user_id"], name: "index_units_on_user_id" end @@ -72,8 +82,11 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_21_230456) do add_foreign_key "quantities", "quantities", column: "parent_id", on_delete: :cascade add_foreign_key "quantities", "users", on_delete: :cascade - add_foreign_key "readouts", "quantities" - add_foreign_key "readouts", "units" + add_foreign_key "readouts", "measurements", on_delete: :cascade + add_foreign_key "readouts", "quantities", column: ["quantity_id", "user_id"], primary_key: ["id", "user_id"] + add_foreign_key "readouts", "quantities", on_delete: :cascade + add_foreign_key "readouts", "units", column: ["unit_id", "user_id"], primary_key: ["id", "user_id"] + add_foreign_key "readouts", "units", on_delete: :cascade add_foreign_key "readouts", "users", on_delete: :cascade add_foreign_key "units", "units", column: "base_id", on_delete: :cascade add_foreign_key "units", "users", on_delete: :cascade