Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In #2317771: Move processor and/or field settings to top-level properties on the index?, 'processors' property was renamed to 'processorPlugins'. The problem is that when the config entity gets serialized and then deserialized, processors was empty becaused it's unset in __sleep() which is the wrong property to unset.
Original report.
Been going over this a couple of times, but it seems like the 'Rendered item' field doesn't index anything anymore.
When I look at the 'rendered_item' column in the index table, the value is always NULL and in the index_{field} table (in this case index_text) there isn't anything there. Happens on vanilla install with also the db defaults module as well (which suprised me a bit especially since I'm seeing tests that actually search, but maybe they are using different fields ?)
I know this has been working at some point, at least up and until 2015-11-30, somewhere after that it looks it got broken somehow, will try to dissect back to figure out which commit might have broken this - unless I'm doing something really stupid ?
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 2642534-21--index_serialization.patch | 2.31 KB | drunken monkey |
| #21 | 2642534-21--index_serialization--tests_only.patch | 1.77 KB | drunken monkey |
Comments
Comment #2
swentel CreditAttribution: swentel commentedComment #3
borisson_I've been look at this issue this morning. I can confirm that this doesn't work as expected.
Comment #4
swentel CreditAttribution: swentel commentedb3a219fd1b7b9d4fb26072a55904d4d965c1634b #2317771: Move processor and/or field settings to top-level properties on the index? broke it
Comment #5
swentel CreditAttribution: swentel commentedsearch_api_language column also is NULL always as well (although that might be 'default' on a monolingual site, still, 'en' seems more appropriate here), so it's not only rendered_item.
Funky, enough, add the title as a field to index and that will work.
Comment #6
borisson_This should "fix" the language and url fields. Rendered item is still a no-no for me, trying to figure that out.
Comment #7
swentel CreditAttribution: swentel commentedI don't think this is the right fix, what I'm seeing is that $processor_settings is empty, fixing that will be the better trick here.
Comment #8
swentel CreditAttribution: swentel commentedWelcome to serialization hell :)
(processors was renamed to processorPlugins which shouldn't be serialized)
Comment #9
swentel CreditAttribution: swentel commentedhopefully one day fixed by #1977206: Default serialization of ConfigEntities in general
Comment #10
borisson_Awesome. Adding a regression test for this would be perfect but I think we can probably add that as a followup and commit this one already.
There is no test for the batch anyway, as far as I can see.
Comment #13
swentel CreditAttribution: swentel commentedUpdating the issue title and summary
Comment #14
swentel CreditAttribution: swentel commentedEven better title :)
Comment #15
borisson_@swentel: We discussed in irc that we could add a unit test, is this what you had in mind?
Comment #16
borisson_Changed the name of
$serializedto$rehydrated_entity, to be clearer and added a test-only patch.Comment #17
swentel CreditAttribution: swentel commentedWoot - we discussed the name in IRC as well, in the end $serialized is probably ok, since that is what has happened to the object, so the patch in #15 is ok, great!
Comment #20
swentel CreditAttribution: swentel commentedRaah, stupid bot :)
Comment #21
drunken monkeyThanks for finding and resolving this problem, that's really a bit silly. All the better that we'll now even get a test for that!
However, there were a few things still wrong with the patch (missing doc comments, "misspelled" "serialization" (coding standards demand American English)). I also think we can do things much simpler by just manually unserializing one of our existing index configs and testing with that – just with
assertEquals(), not with checking properties one by one. That tests more with less code.We should also add such a test for the server, and maybe even for other serializable objects (I'm thinking search queries, and maybe fields).
Comment #24
borisson_We can add tests for query / server serialization later in another issue and get this fix in first.
Comment #25
drunken monkeySure, makes sense. Committed. Thanks again, both of you!
For the follow-up, see #2642814: Add tests for servers to EntitySerializationTest.