Hand the Google client down from the app
No more Google.default: the app builds the client, passes it to each source kind, and the YouTube tests override it in a subclass of the app. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LG4iASZnHb1yEM1JAASiWn
diff --git a/AGENTS.md b/AGENTS.md
index 32aa6ea..34d08a7 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -92,8 +92,9 @@ fly.toml — Fly config for the main demo and PR previews
Fly demos set placeholders in `fly.toml`, since anyone can open them.
Tests set `DATABASE_URL=:memory:` and a dummy Google client in
-`test/test_helper.rb`, and set `RankKing::Google.default` to a subclass
-whose `request` answers without the network.
+`test/test_helper.rb`. The app hands its `google` client to every
+source; the YouTube tests subclass the app to return a `Google`
+subclass whose `request` answers without the network.
## Conventions
diff --git a/lib/rank_king/google.rb b/lib/rank_king/google.rb
index e7b22ef..2df93f8 100644
--- a/lib/rank_king/google.rb
+++ b/lib/rank_king/google.rb
@@ -4,8 +4,6 @@ require "json"
require "net/http"
require "uri"
-require_relative "config"
-
module RankKing
# The Google APIs rank-king calls: OAuth, to connect a user's account,
# and the YouTube Data API, to read their playlists. Every call goes
@@ -26,15 +24,6 @@ module RankKing
PLAYLIST_ITEMS_URL = "https://www.googleapis.com/youtube/v3/playlistItems"
SCOPE = "https://www.googleapis.com/auth/youtube.readonly"
- # The client the app uses, for the OAuth client in CONFIG. Tests set
- # their own.
- singleton_class.attr_writer :default
-
- #: () -> Google
- def self.default
- @default ||= new(client_id: CONFIG.google_client_id, client_secret: CONFIG.google_client_secret)
- end
-
#: (client_id: String, client_secret: String) -> void
def initialize(client_id:, client_secret:)
@client_id = client_id
diff --git a/lib/rank_king/models.rb b/lib/rank_king/models.rb
index 3865b10..38a89e8 100644
--- a/lib/rank_king/models.rb
+++ b/lib/rank_king/models.rb
@@ -25,7 +25,6 @@ module RankKing
one_to_many :items, class: "RankKing::Item", conditions: { archived_at: nil }
one_to_many :axes, class: "RankKing::Axis"
- def source = Sources.fetch(source_kind).for(self)
# Archived items too, which the items association leaves out.
def before_destroy
diff --git a/lib/rank_king/sources/text.rb b/lib/rank_king/sources/text.rb
index bcb9e20..2262dd3 100644
--- a/lib/rank_king/sources/text.rb
+++ b/lib/rank_king/sources/text.rb
@@ -8,19 +8,19 @@ module RankKing
# over the list's items, which matches them by id, so there is nothing
# to fetch and no table of its own.
class Text
- #: (List list) -> Text
- def self.for(list) = new(list)
+ #: (List list, google: Google) -> Text
+ def self.for(list, google:) = new(list)
#: () -> String
def self.title = "Typed list"
- #: (User user) -> _Component
- def self.fields(user) = Views::Sources::TextFields.new
+ #: (User user, google: Google) -> _Component
+ def self.fields(user, google:) = Views::Sources::TextFields.new
# Adds the items typed into the new-list form, of which there must
# be at least two.
- #: (List list, Hash[String, untyped] params) -> bool
- def self.create(list, params)
+ #: (List list, Hash[String, untyped] params, google: Google) -> bool
+ def self.create(list, params, google:)
names = params["items"].to_s.lines.map(&:strip).reject(&:empty?)
return false if names.size < 2
diff --git a/lib/rank_king/sources/youtube.rb b/lib/rank_king/sources/youtube.rb
index 6b73971..bbc3b31 100644
--- a/lib/rank_king/sources/youtube.rb
+++ b/lib/rank_king/sources/youtube.rb
@@ -18,16 +18,15 @@ module RankKing
#: () -> String
def self.title = "YouTube playlist"
- #: (List list) -> YouTube
- def self.for(list) = new(list)
+ #: (List list, google: Google) -> YouTube
+ def self.for(list, google:) = new(list, google:)
# A choice of the user's playlists, fetched as the form is drawn. A
# Google error says so in place of the choice rather than failing
# the whole form.
- #: (User user) -> _Component
- def self.fields(user)
+ #: (User user, google: Google) -> _Component
+ def self.fields(user, google:)
account = user.google_account
- google = Google.default
playlists = account && google.playlists(google.access_token(account.refresh_token))
Views::Sources::YouTubeFields.new(playlists:)
rescue Google::Error => e
@@ -35,19 +34,20 @@ module RankKing
end
# Follows the playlist chosen on the new-list form and fetches it.
- #: (List list, Hash[String, untyped] params) -> bool
- def self.create(list, params)
+ #: (List list, Hash[String, untyped] params, google: Google) -> bool
+ def self.create(list, params, google:)
playlist_id = params["playlist"].to_s[/\A[\w-]+\z/]
return false unless playlist_id && list.user.google_account
DB[TABLE].insert(list_id: list.id, playlist_id:)
- new(list).fetch
+ new(list, google:).fetch
true
end
- #: (List list) -> void
- def initialize(list)
+ #: (List list, google: Google) -> void
+ def initialize(list, google:)
@list = list
+ @google = google
end
#: () -> String
@@ -67,10 +67,9 @@ module RankKing
account = @list.user.google_account
raise Google::Error, "Connect Google in settings to sync this list" unless account
- google = Google.default
- access_token = google.access_token(account.refresh_token)
+ access_token = @google.access_token(account.refresh_token)
# Kept for saved, which caches each video's details by its item.
- @videos = google.playlist_videos(access_token, playlist_id).uniq(&:id).to_h { |video|
+ @videos = @google.playlist_videos(access_token, playlist_id).uniq(&:id).to_h { |video|
[video.id, video] #: [String, Google::Video]
}
entries = @videos.each_value.map { |video| Entry.new(key: video.id, name: video.title) }
diff --git a/lib/rank_king/views/list/show.rb b/lib/rank_king/views/list/show.rb
index 5b57ff7..43f6bf8 100644
--- a/lib/rank_king/views/list/show.rb
+++ b/lib/rank_king/views/list/show.rb
@@ -8,8 +8,9 @@ module RankKing
module Views
module ListViews
class Show < Phlex::HTML
- def initialize(list:, csrf:, axis: nil)
+ def initialize(list:, source:, csrf:, axis: nil)
@list = list
+ @source = source
@csrf = csrf
@axes = list.axes
@selected = axis || @axes.first
@@ -35,7 +36,7 @@ module RankKing
# rather than a new button on the screen. The source decides what
# syncing looks like: its entry opens a dialog holding its own form.
def render_menu
- source = @list.source
+ source = @source
button(class: "icon-button", data_size: "lg", popovertarget: "list-actions", aria_label: "List actions",
style: "--gl-icon: var(--rk-icon-more)")
diff --git a/lib/rank_king/web.rb b/lib/rank_king/web.rb
index 91a0953..1d687e9 100644
--- a/lib/rank_king/web.rb
+++ b/lib/rank_king/web.rb
@@ -52,7 +52,7 @@ module RankKing
r.get "connect" do
state = SecureRandom.hex(16)
session["google_state"] = state
- r.redirect Google.default.authorize_url(redirect_uri:, state:)
+ r.redirect google.authorize_url(redirect_uri:, state:)
end
r.get "callback" do
@@ -61,7 +61,7 @@ module RankKing
# The user said no on Google's side.
r.redirect "/settings" if r.params["error"]
- refresh_token = Google.default.refresh_token(r.params["code"].to_s, redirect_uri:)
+ refresh_token = google.refresh_token(r.params["code"].to_s, redirect_uri:)
if (account = user.google_account)
account.update(refresh_token:)
else
@@ -92,7 +92,7 @@ module RankKing
# down and nothing is made.
list = DB.transaction {
l = List.create(name: name, user_id: user.id, source_kind: source_kind)
- raise Sequel::Rollback unless Sources.fetch(source_kind).create(l, r.params)
+ raise Sequel::Rollback unless Sources.fetch(source_kind).create(l, r.params, google:)
axis_names.each do |n|
Axis.create(list_id: l.id, name: n)
@@ -108,7 +108,7 @@ module RankKing
r.get "new" do
kinds = Sources::KINDS.map { |kind|
source = Sources.fetch(kind)
- [kind, source.title, source.fields(user)] #: [String, String, Sources::_Component]
+ [kind, source.title, source.fields(user, google:)] #: [String, String, Sources::_Component]
}
Views::ListViews::New.new(sources: kinds, csrf: method(:csrf_token)).call
end
@@ -121,7 +121,7 @@ module RankKing
r.is do
r.get do
axis = list.axes_dataset.where(id: r.params["axis"].to_i).first
- Views::ListViews::Show.new(list: list, axis: axis, csrf: method(:csrf_token)).call
+ Views::ListViews::Show.new(list: list, source: source(list), axis: axis, csrf: method(:csrf_token)).call
end
end
@@ -151,7 +151,7 @@ module RankKing
# Submitted from the source's own sync dialog.
r.post "sync" do
- list.source.sync(r.params)
+ source(list).sync(r.params)
r.redirect "/lists/#{list.id}"
end
@@ -200,6 +200,14 @@ module RankKing
private
+ # The Google client every route and source goes through. Tests
+ # subclass the app to hand back a fake one.
+ #: () -> Google
+ def google = Google.new(client_id: CONFIG.google_client_id, client_secret: CONFIG.google_client_secret)
+
+ #: (List list) -> Sources::_Source
+ def source(list) = Sources.fetch(list.source_kind).for(list, google:)
+
# Names from a repeated field (`axes[]`), one per tag.
#: (untyped) -> Array[String]
def names_param(value)
diff --git a/sig/rank_king/google.rbs b/sig/rank_king/google.rbs
index c9b3c2b..4dd1d82 100644
--- a/sig/rank_king/google.rbs
+++ b/sig/rank_king/google.rbs
@@ -1,13 +1,3 @@
-# The default client's writer is defined on the singleton class, which
-# the inline parser cannot read; tests are what call it.
-module RankKing
- class Google
- self.@default: Google?
-
- def self.default=: (Google?) -> void
- end
-end
-
# Video is a Data class, whose members the inline parser cannot read.
module RankKing
class Google
diff --git a/sig/rank_king/models.rbs b/sig/rank_king/models.rbs
index 074ea63..cf0aafc 100644
--- a/sig/rank_king/models.rbs
+++ b/sig/rank_king/models.rbs
@@ -24,7 +24,6 @@ module RankKing
def axes_dataset: () -> Sequel::Dataset[Axis]
def source_kind: () -> String
def synced_at: () -> Time?
- def source: () -> Sources::_Source
end
class Item < Sequel::Model
diff --git a/sig/rank_king/sources.rbs b/sig/rank_king/sources.rbs
index ae193ec..4374991 100644
--- a/sig/rank_king/sources.rbs
+++ b/sig/rank_king/sources.rbs
@@ -35,18 +35,19 @@ module RankKing
# A source kind: a class in Sources, named by a list's source_kind.
interface _SourceClass
- # The source for a list, configured from the kind's own table.
- def for: (List list) -> _Source
+ # The source for a list, configured from the kind's own table. Every
+ # kind is handed the app's Google client, which only YouTube uses.
+ def for: (List list, google: Google) -> _Source
# What the kind is called where a list's source is chosen or shown.
def title: () -> String
# The kind's own fields on the new-list form.
- def fields: (User user) -> _Component
+ def fields: (User user, google: Google) -> _Component
# Sets a new list up from what those fields posted: false, and the
# list is not made, when they are not enough to go on.
- def create: (List list, Hash[String, untyped] params) -> bool
+ def create: (List list, Hash[String, untyped] params, google: Google) -> bool
end
# A view component, as a source's form is.
diff --git a/sig/rank_king/views.rbs b/sig/rank_king/views.rbs
index d8f44f7..ece8ee0 100644
--- a/sig/rank_king/views.rbs
+++ b/sig/rank_king/views.rbs
@@ -42,7 +42,7 @@ module RankKing
end
class Show
- def initialize: (list: List, csrf: Method, ?axis: Axis?) -> void
+ def initialize: (list: List, source: ::RankKing::Sources::_Source, csrf: Method, ?axis: Axis?) -> void
def call: () -> String
end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 0b28ccb..2a1783e 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
ENV["DATABASE_URL"] = ":memory:"
-# A Google client, so the YouTube routes are on; no request leaves the
-# tests, which set RankKing::Google.default to a fake that answers itself.
+# The Google client the app requires. No request reaches Google: the
+# YouTube tests' app subclass hands out a fake one.
ENV["GOOGLE_CLIENT_ID"] = "client-id"
ENV["GOOGLE_CLIENT_SECRET"] = "client-secret"
diff --git a/test/test_youtube.rb b/test/test_youtube.rb
index 3099241..0252a5e 100644
--- a/test/test_youtube.rb
+++ b/test/test_youtube.rb
@@ -11,7 +11,14 @@ require_relative "../lib/rank_king/web"
class TestYouTube < Minitest::Test
include Rack::Test::Methods
- def app = RankKing::Web.app
+ # The app, with the fake Google in place of the real one.
+ class App < RankKing::Web
+ singleton_class.attr_accessor :google
+
+ private def google = self.class.google
+ end
+
+ def app = App.app
LOGIN = "alpha@example.com"
PLAYLIST = "PLabc123"
@@ -76,11 +83,7 @@ class TestYouTube < Minitest::Test
RankKing::DB[:users].delete
@google = FakeGoogle.new
- RankKing::Google.default = @google
- end
-
- def teardown
- RankKing::Google.default = nil
+ App.google = @google
end
def test_settings_offers_to_connect_google