Problem/Motivation
Drupal CMS (aka Starshot) has a Search Track. The search recipes that will be suggested to all Drupal CMS users will include search api module and some other modules from its ecosystem (search_api_exclude, search_api_autocomplete, facets, maybe others in the future).
Config Actions are fundamental part of the recipes. It would be much easier to perform various operations with search api index and search api server if there were config actions for that. The obvious examples of what is needed would be add/remove/rename field from search index, add/remove/update processor, etc .
Drupal core >= 10.3 allows easy implementation of config actions for config entity class methods as ActionMethod. This allows to expose the class methods as config actions. This can already help while not producing any extra code, just expose some methods of search api index/server classes as action methods.
I would like to start a discussion here, what methods should be marked as action methods. And what other config actions can be created to help integrate Search API with recipes. Ideally, all operations that site builder needs to do to make search work on the site would be nice to have as action, but we should start from something.
Remaining tasks
- Create a plan of what methods should/could be exposed as config actions.
- Create list of other useful actions that can be added as config action plugins.
Comments
Comment #2
a.dmitriiev commentedI think the following methods from Index entity class should be exposed as config actions:
setOption(without plural support, as there is alreadysetOptionsmethod in the class)setOptionsremoveDatasourceas only the id is needed for argumentremoveProcessorrenameFieldremoveFieldit would be also nice to have the following methods exposed, but some code adjustments will be needed:
addDatasource, but at the moment the argument is of type Datasource and it can't be used in recipe. Maybe alternatively there could be a methodaddDatasourceByIdAndConfigthat will have datasource id and its config as arguments.setTrackerhas and object as argument so it can't be used as action method, but maybe having a methodsetTrackerByIdAndConfigwith tracker plugin id and config as arguments would make sensesetServerit makes sense to createsetServerById?addProcessorhas object as argument, but maybe new methodaddProcessorByIdAndConfigwould be ok to add as welladdFieldhas Field object as argument, but maybe new methodaddFieldByFieldConfigwould be ok to addMethods that are not in Index class yet, but might be useful:
setServersetReadOnlyMethods for Server entity:
setBackendConfigMissing methods on Server entity, that could be useful:
setBackend- as setBackendConfig method assumes that backend property is already set, maybe it is needed to have backend set method as well.Comment #3
a.dmitriiev commentedComment #4
a.dmitriiev commentedI have prepared the first MR for recipe integration https://www.drupal.org/project/search_api/issues/3484304 . It exposes only already existing methods as config actions to start this feature rolling.
Comment #5
borisson_This is a good and easy first step, I think that one is ok.
For datasource, and server I think adding a new method is a good idea.
Processors and fields are more difficult, because their configuration can be more extensive, but I guess we can go through the same way here as well?
I think tracker is probably not needed, since for 99% the basic tracker is used. So I'm not sure how I feel about creating this new method, however if we already do this same trick for the other 4 I guess it's not too bad?
Comment #6
drunken monkeyGreat initiative, thanks! I already merged your first MR, just making existing methods available seems like a low-hanging fruit.
My thoughts on the rest of your suggestions:
setServerById(): I would just call thatsetServerId()maybe? Would seem like a natural extension from the existinggetServerId(). It’s just a bit unfortunate thatgetServerInstance()andsetServer()don’t follow this sensible pattern.addDatasourceByIdAndConfig(): We already added a separate config action plugin for that in #3456728: Create a config action to add a data source to an index. However, if we add new methods for the other plugin types, doing it for datasources would also make sense for the sake of consistency – not sure how to handle that, we probably don’t want to offer two config actions for the exact same thing.setTrackerByIdAndConfig(),addProcessorByIdAndConfig(),addFieldByFieldConfig: I admit I first balked a bit at thes suggestions, as I dislike such verbose method names and the methods would pretty much duplicate existing ones. I therefore wanted to suggest instead adding dedicated config action plugins to implement this functionaliy, as we already have for datasources (see above).However, on the other hand it does seem a bit strange to have something available to site admins but not to developers. Also, I suspect the DX of those new methods would be better anyways than of the existing ones – I would now say we probably went the wrong way there when first writing the D8 version of the module.
So, I guess I’m now on board with just adding those methods. I can also imagine our own code subsequently drifting towards using those new methods. They certainly seem handier in most situations. (Though the lack of dependency injection for entities continues to be vexing in this regard.)
As mentioned above, though, we should think about what to do with datasources. We will probably want to add the
addDatasourceByIdAndConfig()method in any case, but maybe just not make it available as a config action?setBackend(): This would be a major architectural change as a server’s backend plugin is currently immutable. I would therefore need a very good argument why this would be helpful/needed before considering it.setReadOnly(): Yes, good idea. No clue why this is currently missing.Note that these should all also be added to the interface, in my opinion, which means you’ll have to add them to the
UnsavedIndexConfigurationclass, too. (Just as a tip, as I regularly forget that as well.)setServerseems like a typo, as that method already exists (and is listed by you a few lines above). Do you know what you meant?Comment #7
mxr576Two additional action suggestions:
Comment #8
borisson_I agree with the second one; that sounds useful. The first one however (index items) seems like it shouldn't be happening at all?
Comment #9
mxr576Well, I guess the answer is "it depends". Whoever uses that action should be awareof consequences. However without that it is complicated to set up an immediately usable demo env with recipes, where waiting for cron to index items may not be an option.
Comment #10
strykaizerCreated ticket for #3565685: Create a config action to add fields to an index
Comment #11
clayfreemanWould also be useful to have actions to manage data source bundle includes/excludes, and add/remove type-specific boosts.
Consider the example where someone wants to deliver their search configuration and content types in separate recipes:
Plural variants would be a nice bonus!
Comment #12
drunken monkey@clayfreeman:
That seems a bit too specific to add at this point, when it’s not even possible yet to add fields to an index.
Moreover, I’m not sure how this would work on an architectural level, as the bundle includes/excludes are options specific to the datasource plugin. I don’t think we can/should provide an index-level action to modify these. (Same for the bundle boosts, which is a specific processor plugin.)
Not sure, is there a clean way to provide a config action specific to a certain plugin associated with the entity?