WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 5 years ago

Last modified 5 years ago

#22233 closed defect (bug) (fixed)

update_option() fails when value has nested objects

Reported by: exz Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.4.2
Component: Options, Meta APIs Keywords:
Focuses: Cc:

Description

This gist has an example of how this problem can be reproduced: https://gist.github.com/3923757

If you run the three functions in three separate page loads you will see that the foo attribute of $b doesn't get saved. It seems to be some kind of cache problem.

I've been able to reproduce this in 3.4.2, 3.4.3-alpha and 3.5-beta2

Attachments (1)

22233 update-option-unit-test.patch (2.1 KB) - added by jrf 5 years ago.

Download all attachments as: .zip

Change History (17)

#1 @scribu
8 years ago

  • Component changed from General to Cache
  • Keywords reporter-feedback added; update_option cache removed

So, you have a persistent cache enabled (Memcache, APC etc.), right?

#2 @SergeyBiryukov
8 years ago

Confirmed without a persistent cache.

#3 @exz
8 years ago

Just realized that this could be tested without the need for multiple page loads. I created a unit test for it: https://gist.github.com/3923951

$ phpunit tests/option/option.php 
Installing...
Running as single site... To run multisite, use -c multisite.xml
Not running ajax tests... To execute these, use --group ajax.
PHPUnit 3.6.11 by Sebastian Bergmann.

Configuration read from /Users/rickard/unit-test.svn.wordpress.org/trunk/phpunit.xml

...F

Time: 1 second, Memory: 42.00Mb

There was 1 failure:

1) Tests_Option_Option::test_multiple_calls
Failed asserting that false is true.

/Users/rickard/unit-test.svn.wordpress.org/trunk/tests/option/option.php:92
/usr/bin/phpunit:46

FAILURES!
Tests: 4, Assertions: 31, Failures: 1.

#4 @exz
8 years ago

@scribu I have the memcache module installed but not APC. I verified it using a clean install of 3.4.3-alpha and 3.5-beta

#5 @scribu
8 years ago

  • Keywords reporter-feedback removed

#6 @scribu
8 years ago

  • Severity changed from normal to minor

Confirmed. It seems this check if ( $newvalue === $oldvalue ) isn't enough. PHP probably just compares the internal object ids, rather than the values.

A possible solution would be to make the cloning recursive:

if ( is_object($newvalue) )
	$newvalue = clone $newvalue;

#7 @scribu
8 years ago

  • Component changed from Cache to General

#8 @scribu
8 years ago

  • Summary changed from Multiple calls to update_option with the same array only saves the first time to update_option() fails when value has nested objects

#10 @knutsp
8 years ago

  • Cc knut@… added

#11 @nacin
6 years ago

  • Component changed from General to Options and Meta

#12 @chriscct7
5 years ago

  • Keywords needs-patch needs-testing added
  • Severity changed from minor to normal

#13 @jrf
5 years ago

  • Keywords needs-patch needs-testing removed
  • Resolution set to worksforme
  • Status changed from new to closed

I've been trying to reproduce this issue in order to fix it and have not been able to.
Went back as far as WP 3.7-RC1 to run the unit test and it just keeps passing.

Probably got fixed "accidentally" somewhere along the way between WP 3.5 and WP 3.7, though I can't see anything in the code to pinpoint what would have fixed it.

@exz do you still have a problem with this ? and if so, could you provide a unit test which will fail on trunk ?

If not, I suggest this ticket should be closed, though it might be a good idea to add the unit test to the test suite all the same.

I'll upload a patch which will add @exz's unit test for this. Props @exz.

#14 @jrf
5 years ago

  • Keywords reporter-feedback added

#15 @swissspidy
5 years ago

  • Milestone Awaiting Review deleted

Closing, as the issue could not be reproduced. Also, OP hasn't been active on dotorg for over 2 years and therefore is unlikely to respond in the near future.

#16 @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback removed
  • Milestone set to 3.6
  • Resolution changed from worksforme to fixed

Fixed in [24760].

Note: See TracTickets for help on using tickets.