feat(core): ChannelAware ProductVariants

[hendrik-advantitge]

Linked to issue #519.
Functional part of transferring from ChannelAware Products to ChannelAware ProductVariants is done.

Tasks that still need to be done:

Fix DefaultSearchPlugin
Fix ElasticSearchPlugin
Adjust AdminUI
Fix FastImporter service
Fix existing Product tests that are now broken
Write new tests
Would you be able to take some of these tasks off my hands if I focus on writing the new tests?
Any feedback on the implementation is of course welcome.
Thank you very much in advance!

[michaelbromley]

I'm going to pull down the work so far and take a look around.

Assuming the changes seem suitable to merge into Vendure, I could then work on:

• Fixing DefaultSearchPlugin & ElasticSearchPlugin
• Adjusting Admin UI

In that case it would be good if I could push directly to your PR branch. It seems that this would be possible, see this guide.

[michaelbromley]

I'm wondering about any potential performance implications of this part. Imagine that a Product has 100 variants. To look up that product we then need to join 100 * (# channels) rows to check whether the Product is available on the current Channel.

In the original proposal we discussed that edge-case where a Product is created without any variants, and it therefore belongs to no Channel. One of my proposals was to keep a reference to the Channels on the Product. I'm now wondering whether it is worthwhile to keep it for reasons of performance and also developer ergonomics.

Of course this perf thing is just a suspicion. It needs to be tested. I'll put together a load test so we can have some objective idea about whether or not it really matters to do it this way.

[michaelbromley]

Re performance of product lookup:

As stated in the review comment, I wanted to investigate the perf impact of needing to join all variants (in order to determine channel) when looking up a product.

Load test setup

I constructed the following load test:

• 5 Products, each with between 2 - 40 variants:
• A load testing script which makes the following query for 1 minute with up to 50 virtual users:

  query {
      products {
          items {
              id
              name
              slug
              description
              languageCode
          }
      }
  }

Results:

next branch

The results from the current next branch (3e33bbc)

    checks.....................: 100.00% ✓ 3474 ✗ 0
    data_received..............: 6.2 MB  103 kB/s
    data_sent..................: 1.4 MB  23 kB/s
    http_req_blocked...........: avg=13.19µs  min=0s      med=0s       max=1.04ms   p(90)=0s       p(95)=0s
    http_req_connecting........: avg=7.15µs   min=0s      med=0s       max=1.04ms   p(90)=0s       p(95)=0s
    http_req_duration..........: avg=425.77ms min=19.98ms med=432.99ms max=985ms    p(90)=719ms    p(95)=799.01ms
    http_req_receiving.........: avg=106.45µs min=0s      med=0s       max=1.02ms   p(90)=969.92µs p(95)=1ms
    http_req_sending...........: avg=37.9µs   min=0s      med=0s       max=1.01ms   p(90)=0s       p(95)=0s
    http_req_tls_handshaking...: avg=0s       min=0s      med=0s       max=0s       p(90)=0s       p(95)=0s
    http_req_waiting...........: avg=425.62ms min=18.98ms med=432.99ms max=985ms    p(90)=719ms    p(95)=799.01ms
    http_reqs..................: 3474    57.899948/s
    iteration_duration.........: avg=425.99ms min=19.98ms med=433ms    max=986.01ms p(90)=719.01ms p(95)=799.01ms
    iterations.................: 3473    57.883282/s
    vus........................: 49      min=1  max=49
    vus_max....................: 50      min=50 max=50

PR branch

The results from this PR branch (4c1a2be)

    checks.....................: 100.00% ✓ 2501 ✗ 0
    data_received..............: 4.5 MB  74 kB/s
    data_sent..................: 1.0 MB  17 kB/s
    http_req_blocked...........: avg=17.51µs  min=0s      med=0s       max=1.02ms p(90)=0s      p(95)=0s
    http_req_connecting........: avg=12.72µs  min=0s      med=0s       max=1.02ms p(90)=0s      p(95)=0s
    http_req_duration..........: avg=589.46ms min=28.01ms med=590.99ms max=1.14s  p(90)=1s      p(95)=1.05s
    http_req_receiving.........: avg=113.78µs min=0s      med=0s       max=1.05ms p(90)=971.5µs p(95)=1ms
    http_req_sending...........: avg=29.17µs  min=0s      med=0s       max=1.01ms p(90)=0s      p(95)=0s
    http_req_tls_handshaking...: avg=0s       min=0s      med=0s       max=0s     p(90)=0s      p(95)=0s
    http_req_waiting...........: avg=589.31ms min=28.01ms med=590.99ms max=1.14s  p(90)=1s      p(95)=1.05s
    http_reqs..................: 2501    41.68266/s
    iteration_duration.........: avg=589.83ms min=29.03ms med=591.99ms max=1.14s  p(90)=1s      p(95)=1.05s
    iterations.................: 2501    41.68266/s
    vus........................: 49      min=1  max=49
    vus_max....................: 50      min=50 max=50

Conclusion

The test shows a performance regression from 57 req/s to 41 req/s, or a 28% decrease in throughput for this query. I think this is significant enough to consider alternative ways of handling the Product - Channel relation. For example, keeping the relation and updating it based on the channels of the variants, so the calculation only has to be done once when the variants are changed, not on each query.

[hendrik-advantitge]

Hi Michael
This performance regression is indeed significant enough to change our approach on this. I will keep the Channel relation on product, and update it based on the Channels of the Variants. This also removes the need to always have at least one variant defined.
I will follow the guide to give you access to my branch.
Kind regards, Hendrik