Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I noticed that "mongodb_field_storage" and "entity_translation" modules have compatibility issues for translatable field values when the field instance allows only a single value.
PS. I marked this as critical because under the given conditions, it causes data loss.
Comment | File | Size | Author |
---|---|---|---|
#4 | mongodb-entitytranslation-2197361-1.patch | 3.74 KB | math3usmartins |
Comments
Comment #1
math3usmartins CreditAttribution: math3usmartins commentedThis patch helps to explain what I think is wrong with the current source code.
Basically, the current code causes data loss because it forces a single/flat value storage when the field instance is set to support only single value per entity (field instance config form). However, with translatable field values, it must store multiple values (a value for each translation).
So currently, this compatibility issue causes something like this:
1. submitted data (Drupal):
2. stored values (mongodb)
The reason is that the condition "if ($field['cardinality'] == 1)" isn't enough to flatten the stored information. We may need both "en" and "pt-br" keys, for example.
By the way, I think we should never flatten the stored information. In fact there's probably a backward compatibility with already existing sites and field values depending on the field instance settings, because the patch changes the way values are parsed/loaded from the mongodb collection.
This backward compatibility can be solved using an updated_N_hook() to adjust already existing content, but this may raise other issues like long execution time for sites with large amount of data -- which should be the case for those who chose mongodb.
PS. I couldn't test this patch properly but I wanted to report the bug quickly and share the code ASAP. Anyway, I'm sure the Drupal community will find a good solution.
Best regards,
Comment #3
math3usmartins CreditAttribution: math3usmartins commentedfix typo on the issue title.
Comment #4
math3usmartins CreditAttribution: math3usmartins commentedfix relative path within the patch file.
Comment #5
math3usmartins CreditAttribution: math3usmartins commentedComment #7
math3usmartins CreditAttribution: math3usmartins commented4: mongodb-entitytranslation-2197361-1.patch queued for re-testing.
Comment #9
marcingy CreditAttribution: marcingy commentedSo the issue you are going to run into is that mongo only allows one array element in an index. This approach is going to make multi element indexing pretty much impossible. Also as this is doesn't affect drupal it isn't critical
Comment #10
math3usmartins CreditAttribution: math3usmartins commentedActually, MongoDB follows the JSON structure, so it's very flexible.
The error isn't caused by a limitation on the mongodb side. I believe it's simply that this module wanted to simplify/flatten the data structure when there's only a single value for the field, but that simplification causes a conflict with i18n + entity_translation (translation per field) because it may need multiple values (one for each translation).
See the original version where the module changes the data structure right before saving it into mongodb:
Basically I think this module shouldn't change the data structure. And I think it's not really necessary to change the data structure. Moreover, it causes an additional process on write/load, to alter/revert the changes.
Comment #11
marcingy CreditAttribution: marcingy commentedMongo in terms of storage is flexible I agree. So you end with the structure
now you can index field_custom_acme but that is an array or you can add an index for field_custom_acme.en.value and an index for field_custom_acme.a.value. The latter of course works but you only have 64 indexes on a collection and for single language it matters little beyond the backward compatibility break which to be honest is likely to not be allowed to happen given how this could pretty much destroy a large site in terms of how long the update hook would take to run. I am not sure of the answer but I am not sure how this can be supported under d7, of course for d8 it will be as the module you reference is now in core.
Comment #12
math3usmartins CreditAttribution: math3usmartins commentedJust to make it clear, this structure works:
And that is the goal of the patch #2197361-4: Make the module compatible with translatable field values (entity_translation)
Comment #13
math3usmartins CreditAttribution: math3usmartins commentedTo keep backward compatibility and support the entity_translation module for new sites, we can do something like this:
The module would check that variable during the write/load functions.
Sites using i18n + entity_translation must change that variable to FALSE.
That's basically introducing a config variable to support backward compatibility.
Just like what we had in the core:
https://drupal.org/drupal-7.21-release-notes
Caused by this:
https://drupal.org/drupal-7.20-release-notes
That's ugly, but helps. Anyway, that's just an idea to move on.
Comment #14
marcingy CreditAttribution: marcingy commentedThe structure in #12 is going to be an issue as you are now going to have to index arrays of arrays to do anything and as I say mongo has a limit of one array of arrays per index. So while it might be ok from the question of can mongo create the json - the answer for which is yes, it is going to limit how complex of indexes you can create.