Problem/Motivation

Part of #2120003: [META] Create sensible limits for the maximum length of configuration object filename components

The config entity machine_name (also called id) property may form a part of a config entity's file name -- the yml file that represents an instance of the entity. The file name length is limited to 255 characters by most operating systems. To be safe, we limit the name to 250 characters and leave 5 for a file extension. The file name is potentially composed of 5 pieces. Each part may therefore only consume 20% of the available characters, so the config entity machine_name can therefore only be a maximum of 50 characters long.

Here is an example of an config entity machine_name in Core using the entity.view_mode.node.teaser.yml config entity.

id: node.teaser
label: Teaser
status: true
cache: true
targetEntityType: node

Proposed resolution

We need to look at imposing limits at install, import and the through the UI. Machine names for config entities are often created through forms.

Config entities with a machine_name greater than 50 characters must:

  1. Fail installation
  2. Prevent the owning module from installing as well.

This will keep contrib authors from creating config entity types with config_prefix greater than 50 characters and discourage adding them to D.O.

When creating a config entity, such as a field or a content type, through a form with a machine_name candidate greater than 50 characters, the form must:

  1. Fail validation
  2. Inform the user that the machine_name must be short than 50 characters.

Some config entities already impose machine_name length limits. Check for existing constants while working on this issue.

User interface changes

machine_name fields will fail validation if they contain a string greater than 50 characters.

API changes

Perhaps additional methods.

Original report by @jessebeach

Files: 
CommentFileSizeAuthor
#45 taxonomy.png80.8 KBjessebeach
#42 config-2220757-42-interdiff.txt3.42 KBzserno
#42 config-2220757-42.patch5.58 KBzserno
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,920 pass(es). View
#37 interdiff.txt2.14 KBxjm
#37 config-2220757-37.patch5.24 KBxjm
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,921 pass(es). View
#32 interdiff.txt6.03 KBxjm
#32 config-2220757-32.patch5.22 KBxjm
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,883 pass(es). View
#26 2220757-26.patch4.46 KBKarol Haltenberger
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View
#23 2220757-23.patch3.76 KBKarol Haltenberger
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,800 pass(es). View
#21 2220757-21.patch1.64 KBKarol Haltenberger
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,737 pass(es). View
#18 2220757-18.patch2.11 KBKarol Haltenberger
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 60,630 pass(es), 150 fail(s), and 176 exception(s). View
#16 2220757-16.patch3.98 KBKarol Haltenberger
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,723 pass(es). View
#15 2220757-15.patch3.4 KBdrifter
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,736 pass(es). View
#14 2220757-14.patch1.73 KBdrifter
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,835 pass(es). View

Comments

Berdir’s picture

The problem here and in other related issues (like comment bundle length) are composite ID's where the ID is not the same as the machine name entered in the UI.

The ID of a field instance is created based on 3 parts: $this->entity_type . '.' . $this->bundle . '.' . $this->field->name

If you want to limit the ID of config entities, and have an entity type and a bundle on that with each 25 characters, it's impossible to create fields for that.

Bundles are usually fairly short, but comment is an interesting example where the bundle consists of the field id of it's comment field.

So for example he comment_body field instance on nodes has an ID like this: "comment.node__comment.comment_body". Those are all very short, so it's only 34 characters, but let's assume that we're going to add a comment field named comment to a commerce_order_line_item entity. That's already 54 and too long. We could shorten the field name to just body, because we no longer need to separate it from the node body field by name, which would work for this, but with the default field prefix, you get "comment.commerce_order_line_item__comment.field_", which is 48 characters, which leaves users exactly 2 characters to name their fields on comments on commerce order line items ;)

alexpott’s picture

Re #1 I don't think we should apply a general rule - also I took this issue to be proposing that in the case of fields and their instances it'd be the $this->field->name part that would be limited. As this is already limited to 32 nothing needs to be done for fields / field instances.

xjm’s picture

Isn't this a duplicate of #1191434: Standardize entity type is a maximum of 32 characters (not 64 or 128)? The point of declaring it at the entity level is that it then propagates down to all child classes, including config entities and fields.

Berdir’s picture

It's related but not the same.

*Some* config entities have ID's that relate to entity types or bundles, but most don't. This is about a default verification/validation for everyhting.. views, actions, blocks... as I understand it?

#2: Hm, maybe, but that can't be enforced globally/by default then as we don't know which property we need to check (instead of just id()) and it would theoretically mean that a field instance ID could be 132 characters (or at least 3 * 32, if we limit bundle and entity_type to 32 as proposed by @xjm there).

jhodgdon’s picture

But... Looking at #2120003: [META] Create sensible limits for the maximum length of configuration object filename components, the point is to limit **each part (between the "."s) of the configuration file name** to 50 characters, not the whole thing.

So in the case of a field instance config or field config, you have:
(field config): field .field .node .field_my_entered_machine_name
(instance config): field .instance .node .article .field_my_entered_machine_name

Each part of this is limited to 50 characters:
- The module name "field", OK.
- The config ID, "field" or "instance", OK.
- The entity type, "node", OK.
- The entity bundle, "article", OK.
- The field's machine name, which we need to limit to 50 characters (including the "field_" which the Field UI module puts in there).

Comment #1 here seems to think that the whole thing node.body or node.article.body is limited to 50 characters, but that is not the proposal. The proposal is (unless I am mistaken) that the machine name itself -- the part that the user enters + the standard field_ prefix from Field UI, needs to be 50 characters or less. This doesn't sound like a huge problem?

Berdir’s picture

Ok, yes, that seems fine, I misunderstood that a bit.

We have a different understanding of name component I guess. For me, the config entity ID is a single component, because it can not be separated further. That it uses "." is basically an implementation detail, it could also be a different character. And nothing is stopping me from creating an entity type that has 4 parts that make up the ID?

So for the way I understand it, we should say that a config entity ID must not be longer than 149 characters (shouldn't we include the "." in our calculations too, 50 always includes the "." as well?) , and is free to split that up.

Which means that for a field_instance, we know that with with the decision in #1709960: declare a maximum length for entity and bundle machine names to limit entity type and bundle to 32, entity_type.bundle. can have a maximal length of 32 + 1 + 32 + 1 => 66, Which means that field names could be up to 84 characters long and we'd still be good. And since they are already limited to 32 characters, we're all set and someone could even come up with a config entity that is entity_type.bundle.field_name.whatever ;)

And a view could have a machine name that is up to 149 characters long.

jessebeach’s picture

Title: Limit the machine name of config entities to 50 characters » Limit the machine name of config entities to 128 characters
Status: Active » Closed (duplicate)
jessebeach’s picture

Status: Closed (duplicate) » Active

Derp, sorry, wrong issue. #7 was incorrect.

jessebeach’s picture

Keep in mind that if entity and bundle machine names have an upper limit of 50 chars each, that leaves 28 chars for the instance name in the following classes:

  1. EntityViewDisplay
  2. EntityFormDisplay
  3. FieldInstanceConfig

The instance name being something like "default" or "teaser" or "search_result".

Field names are also then limited to 28 characters as well.

#1709960: declare a maximum length for entity and bundle machine names

xjm’s picture

Re. #4, right, sorry, I was confusing two issues. So this issue is about specifying a maximum length for Config::$id, e.g. in:
views.view.frontpage.yml
field.instance.node.article.default.yml
entity.form_display.node.article.default.yml
The following parts would be the part capped for this issue in each case:
frontpage
node.article.default
node.article.default

The goal of #1862600: Entity type names/IDs are not properly namespaced by owner (e.g., taxonomy.term vs. taxonomy_term) is to change it so that the entity type ID becomes a compound of module_name.what_we_currently_call_config_prefix_in_the_entity_but_not_the_method_name, e.g. views.view or node.article. If module_name is capped at 50 (as it is in HEAD), then we would need the combination of what_we_currently_call_config_prefix_in_the_entity_but_not_the_method_name and the ID to add up to 200 characters max. If we make what_we_currently_call_config_prefix_in_the_entity_but_not_the_method_name 32 characters as proposed in #2120003: [META] Create sensible limits for the maximum length of configuration object filename components, then that would leave 168 166 characters for the rest. (Edit: taking into account the dots.)

xjm’s picture

Title: Limit the machine name of config entities to 128 characters » Limit length of Config::$id to something >= 166 characters
alexpott’s picture

Title: Limit length of Config::$id to something >= 166 characters » Limit length of Config::$id to something <= 166 characters
jessebeach’s picture

then we would need the combination of what_we_currently_call_config_prefix_in_the_entity_but_not_the_method_name and the ID to add up to 200 characters max

200 seems like a typo here. Is that what you meant xjm?

drifter’s picture

Status: Active » Needs review
FileSize
1.73 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,835 pass(es). View

So if I understand correctly, a config name can be up to 250 characters, of which 50 characters is the module name, 32 characters is the prefix, and the rest (up to 166 characters) is the ID (sometimes called machine name).

I'm new to this so this could be the wrong place to put this, but here's a check that raises an exception before saving the config entity if the limit is exceeded.

drifter’s picture

FileSize
3.4 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,736 pass(es). View

Also trying to add a test for this...

Karol Haltenberger’s picture

FileSize
3.98 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,723 pass(es). View

Testing for Config entity "prefix" length (83) and "id" length (166) in ConfigStorageController::save()

This patch includes the changes from 2220757-15.patch

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
    @@ -80,6 +80,21 @@
       /**
    +   * The maximum length of a configuration object ID.
    +   *
    +   * Many filesystems (including HFS, NTFS, and ext4) have a maximum file name
    +   * length of 255 characters. To ensure that no configuration objects
    +   * incompatible with this limitation are created, we enforce a maximum name
    +   * length of 250 characters (leaving 5 characters for the file extension).
    +   *
    +   * @see http://en.wikipedia.org/wiki/Comparison_of_file_systems
    +   *
    +   * 50 characters of the 250 is reserved for the module name,
    +   * 32 characters for the config prefix, that leaves 166 characters for the ID length.
    +   */
    +  const MAX_ID_LENGTH = 166;
    

    Not sure if we want to copy the documentation from ConfigBase. Maybe just add a @see to it?

    The description should also not be wider than 80 characters.

  2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
    @@ -259,6 +274,13 @@ public function toArray() {
    +    if (strlen($this->id()) > self::MAX_ID_LENGTH) {
    +      throw new \Drupal\Core\Config\ConfigNameException(String::format('Config entity machine name (id) @name exceeds maximum allowed length of @length characters.', array(
    +        '@name' => $this->id(),
    +        '@length' => self::MAX_ID_LENGTH,
    +      )));
    +    }
    

    Just discussed this with @alexpott and we wondering who and where this check should live.

    We thought that it makes more sense that ConfigStorageController::save() does this because this is a storage limitation. For example, @timplunkett right now is working on a key value storage which has only allows 128 character long ID's, so he needs a different check. And someone could experiment with a different storage controller in contrib that allows longer ID's.

  3. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php
    @@ -288,6 +291,15 @@ public function delete(array $entities) {
    +    //check config entity prefix length, should not be > 83 (MAX_PREFIX_LENGTH)
    +    //NOTE: $this->getConfigPrefix() returns the content entity prefix plus a '.'
    +    if (strlen($prefix) > self::MAX_PREFIX_LENGTH+1) {
    +      throw new ConfigNameException(String::format('Config entity prefix ({extension}.{identifier}) @name exceeds maximum allowed length of @length characters.', array(
    +       '@name' => rtrim($prefix, '.'),
    +       '@length' => self::MAX_PREFIX_LENGTH,
    +     )));
    +    }
    

    We have a separate issue for this, this shouldn't be checked here.

  4. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php
    @@ -315,6 +327,14 @@ public function save(EntityInterface $entity) {
     
    +    //check config entity id length, should not be > 166 (ConfigEntityBase::MAX_ID_LENGTH)
    +   if (strlen($entity->{$this->idKey}) > ConfigEntityBase::MAX_ID_LENGTH) {
    +     throw new ConfigNameException(String::format('Config entity machine name (id) @name exceeds maximum allowed length of @length characters.', array(
    +       '@name' => $entity->{$this->idKey},
    +       '@length' => ConfigEntityBase::MAX_ID_LENGTH,
    +     )));
    +   }
    

    Actually we have the check here as well? Why both? :)

    Comment nitpick: Should have a space after //, start with an uppercase character, not be longer than 180 characters and end with a ".".

Karol Haltenberger’s picture

Status: Needs work » Needs review
FileSize
2.11 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 60,630 pass(es), 150 fail(s), and 176 exception(s). View

We thought that it makes more sense that ConfigStorageController::save() does this because this is a storage limitation.

Actually we have the check here as well? Why both? :)

config entity id length checking in ConfigStorageController::save() only

For example, @timplunkett right now is working on a key value storage which has only allows 128 character long ID's, so he needs a different check. And someone could experiment with a different storage controller in contrib that allows longer ID's.

checking is based on an optional constant MAX_ID_LENGTH introduced to the storage service (see Drupal\Core\Config\FileStorage)

Comments / descriptions updated

Berdir’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Sorry, maybe I wasn't clear. The constant has to live in the config entity storage controller, *not* in the config storage. I was talking about a different entity storage controller, not a different config storage. So the dynamic stuff there isn't necessary, just use your own constant.

This also needs a test, you should probably extend ConfigEntityTest::testCRUD().

The last submitted patch, 18: 2220757-18.patch, failed testing.

Karol Haltenberger’s picture

Status: Needs work » Needs review
FileSize
1.64 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,737 pass(es). View

Constant moved back and value corrected

As for the tests, I'll try to write some as soon as I learn how :)

Berdir’s picture

I'm in the Margrathea room in Szeged if you're here, happy to show you how to get started with tests.

Karol Haltenberger’s picture

FileSize
3.76 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,800 pass(es). View

First attempt at tests.

jessebeach’s picture

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php
@@ -37,6 +38,15 @@
+  /**
+   * The maximum length of a config entity ID.
+   *
+   * @see \Drupal\Core\ConfigBase::MAX_NAME_LENGTH
+   */
+
+  const MAX_ID_LENGTH = 166;

Please remove this extra line in your next patch.

jessebeach’s picture

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php
    @@ -37,6 +38,15 @@
    +
    
    +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    // Test with a short id
    +    $short_id = entity_create('config_test', array(
    +      'id' => $this->randomName(),
    +    ));
    

    Mention this defaults to 8 characters in a comment.

  2. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    try {
    +      $short_id->save();
    +      $this->pass("ConfigNameException was not thrown for short id.");
    +    }
    

    The message should just be: "config_test entity with ID length @len was saved."

  3. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    catch (ConfigNameException $e) {
    +      $this->fail("ConfigNameException was thrown despite short id.");
    +    }
    

    Maybe just pass through the Exception message?

  4. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    try {
    +      $just_id->save();
    +      $this->pass("ConfigNameException was not thrown for just about right length id.");
    +    }
    

    Use the message: "config_test entity with ID length @len was saved."

  5. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    catch (ConfigNameException $e) {
    +      $this->fail("ConfigNameException was thrown despite id being <= 166.");
    +    }
    

    Just pass through the exception message.

  6. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    try {
    +      $status = $long_id->save();
    +      $this->fail("ConfigNameException was not thrown despite too long id.");
    +    }
    

    "config_test entity saved successfully despite having an ID that exceeds the maximum length of @len"

  7. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +143,46 @@ function testCRUD() {
    +    catch (ConfigNameException $e) {
    +      $this->pass("ConfigNameException was thrown for too long id. Woohoo!");
    +    }
    

    Although I love enthusiasm, too, let's keep the messages fairly neutral :)

    "config_test entity with an id length of @len failed to save"

Replace instances of @len in my comments with the length of the ID that was generated.

Looking good so far!

Karol Haltenberger’s picture

FileSize
4.46 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es). View

- Updated the patch with the recent class name changes.
- Changed the comments to the proposed ones, with some modifications.
- Using the actual constant from the class rather than fixed lengths.

xjm’s picture

Thanks @Karol Haltenberger! Can you be sure to post interdiffs when you create new patches? See: https://drupal.org/documentation/git/interdiff

Berdir’s picture

Status: Needs review » Needs work

We just discussed that this should use the exception class from #2220739: Custom block types can be 64 characters long but the custom_block only supports 32. (ConfigEntityIdLengthException)

Note that #1709960: declare a maximum length for entity and bundle machine names will add and use the same exception class, but we can continue by adding it to both issues, then we can simply re-roll the other one once one is in. No need to postpone issues.

jessebeach’s picture

Assigned: Unassigned » xjm
xjm’s picture

I'll reroll to add it, plus clean up the following nitpicky things. :)

  1. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +    // Verify that a configuration entity can be saved with
    +    // an ID of maximum allowed length, but not longer.
    

    Wrapping a bit early.

  2. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +    // Test with a short ID
    +    // By default randomName() generates a 8 character long name
    

    Needs periods at the end of sentences. Also could be wrapped into one comment.

  3. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +    $config_test_idlen = entity_create('config_test', array(
    

    Usually we don't use abbreviations like "idlen". Also, this variable does not store an ID length, it stores a configuration entity object, so we could call it something like $test_config_entity.

  4. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +    // Test with an ID of maximum allowed length
    ...
    +    // Test with an ID exeeding the maximum allowed length
    

    Also need periods. :)

xjm’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
77.7 KB
47.3 KB

Attached uses the new exception class and cleans up the things in #30. I also tested it manually!

Installing with default config with a too-long ID

Trying to synch config with a too-long ID

xjm’s picture

FileSize
5.22 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,883 pass(es). View
6.03 KB

Let's actually attach the patch, eh?

xjm’s picture

Assigned: xjm » Unassigned
xjm’s picture

Assigned: Unassigned » jessebeach

@berdir said he was +1 with the current version of the patch, and suggested we let @jessebeach take a final look at it as well.

jessebeach’s picture

I'll have a look tonight/tomorrow morning after the closing dinner event. I'd like to do some manual testing as well as code review.

xjm’s picture

I found another couple non-blocking coding standards cleanups.

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityIdLengthException.php
    @@ -0,0 +1,15 @@
    + * Exception thrown when a configuration entity ID is too long.
    

    Should start with a verb.

  2. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +      'id' => $this->randomName(ConfigEntityStorage::MAX_ID_LENGTH+1),
    

    There should be spaces around the +.

  3. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +      $this->fail(String::format("config_test entity with ID length @len, exceeding the maximum allowed length of @max saved successfully", array(
    ...
    +      $this->pass(String::format("config_test entity with ID length @len, exceeding the maximum allowed length of @max failed to save", array(
    

    Grammatically, there should either be a comma after @max, or no comma after @len.

Patched patch inc.

xjm’s picture

FileSize
5.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,921 pass(es). View
2.14 KB
Berdir’s picture

  1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php
    @@ -38,6 +39,13 @@
       /**
    +   * The maximum length of a configuration entity ID.
    +   *
    +   * @see \Drupal\Core\ConfigBase::MAX_NAME_LENGTH
    +   */
    +  const MAX_ID_LENGTH = 166;
    

    In the config prefix issue (ConfigEntityType::PREFIX_LENGTH), we added an explanation why the prefix is limited to 83, we should probably do the inverse here.

  2. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php
    @@ -142,6 +145,56 @@ function testCRUD() {
    +    // Test with a short ID. By default randomName() generates an 8 character
    +    // name.
    +    $id_length_config_test = entity_create('config_test', array(
    +      'id' => $this->randomName(),
    

    Would be easier to just make 8 or something else explicit instead of a long explanation....

zserno’s picture

According to https://drupal.org/node/1709960#comment-8631877, please put ConfigEntityIdLengthException in Drupal\Core\Config\Entity\Exception. See latest patch there as an example.

jessebeach’s picture

Rerolling and addressing feedback now.

jessebeach’s picture

Assigned: jessebeach » Unassigned

zserno has been working on this. I'm unassigning myself.

zserno’s picture

FileSize
5.58 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,920 pass(es). View
3.42 KB

Thanks jessebeach. Rerolled according to feedback at #38 and #39.

xjm’s picture

Component: base system » configuration system

+1, all that looks good to me.

jessebeach’s picture

So, setting FieldConfig::NAME_MAX_LENGTH to some high value, like 10000, let's me create a field name with more than the intended limit of 32 chars. Saving the field yields a database error:

There was a problem creating field aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz: SQLSTATE[42000]: Syntax error or access violation: 1059 Identifier name 'field_aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz_value' is too long: CREATE TABLE {node__442550c0f0} ( `bundle` VARCHAR(128) NOT NULL DEFAULT '' COMMENT 'The field instance bundle to which this row belongs, used when deleting a field instance', `deleted` TINYINT NOT NULL DEFAULT 0 COMMENT 'A boolean indicating whether this data item has been deleted', `entity_id` INT unsigned NOT NULL COMMENT 'The entity id this data is attached to', `revision_id` INT unsigned NULL DEFAULT NULL COMMENT 'The entity revision id this data is attached to, or NULL if the entity type is not versioned', `langcode` VARCHAR(32) NOT NULL DEFAULT '' COMMENT 'The language code for this data item.', `delta` INT unsigned NOT NULL COMMENT 'The sequence number for this data item, used for multi-value fields', `field_aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz_value` VARCHAR(255) NULL DEFAULT NULL, `field_aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz_format` VARCHAR(255) NULL DEFAULT NULL, PRIMARY KEY (`entity_id`, `deleted`, `delta`, `langcode`), INDEX `bundle` (`bundle`), INDEX `deleted` (`deleted`), INDEX `entity_id` (`entity_id`), INDEX `revision_id` (`revision_id`), INDEX `langcode` (`langcode`), INDEX `field_aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz_format` (`field_aaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuuvvvwwwxxxyyyzzz_format`) ) ENGINE = InnoDB DEFAULT CHARACTER SET utf8 COMMENT 'Data storage for node field field_aaabbbcccdddeeefffggghhhii'; Array ( )

Now, changing the FieldConfig NAME_MAX_LENGTH constant is a contrived scenario. I don't think we need to code to this case in this issue. I just wanted to mention that because FieldConfig does its table creation in pre-save, we never get to the bundle checks we introduce in this issue.

jessebeach’s picture

FileSize
80.8 KB

A taxonomy vocabulary with a 170 character name throws our \Core\Config\Entity\Exception\ConfigEntityIdLengthException.

Screenshot of a ConfigEntityIdLengthException

Yay!

jessebeach’s picture

Status: Needs review » Reviewed & tested by the community

Alright, the clicky-clicky testing has convinced me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php
@@ -38,6 +39,19 @@
   /**
+   * Length limit of the configuration entity ID.
+   *
+   * Most file systems limit a file name's length to 255 characters. In
+   * order to leave sufficient characters to construct a configuration prefix,
+   * the configuration entity ID is limited to 166 characters which
+   * leaves 83 characters for the configuration prefix. 5 characters are
+   * reserved for the file extension.
+   *
+   * @see \Drupal\Core\Config\ConfigBase::MAX_NAME_LENGTH
+   */
+  const MAX_ID_LENGTH = 166;

That is the weirdest, most arbitrary limit for something I've ever seen, and leaves another weird, arbitrary limit (83) to work with. And unfortunately, the docs that are here do not in any way help me to understand how we arrived at that limit, and why we shouldn't change that some day in the future to something less weird and arbitrary.

Asking around in IRC, xjm said that this was arrived at because 50 was reserved for the module name, 32 for the entity ID, plus a dot. I think we could really do to explain this to people because I can't be the only one who sees this and goes "Huh?!"

xjm spun off a sub-issue for this, as well as to unify the docs of these constants more generally: #2229931: Improve documentation related to config object filename component limits

The only other very minor thing here is we don't generally abbreviate things so all the places that are @len should be @length. Fixed on commit.

Committed and pushed to 8.x. Thanks!

  • Commit c0d5ab2 on 8.x by webchick:
    Issue #2220757 by Karol Haltenberger, xjm, drifter, zserno: Limit length...

Status: Fixed » Closed (fixed)

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