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:

  1. Check against the Database if already exists beans with a certain delta
  2. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plopesc created an issue. See original summary.

plopesc’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.16 KB

Attaching patch with the proposed solution.

fran seva’s picture

Status: Needs review » Reviewed & tested by the community

Hi @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.

skwashd’s picture

skwashd’s picture

Status: Reviewed & tested by the community » Fixed

@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.

  • skwashd committed 15d1429 on 7.x-1.x authored by plopesc
    Issue #2702495 by plopesc: Get rid of bean_load_delta() in Bean entity...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.