Adding a method `get_like` to PluginStore class

I want to submit a PR to add a method get_like to the PluginStore.

get_like will fetch all the records for a given plugin, whose key starts with the same word.

Usecase
If a plugin has two or more entities they want to store say apples and oranges, they’ll store the data as apple_1, apple_2… and orange_1, orange_2 from a plugin names fruits.

To fetch all the oranges, one can simply call PluginStore.get_like(‘fruits’, ‘orange’) etc.

What does the team and plugin devs think about this?

I don’t think I’ve personally needed this pattern. Have you seen it in public plugins, or just the one you are creating?

Also it’s not too long to type:

PluginStoreRow.where("plugin_name = 'fruits' AND key LIKE 'orange%')

I would recommend adding a helper function or class to your plugin if you do this often. Also be sure to make sure you have appropriate indexes for these queries.

3 Likes

I was working on this feature for the custom-wizards plugin and I wanted to fetch all the wizards.
The key used is a string which is unique per wizard. This case was handled by fetching all the rows for the plugin and filtering them based on a for the value which is fine for small number of records but not ideal for a big plugin. It looks like plugin_store_rows is meant to store settings-like data for the plugin.

I personally think, this would open the idea of storing non-settings like data in the plugin_store_rows table.

(Forgive me if it sounds strange)

In general I recommend creating your own tables via migrations if the PluginStoreRow can’t be queried the way you want to. This is now commonly done in several plugins and works great!

3 Likes

Ohh. I wasn’t aware of this. Thanks a lot.

1 Like

The pattern is to use the key field in the plugin_store_rows table to store both a namespace and a unique identifier, i.e.:

<namespace>_<id>

This pattern is seen less in core Discourse plugins these days with a general decline in the use of PluginStore for example:

However it is still used in places, including the core Discourse codebase itself, e.g. in the Reviewables model.

I also use the pattern in a number of plugins.

The main reason the pattern is used is because the plugin_store_rows table is used by multiple plugins (and some core services), so the identifying columns, i.e. id and plugin_name, can’t be used for identification internally within each system using the PluginStore. So a string-based system is used in the key column instead.

In terms of changing the database structure from within a plugin, @gdpelican has a good post on this:

Personally, I’m still quite wary of doing this as working on a third party plugin you have no control over namespacing, whether your plugin is removed and what potentially conflicting changes are made to core Discourse.

As @gdpelican mentions, you need to provide a way for your plugin user to remove the db changes if they uninstall the plugin.

Provide a method for users to clean up your database changes if they don’t want your plugin anymore. I did it with a rake task.

I feel this is too in the weeds for most plugin users and poses a risk if they’re not aware of this detail.

Moreover, I haven’t found a real need to go outside the bounds of the PluginStore and CustomFields yet.

All that said, personally I’d be in favour of a new method along these lines in PluginStore, as I find the pattern useful.

@david Would be interested in your thoughts on the above as well.

5 Likes

As @eviltrout has said, we’re using migrations and dedicated tables in a number of plugins now, with great success. Having the ability to enforce database constraints has helped improve performance (lookups in any column) and data integrity (through unique indexes). These two things have proved especially important at the scale of some of our hosted customers - not really something I considered before joining the team.

The first substantial plugin I worked on was chat-integration, and I implemented a very fiddly “fake activerecord”, which leans on the plugin store. In hindsight, dedicated tables would have been a far better choice, and I might look at migrating the plugin to that in future.

I would agree, when it comes to modifying core tables. Adding/modifying columns on existing tables could have unintended consequences later down the line, and will stick around even if the plugin is uninstalled. I would strongly recommend against doing this.

Dedicated tables, on the other hand, are fairly low risk. If the plugin is uninstalled, they will just stick around, without any negative side effects (as long as you don’t introduce any foreign key constraints). Leaving data lying around is “no worse” than using the plugin store.

In terms of cleanup, we could look at providing rake tasks which “reverse” plugin migrations to cleanup. But to be honest, I don’t think this will see much use. My assumption is that people rarely uninstall plugins, and when they do, they would rather keep the data around in case they want to reinstall it again.

6 Likes

As the author of that code I should maybe clarify it a little bit. The priority ids are constants and not relational data. I would absolutely recommend against using something_id when the ids are not known in advance. In this case each priority is thought of as a singleton, and I figured that any table I’d create would essentially be a clone of the PluginStore anyway!

3 Likes