diff --git a/tripal/src/TripalField/TripalFieldItemBase.php b/tripal/src/TripalField/TripalFieldItemBase.php index 4fe11e6f0..87c65a927 100644 --- a/tripal/src/TripalField/TripalFieldItemBase.php +++ b/tripal/src/TripalField/TripalFieldItemBase.php @@ -6,6 +6,7 @@ use Drupal\tripal\TripalField\Interfaces\TripalFieldItemInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Field\FieldStorageDefinitionInterface; +use Drupal\field\Entity\FieldStorageConfig; use Drupal\tripal\TripalStorage\IntStoragePropertyType; use Drupal\tripal\TripalStorage\VarCharStoragePropertyType; use Drupal\tripal\TripalStorage\TextStoragePropertyType; @@ -663,19 +664,35 @@ public static function discover(TripalEntityType $bundle, string $field_id, arra * an appropriate field name. It removes will always include the bundle * type as the prefix, followed by an underscore, followed by extra text * provided. It ensures that only alphanumeric values are present in the - * name and that it doesn't exceed 50 characters. + * name and that it doesn't exceed Drupal's maximum length. * * @param \Drupal\tripal\Entity\TripalEntityType TripalEntityType $bundle * The TripalEntityType object with information about the bundle. * @param string $extra * Extra text to add to the field name after the bundle name. + * @param int $cvterm_id + * The cvterm_id value, only used when we need to truncate a long name. + * You may leave this set to the default of zero if the cvterm_id is + * not readily available, in which case a random unique ID is used, + * however we recommend using the actual cvterm_id if it is available + * so that the field name generated is predictable and reproducible. + * * @return string * The generated field name. */ - public static function generateFieldName(TripalEntityType $bundle, string $extra) { + public static function generateFieldName(TripalEntityType $bundle, string $extra, int $cvterm_id = 0) { + $max_length = FieldStorageConfig::NAME_MAX_LENGTH; $field_name = strtolower($bundle->getID() . '_' . $extra); - $field_name = preg_replace('/[^\w]/', '_', $field_name); - $field_name = substr($field_name, 0, 50); + $field_name = preg_replace('/[^\w]/u', '_', $field_name); + // If name is longer than Drupal allows, truncate and include the + // cvterm_id as a suffix to guarantee uniqueness. + if (mb_strlen($field_name) > $max_length) { + if (!$cvterm_id) { + $cvterm_id = uniqid(); + } + $truncate_to = $max_length - strlen($cvterm_id) - 1; + $field_name = mb_substr($field_name, 0, $truncate_to) . '_' . $cvterm_id; + } return $field_name; } } diff --git a/tripal/tests/modules/tripal_test/src/Plugin/Field/FieldType/TripalTestTextTypeItem.php b/tripal/tests/modules/tripal_test/src/Plugin/Field/FieldType/TripalTestTextTypeItem.php index 49f2d99e4..47d8f9b76 100644 --- a/tripal/tests/modules/tripal_test/src/Plugin/Field/FieldType/TripalTestTextTypeItem.php +++ b/tripal/tests/modules/tripal_test/src/Plugin/Field/FieldType/TripalTestTextTypeItem.php @@ -44,12 +44,8 @@ public static function tripalTypes($field_definition) { */ public static function discover(TripalEntityType $bundle, string $field_id, array $field_definitions) : array { - // Initialize with an empty field list. - $field_list = []; - - // Create a valid field. - $field_list[] = [ - 'name' => self::generateFieldName($bundle, 'test_field'), + $base_field = [ + 'name' => self::generateFieldName($bundle, 'test_field', 0), 'content_type' => $bundle->getID(), 'label' => 'Test', 'type' => self::$id, @@ -82,40 +78,30 @@ public static function discover(TripalEntityType $bundle, string $field_id, arra ], ]; + // Initialize with an empty field list. + $field_list = []; + + // Create a valid field. + $field_list[] = $base_field; + + // The same field but with a long name including spaces and unicode that + // will be truncated to 32 characters: 'organism__test_field_but_with__1' + // cvterm_id is passed and should appear at the end of the field name + $field_2 = $base_field; + $field_2['name'] = self::generateFieldName($bundle, '🙈test field_but with_a very_very long_name', 1); + $field_list[] = $field_2; + + // The same except cvterm_id is not passed, a random unique id should be appended + $field_3 = $base_field; + $field_3['name'] = self::generateFieldName($bundle, '🙈test field_but with_a very_very long_name'); + $field_list[] = $field_3; + // Create an invalid field. - $field_list[] = [ - 'name' => self::generateFieldName($bundle, 'test_field2'), - 'content_type' => $bundle->getID(), - 'label' => 'Test', - 'type' => 'this_type_does_not_exist', - 'description' => 'A test field', - 'cardinality' => 1, - 'required' => TRUE, - 'storage_settings' => [ - 'storage_plugin_id' => 'drupal_sql_storage', - 'storage_plugin_settings' => [], - 'max_length' => 255, - ], - 'settings' => [ - 'termIdSpace' => 'OBI', - 'termAccession' => '0100026' - ], - 'display' => [ - 'view' => [ - 'default' => [ - 'region' => 'content', - 'label' => 'above', - 'weight' => 10, - ], - ], - 'form' => [ - 'default' => [ - 'region' => 'content', - 'weight' => 10 - ], - ], - ], - ]; + $field_4 = $base_field; + $field_4['name'] = self::generateFieldName($bundle, 'test_field4', 0); + $field_4['type'] = 'this_type_does_not_exist'; + $field_list[] = $field_4; + return $field_list; } } diff --git a/tripal/tests/src/Functional/Services/TripalFieldCollectionTest.php b/tripal/tests/src/Functional/Services/TripalFieldCollectionTest.php index 7a9b8ebf9..c4eef6d95 100644 --- a/tripal/tests/src/Functional/Services/TripalFieldCollectionTest.php +++ b/tripal/tests/src/Functional/Services/TripalFieldCollectionTest.php @@ -273,14 +273,27 @@ public function testTripalFieldCollection() { // those fields as expected. // Make sure we see a valid field - //print_r($discovered_fields); $this->assertTrue(array_key_exists('organism_test_field', $discovered_fields['new']), "Missing the 'organism_test_field' key in the 'new' array returned by the discover() function."); + // Make sure we see a properly truncated field name with + // spaces and unicode character replaced with underscores + $this->assertTrue(array_key_exists('organism__test_field_but_with__1', $discovered_fields['new']), "Missing the 'organism__test_field_but_with__1' key in the 'new' array returned by the discover() function."); + + // Same as above, but cvterm_id was not passed. The field name should + // end with a unique id + $found = FALSE; + foreach ($discovered_fields['new'] as $field) { + if (preg_match('/^organism__test_fie_[0-9a-f]{13}$/', $field['name'])) { + $found = TRUE; + } + } + $this->assertTrue($found, "Missing the 'organism__test_fie_[0-9a-f]{13}' key in the 'new' array returned by the discover() function."); + // Make sure we have an invalid field. We don't need to test every // case where a field may be invalid. That happens above. We just need to // make sure that if a field is invalid that it is listed in the invalid // section with the reason included. - $this->assertTrue(array_key_exists('organism_test_field2', $discovered_fields['invalid']), "The 'organism_test_field2' key should be in the 'invalid' array returned by the discover() function."); + $this->assertTrue(array_key_exists('organism_test_field4', $discovered_fields['invalid']), "The 'organism_test_field4' key should be in the 'invalid' array returned by the discover() function."); // Now add the valid field to the content type. and check to make sure // it is listed as `existing` afterwards when discover() is called again. diff --git a/tripal_chado/src/Plugin/Field/FieldType/ChadoOrganismTypeDefault.php b/tripal_chado/src/Plugin/Field/FieldType/ChadoOrganismTypeDefault.php index 1844f0a7e..ffc541413 100644 --- a/tripal_chado/src/Plugin/Field/FieldType/ChadoOrganismTypeDefault.php +++ b/tripal_chado/src/Plugin/Field/FieldType/ChadoOrganismTypeDefault.php @@ -312,7 +312,7 @@ public static function discover(TripalEntityType $bundle, string $field_id, arra // Create a field entry in the list. $field_list[] = [ - 'name' => self::generateFieldName($bundle, 'organism'), + 'name' => self::generateFieldName($bundle, 'organism', 0), 'content_type' => $bundle->getID(), 'label' => 'Organism', 'type' => self::$id, diff --git a/tripal_chado/src/Plugin/Field/FieldType/ChadoPropertyTypeDefault.php b/tripal_chado/src/Plugin/Field/FieldType/ChadoPropertyTypeDefault.php index 4e59f80ad..3c51967b2 100644 --- a/tripal_chado/src/Plugin/Field/FieldType/ChadoPropertyTypeDefault.php +++ b/tripal_chado/src/Plugin/Field/FieldType/ChadoPropertyTypeDefault.php @@ -272,7 +272,7 @@ public static function discover(TripalEntityType $bundle, string $field_id, arra // Create a field entry for each property type. foreach ($results as $recprop) { $field_list[] = [ - 'name' => self::generateFieldName($bundle, $recprop->cvterm_name), + 'name' => self::generateFieldName($bundle, $recprop->cvterm_name, $recprop->cvterm_id), 'content_type' => $bundle->getID(), 'label' => ucwords($recprop->cvterm_name), 'type' => self::$id,