About "is_searchable" + normalizer

[johndodev]

Hello,

I'm not sure it's a bug, I aked in discord but I have no answer, but maybe it's not a bug but something improvable.

Original post is :

About last version of meili with symfony bundle.

It seems that the normalization process (custom normalizer) still occurs event if "is_searchable" return false.
So, I wonder what I need to do in the normalizer.

For example, my "is_searchable" function handle non validated (not full for example) entities, so they can't really be serialized.
But as the normalizer are still called, I have to duplicate the logic, and return something (empty array ?).

So first question is, am I wrong about that first assumption (maybe problem is my code) ?

Then, questions are, what is the behaviour of normalization process ? I mean, what happens if I return an empty array, or null, or false ?
Is "is_searchable" still usefull if I handle the logic in the normalizer ?

Thanks !

[norkunas]

Hello, can you please specify about what custom normalizer are you talking about? and give some snippet?

[johndodev]

So, I dug a little.

In the engine's remove function, if the normalizer returns an empty array, the entity is silently ignored (engine line 76).
However, I fail to understand the need to normalize the entity there, as the data are not used for removal.

So, if I understand correctly, index_if is used on postUpdate and postPersist events ("doctrineSubscribedEvents" config).

For postPersist, if index_if return false, it will call remove but it's useless as the entity is not yet indexed -> if the normalizer return empty array, it's skipped, but the normalizer has to manually re call the index_if method for that.

For postUpdate, it index_if return false, the entity has to be removed because it may have been earlier. However, there should be no need to normalize the entity, as it can be expansive and the datas are not used, perhaps even not normalizable. So at this point, the way is to re call index_if in the normalizer, and return a non-empty array for the entity to be removed.

I think there is a logic problem here. So, I think.

• the remove function should not call the normalize (remove theses lines : https://github.com/meilisearch/meilisearch-symfony/blob/main/src/Engine.php#L75-L78),
• the postPersist event should call the index_if method in some way, and complity skip the index() call if index_if return false.

Can someone check if I'm wrong somewhere ?

---

@norkunas You replied while I was writing. Let me know if you still need some code, but it's pretty basic : my entity has an indexIf method, configured in the index_if of meilisearch.yaml, and I have a SF custom normalizer just for meili.

[norkunas]

For postPersist, if index_if return false, it will call remove but it's useless as the entity is not yet indexed -> if the normalizer return empty array, it's skipped, but the normalizer has to manually re call the index_if method for that.

I'm not sure why it was done like this, but please open a PR, we'll see if it breaks something :)

[johndodev]

For that, I think "shouldBeIndexed" should be part of the SearchService interface, which is not currently, so it may cause compatibility issues.. Or we can check "SearchService instanceOf MeiliSearchService" in the listener, but it's a bit dirty..

[johndodev]

If I may I'd like to reopen, as there is still a second subject here, but I can open a new issue if required.

It's about an useless call of delete at a newly persisted entity which is not indexable (via the configurable "index_if" option) :

Full process example

• index if is configured :

-   name: somename
        class: App\Entity\SomeEntity
        index_if: isSearchIndexable

• persist an newly created entity which return false in its "isSearchIndexable()"
• DoctrineEventSubscriber try to index it there
• index() function understand it should'nt index it, as it's not "shouldBeIndexed()", but, then, call remove() there.

Problem is, as it's a new Entity (postPersist), removing it is useless, the entity cannot have been previously indexed.

My suggestion was to test "shouldBeIndexed()" directly in the DoctrineEventSubscriber.php file, but as the class receive an interface (SearchService), and "shouldBeIndexed()" is not part of it, we are more or less stuck.

[johndodev]

[I updated previous post to fix function name (shouldBeIndexed)]

So, I'm not sure if you want to fix that soon, and what direction to take. There are few possibilities, so before making a PR I guess we have to talk about it. Some ideas :

• make an "update()" function in SearchService interface, which will keep the "remove call" if entity is not "shouldBeIndexed()", and remove the "remove call" index in the index() function.
• call "if shouldBeIndexed()" in the DoctrineEventSubscriber.php here, but this involves either add "shouldBeIndexed()" function in the SearchService interface - bc break, or create a new interface just for that function, but it would add complexity for not much.
• add a third default boolean parameter like "bool $removeNotIndexable" but seems crappy, like any other options I think of.