Only support entities with a numeric entity_id [#1448324]

From #1343934: Meta Tags & WYSIWYG Incompatibility? and #1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID (see comment #67) it seems that the API requirement of entity_id to be numeric only applies to fieldables entities. So entity types that are not intended to have fields can have a non-numeric entity_id.

Metatag uses a numeric entity_id and that makes it incompatible with modules that do not use numeric ids, like WYSIWYG.

Comment	File	Size	Author
#11	1448324-metatag-entity-IDs-must-be-numeric.patch	2.76 KB	Dave Reid
#11
#9	metatag-n1448324-9.patch	2.97 KB	DamienMcKenna
#9
#1	1448324-metatag-entity-id.patch	1.17 KB	Pedro Lozano
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Pedro Lozano CreditAttribution: Pedro Lozano commented 21 February 2012 at 16:27

File	Size
1448324-metatag-entity-id.patch	1.17 KB

Since it seems way more complicated to make WYSIWYG to use a numeric entity_id than change Metatags to use a string entity_id, I've made a patch myself with an update function.

Comment #2

Dave Reid

he/him

English

Nebraska USA

CreditAttribution: Dave Reid commented 21 February 2012 at 17:03

No, we cannot just make this change since it also changes the size of table indexes by changing this to from an integer to a varchar. If we must fix this in Meta tags, then I would much rather just add an exclusion for the stupid wysiwyg profile entity types in metatag_entity_load().

Comment #3

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 21 February 2012 at 17:35

Status:

Needs review

» Needs work

In talking with Pedro about it on IRC I'd also recommend just doing a check to exclude all entities that have non-numeric IDs; changing to a non-numeric ID introduces complications, e.g. varchar columns in UTF-8 tables have to be small in order to be indexable.

Comment #4

kalabro

she/her

CreditAttribution: kalabro commented 16 March 2012 at 12:04

The same problem with Commerce Coupon (commerce_coupon) module.

Comment #5

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 24 March 2012 at 22:31

Title:

Incompatibility with non fieldable entities, entity_id should be a string

» Support for entities with a string ID

Comment #6

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 27 March 2012 at 12:07

Drupal core, e.g. the Field module, uses an unsigned integer value for storing the entity_id, so Metatags needs to do the same and ignore other data.

Comment #7

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 27 March 2012 at 12:08

Issue tags:

+D7 stable release blocker

Tagging.

Comment #8

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 27 March 2012 at 12:09

Title:

Support for entities with a string ID

» Only support entities with a numeric entity_id

Comment #9

DamienMcKenna

NH, USA

CreditAttribution: DamienMcKenna commented 27 March 2012 at 12:53

Status:

Needs work

» Needs review

File	Size
metatag-n1448324-9.patch	2.97 KB

An initial pass that needs some testing. Could someone let me know of any existing contrib modules that use string entity IDs so I can test it further? Thanks.

Comment #10

Dave Reid

he/him

English

Nebraska USA

CreditAttribution: Dave Reid commented 27 March 2012 at 14:09

Status:

Needs review

» Needs work

+++ b/metatag.moduleundefined
@@ -387,7 +394,9 @@ function metatag_entity_update($entity, $entity_type) {
@@ -405,7 +414,7 @@ function metatag_field_attach_view_alter(&$output, $context) {

@@ -405,7 +414,7 @@ function metatag_field_attach_view_alter(&$output, $context) {
   $entity = $context['entity'];
   list($entity_id, $revision_id, $bundle) = entity_extract_ids($entity_type, $entity);
 
-  if (metatag_entity_supports_metatags($entity_type, $bundle) && $context['view_mode'] == 'full' && _metatag_entity_is_page($entity_type, $context['entity'])) {
+  if (is_numeric($entity_id) && metatag_entity_supports_metatags($entity_type, $bundle) && $context['view_mode'] == 'full' && _metatag_entity_is_page($entity_type, $context['entity'])) {
     $cid = "output:{$entity_type}:{$entity_id}";
 
     if ($cache = cache_get($cid, 'cache_metatag')) {
@@ -792,7 +801,7 @@ function metatag_field_attach_delete_bundle($entity_type, $bundle) {

@@ -792,7 +801,7 @@ function metatag_field_attach_delete_bundle($entity_type, $bundle) {
 function metatag_field_attach_form($entity_type, $entity, &$form, &$form_state, $langcode) {
   list($entity_id, $revision_id, $bundle) = entity_extract_ids($entity_type, $entity);
 
-  if (!metatag_entity_supports_metatags($entity_type, $bundle)) {
+  if (!is_numeric($entity_id) || !metatag_entity_supports_metatags($entity_type, $bundle)) {
     return;

These changes should be unnecessary because if an entity is fieldable it has to be using numeric IDs.