Posted by Georgique on April 10, 2012 at 6:39am
86 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | entity system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | Needs tests |
Issue Summary
PHP 5.4
Got notice when saving node:Notice: Array to string conversion in function DrupalDefaultEntityController->cacheGet() (string 368 in file /home/george/www/inti.kz.dev/includes/entity.inc).
Looked into code and don't understand why I see it. So can't make patch myself.
Comments
#1
I found it.
Function array_diff_assoc() converts all elements of input arrays to string. But $entity_values can and contains elements which are arrays too.
So solution is serialize each element of array before comparing. Patch is attached.
#2
I havent checked, bu this looks like something that would have to be fixed in D8 first and then backported
#3
Pretty sure this needs a test then... not sure I fully understand it.
#4
I don't think a notice is a major, bumping down.
#5
Can we get a wrapper? This is starting to pop up on other modules like Views, Features and even core itself.
#6
There is the same problem with using array_diff() also. I've checked. It should be repaired in many places.
#7
got same issue in D7.14
#8
.
#9
CNW for tests.
#10
The patch from #1 works and is needed in Drupal 7.14.
Please move forward.
#11
Core patches need tests before being considered finished. See http://drupal.org/core-gates#testing
#12
Actually, I think using
array_diff_assoc()was wrong in the first place, because that means we don't support array values for $conditions, like we do (by chance) when loading entities from the database.Attaching patches for 8.x and 7.x that should fix this properly. I can provide some tests on Friday if we agree on the approach.
#13
Reviewed the patch for Drupal 7, looks great. I tested it and it works fine.
I did not have the time to test for Drupal 8.
#14
Patch #12 works perfectly for me on Drupal 7
#15
The same patch was successfully applied to Entity.module in #1514764: Array to string conversion error from line 625 of entity/includes/entity.controller.inc.
#16
@amateescu can you write a test for Drupal 8, so we are sure this works?
#17
Same problem here on a fresh install of 7.15
Patch #12 seems to solve it.
#18
#12: 1525176-fix_entity_conditions.patch queued for re-testing.
#19
The last submitted patch, 1525176-fix_entity_conditions.patch, failed testing.
#20
This patch works for Entity 7.x-1.0-rc3. This is the first time I post a patch, so hopefully I haven't done anything wrong.
#21
#22
The last submitted patch, entity-1525176-#20.patch, failed testing.
#23
#1 and #12 worked for me with Drupal 7.16 (not a fresh install).
Please note that this is a serious issue, more than just a PHP warning. I was unable to actually update my nodes. Following the long string of PHP warnings Drupal had a notification saying that the changes were not saved.
This problem appeared at some point on a MAMP sandbox server running PHP 5.4.4. I'm not sure when exactly because most of the node creation and updating is automated.
Thanks to amateescu.
#24
Can anyone tell me what the status of this problem is? I downloaded Crommerce and have the very same issues described above. I only found one download of Commerce 7 that would actually work on my WAMP but it has this error all over the place. If I can't find the solution, I will have to go back to Ubercart. I didn't experience it until I downloaded the Commerce distribution. I am a newbie and not sure how to find out more information or a solution. Thanks everyone.
#25
Restoring version, category and priority.
#26
FYI, http://drupal.org/node/1829260#comment-6677906 points out a PHP problem with array_diff_assoc.
I have this issue in 7.16. What is the consensus: is it safe to ignore these warning messages? If not, perhaps this should be changed back to 7.x and 'major'.
#27
#26 it should be fixed for Drupal 8 first
#28
I don't understand why it should be fixed for D8 first, if it is causing problems for D7 sites currently in use and is an actual problem (not just faulty warning messages) . Please don't misunderstand--I'm not being confrontational, but as an outsider to the process, it would seem that the current sites being used should have priority over the future dev project for non-trivial bugs.
#29
@mark123 we fix d8 first in order to prevent the error from showing up in the next release. Once it's fixed, we backport the patch so d7 (and d6) get the fix as well.
#30
Does anyone know when this fix might be available? I am just looking for a rough estimate (ballpark figure). I am new to Drupal and I am not really sure how long NORMAL fixes seem to take. Should I look at another solution or hang out for this fix. I am not going live for at least 3 to 4 more weeks. Thanks everyone for helping a newbie!
#31
Changing line from the patch in post #1 in entity.inc worked for me. I do not get those messages anymore. Thanks.
#32
Fixing version.
#33
Since 7.18 is current, I guess this issue moves to it too.
#34
Patch #12 works fine on Drupal 7.18
#35
#12: 1525176-fix_entity_conditions.patch queued for re-testing.
#36
So will this patch be added to the next Drupal 7.19 release ?
#37
Nope
a) no patch gets committed until it passes all the tests (aka turns Green in the issue queue) and is marked as Reviewd & Tested by the Community.
b) bugs are always fixed in the latest branch (in this case 8.x) first so that we dont forget to put the fix in there and thus re-introduce the bug
Neither of those two things have happened yet. If you want to help move this issue along so it can get into D7, then the next step would be to create a patch against Drupal8 that passes all the tests.
#38
Given that:
It seems appropriate to raise the priority of this issue to at least 'major', and perhaps 'critical'. I won't make the change myself, but leave it to those more involved with the solution to this issue to decide.
#39
Tagging.
#40
Marked #1829260: PHP array_diff* API isn't recommended for multidimensional arrays as a duplicate of this issue.
#41
If this is preventing nodes from being saved (and if that is reproducible with Drupal core alone) this should be moved to major or probably critical. Nothing to do with Drupal 7 vs. Drupal 8 most likely; a problem like that would be equally critical for Drupal 8 too :)
However, it's not clear to me from reading this issue whether there are actually a reproducible set of steps (starting from a fresh installation of Drupal core) that result in the node not being saved at all...
#42
#1: cache_get.patch queued for re-testing.
#43
It looks like the buggy code was removed from Drupal 8 in August (see #1184272).
#44
does this mean we can get it for 7 ?
#45
Yes, this code was removed in #1184272-81: Remove deprecated $conditions support from entity controller
<?php- foreach ($entities as $entity) {
- $entity_values = (array) $entity;
- if (array_diff_assoc($conditions, $entity_values)) {
- unset($entities[$entity->{$this->idKey}]);
- }
- }
?>
Needs work as the patch doesn't apply and also from #11 this needs tests
#46
Wonder if #1850798: Add a recursive version of array_diff_assoc() would help here?
#47
.
#48
Is there an estimated time for this fix in D7 (7-21+)?
Is the Dev version of D7 fixed ?
I would gladly move on to D8, but there are not enough modules ported to D8 yet.
Patrick
#49
this works on 7.21, Thanks!
#50
Thanks. #12 1525176-fix_entity_conditions-D7-do-not-test.patch works on drupal 7.22
#51
OK thank you.
Patch applied to production version of http://www.ExpertWitness.com
I will report back in a few days with feed back.
Patrick
#52
Hi!
I'm having this issue. It happens since I use "tags". My problem is that I can't apply the #12 1525176-fix_entity_conditions-D7-do-not-test.patch
I'm using Drupal 7.21 on localhost with MAMP.
I attach the screenshot.
Thanks in advance!
#53
@mellamoanton #50 says it works on Drupal 7.22, did you try upgrading then applying?
#54
My apologies, I don't know why I was trying apply patch on Entity module, not Drupal core. Anyway, I could not apply the patch correctly –I don't know why–, so I did it manually –were a few lines–. Now, everything is perfect.
Thanks!
#55
#12 works for me on 7.22.
#56
#12 1525176-fix_entity_conditions-D7-do-not-test.patch works for me on Drupal 7.22 with no fresh install (is a blog with a lot of modules preactivated and configured). Thanks.
So, are this patch will be included in next versión of Drupal 7.x (for example 7.23) or in Drupal 7.x-dev?
Sorry my english ;)
#57
So to clarify, what are we waiting on getting this in Drupal 7? I can try to help out if possible.
#58
Seems to be all good here.
I applied the patch #68 at issue:
==> http://drupal.org/node/1850798
this morning on my production drupal site on
==> http://www.ExpertWitness.com
Patrick
#59
As said before, we are waiting for a test. Without a test that exposes this bug and proves that the patch fixes it and will ensure that it won't get broken again, this will not get in.
#60
For me the error went away with 1525176-fix_entity_conditions-D7-do-not-test.patch from #12 too.
#61
I tried applying the one-liner patch proposed in #1 (in line 369 on D7.22) and the error has gone away for the moment. I will upgrade to the next version when the issue is fixed definitively!
#62
re-rolling the patch so it get's tested for D7..
edit: saved too quickly, see next comment.
#63
re-rolling the patch so it get's tested for D7..
#64
#63 worked great for me!
#65
also replace the "array_diff_assoc" with 'array_diff_assoc_recursive' and copy this function in a module or in the core:
function array_diff_assoc_recursive($array1, $array2) {foreach ($array1 as $key => $value) {
if (is_array($value)) {
if (!isset($array2[$key])) {
$difference[$key] = $value;
} elseif (!is_array($array2[$key])) {
$difference[$key] = $value;
} else {
$new_diff = array_diff_assoc_recursive($value, $array2[$key]);
if (count($new_diff) > 0) {
$difference[$key] = $value;
}
}
} elseif (!array_key_exists($key, $array2) || $array2[$key] != $value) {
$difference[$key] = $value;
}
}
return !isset($difference) ? array() : $difference;
}
damned php 5.4!