Incorrect package caching triggers conflicts with various extensions

[jimjasson]

Describe the bug

When calculating rates for packages, Subscriptions uses a caching mechanism to store rates per package and avoid calling WC()->shipping->calculate_shipping_for_package multiple times: https://github.com/Automattic/woocommerce-subscriptions-core/blob/trunk/includes/class-wc-subscriptions-cart.php#L1257-L1267

WC()->shipping->calculate_shipping_for_package has its own caching mechanism that uses the entire $package in order to produce the hash key to avoid calculating rates multiple times: https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/includes/class-wc-shipping.php#L333

Subscriptions uses only some of the $package keys to create the hash key (contents/contents cost and applied coupons): https://github.com/Automattic/woocommerce-subscriptions-core/blob/trunk/includes/class-wc-subscriptions-cart.php#L1277

The reason behind this seems to be that Subscriptions wants to apply the same Shipping rates to all recurring packages in the cart to boost performance. However, this triggers conflicts with Conditional Shipping and Payments, Product Bundles, and Composite Products.

For Conditional Shipping and Payments:

This plugin can be used to filter shipping rates in recurring packages based on their billing schedule. When Subscriptions is caching the package, it doesn't take into account its recurring schedule. Therefore, packages with identical contents but different schedules end up having the same rates. This prevents Conditional Shipping and Payments from disabling specific rates per package.

For Product Bundles/ Composite Products together with All Products for WooCommerce Subscriptions:

All Products for WooCommerce Subscriptions enables content switching for subscribed Bundles. These switches are always considered cross-grades, as proration for content switching is not supported.

When switching a subscribed bundle, the initial package with $0 subtotal (cross-grade) gets cached and the rates calculated for this package are passed down to the recurring package as well (as contents, content totals, and applied coupons are identical). If a Free Shipping Method has a minimum order restriction, then it doesn't show up for the recurring package because Subscriptions uses the rates of the initial package for the recurring one as well -- and the subtotal of the initial package is $0.

Therefore, it seems that the optimization Subscriptions has implemented for caching packages, by creating hashes using only some of its keys, is causing more issues than benefits. A couple of solutions here are:

• When caching packages, cache the entire package similarly to what core does.
• Fall back the core package caching system.

To Reproduce

See linked issues for detailed replication steps:

• https://github.com/woocommerce/woocommerce-product-bundles/issues/1134
• https://github.com/woocommerce/woocommerce-conditional-shipping-and-payments/issues/534

[wyter]

5896670-zen

[glagonikas]

@jimjasson can you please confirm if restrictions for PB switches will be resolved within the scope of this task?

So in our case, we sell PBs subscriptions and offer free shipping to everyone who spends at least £5 on subscriptions.

We've set up the following rule thinking that it will stop offering free shipping to subs below £5 but have obviously come across this bug.

Whenever this rule is enabled, we lose free shipping on swiching, regardless of shipping debug being on or off.

[jimjasson]

Hey @glagonikas 👋

This issue causes Conditional Shipping and Payments to get a cached package instead of the actual recurring package when evaluating its restrictions. So, any restrictions you set up for the recurring package might apply to a different, cached package -- therefore, you may see that Package Total is higher than 5 in your cart, but the Restriction is not kicking in because it is looking at a different package.

I think that it is best to take it one step at a time here. Once this issue is resolved and Conditional Shipping and Payments gets access to the correct package when evaluating its Restrictions, we need to check if there are any additional issues on your site. If there are, we'd be happy to help you further investigate them through support.

[james-allan]

Thanks for the thorough issue write up @jimjasson.

Therefore, it seems that the optimization Subscriptions has implemented for caching packages, by creating hashes using only some of its keys, is causing more issues than benefits. A couple of solutions here are:

• When caching packages, cache the entire package similarly to what core does.
• Fall back the core package caching system.

I've done a bit of digging into the background of this shipping package caching mostly to see if I could find a set of steps I could use to confirm removing it wouldn't lead to any regression.

The original issue and PR can be found on the WC Subscriptions repo:

• Issue - https://github.com/woocommerce/woocommerce-subscriptions/issues/1415
• PR - https://github.com/woocommerce/woocommerce-subscriptions/pull/1485

From the discussion on the issue, the rationale for this cache was for purely performance reasons. The issue and PR were originally filed in May-Jun 2016. It's hard to tell, but it seems WC core's caching was also introduced around that time in 2.6.0 @ 2016-06-14 or variations of a cache layer existed before that time. 🤷

Given the amount of time that has passed since this cache was introduced, I think at the very least, we should bypass our cache and test to see if there's any issues that arise.

Subscriptions uses only some of the $package keys to create the hash key (contents/contents cost and applied coupons): https://github.com/Automattic/woocommerce-subscriptions-core/blob/trunk/includes/class-wc-subscriptions-cart.php#L1277

The reason behind this seems to be that Subscriptions wants to apply the same Shipping rates to all recurring packages in the cart to boost performance. However, this triggers conflicts with Conditional Shipping and Payments, Product Bundles, and Composite Products.

Following the conversation on the original issue, the reason behind our limited data set to make up the cache key was because some package values changed on the package between requests. WC seems to have gotten around this by unsetting the product data, we took a different approach.

I've reached EOD today, but locally I've bypassed our get_calculated_shipping_for_package() function and will see if I notice any issues.