We do not provide locking with the order entity controller. From #2240427: Improve commerce entities locking

Damien Tournoud
@vadym.kononenko: if commerce_entitycache replaces the commerce_order entity controller, it probably doesn't provide locking, which is *extremely* unsafe. Don't use it.

CommentFileSizeAuthor
#3 don_t_enable_entity-2653788-3.patch1.32 KBmglaman

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue tags: +Commerce Sprint
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs review
StatusFileSize
new don_t_enable_entity-2653788-3.patch1.32 KB

Update hook to disable order entity cache, and keep settings in case someone feels adventurous. And opens door to have it fixed.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work

3:28 PM mglaman: we should probably update the readme to match and we'd have to be clear on the module page we were disabling something on people, but otherwise yah
3:28 PM mglaman: oh and you spelled commerce wrong :)
3:29 PM mglaman: also, maybe link to this, https://www.drupal.org/node/2240427#comment-10709112 in the comment, which explains why in a bit more detail

Update project page, readme.

vulfox’s picture

vulfox commented

I'm currently looking at using this at our site. Would it be better to wait for this update? Is the current behaviour very unsafe?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

vulfox, you can use the module. I'd just recommend running "variable_set('commerce_entitycache_cache_orders', FALSE);" in an update hook, or apply this patch. It's not necessarily unsafe, but could be problematic in a large Drupal Commerce site. The module removes the locking features for orders provided by Drupal Commerce, which is technically a regression.

Thanks for commenting, forgot I need to button this up.

liamanderson’s picture

liamanderson commented
Status: Needs work » Reviewed & tested by the community

Confirm patch from #3 is working.

  • mglaman committed e244452 on 7.x-1.x 
    Issue #2653788 by mglaman, liamanderson: Don't enable entity cache for...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.