Merge pull request #2845 from SamantazFox/more-db-improvements

More db improvements

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_integrity(CONFIG)
-  Invidious::Database.check_enum(PG_DB, "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)
-
-  if CONFIG.cache_annotations
-    Invidious::Database.check_table(PG_DB, "annotations", Annotation)
-  end
-end
 
 # Resolve player dependencies. This is done at compile time.
 #

src/invidious/database/base.cr

@@ -3,26 +3,52 @@ require "pg"
 module Invidious::Database
   extend self
 
-  def check_enum(db, enum_name, struct_type = nil)
+  # 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
 
-    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 +56,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\((?<types>[\d\D]*?)\);/)
       .try &.["types"].split(",").map(&.strip).reject &.starts_with?("CONSTRAINT")
 
@@ -41,14 +67,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 +85,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 +117,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

src/invidious/database/channels.cr

@@ -35,21 +35,31 @@ module Invidious::Database::Channels
   def update_author(id : String, author : String)
     request = <<-SQL
       UPDATE channels
-      SET updated = $1, author = $2, deleted = false
+      SET updated = now(), author = $1, deleted = false
-      WHERE id = $3
+      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 = now()
+      WHERE id = $1
+    SQL
+
+    PG_DB.exec(request, id)
   end
 
   def update_mark_deleted(id : String)
     request = <<-SQL
       UPDATE channels
-      SET updated = $1, deleted = true
+      SET updated = now(), deleted = true
-      WHERE id = $2
+      WHERE id = $1
     SQL
 
-    PG_DB.exec(request, Time.utc, id)
+    PG_DB.exec(request, id)
   end
 
   # -------------------
@@ -67,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
 
@@ -117,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)

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
+      SET subscribed = now()
-      WHERE id = $2
+      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
+          updated = now()
-      WHERE id = $3
+      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,28 +83,24 @@ module Invidious::Database::Playlists
       UPDATE playlists
       SET index = array_remove(index, $1),
           video_count = cardinality(index) - 1,
-          updated = $2
+          updated = now()
-      WHERE id = $3
+      WHERE id = $2
     SQL
 
-    PG_DB.exec(request, index, Time.utc, id)
+    PG_DB.exec(request, index, id)
   end
 
   # -------------------
   #  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)
-      return PG_DB.query_one(request, id, as: InvidiousPlaylist)
-    else
-      return PG_DB.query_one?(request, id, as: InvidiousPlaylist)
-    end
   end
 
   def select_all(*, author : String) : Array(InvidiousPlaylist)

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
 
   # -------------------

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
+      SET notifications = '{}', updated = now()
-      WHERE email = $2
+      WHERE email = $1
     SQL
 
-    PG_DB.exec(request, Time.utc, user.email)
+    PG_DB.exec(request, user.email)
   end
 
   # -------------------

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

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)
-      playlist = Invidious::Database::Playlists.select(id: plid, raise_on_fail: true)
+    if !playlist || playlist.author != user.email
-      if !playlist || playlist.author != user.email
-        return env.redirect referer
-      end
-    rescue ex
       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)
-      playlist = Invidious::Database::Playlists.select(id: plid, raise_on_fail: true)
+    if !playlist || playlist.author != user.email
-      if !playlist || playlist.author != user.email
-        return env.redirect referer
-      end
-    rescue ex
       return env.redirect referer
     end