What are appropriate uses of PluginStore?

mcdanlj · June 9, 2020, 9:18pm

It looks like PluginStore would be the easiest way for me to implement remembering Slack thread IDs for threaded responses — but I notice that the only use of it in all of discourse-chat-integration is orphaned; nothing actually uses the value that is looked up.

Does this mean that it’s deprecated or just that it’s being done another way?

I’d be storing only one value per topic max, so this seems relatively low cardinality, all things considered. Is this reasonable for PluginStore?

Falco · June 9, 2020, 9:32pm

If you are storing up to one value per topic, using a topic custom field may be better for this use case.

PluginStore is a simple KV store that is used to plugins where the cardinality of the data means it is a bad fit for existing custom fields tables. In the last years, we are moving plugins to their own tables to get better data reliability where it makes sense, like we did with the poll plugin. You can still use it if it makes sense for your use case.

mcdanlj · June 9, 2020, 9:43pm

Thanks, sounds like I should do that. Because I’m new here, any chance you could give me an example of a topic custom field in a plugin for me to learn from without being a pest?

Falco · June 9, 2020, 9:49pm

The voting plugin uses topic custom fields a little, like in discourse-voting/plugin.rb at master · discourse/discourse-voting · GitHub and discourse-voting/voting_ensure_consistency.rb at master · discourse/discourse-voting · GitHub

mcdanlj · June 9, 2020, 10:08pm

I think that points me in the right direction, much appreciated!

mcdanlj · June 12, 2020, 3:49pm

For the next person who is new to this and finds this, some of the mistakes I made as someone completely new to Ruby on Rails:

I thought that the isolate_namespace was part of the pattern to copy, and didn’t realize that the namespace it refers to is the route namespace nothing in the database. This caused me trouble.
Setting the custom field doesn’t save it. You can save the object it’s part of (e.g. topic.save!) or just the custom fields (e.g. topic.save_custom_fields)

fzngagan · July 2, 2020, 6:28am

yes, the HasCustomFields mixin has some perks like save_custom_fields, auto type-casting etc.

sam · July 2, 2020, 7:35am

I can see us removing both plugin store and custom fields long term and replacing/amending with a far more restricted and controlled system, they cause more trouble than they do help things.

The one place we use custom fields and really need them these days it to flag to serializers that “special” information exists on a post, or in exceptional cases when you are decorating 50% of the posts in the system you would also put data there.

custom fields are too flexible and have tons of concurrency issues.

The best way of going about adding rich data is by having a plugin create the tables it needs and owns, this should be the first thing you do, if you get stuck cause N+1 and other issues then reach to custom fields to help avoiding them.

mcdanlj · July 2, 2020, 11:46am

Wow, wouldn’t that break large swathes of the existing plugin ecosystem?

My contribution to discourse-chat-integration (to support threads, see topic) used custom fields based on the advice I got here.

spirobel · July 2, 2020, 1:51pm

how do you usually create models and migrations for plugins? do you use the main rails generator and copy them to the plugin folder? I created myself this model and migration generator, that create models and migrations with a plugin specific prefix. GitHub - spirobel/discourse at plugin_model_and_migrations maybe thats of use for others as well.

sam · July 3, 2020, 12:55am

I recommend looking at the discourse-policy and the polls plugin for some examples on how we do migrations. That is the pattern you should follow.

The change will happen over time and we will offer guidance. Current system of heavy reliance on plugin store and custom fields has lead to countless regular weekly issues.

spirobel · July 3, 2020, 3:10pm

I looked at the polls plugin and based on that I created a model generator for plugins and also this migrations generator:

github.com

spirobel/discourse/blob/plugin_model_and_migrations/lib/generators/plugin/migration_generator.rb

# frozen_string_literal: true

require "rails/generators/active_record/migration/migration_generator"
class Plugin::MigrationGenerator < ActiveRecord::Generators::MigrationGenerator
  desc "Expecting NAME to start with name of plugin: Pluginname_Migrationname"
  source_root "#{Gem.loaded_specs["activerecord"].full_gem_path}/lib/rails/generators/active_record/migration/templates"
  private

  def db_migrate_path
    plugin_folder = 'plugins/' + name.split("_").first
    plugin_folder = plugin_folder.underscore.dasherize
    unless File.directory?(Rails.root.join(plugin_folder))
     print "#{plugin_folder} does not exit. Expecting NAME to start with name of plugin: Pluginname_Migrationname"
     exit
    end
    plugin_folder + "/db/migrate"
  end
end

If I understood it correctly one of the reasons the pluginstore was created in the first place, was the concern that if plugins created their own tables, the names might collide. The generators I created will prefix the names of models and migrations with the name of the plugin. I mainly made these generators because migrations need to have this date number in front to create a predictable order. thats why I didnt want to create them manually.

sam · July 6, 2020, 2:25am

Collision though theoretically a problem was not a big motivation. The big motivator was that it was a bit unclear how to run migrations and we wanted something very low friction. These days creating migrations in plugins is trivial, you simply create a file in the migration folder.

Collision can still happen with post custom fields anyway.