Hello.
I found bug in process of working bean with EntityAPI. It will be happened when bean have numeric delta.
Delta will be set from title, when bean block is saving.
Description:
For example - you want to delete bean block with "delta" = 5 and real "bid" of this block is 3.
Part of callstack:
...
// Delete selected bean.
BeanEntityAPIController->delete();
// Load bean before delete.
EntityAPIControllerExportable->load();
in this function used snippet of code:
// Only do something if loaded by names.
if (!$ids || $this->nameKey == $this->idKey || is_numeric(reset($ids))) {
return parent::load($ids, $conditions);
}
In case with beans variable $ids contains "delta" of bean. In our example it's "5". 5 is numeric and in this case, entityAPI will load bean via "idKey" (bid). And when I will try to remove bean with delta = 5 and real bid = 3, entityAPI will load bean with bid = 5. If this bean is exists, it will be removed. If not, then no actions and I cannot remove my bean.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | bean-avoid-numeric-delta-2036137-4.patch | 848 bytes | eugene.ilyin |
| #1 | bean-avoid-numeric-delta-2036137-1.patch | 543 bytes | eugene.ilyin |
Comments
Comment #1
eugene.ilyin commentedThis case also used for menu and I think that easy way to solve this problem - add prefix for delta.
This patch will solve this problem.
If you will apply my patch, please specify me as author of commit. Also I can attach script for repair deltas for old beans, but I don't know the best way for run this script. It can be hook_update, but it use batch. Hook_update can be runned from drush and I cannot make batch, working from drush and located in .install file (not as drush command).
Thank you for your module.
Comment #2
eugene.ilyin commentedComment #3
eugene.ilyin commentedvery long testing : -\
Comment #4
eugene.ilyin commentedLittle fix in this patch
Comment #5
saltednutThe tests are broken, I think. This is a great catch. I could easily delete bean with bid 1 simply by deleting bean with bid 2 but having a delta of 1. Wow. :(
Nice work here.
Some things needed to be added to get this in.
bb as a prefix introduces a whole new problem for the bean namespace. The prefix can just be "bean"
Also, I think we only need to append the prefix when the delta provided is purely numeric.
I added an if statement to take care of these problems and fixed the name but I'm giving eugene.ilyin commit credit for doing the majority of the work here.
Note that this still doesn't fix the heart of the issue - which is a flaw to Entity API. But we can't have it messing up Bean's UX so we'll account for it.
You can still delete beans errantly by providing bid in the url for say, /block/1/delete -- but 90% won't try to delete Beans this way.
http://drupalcode.org/project/bean.git/blobdiff/3227c5a47e74092406464ae3...
Comment #6
saltednutJust realized I errantly created a reversion here where the max_length wasn't always trimming correctly. Fixed in followup.