Skip to content

Add support for server addons #454

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

Merged
merged 1 commit into from
Sep 20, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions lib/ruby_lsp/ruby_lsp_rails/addon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,32 @@ def initialize
# the real client is initialized, features that depend on it will not be blocked by using the NullClient
@rails_runner_client = T.let(NullClient.new, RunnerClient)
@global_state = T.let(nil, T.nilable(GlobalState))
@addon_mutex = T.let(Mutex.new, Mutex)
@client_mutex = T.let(Mutex.new, Mutex)
@client_mutex.lock

Thread.new do
@addon_mutex.synchronize do
# We need to ensure the Rails client is fully loaded before we activate the server addons
@client_mutex.synchronize { @rails_runner_client = RunnerClient.create_client }
end
end
end

sig { returns(RunnerClient) }
attr_reader :rails_runner_client
def rails_runner_client
@addon_mutex.synchronize { @rails_runner_client }
end

sig { override.params(global_state: GlobalState, message_queue: Thread::Queue).void }
def activate(global_state, message_queue)
@global_state = global_state
$stderr.puts("Activating Ruby LSP Rails addon v#{VERSION}")
# Start booting the real client in a background thread. Until this completes, the client will be a NullClient
Thread.new { @rails_runner_client = RunnerClient.create_client }
register_additional_file_watchers(global_state: global_state, message_queue: message_queue)

@global_state.index.register_enhancement(IndexingEnhancement.new)

# Start booting the real client in a background thread. Until this completes, the client will be a NullClient
@client_mutex.unlock
end

sig { override.void }
Expand Down
8 changes: 8 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ def initialize
raise InitializationError, @stderr.read
end

sig { params(server_addon_path: String).void }
def register_server_addon(server_addon_path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need both inherited and register_server_addon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 perhaps we don't.... @vinistock ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I wanted to bring up for context that we need a way to limit who can use our addon. Right now we can do it in the addon while triggering register_server_addon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They run on different Ruby processes, so I'm not sure how we could combine theses two.

Also, why do we need to limit who uses the addon?

Copy link
Contributor

@KaanOzkan KaanOzkan Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it wasn't necessary but now I see that inherited is only triggered through the manual require we do in the server so we also need register_server_addon.

Also, why do we need to limit who uses the addon?

We need to do some testing first on a partial rollout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do some testing first on a partial rollout.

We can manage that on the Tapioca side though, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, there's no way to perform a partial rollout. But we could inform addons if experimental features are enabled so that they can decide whether to do something or not.

That would give you some control.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said above, with this implementation, rollout can be controlled by the individual addons through the trigger of register_server_addon. It's not an issue.

send_notification("server_addon/register", server_addon_path: server_addon_path)
rescue IncompleteMessageError
$stderr.puts("Ruby LSP Rails failed to register server addon #{server_addon_path}")
nil
end

sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
def model(name)
make_request("model", name: name)
Expand Down
42 changes: 42 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,41 @@

module RubyLsp
module Rails
class ServerAddon
@server_addon_classes = []
@server_addons = {}

class << self
# We keep track of runtime server addons the same way we track other addons, by storing classes that inherit
# from the base one
def inherited(child)
@server_addon_classes << child
super
end

# Delegate `request` with `params` to the server addon with the given `name`
def delegate(name, request, params)
@server_addons[name]&.execute(request, params)
end

# Instantiate all server addons and store them in a hash for easy access after we have discovered the classes
def finalize_registrations!
until @server_addon_classes.empty?
addon = @server_addon_classes.shift.new
@server_addons[addon.name] = addon
end
end
end

def name
raise NotImplementedError, "Not implemented!"
end

def execute(request, params)
raise NotImplementedError, "Not implemented!"
end
end

class Server
def initialize(stdout: $stdout, override_default_output_device: true)
# Grab references to the original pipes so that we can change the default output device further down
Expand Down Expand Up @@ -60,6 +95,13 @@ def execute(request, params)
write_response(route_location(params.fetch(:name)))
when "route_info"
write_response(resolve_route_info(params))
when "server_addon/register"
require params[:server_addon_path]
ServerAddon.finalize_registrations!
when "server_addon/delegate"
server_addon_name = params.delete(:server_addon_name)
request_name = params.delete(:request_name)
write_response(ServerAddon.delegate(server_addon_name, request_name, params))
end
rescue => e
write_response({ error: e.full_message(highlight: false) })
Expand Down
21 changes: 21 additions & 0 deletions test/ruby_lsp_rails/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,27 @@ def <(other)
assert_equal "/users(.:format)", result[:path]
end

test "server addons" do
File.write("server_addon.rb", <<~RUBY)
class TapiocaServerAddon < RubyLsp::Rails::ServerAddon
def name
"Tapioca"
end

def execute(request, params)
{ request:, params: }
end
end
RUBY

@server.execute("server_addon/register", server_addon_path: File.expand_path("server_addon.rb"))

@server.execute("server_addon/delegate", server_addon_name: "Tapioca", request_name: "dsl")
assert_equal(response, { params: {}, request: "dsl" })
ensure
FileUtils.rm("server_addon.rb")
end

test "prints in the Rails application or server are automatically redirected to stderr" do
stdout = StringIO.new
server = RubyLsp::Rails::Server.new(stdout: stdout)
Expand Down
Loading