From 818bf1355d094758d9c85a920e3b8a0bc60e5247 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 26 Aug 2013 12:52:36 +1000 Subject: [PATCH] PluginStore for plugin specific bits of storage Amended plugin interfaces so they work with the vk sample --- app/assets/stylesheets/application.css.erb | 2 + app/models/plugin_store.rb | 40 +++++++++++++++ app/models/plugin_store_row.rb | 2 + config/application.rb | 4 +- ...20130826011521_create_plugin_store_rows.rb | 15 ++++++ lib/plugin/auth_provider.rb | 6 ++- lib/plugin/instance.rb | 6 +-- spec/components/plugin/instance_spec.rb | 1 - spec/models/plugin_store_spec.rb | 49 +++++++++++++++++++ 9 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 app/models/plugin_store.rb create mode 100644 app/models/plugin_store_row.rb create mode 100644 db/migrate/20130826011521_create_plugin_store_rows.rb create mode 100644 spec/models/plugin_store_spec.rb diff --git a/app/assets/stylesheets/application.css.erb b/app/assets/stylesheets/application.css.erb index 92f4d2d65..78d8f1182 100644 --- a/app/assets/stylesheets/application.css.erb +++ b/app/assets/stylesheets/application.css.erb @@ -9,6 +9,8 @@ //= require_tree ./application //= require ./foundation/helpers <% + # TODO this is very tricky, we want to add a dependency here on files that may not yet exist + # otherwise in dev we are often stuck nuking the tmp/cache directory DiscoursePluginRegistry.stylesheets.each do |css| require_asset(css) end diff --git a/app/models/plugin_store.rb b/app/models/plugin_store.rb new file mode 100644 index 000000000..82a30c711 --- /dev/null +++ b/app/models/plugin_store.rb @@ -0,0 +1,40 @@ +# API to wrap up plugin store rows +class PluginStore + def self.get(plugin_name, key) + if row = PluginStoreRow.where(plugin_name: plugin_name, key: key).first + cast_value(row.type_name, row.value) + end + end + + def self.set(plugin_name, key, value) + hash = {plugin_name: plugin_name, key: key} + row = PluginStoreRow.where(hash).first || row = PluginStoreRow.new(hash) + + row.type_name = determine_type(value) + # nil are stored as nil + row.value = + if row.type_name == "JSON" + value.to_json + elsif value + value.to_s + end + + row.save + end + + protected + + + def self.determine_type(value) + value.is_a?(Hash) ? "JSON" : value.class.to_s + end + + def self.cast_value(type, value) + case type + when "Fixnum" then value.to_i + when "TrueClass", "FalseClass" then value == "true" + when "JSON" then ActiveSupport::HashWithIndifferentAccess.new(::JSON.parse(value)) + else value + end + end +end diff --git a/app/models/plugin_store_row.rb b/app/models/plugin_store_row.rb new file mode 100644 index 000000000..a18cd005b --- /dev/null +++ b/app/models/plugin_store_row.rb @@ -0,0 +1,2 @@ +class PluginStoreRow < ActiveRecord::Base +end diff --git a/config/application.rb b/config/application.rb index 6fb47f369..8fff3fd07 100644 --- a/config/application.rb +++ b/config/application.rb @@ -119,9 +119,9 @@ module Discourse # attr_accessible. config.active_record.whitelist_attributes = false + require 'plugin' + require 'auth' unless Rails.env.test? - require 'plugin' - require_dependency 'auth' Discourse.activate_plugins! end diff --git a/db/migrate/20130826011521_create_plugin_store_rows.rb b/db/migrate/20130826011521_create_plugin_store_rows.rb new file mode 100644 index 000000000..acf0e2c38 --- /dev/null +++ b/db/migrate/20130826011521_create_plugin_store_rows.rb @@ -0,0 +1,15 @@ +class CreatePluginStoreRows < ActiveRecord::Migration + def change + create_table :plugin_store_rows do |table| + table.string :plugin_name, null: false + table.string :key, null: false + table.string :type_name, null: false + # not the most efficient implementation but will do for now + # possibly in future we can add more tables so int and boolean etc values are + # not stored in text + table.text :value + end + + add_index :plugin_store_rows, [:plugin_name, :key], unique: true + end +end diff --git a/lib/plugin/auth_provider.rb b/lib/plugin/auth_provider.rb index a7c90a2de..367f9a651 100644 --- a/lib/plugin/auth_provider.rb +++ b/lib/plugin/auth_provider.rb @@ -1,5 +1,9 @@ class Plugin::AuthProvider - attr_accessor :type, :glyph, :background_color, :name, :title, + attr_accessor :glyph, :background_color, :title, :message, :frame_width, :frame_height, :authenticator + def name + authenticator.name + end + end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index cc2e8f4f6..a9ec57708 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -153,14 +153,12 @@ class Plugin::Instance end end - def auth_provider(type, opts) + def auth_provider(opts) @auth_providers ||= [] provider = Plugin::AuthProvider.new - provider.type = type - [:name, :glyph, :background_color, :title, :message, :frame_width, :frame_height, :authenticator].each do |sym| + [:glyph, :background_color, :title, :message, :frame_width, :frame_height, :authenticator].each do |sym| provider.send "#{sym}=", opts.delete(sym) end - provider.name ||= type.to_s @auth_providers << provider end diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb index cfb387c3b..bf019cbf8 100644 --- a/spec/components/plugin/instance_spec.rb +++ b/spec/components/plugin/instance_spec.rb @@ -31,7 +31,6 @@ describe Plugin::Instance do plugin.auth_providers.count.should == 1 auth_provider = plugin.auth_providers[0] - auth_provider.options.should == {:identifier => 'https://zappa.com'} auth_provider.type.should == :open_id # calls ensure_assets! make sure they are there diff --git a/spec/models/plugin_store_spec.rb b/spec/models/plugin_store_spec.rb new file mode 100644 index 000000000..e08788c5b --- /dev/null +++ b/spec/models/plugin_store_spec.rb @@ -0,0 +1,49 @@ +require "spec_helper" +require_dependency "plugin_store" + +describe PluginStore do + def set(k,v) + PluginStore.set("my_plugin", k, v) + end + + def get(k) + PluginStore.get("my_plugin", k) + end + + it "sets strings correctly" do + set("hello", "world") + expect(get("hello")).to eq("world") + + set("hello", "world1") + expect(get("hello")).to eq("world1") + end + + it "sets fixnums correctly" do + set("hello", 1) + expect(get("hello")).to eq(1) + end + + it "sets bools correctly" do + set("hello", true) + expect(get("hello")).to eq(true) + + set("hello", false) + expect(get("hello")).to eq(false) + + set("hello", nil) + expect(get("hello")).to eq(nil) + end + + + it "handles hashes correctly" do + + val = {"hi" => "there", "1" => 1} + set("hello", val) + result = get("hello") + + expect(result).to eq(val) + + # ensure indiff access holds + expect(result[:hi]).to eq("there") + end +end