From 848e66322a39c1ca9f5c1fec4a3aefcf193b783c Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Wed, 13 Sep 2023 13:38:34 +0800 Subject: [PATCH 1/2] Add plugin statistics endpoint --- .../plugin_manager/statistics_controller.rb | 39 ++++++ .../discourse_plugin_statistics_discourse.rb | 3 + .../discourse_plugin_statistics_plugin.rb | 4 + config/routes.rb | 1 + ...e_discourse_plugin_statistics_discourse.rb | 11 ++ ...eate_discourse_plugin_statistics_plugin.rb | 14 ++ plugin.rb | 3 + .../discourse_plugin_statistics_discourse.rb | 7 + .../discourse_plugin_statistics_plugin.rb | 7 + .../statistics_controller_spec.rb | 120 ++++++++++++++++++ 10 files changed, 209 insertions(+) create mode 100644 app/controllers/plugin_manager/statistics_controller.rb create mode 100644 app/models/discourse_plugin_statistics_discourse.rb create mode 100644 app/models/discourse_plugin_statistics_plugin.rb create mode 100644 db/migrate/20230913021018_create_discourse_plugin_statistics_discourse.rb create mode 100644 db/migrate/20230913021539_create_discourse_plugin_statistics_plugin.rb create mode 100644 spec/fabricators/discourse_plugin_statistics_discourse.rb create mode 100644 spec/fabricators/discourse_plugin_statistics_plugin.rb create mode 100644 spec/requests/plugin_manager/statistics_controller_spec.rb diff --git a/app/controllers/plugin_manager/statistics_controller.rb b/app/controllers/plugin_manager/statistics_controller.rb new file mode 100644 index 0000000..f0bd864 --- /dev/null +++ b/app/controllers/plugin_manager/statistics_controller.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class PluginManager::StatisticsController < ApplicationController + skip_before_action :check_xhr, :preload_json, :verify_authenticity_token + + def create + received_at = Time.now + + if discourse = DiscoursePluginStatisticsDiscourse.find_by(host: discourse_params[:host]) + discourse.update!(discourse_params) + else + discourse = DiscoursePluginStatisticsDiscourse.create!(discourse_params) + end + raise Discourse::InvalidParameters.new('invalid discourse') unless discourse + + plugin_params[:plugins].each do |plugin| + if ::PluginManager::Plugin.exists?(plugin[:name]) + DiscoursePluginStatisticsPlugin.create!( + received_at: received_at, + discourse_id: discourse.id, + **plugin.to_h + ) + end + end + + render json: success_json + end + + protected + + def discourse_params + params.require(:discourse).permit(:host, :branch, :sha) + end + + def plugin_params + params.require(:plugins) + params.permit(plugins: %i(name branch sha data)) + end +end diff --git a/app/models/discourse_plugin_statistics_discourse.rb b/app/models/discourse_plugin_statistics_discourse.rb new file mode 100644 index 0000000..6c2a8d0 --- /dev/null +++ b/app/models/discourse_plugin_statistics_discourse.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true +class DiscoursePluginStatisticsDiscourse < ActiveRecord::Base +end diff --git a/app/models/discourse_plugin_statistics_plugin.rb b/app/models/discourse_plugin_statistics_plugin.rb new file mode 100644 index 0000000..8ead880 --- /dev/null +++ b/app/models/discourse_plugin_statistics_plugin.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true +class DiscoursePluginStatisticsPlugin < ActiveRecord::Base + belongs_to :discourse, class_name: "DiscoursePluginStatisticsDiscourse", foreign_key: "discourse_id" +end diff --git a/config/routes.rb b/config/routes.rb index 6d7a6df..c335f90 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,6 +18,7 @@ class PluginManager::Engine < ::Rails::Engine put 'plugin/:plugin_name' => 'plugin#save', constraints: StaffConstraint.new delete 'plugin/:plugin_name' => 'plugin#delete', constraints: StaffConstraint.new post 'user/register' => 'plugin_user#register', constraints: { format: 'json' } + post 'statistics' => 'statistics#create', constraints: { format: 'json' } end Discourse::Application.routes.prepend do diff --git a/db/migrate/20230913021018_create_discourse_plugin_statistics_discourse.rb b/db/migrate/20230913021018_create_discourse_plugin_statistics_discourse.rb new file mode 100644 index 0000000..b9c14ab --- /dev/null +++ b/db/migrate/20230913021018_create_discourse_plugin_statistics_discourse.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +class CreateDiscoursePluginStatisticsDiscourse < ActiveRecord::Migration[7.0] + def change + create_table :discourse_plugin_statistics_discourses do |t| + t.string :host + t.string :branch + t.string :sha + t.timestamps + end + end +end diff --git a/db/migrate/20230913021539_create_discourse_plugin_statistics_plugin.rb b/db/migrate/20230913021539_create_discourse_plugin_statistics_plugin.rb new file mode 100644 index 0000000..8ee8ce8 --- /dev/null +++ b/db/migrate/20230913021539_create_discourse_plugin_statistics_plugin.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class CreateDiscoursePluginStatisticsPlugin < ActiveRecord::Migration[7.0] + def change + create_table :discourse_plugin_statistics_plugins do |t| + t.integer :discourse_id + t.datetime :received_at + t.string :name + t.string :branch + t.string :sha + t.json :data + t.timestamps + end + end +end diff --git a/plugin.rb b/plugin.rb index 1389628..e106877 100644 --- a/plugin.rb +++ b/plugin.rb @@ -50,9 +50,12 @@ ../app/jobs/scheduled/update_plugin_test_statuses.rb ../app/jobs/scheduled/update_plugins.rb ../app/jobs/regular/send_plugin_notification.rb + ../app/models/discourse_plugin_statistics_discourse.rb + ../app/models/discourse_plugin_statistics_plugin.rb ../app/controllers/plugin_manager/plugin_controller.rb ../app/controllers/plugin_manager/plugin_status_controller.rb ../app/controllers/plugin_manager/plugin_user_controller.rb + ../app/controllers/plugin_manager/statistics_controller.rb ../app/serializers/plugin_manager/log_serializer.rb ../app/serializers/plugin_manager/plugin_serializer.rb ../app/serializers/plugin_manager/plugin_user_serializer.rb diff --git a/spec/fabricators/discourse_plugin_statistics_discourse.rb b/spec/fabricators/discourse_plugin_statistics_discourse.rb new file mode 100644 index 0000000..425590a --- /dev/null +++ b/spec/fabricators/discourse_plugin_statistics_discourse.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:discourse_plugin_statistics_discourse) do + host { "forum.external.com" } + branch { "main" } + sha { sequence(:sha) { |i| "#{i}123456" } } +end diff --git a/spec/fabricators/discourse_plugin_statistics_plugin.rb b/spec/fabricators/discourse_plugin_statistics_plugin.rb new file mode 100644 index 0000000..6993808 --- /dev/null +++ b/spec/fabricators/discourse_plugin_statistics_plugin.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:discourse_plugin_statistics_plugin) do + host { "forum.external.com" } + branch { "main" } + sha { sequence(:sha) { |i| "#{i}123456" } } +end diff --git a/spec/requests/plugin_manager/statistics_controller_spec.rb b/spec/requests/plugin_manager/statistics_controller_spec.rb new file mode 100644 index 0000000..b52d864 --- /dev/null +++ b/spec/requests/plugin_manager/statistics_controller_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +describe PluginManager::StatisticsController do + let(:registered_plugin) { compatible_plugin } + let(:non_registered_plugin) { third_party_plugin } + let(:plugin_sha) { "12345678910" } + let(:plugin_branch) { "plugin_branch" } + let(:plugin_data) do + { + data_key_1: "data-val-1", + data_key_2: "data-val-2" + } + end + let(:discourse_sha) { "678910" } + let(:discourse_branch) { "discourse_branch" } + let(:discourse_host) { "forum.external.com" } + let(:params) do + { + discourse: { + host: discourse_host, + branch: discourse_branch, + sha: discourse_sha + }, + plugins: [ + { + name: registered_plugin, + branch: plugin_branch, + sha: plugin_sha, + data: plugin_data + } + ] + } + end + + before do + stub_github_plugin_request + stub_github_user_request + setup_test_plugin(registered_plugin) + freeze_time + end + + describe "#process" do + it "requires valid params" do + post "/plugin-manager/statistics" + expect(response).not_to be_successful + end + + context "with a new discourse" do + it "creates a new discourse record" do + post "/plugin-manager/statistics", params: params + expect(response).to be_successful + expect( + DiscoursePluginStatisticsDiscourse.exists?( + host: discourse_host, + branch: discourse_branch, + sha: discourse_sha + ) + ).to eq(true) + end + end + + context "with an existing discourse" do + let!(:discourse) { Fabricate(:discourse_plugin_statistics_discourse, host: discourse_host) } + + it "updates the existing discourse record" do + new_sha = "11121314" + params[:discourse][:sha] = new_sha + post "/plugin-manager/statistics", params: params + expect(response).to be_successful + expect( + DiscoursePluginStatisticsDiscourse.exists?( + host: discourse_host, + branch: discourse_branch, + sha: new_sha + ) + ).to eq(true) + expect( + DiscoursePluginStatisticsDiscourse.where(host: discourse_host).size + ).to eq(1) + end + end + + context "with a registered plugin" do + it "saves a new plugin record" do + post "/plugin-manager/statistics", params: params + expect(response).to be_successful + + discourse = DiscoursePluginStatisticsDiscourse.find_by(host: discourse_host) + expect( + DiscoursePluginStatisticsPlugin.exists?( + received_at: Time.now, + discourse_id: discourse.id, + name: registered_plugin, + branch: plugin_branch, + sha: plugin_sha + ) + ).to eq(true) + end + end + + context "with a non-reigstered plugin" do + it "does not save a new plugin record" do + params[:plugins][0][:name] = non_registered_plugin + post "/plugin-manager/statistics", params: params + expect(response).to be_successful + + discourse = DiscoursePluginStatisticsDiscourse.find_by(host: discourse_host) + expect( + DiscoursePluginStatisticsPlugin.exists?( + received_at: Time.now, + discourse_id: discourse.id, + name: non_registered_plugin, + branch: plugin_branch, + sha: plugin_sha + ) + ).to eq(false) + end + end + end +end From 6f0da7d3777e62541806eff224115e964630d37b Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Wed, 13 Sep 2023 17:57:42 +0800 Subject: [PATCH 2/2] Fix failing tests --- lib/plugin_manager/plugin.rb | 6 +++--- spec/plugin_helper.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/plugin_manager/plugin.rb b/lib/plugin_manager/plugin.rb index 043ef09..2af5e6f 100644 --- a/lib/plugin_manager/plugin.rb +++ b/lib/plugin_manager/plugin.rb @@ -446,15 +446,15 @@ def self.build_local_paths(root_path) end def self.get_local_sha(path) - PluginManager.run_shell_cmd('git rev-parse HEAD', chdir: path) + PluginManager.run_shell_cmd('git rev-parse HEAD', { chdir: path }) end def self.get_local_branch(path) - PluginManager.run_shell_cmd('git rev-parse --abbrev-ref HEAD', chdir: path) + PluginManager.run_shell_cmd('git rev-parse --abbrev-ref HEAD', { chdir: path }) end def self.get_local_url(path) - PluginManager.run_shell_cmd('git config --get remote.origin.url', chdir: path) + PluginManager.run_shell_cmd('git config --get remote.origin.url', { chdir: path }) end def self.excluded_local_plugins diff --git a/spec/plugin_helper.rb b/spec/plugin_helper.rb index 7d30d6b..46f6950 100644 --- a/spec/plugin_helper.rb +++ b/spec/plugin_helper.rb @@ -97,9 +97,9 @@ def api_token end def stub_plugin_git_cmds(dir, plugin_url) - Open3.expects(:capture3).with("git rev-parse HEAD", { chdir: dir }).returns(plugin_sha).at_least_once - Open3.expects(:capture3).with("git rev-parse --abbrev-ref HEAD", { chdir: dir }).returns(plugin_branch).at_least_once - Open3.expects(:capture3).with("git config --get remote.origin.url", { chdir: dir }).returns(plugin_url || "https://github.com/paviliondev/discourse-compatible-plugin.git") + PluginManager.expects(:run_shell_cmd).with("git rev-parse HEAD", { chdir: dir }).returns(plugin_sha).at_least_once + PluginManager.expects(:run_shell_cmd).with("git rev-parse --abbrev-ref HEAD", { chdir: dir }).returns(plugin_branch).at_least_once + PluginManager.expects(:run_shell_cmd).with("git config --get remote.origin.url", { chdir: dir }).returns(plugin_url || "https://github.com/paviliondev/discourse-compatible-plugin.git") Discourse.expects(:git_branch).returns(discourse_branch).at_least_once Discourse.expects(:git_version).returns(discourse_sha).at_least_once end