PDOException on duplicate node type machine name

Why the hell would anyone do this crap anyway.

Typos and other user errors. It's easy enough to make a mistake, then you want to put it back how it was to fix the mistake. This bug prevents you from fixing it.

you've changed the machine name of a type created by a module and now the module is out-of-sorts 'cause it won't find its content type.

Naw, that's already dealt with. The "base" and "module" fields of {node_type} aren't editable. The module can keep track of its stuff that way.

Also, if the module really doesn't want anyone modifying the machine names for its types, it can set the "locked" field in {node_type} for its node types.

Also, if the module needs to know about node type changes, it can implement hook_node_type_update(). Ubercart does exactly this.

I think the machine name should not be allowed to be changed, especially if there is content.

The content issue is dealt with. Changing the machine name ("type" field) from admin/structure/types also updates the "type" field in the {node} table for all records that match the old type.

If no content is presently using the machine name of the change from type then the change from content type record needs to be removed; otherwise the change needs to check for an existing row and update it as active once again.

See my paragraph above. If you use admin/structure/types to change the type machine name, there will be no content with the old type. All the matching nodes get updated to the new type.

I don't see any reason to keep the old, disabled record around. It might be worth digging around in the code to find if there is a reason, but it might simply be a leftover from some earlier mechanism that never got cleaned up. The UI certainly has nothing that lets you revert to the saved record.

Ok, I'm digging in the (7.16) code a bit.

The error check for a duplicate machine name should go into node_type_form_validate(). Something like this would get rid of that annoying PDOException:

  if ($old_type != $type->type && array_key_exists($type->type, $types) {
    form_set_error('type', t('The machine-readable name %name is already taken.', array('%name' => $type->type)));
  }

I haven't found the bit that makes the disabled record...still digging....

After further digging, I couldn't find code that explicitly creates a disabled record. The records are created by node_type_save(), and that explicitly overwrites the existing record (and doesn't create a new one):

    db_update('node_type')
      ->fields($fields)
      ->condition('type', $existing_type)
      ->execute();

In other words, the disabled entry is not supposed to be there.

My current best guess is that this is related to _node_types_build() in combination with some sort of a sync, cache, or module problem. I'll keep checking. Also, see #1832640: Changing node content type machine name fails until manual cache reset. It may be related.

Ok, after updating just once, it's become slightly clearer.

Created a node type via a module (Ubercart). Machine name (type) is "book".

Go to admin/structure/types. Edit the node type. Change the machine name to "fred_book".

admin/structure/types still shows the type as "book", not "fred_book".

{node_type} contains the following record:
type: fred_book
base: uc_product
module: uc_product
custom: 0
locked: 0
disabled: 1
orig_type: book

{node_type} does NOT contain a record with "type: book". There's just the one record (disabled) record.

The only code I found that could set the disabled field was in _node_types_build() by way of node_types_rebuild(). It appears to mark node type records as disabled if they are defined by a module but no module claims them. So, a possible sequence of events is:

1. node_type_save() correctly overwrites record.
2. node_type_save() correctly invokes hook_node_type_update(). The module now knows about the new type and not the old type.
3. node_type_save() calls node_type_cache_reset(), but this somehow doesn't update the cache.
4. node_type_form_submit() calls node_types_rebuild() calls _node_types_build()
5. _node_types_build() gets its data from the (incorrect) cache, so it thinks the updated record in the DB is abandoned, and disables it.

Could that happen? If so, this is probably caused by #1832640: Changing node content type machine name fails until manual cache reset. I won't mark this as a duplicate, because the error check still needs to be added to get rid of the PDOException.

After more testing, I've concluded that #5 is essentially correct, except it's not exactly a caching issue. It appears to be related to a bug in a module providing out-of-date info via hook_node_info(). So, only the error checking of #3 is left. I'll update the title.

Presumably, this needs to be patched to 8.x-dev, then backported to 7.x-dev, right?