Problem/Motivation
We have a project were we make use of Domain Access and Domain Entity modules to give access beans only in specific domains.
Everything worked fine, but when the number of beans has increased, we noticed an error when creating new ones.
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'test' for key 'delta': INSERT INTO {bean} (vid, delta, label, title, type, view_mode, data, uid, created, changed) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9); Array ( [:db_insert_placeholder_0] => 0 [:db_insert_placeholder_1] => test [:db_insert_placeholder_2] => Test [:db_insert_placeholder_3] => Test [:db_insert_placeholder_4] => inspirations_introduction [:db_insert_placeholder_5] => default [:db_insert_placeholder_6] => a:1:{s:9:"view_mode";s:7:"default";} [:db_insert_placeholder_7] => 331 [:db_insert_placeholder_8] => 1460042381 [:db_insert_placeholder_9] => 1460042381 ) in drupal_write_record() (line 7334 of /my/path/www/includes/common.inc).
Then we realized that Bean module was not able to detect we already had a bean with delta test
and renamed our new bean to test-0
. We realized there was a conflict between Bean & Domain entity module because original "test" bean was not visible in the domain we were creating the new "test" bean.
After some debug we discover that problem was in bean_load_delta()
function, which is called from Bean::checkDelta()
method. This function does 2 things:
- Check against the Database if already exists beans with a certain delta
- Load those beans and return them
First step is completely OK and the query return the already existing Bean ID. So things are good so far. However, when trying to load the entity, Domain entity module alters the load entity query to hide beans not available for the current domain, so instead of returning the entity, it returns FALSE
.
Given that step 1 works as expected, and we don't really need in Bean::checkDelta()
the loaded entity, we propose to create a new method in Bean class Bean::deltaExists()
where we just make the first query against the DB to ensure if already exists a bean using that delta.
Besides of that, this would increase performance because we would skip an unnecessary call to bean_load()
function.
Proposed resolution
- Get rid of
bean_load_delta()
call in Bean class - Create a specific method in the class
Remaining tasks
Create patch- Review
- Commit patch
User interface changes
None
API changes
None
Data model changes
We would add a new method in Bean
class decoupling it from bean.module
file and increasing performance.
Comment | File | Size | Author |
---|---|---|---|
#2 | check_delta-2702495-2.patch | 1.16 KB | plopesc |
|
Comments
Comment #2
plopescAttaching patch with the proposed solution.
Comment #3
fran seva CreditAttribution: fran seva as a volunteer and at Bluespark commentedHi @plopesc. The code looks good and the idea is good because we decouple code that could be needed to integrate with other modules and improve performance. Maybe, modify bean_load_delta to use Bean::deltaExists() could be good to avoid duplicate code but I think that should be another issue.
Comment #4
skwashd CreditAttribution: skwashd at Dave Hall Consulting for Dave Hall Consulting commentedComment #5
skwashd CreditAttribution: skwashd at Dave Hall Consulting for Pfizer, Inc. commented@plopesc, thanks for the patch. I've committed it. This will be included in the next Bean release in the coming days.
@fran seva I'm happy to see your suggestion handled in a followup issue.