qtranxf_filter_postmeta overwrites metavalues changed during the request

[soft79]

qtranxf_filter_postmeta seems to use it's only little caching for translated metadata. This works fine as long as the original metadata isn't changed during the request.

An example: in WooCommerce when an order is placed, the stock-level for products is changed. After doing so, some checks are performed, for example to mark an item as 'out-of-stock'. But since the q-X 3.2.3 qtranxf_filter_postmeta was added, this doesn't work as it should.

A solution could be adding a hash of the objects' metadata to the cache key:

OLD (qtranslate-frontend.php version 3.2.9.9.3):

    $meta_type = 'post';
    $lang = $q_config['language'];
    $cache_key = $meta_type . '_meta';
    $cache_key_lang = $cache_key . $lang;
    $meta_cache = wp_cache_get( $object_id, $cache_key_lang );
    if( !$meta_cache ){
        $meta_cache = wp_cache_get($object_id, $cache_key);

SUGGESTION:

    $meta_type = 'post';
    $lang = $q_config['language'];
    $cache_key = $meta_type . '_meta';
    $meta_original = wp_cache_get($object_id, $cache_key); //Required to detect changes to the metadata during current request
    if (!$meta_original)
            $meta_cache = false;
    else {
        $cache_key_lang = $cache_key . "_" .  md5(serialize($meta_original)) . "_" . $lang;
        $meta_cache = wp_cache_get( $object_id, $cache_key_lang );
    }
    if( !$meta_cache ){
        $meta_cache = $meta_original; //wp_cache_get($object_id, $cache_key);

...and at the end of the function don't execute wp_cache_add if $cache_key_lang is not set;

I don't know if this is the best solution, what do you think?

Is it actually a good/safe idea to use the cache trick that is used by qtranxf_filter_postmeta?

This support thread is related: https://wordpress.org/support/topic/filter-qtranxf_filter_postmeta-issue

[johnclause]

Thank you very much for reporting this. I did changes similar like you suggested and it passed may testing so far. Could you please try the latest https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip and see if you can reproduce the problem again?

To save the clicks, I am closing it now, we can still write to closed issue and we may re-open it if needed.

[soft79]

Sorry John, this doesn't solve it for the WooCommerce stock issue.

I think this happens:

1. postmeta cache gets flushed by WooCommerce
2. stock is reread (and at this moment recached by qTr-X)
3. stock is changed by WooCommerce, but since qTr-X cache has just been made, qTr-X will use the cache (thus unchanged) stock value.

[johnclause]

Does not it clear cache on each update automatically? I did not check that ...

How are we going to solve this? Is there a filter in update_meta, which we could employ to clear cache?

[soft79]

I justed checked what really happens.

WP does clear the cache after updating _stock in the database, but right before fetching the updated _stock value another post_meta is fetched (_backorders) and thus the wp_cache is written again.

After that _stock is fetched, but since wp_cache_get fetches a value, qTr-X will use the obsolete cached value.

There is an action that is called after updating meta: do_action( "updated_{$meta_type}_meta", $meta_id, $object_id, $meta_key, $_meta_value ); in meta.php

And if metatype is post: do_action( 'update_postmeta', $meta_id, $object_id, $meta_key, $meta_value );

So this could be used to invalidate the qTr-X cache somehow I guess ?!?!

[johnclause]

@josk79: thanks a lot for the research. I added 'updated_postmeta' action, but did not test it, have to run now. Could you test it: https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip

[soft79]

That doesn't work either.

Either the qTranslate-X cache of the object has to be removed when the action update_postmeta is called by WP or some other trick must be used (like my hash trick).

Some other trick might be saving the qTr-X cache as if it were metadata itself. So instead of storing it as a new cache postmeta_en, why not saving it within the postmeta for example as _qtranx_cache_en. This way it will be atomatically deleted when wp_cache_delete is called. Might that work?

If I have time and concentration, I can try it tomorrow, but would like your opinion first.

[johnclause]

I am puzzled. I tested it and could not fail it. Function 'qtranxf_updated_postmeta' deletes QTX cache on update of meta and next time 'get_' is called, the cache has to be re-created. I do not see a way around. Could you give me a sequence of PHP lines I can execute which show the problem?

There is function qtranxf_test_meta_cache in https://github.com/qTranslate-Team/qtranslate-x/blob/master/dev/qtx-tests.php, which I used to test it. Could you modify it or create a similar function so that it would show the problem?

[soft79]

Do you have an environment with WooCommerce by any change? Then it's easy to reproduce, simply by placing an order.

update_post_meta is not called by WooCommerce when changes the stock.

This is the code responsible for stock-changing:

/**
 * Set stock level of the product.
 *
 * Uses queries rather than update_post_meta so we can do this in one query (to avoid stock issues).
 * We cannot rely on the original loaded value in case another order was made since then.
 *
 * @param int $amount (default: null)
 * @param string $mode can be set, add, or subtract
 * @return int new stock level
 */
public function set_stock( $amount = null, $mode = 'set' ) {
    global $wpdb;

    if ( ! is_null( $amount ) && $this->managing_stock() ) {

        // Ensure key exists
        add_post_meta( $this->id, '_stock', 0, true );

        // Update stock in DB directly
        switch ( $mode ) {
            case 'add' :
                $wpdb->query( $wpdb->prepare( "UPDATE {$wpdb->postmeta} SET meta_value = meta_value + %f WHERE post_id = %d AND meta_key='_stock'", $amount, $this->id ) );
            break;
            case 'subtract' :
                $wpdb->query( $wpdb->prepare( "UPDATE {$wpdb->postmeta} SET meta_value = meta_value - %f WHERE post_id = %d AND meta_key='_stock'", $amount, $this->id ) );
            break;
            default :
                $wpdb->query( $wpdb->prepare( "UPDATE {$wpdb->postmeta} SET meta_value = %f WHERE post_id = %d AND meta_key='_stock'", $amount, $this->id ) );
            break;
        }

        // Clear caches

        wp_cache_delete( $this->id, 'post_meta' );
        delete_transient( 'wc_low_stock_count' );
        delete_transient( 'wc_outofstock_count' );
        unset( $this->stock );

        // Stock status
        $this->check_stock_status();

        // Trigger action
        do_action( 'woocommerce_product_set_stock', $this );
    }

    return $this->get_stock_quantity();
}

Note that during check_stock_status(), the _stock is reread. You reckon that Woothemes should call updated_postmeta after the unset?

[johnclause]

How exactly does it get re-read? Don't they use get_post_meta? Otherwise it should still work.
This seems specific to Woocommerce only, otherwise could you setup a simple example to demonstrate the problem?

[soft79]

I made an environment, how can I send you credentials (privately?)

[ghost]

Please note that this problem is not limited to Woocommerce only, it affects other plugins too, particularly http://www.github.com/Teplitsa/Leyka

[johnclause]

Thank you. Please, use qtranslateteam at g m ail d c om.
@Axumata: are you referring to the latest version: https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip?

[soft79]

I Sent you the credentials.

In WP-Admin - WooCommerce - Orders you'll find 2 orders:
Lets take order 11 as an example. In its order notes it says that stock was reduced from 105 to 100. Actually it was reduced 100 to 95.

Stock is changed in woocommerce/includes/abstracts/abstract-wc-product.php:223

This function executes wp_cache_delete, which should invalidate the qTr cache. But as you can see check_stock_status is called which retrieves two metas (indirectly with get_post_meta, (see __get):

first it retrieves _backorders, this will cause the postmeta to be cached by wp
second it retrieves _stock, but since a wp_cache exists at this time, qTranslate-X will use it's old invalid cache.

[ghost]

@johnclause yes i use the latest version

[johnclause]

Thank you, @josk79, the problem was that I forgot to change function wp_cache_add to wp_cache_set in qtranxf_filter_postmeta and old value of cache was not replaced with wp_cache_add.

All, please, test the latest version https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip and make sure to delete previous version of plugin before uploading the new one, as the version number has not been changed, it is still 3.2.9.9.6.

Could you confirm with your testing if this is ok now? Thank you.

[soft79]

Lookin' good! WooCommerce stock management now works fine.

Thanks!

[ghost]

Unfortunately, the problem still persists wnen using both qtranslate-x and Leyka plugin. I've dropped you an email with the problem details and some screenshots.

[johnclause]

Thank you very much @Axumata for providing the testing environment.

Indeed, there was one more problem, different and unrelated to the topic of this issue, but since it should be solved by now, we will not open a new issue. I had to add a couple more lines of code in function qtranxf_filter_postmeta with an extensive explanation:

if(!$meta_key){
if($single){
/**
@SInCE 3.2.9.9.7
The code executed after a call to this filter in /wp-includes/meta.php,
in function get_metadata, is apparently designed having non-empty $meta_key in mind:
if ( $single && is_array( $check ) ){
return $check[0];
}else
return $check;
Following the logic of the code "if ( !$meta_key ) return $meta_cache;",
a few lines below in the same function, the code above rather have to be:
if ( $meta_key && $single && is_array( $check ) ){
return $check[0];
}else
return $check;
The line below offsets this imperfection.
If WP ever fixes that place, this block of code will have to be removed.
*/
return array($meta_cache);
}
return $meta_cache;
}

Please, @Axumata & @josk79, could you both re-test again the latest version from GitHub: https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip. For @Axumata, I already uploaded the new version to his testing site. This time, I increased the version conter and even made another alpha release: https://github.com/qTranslate-Team/qtranslate-x/releases/tag/3.2.9.9.7 This one will probably go as 3.3, unless sombody reports something else again.

[soft79]

Just tested on two sites. It's still working!

[johnclause]

Thank you! I guess we can close this issue now, we can still write to closed issue and we may re-open it if needed.

[ghost]

@johnclause thanks for the fix bug. 3.2.9.9.7 working fine!
Special thanks for the great plugin!