vuestorefront / vue-storefront-1

The open-source frontend for any eCommerce. Built with a PWA and headless approach, using a modern JS stack. We have custom integrations with Magento, commercetools, Shopware and Shopify and total coverage is just a matter of time. The API approach also allows you to merge VSF with any third-party tool like CMS, payment gateways or analytics. Newest updates: https://blog.vuestorefront.io. Always Open Source, MIT license.
https://www.vuestorefront.io
MIT License
19 stars 13 forks source link

Changing the way attributes are persisted / aggregated #383

Open dkarlovi opened 4 years ago

dkarlovi commented 4 years ago

Current behavior

You need to define all your attributes in the root of your Elasticsearch document, like so

"name": "Something",
"attribute1": 123,
"attribute2": 938,

In the case you're using ES < 7 (one index for everything) or if you have a lot of attributes (in our case - 903, not all attributes for all products of course), you'll run into the issue of Elasticsearch disallowing more than 1000 fields per index by default, changing that limit can lead to performance and memory usage problems.

Expected behavior

Allow storing arbitrary number of attributes, for example using an array instead:

"attributes": [
    {"name": "attribute1", "value": 123},
    {"name": "attribute2", "value": 938}
],

Additional benefit of this approach is: it doesn't clutter the mapping, but also makes mapping attributes possible with libs like ONGR Elasticsearch (used in Coreshop bridge), which maps only explicitly annotated properties by default, you cannot add them on the fly.

Steps to reproduce the issue

Not sure how I'd do that, just create a 100 products, each with 10 different attributes attached,

Version of Vue Storefront

Can you handle fixing this bug by yourself?

Which Release Cycle state this refers to? Info for developer. (doesn't apply to Next)

Pick one option.

Additional information

Somewhat related to vuestorefront/vue-storefront-1#254, doing this would require changing the aggregations, so it's a good direction to not require naming the filters at all, since you can construct filters from what the search matched (and enrich it with attribute metadata, which is done already).

dkarlovi commented 4 years ago

Followup here: we need about 450 filters added, this is currently too much because the URL generated toward VSF API is too large (getting HTTP error 414).

We've worked around that by adding the filters in server-side, just before building the query. It means the filters don't always need to travel from the client to the server (being static, configured in the config file). The issue we're still getting all the aggregations back, even the empty ones, that one can be patched out.

What would really help is:

  1. removing the attribute values from the root object, keep the attribute values in an array instead (as noted above)
  2. aggregate on that field alone, meaning the buckets will be created naturally by Elasticsearch, without enumerating anything
  3. postprocessing can remove attributes which are not to be exported, if required

This would fix all the issues:

  1. Dynamic filters in vuestorefront/vue-storefront-1#254
  2. too much fields issue reported here
PawFV commented 3 years ago

Any update on this? @gibkigonzo