search

algolia / algoliasearch-client-php

⚡️ A fully-featured and blazing-fast PHP API client to interact with Algolia.
https://www.algolia.com/doc/api-client/php/getting-started/
MIT License
671 stars 116 forks source link

feat: always cast decompoundedAttributes to object #681

Closed DevinCodes closed 7 months ago

DevinCodes commented 3 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? no

Describe your change

The engine only accepts objects when setting the decompoundedAttributes setting when performing a setSettings. This normally goes well since PHP encodes associative arrays to objects. If the array is empty, however, it will be encoded as an array. This PR introduces a change so that during setSettings, we always cast decompoundedAttributes to an object.

bsuravech commented 3 years ago

@DevinCodes Can you elaborate why the php client has to make exceptions for the decompoundedAttributes on empty values? I feel that the search API shouldn't be returning empty/null values and should save empty objects as null to prevent this error from occurring.

If we're trying to fix this on the php client level, all customers who accidentally play with this attribute will encounter this error and will need to update their client version to compensate. Those on 1.x would also need to patch as well.

DevinCodes commented 3 years ago

Hi @bsuravech ,

You can find more details in this related HelpScout ticket (Algolia-only), but I see you've been active on this thread as well.

Basically, we need to support empty objects for the scenario where a user does a getSettings on index A to perform a setSettings on index B. If we don't add support for empty objects, this will not work as expected if the API wouldn't return the field. To me it makes sense to have this particularity (which, AFAIK, only occurs in PHP) handled by the API client, instead of adding an exception for this on the search API.

Users that encounter this bug indeed need to update their API client version. I'll check internally what the strategy regarding v1.x is, though, before adding it there as well 🙂

bsuravech commented 3 years ago

@DevinCodes I'm trying to suss out why we need to return an empty attribute value in settings from Search API. I understand it would be "easier" from our side to fix as it only effects the PHP client but it would be on our customers to find a solution and would impact support should there be more cases.

I feel it would be easier for the engine to just not return an unset/empty value on an attribute. There's not issue on this value when there is a value configured. AFAIK, the issue is when the value has been cleared on the dashboard for this attribute. What's the logic in returning an empty attribute?