From 5ad2fc64b4d34cb2cb5e0fcf0fd3777b35058a75 Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Thu, 6 Jan 2022 22:01:09 +0100 Subject: [PATCH 1/6] DB: Move a forgotten 'UPDATE channels' statement --- src/invidious/database/channels.cr | 10 ++++++++++ src/invidious/routes/feeds.cr | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/invidious/database/channels.cr b/src/invidious/database/channels.cr index 134cf59d2..b45527333 100644 --- a/src/invidious/database/channels.cr +++ b/src/invidious/database/channels.cr @@ -42,6 +42,16 @@ module Invidious::Database::Channels PG_DB.exec(request, Time.utc, author, id) end + def update_subscription_time(id : String) + request = <<-SQL + UPDATE channels + SET subscribed = $1 + WHERE id = $2 + SQL + + PG_DB.exec(request, Time.utc, id) + end + def update_mark_deleted(id : String) request = <<-SQL UPDATE channels diff --git a/src/invidious/routes/feeds.cr b/src/invidious/routes/feeds.cr index fd8c25ce9..c323cdf78 100644 --- a/src/invidious/routes/feeds.cr +++ b/src/invidious/routes/feeds.cr @@ -362,7 +362,7 @@ module Invidious::Routes::Feeds end if ucid = HTTP::Params.parse(URI.parse(topic).query.not_nil!)["channel_id"]? - PG_DB.exec("UPDATE channels SET subscribed = $1 WHERE id = $2", Time.utc, ucid) + Invidious::Database::Channels.update_subscription_time(ucid) elsif plid = HTTP::Params.parse(URI.parse(topic).query.not_nil!)["playlist_id"]? Invidious::Database::Playlists.update_subscription_time(plid) else From a6c9b263da170eb8180f3b609ce7b4cc62ef4b0e Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Thu, 6 Jan 2022 23:33:52 +0100 Subject: [PATCH 2/6] DB: don't pass PG_DB to check_table/check_enum --- src/invidious.cr | 20 +++++++++--------- src/invidious/database/base.cr | 38 +++++++++++++++++----------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/invidious.cr b/src/invidious.cr index 7a324bd12..96e5d348e 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -113,19 +113,19 @@ LOGGER = Invidious::LogHandler.new(OUTPUT, CONFIG.log_level) # Check table integrity if CONFIG.check_tables - Invidious::Database.check_enum(PG_DB, "privacy", PlaylistPrivacy) + Invidious::Database.check_enum("privacy", PlaylistPrivacy) - Invidious::Database.check_table(PG_DB, "channels", InvidiousChannel) - Invidious::Database.check_table(PG_DB, "channel_videos", ChannelVideo) - Invidious::Database.check_table(PG_DB, "playlists", InvidiousPlaylist) - Invidious::Database.check_table(PG_DB, "playlist_videos", PlaylistVideo) - Invidious::Database.check_table(PG_DB, "nonces", Nonce) - Invidious::Database.check_table(PG_DB, "session_ids", SessionId) - Invidious::Database.check_table(PG_DB, "users", User) - Invidious::Database.check_table(PG_DB, "videos", Video) + Invidious::Database.check_table("channels", InvidiousChannel) + Invidious::Database.check_table("channel_videos", ChannelVideo) + Invidious::Database.check_table("playlists", InvidiousPlaylist) + Invidious::Database.check_table("playlist_videos", PlaylistVideo) + Invidious::Database.check_table("nonces", Nonce) + Invidious::Database.check_table("session_ids", SessionId) + Invidious::Database.check_table("users", User) + Invidious::Database.check_table("videos", Video) if CONFIG.cache_annotations - Invidious::Database.check_table(PG_DB, "annotations", Annotation) + Invidious::Database.check_table("annotations", Annotation) end end diff --git a/src/invidious/database/base.cr b/src/invidious/database/base.cr index 6e49ea1ab..a6b38f1ca 100644 --- a/src/invidious/database/base.cr +++ b/src/invidious/database/base.cr @@ -3,26 +3,26 @@ require "pg" module Invidious::Database extend self - def check_enum(db, enum_name, struct_type = nil) + def check_enum(enum_name, struct_type = nil) return # TODO - if !db.query_one?("SELECT true FROM pg_type WHERE typname = $1", enum_name, as: Bool) + if !PG_DB.query_one?("SELECT true FROM pg_type WHERE typname = $1", enum_name, as: Bool) LOGGER.info("check_enum: CREATE TYPE #{enum_name}") - db.using_connection do |conn| + PG_DB.using_connection do |conn| conn.as(PG::Connection).exec_all(File.read("config/sql/#{enum_name}.sql")) end end end - def check_table(db, table_name, struct_type = nil) + def check_table(table_name, struct_type = nil) # Create table if it doesn't exist begin - db.exec("SELECT * FROM #{table_name} LIMIT 0") + PG_DB.exec("SELECT * FROM #{table_name} LIMIT 0") rescue ex LOGGER.info("check_table: check_table: CREATE TABLE #{table_name}") - db.using_connection do |conn| + PG_DB.using_connection do |conn| conn.as(PG::Connection).exec_all(File.read("config/sql/#{table_name}.sql")) end end @@ -30,7 +30,7 @@ module Invidious::Database return if !struct_type struct_array = struct_type.type_array - column_array = get_column_array(db, table_name) + column_array = get_column_array(PG_DB, table_name) column_types = File.read("config/sql/#{table_name}.sql").match(/CREATE TABLE public\.#{table_name}\n\((?[\d\D]*?)\);/) .try &.["types"].split(",").map(&.strip).reject &.starts_with?("CONSTRAINT") @@ -41,14 +41,14 @@ module Invidious::Database if !column_array[i]? new_column = column_types.select(&.starts_with?(name))[0] LOGGER.info("check_table: ALTER TABLE #{table_name} ADD COLUMN #{new_column}") - db.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") + PG_DB.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") next end # Column doesn't exist if !column_array.includes? name new_column = column_types.select(&.starts_with?(name))[0] - db.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") + PG_DB.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") end # Column exists but in the wrong position, rotate @@ -59,29 +59,29 @@ module Invidious::Database # There's a column we didn't expect if !new_column LOGGER.info("check_table: ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]}") - db.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") + PG_DB.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") - column_array = get_column_array(db, table_name) + column_array = get_column_array(PG_DB, table_name) next end LOGGER.info("check_table: ALTER TABLE #{table_name} ADD COLUMN #{new_column}") - db.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") + PG_DB.exec("ALTER TABLE #{table_name} ADD COLUMN #{new_column}") LOGGER.info("check_table: UPDATE #{table_name} SET #{column_array[i]}_new=#{column_array[i]}") - db.exec("UPDATE #{table_name} SET #{column_array[i]}_new=#{column_array[i]}") + PG_DB.exec("UPDATE #{table_name} SET #{column_array[i]}_new=#{column_array[i]}") LOGGER.info("check_table: ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") - db.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") + PG_DB.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") LOGGER.info("check_table: ALTER TABLE #{table_name} RENAME COLUMN #{column_array[i]}_new TO #{column_array[i]}") - db.exec("ALTER TABLE #{table_name} RENAME COLUMN #{column_array[i]}_new TO #{column_array[i]}") + PG_DB.exec("ALTER TABLE #{table_name} RENAME COLUMN #{column_array[i]}_new TO #{column_array[i]}") - column_array = get_column_array(db, table_name) + column_array = get_column_array(PG_DB, table_name) end else LOGGER.info("check_table: ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") - db.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") + PG_DB.exec("ALTER TABLE #{table_name} DROP COLUMN #{column_array[i]} CASCADE") end end end @@ -91,14 +91,14 @@ module Invidious::Database column_array.each do |column| if !struct_array.includes? column LOGGER.info("check_table: ALTER TABLE #{table_name} DROP COLUMN #{column} CASCADE") - db.exec("ALTER TABLE #{table_name} DROP COLUMN #{column} CASCADE") + PG_DB.exec("ALTER TABLE #{table_name} DROP COLUMN #{column} CASCADE") end end end def get_column_array(db, table_name) column_array = [] of String - db.query("SELECT * FROM #{table_name} LIMIT 0") do |rs| + PG_DB.query("SELECT * FROM #{table_name} LIMIT 0") do |rs| rs.column_count.times do |i| column = rs.as(PG::ResultSet).field(i) column_array << column.name From c78f84d5c6677551ca32d57187887bfb37d77750 Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Thu, 6 Jan 2022 23:47:30 +0100 Subject: [PATCH 3/6] DB: Move integrity check to the base.cr file --- src/invidious.cr | 17 +---------------- src/invidious/database/base.cr | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/invidious.cr b/src/invidious.cr index 96e5d348e..d3ad18bd1 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -112,22 +112,7 @@ OUTPUT = CONFIG.output.upcase == "STDOUT" ? STDOUT : File.open(CONFIG.output, mo LOGGER = Invidious::LogHandler.new(OUTPUT, CONFIG.log_level) # Check table integrity -if CONFIG.check_tables - Invidious::Database.check_enum("privacy", PlaylistPrivacy) - - Invidious::Database.check_table("channels", InvidiousChannel) - Invidious::Database.check_table("channel_videos", ChannelVideo) - Invidious::Database.check_table("playlists", InvidiousPlaylist) - Invidious::Database.check_table("playlist_videos", PlaylistVideo) - Invidious::Database.check_table("nonces", Nonce) - Invidious::Database.check_table("session_ids", SessionId) - Invidious::Database.check_table("users", User) - Invidious::Database.check_table("videos", Video) - - if CONFIG.cache_annotations - Invidious::Database.check_table("annotations", Annotation) - end -end +Invidious::Database.check_integrity(CONFIG) # Start jobs diff --git a/src/invidious/database/base.cr b/src/invidious/database/base.cr index a6b38f1ca..0fb1b6af0 100644 --- a/src/invidious/database/base.cr +++ b/src/invidious/database/base.cr @@ -3,6 +3,32 @@ require "pg" module Invidious::Database extend self + # Checks table integrity + # + # Note: config is passed as a parameter to avoid complex + # dependencies between different parts of the software. + def check_integrity(cfg) + return if !cfg.check_tables + Invidious::Database.check_enum("privacy", PlaylistPrivacy) + + Invidious::Database.check_table("channels", InvidiousChannel) + Invidious::Database.check_table("channel_videos", ChannelVideo) + Invidious::Database.check_table("playlists", InvidiousPlaylist) + Invidious::Database.check_table("playlist_videos", PlaylistVideo) + Invidious::Database.check_table("nonces", Nonce) + Invidious::Database.check_table("session_ids", SessionId) + Invidious::Database.check_table("users", User) + Invidious::Database.check_table("videos", Video) + + if cfg.cache_annotations + Invidious::Database.check_table("annotations", Annotation) + end + end + + # + # Table/enum integrity checks + # + def check_enum(enum_name, struct_type = nil) return # TODO From 714a0013320f6126f6ac918e65264919582181b4 Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Fri, 7 Jan 2022 18:05:59 +0100 Subject: [PATCH 4/6] DB: playlists: make that 'insert' never raises --- src/invidious/database/playlists.cr | 8 ++------ src/invidious/routes/playlists.cr | 16 ++++------------ 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/invidious/database/playlists.cr b/src/invidious/database/playlists.cr index 7a5f61dcf..a37310d6e 100644 --- a/src/invidious/database/playlists.cr +++ b/src/invidious/database/playlists.cr @@ -94,17 +94,13 @@ module Invidious::Database::Playlists # Salect # ------------------- - def select(*, id : String, raise_on_fail : Bool = false) : InvidiousPlaylist? + def select(*, id : String) : InvidiousPlaylist? request = <<-SQL SELECT * FROM playlists WHERE id = $1 SQL - if raise_on_fail - return PG_DB.query_one(request, id, as: InvidiousPlaylist) - else - return PG_DB.query_one?(request, id, as: InvidiousPlaylist) - end + return PG_DB.query_one?(request, id, as: InvidiousPlaylist) end def select_all(*, author : String) : Array(InvidiousPlaylist) diff --git a/src/invidious/routes/playlists.cr b/src/invidious/routes/playlists.cr index d437b79c5..1c4f1bef8 100644 --- a/src/invidious/routes/playlists.cr +++ b/src/invidious/routes/playlists.cr @@ -151,12 +151,8 @@ module Invidious::Routes::Playlists page = env.params.query["page"]?.try &.to_i? page ||= 1 - begin - playlist = Invidious::Database::Playlists.select(id: plid, raise_on_fail: true) - if !playlist || playlist.author != user.email - return env.redirect referer - end - rescue ex + playlist = Invidious::Database::Playlists.select(id: plid) + if !playlist || playlist.author != user.email return env.redirect referer end @@ -235,12 +231,8 @@ module Invidious::Routes::Playlists page = env.params.query["page"]?.try &.to_i? page ||= 1 - begin - playlist = Invidious::Database::Playlists.select(id: plid, raise_on_fail: true) - if !playlist || playlist.author != user.email - return env.redirect referer - end - rescue ex + playlist = Invidious::Database::Playlists.select(id: plid) + if !playlist || playlist.author != user.email return env.redirect referer end From ce4a52325b6ed77a9829d46621808ec147e7e7c2 Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Wed, 26 Jan 2022 01:49:29 +0100 Subject: [PATCH 5/6] db: use now() function instead of passing Time.utc --- src/invidious/database/channels.cr | 18 +++++++++--------- src/invidious/database/playlists.cr | 18 +++++++++--------- src/invidious/database/sessions.cr | 4 ++-- src/invidious/database/users.cr | 6 +++--- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/invidious/database/channels.cr b/src/invidious/database/channels.cr index b45527333..e35b981d6 100644 --- a/src/invidious/database/channels.cr +++ b/src/invidious/database/channels.cr @@ -35,31 +35,31 @@ module Invidious::Database::Channels def update_author(id : String, author : String) request = <<-SQL UPDATE channels - SET updated = $1, author = $2, deleted = false - WHERE id = $3 + SET updated = now(), author = $1, deleted = false + WHERE id = $2 SQL - PG_DB.exec(request, Time.utc, author, id) + PG_DB.exec(request, author, id) end def update_subscription_time(id : String) request = <<-SQL UPDATE channels - SET subscribed = $1 - WHERE id = $2 + SET subscribed = now() + WHERE id = $1 SQL - PG_DB.exec(request, Time.utc, id) + PG_DB.exec(request, id) end def update_mark_deleted(id : String) request = <<-SQL UPDATE channels - SET updated = $1, deleted = true - WHERE id = $2 + SET updated = now(), deleted = true + WHERE id = $1 SQL - PG_DB.exec(request, Time.utc, id) + PG_DB.exec(request, id) end # ------------------- diff --git a/src/invidious/database/playlists.cr b/src/invidious/database/playlists.cr index a37310d6e..c6754a1ef 100644 --- a/src/invidious/database/playlists.cr +++ b/src/invidious/database/playlists.cr @@ -59,11 +59,11 @@ module Invidious::Database::Playlists def update_subscription_time(id : String) request = <<-SQL UPDATE playlists - SET subscribed = $1 - WHERE id = $2 + SET subscribed = now() + WHERE id = $1 SQL - PG_DB.exec(request, Time.utc, id) + PG_DB.exec(request, id) end def update_video_added(id : String, index : String | Int64) @@ -71,11 +71,11 @@ module Invidious::Database::Playlists UPDATE playlists SET index = array_append(index, $1), video_count = cardinality(index) + 1, - updated = $2 - WHERE id = $3 + updated = now() + WHERE id = $2 SQL - PG_DB.exec(request, index, Time.utc, id) + PG_DB.exec(request, index, id) end def update_video_removed(id : String, index : String | Int64) @@ -83,11 +83,11 @@ module Invidious::Database::Playlists UPDATE playlists SET index = array_remove(index, $1), video_count = cardinality(index) - 1, - updated = $2 - WHERE id = $3 + updated = now() + WHERE id = $2 SQL - PG_DB.exec(request, index, Time.utc, id) + PG_DB.exec(request, index, id) end # ------------------- diff --git a/src/invidious/database/sessions.cr b/src/invidious/database/sessions.cr index d5f85dd61..965870829 100644 --- a/src/invidious/database/sessions.cr +++ b/src/invidious/database/sessions.cr @@ -10,12 +10,12 @@ module Invidious::Database::SessionIDs def insert(sid : String, email : String, handle_conflicts : Bool = false) request = <<-SQL INSERT INTO session_ids - VALUES ($1, $2, $3) + VALUES ($1, $2, now()) SQL request += " ON CONFLICT (id) DO NOTHING" if handle_conflicts - PG_DB.exec(request, sid, email, Time.utc) + PG_DB.exec(request, sid, email) end # ------------------- diff --git a/src/invidious/database/users.cr b/src/invidious/database/users.cr index 53724dbfc..26be42702 100644 --- a/src/invidious/database/users.cr +++ b/src/invidious/database/users.cr @@ -143,11 +143,11 @@ module Invidious::Database::Users def clear_notifications(user : User) request = <<-SQL UPDATE users - SET notifications = '{}', updated = $1 - WHERE email = $2 + SET notifications = '{}', updated = now() + WHERE email = $1 SQL - PG_DB.exec(request, Time.utc, user.email) + PG_DB.exec(request, user.email) end # ------------------- From 67dd2b419a28510e6d89991e86e5d0aa97cac273 Mon Sep 17 00:00:00 2001 From: Samantaz Fox Date: Wed, 26 Jan 2022 17:30:54 +0100 Subject: [PATCH 6/6] db: use prepared statements rather than crafted argument list --- src/invidious/database/channels.cr | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/invidious/database/channels.cr b/src/invidious/database/channels.cr index e35b981d6..df44e485d 100644 --- a/src/invidious/database/channels.cr +++ b/src/invidious/database/channels.cr @@ -77,14 +77,13 @@ module Invidious::Database::Channels def select(ids : Array(String)) : Array(InvidiousChannel)? return [] of InvidiousChannel if ids.empty? - values = ids.map { |id| %(('#{id}')) }.join(",") request = <<-SQL SELECT * FROM channels - WHERE id = ANY(VALUES #{values}) + WHERE id = ANY($1) SQL - return PG_DB.query_all(request, as: InvidiousChannel) + return PG_DB.query_all(request, ids, as: InvidiousChannel) end end @@ -127,11 +126,11 @@ module Invidious::Database::ChannelVideos request = <<-SQL SELECT * FROM channel_videos - WHERE id IN (#{arg_array(ids)}) + WHERE id = ANY($1) ORDER BY published DESC SQL - return PG_DB.query_all(request, args: ids, as: ChannelVideo) + return PG_DB.query_all(request, ids, as: ChannelVideo) end def select_notfications(ucid : String, since : Time) : Array(ChannelVideo)