From e31c42eccc0a3c4ac6613364b35824a6753a3cb0 Mon Sep 17 00:00:00 2001 From: Tim Popham Date: Sat, 3 Sep 2011 20:50:30 -0700 Subject: [PATCH 1/2] Expecting a data structure including disabled fields, and since the spec calls for active fields alone, we gotta filter out the disabled ones. --- modules/node/node.module | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/modules/node/node.module b/modules/node/node.module index 6abfcb2..bbe28d9 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -382,7 +382,8 @@ function _node_extract_type($node) { * @see node_type_get_type() */ function node_type_get_types() { - return _node_types_build()->types; + return array_filter(_node_types_build()->types, + create_function('$nt','return !$nt->disabled'); } /** -- 1.7.6.1 From f110e6b981141696918e8c271ad2ffe3d8d9eef1 Mon Sep 17 00:00:00 2001 From: Tim Popham Date: Sun, 4 Sep 2011 21:48:02 -0700 Subject: [PATCH 2/2] Reimplemented _node_types_build(). I would like to refine the specs of a few others, as the module seems very vague, but I will wait to see what sort of tests fail to get a better feel for what precisely to spec. --- modules/node/node.module | 179 +++++++++++++++++++++++++++++++--------------- 1 files changed, 120 insertions(+), 59 deletions(-) diff --git a/modules/node/node.module b/modules/node/node.module index bbe28d9..c7314f8 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -383,7 +383,7 @@ function _node_extract_type($node) { */ function node_type_get_types() { return array_filter(_node_types_build()->types, - create_function('$nt','return !$nt->disabled'); + create_function('$nt','return !$nt->disabled;')); } /** @@ -646,27 +646,45 @@ function node_type_update_nodes($old_type, $type) { } /** - * Builds and returns the list of available node types. - * - * The list of types is built by invoking hook_node_info() on all modules and - * comparing this information with the node types in the {node_type} table. - * These two information sources are not synchronized during module installation - * until node_types_rebuild() is called. + * Get the list of all node types that have not been deleted. * + * This function provides the `node_type' table state if $rebuild==TRUE. + * Otherwise, the returned data structure will contain node types from freshly + * enabled modules that implement hook_node_info(). The returned node type's + * `disabled' property indicates if the type's hook_node_info() hook is + * available (that is, if the implementing module is disabled or no longer + * implements hook_node_info() for the node type). Node types built without + * hook_node_info(), with node_type_save() alone that is, have their `disabled' + * property fixed to 0. In short, this function returns what the `node_type' + * table state should be, and if $rebuild==TRUE, then the node_types table is as + * it should be. * @param $rebuild * TRUE to rebuild node types. Equivalent to calling node_types_rebuild(). * @return - * Associative array with two components: - * - names: Associative array of the names of node types, keyed by the type. - * - types: Associative array of node type objects, keyed by the type. - * Both of these arrays will include new types that have been defined by - * hook_node_info() implementations but not yet saved in the {node_type} - * table. These are indicated in the type object by $type->is_new being set - * to the value 1. These arrays will also include obsolete types: types that - * were previously defined by modules that have now been disabled, or for - * whatever reason are no longer being defined in hook_node_info() - * implementations, but are still in the database. These are indicated in the - * type object by $type->disabled being set to TRUE. + * An object with two properties: + * - names: Associative array of the names corresponding to active or disabled + * node types, keyed by node type. This property does not reflect the state + * of the `node_type' table unless $rebuild==TRUE. The property is a thin + * version of the `types' property. See below for exactly what membership in + * this array means. + * - types: Associative array of node type objects, keyed by node type. + * Like the `names' array, this data structure does not necessarily reflect + * the state of the `node_type' table. The node type objects contain fresher + * data. + * - disabled: The $type->disabled flag takes a value of 1 for any type in + * the `node_type' table implemented by hook_node_info() whose module + * - no longer implements hook_node_info() + * - has been disabled. + * The $type->disabled flag takes a value of 0 otherwise. + * - is_new: The $type->is_new flag does not appear in the `node_type' + * table. The property takes a value of 1 for active node types + * implemented by hook_node_info() that do not yet appear in the + * `node_type' table, and 0 otherwise. If $rebuild==TRUE, then + * $type->is_new==0 for all node types. + * - new_state: The $type->new_state flag does not appear in the `node_type' + * table. The property takes a value of 1 for types within the data + * structure that differ from their corresponding entries in the + * `node_type' table */ function _node_types_build($rebuild = FALSE) { $cid = 'node_types:' . $GLOBALS['language']->language; @@ -682,65 +700,108 @@ function _node_types_build($rebuild = FALSE) { } } - $_node_types = (object) array('types' => array(), 'names' => array()); - + // Gather node types from the hook_node_info() implementations of active + // modules. + $active_types = (object) array('types' => array(), 'names' => array()); foreach (module_implements('node_info') as $module) { - $info_array = module_invoke($module, 'node_info'); - foreach ($info_array as $type => $info) { - $info['type'] = $type; - $_node_types->types[$type] = node_type_set_defaults($info); - $_node_types->types[$type]->module = $module; - $_node_types->names[$type] = $info['name']; - } - } - $query = db_select('node_type', 'nt') + foreach (module_invoke($module, 'node_info') as $type => $node_info) { + $node_info['type'] = $type; + $active_types->types[$type] = node_type_set_defaults($node_info); + $active_types->types[$type]->module = $module; + // active_type not in node_type table => active_type is new, so clobber + // this default value if the type is found in the database. + $active_types->types[$type]->is_new = 1; + // active_type is new => active_type is not disabled, so be consistent + // with other defaults for now + $active_types->types[$type]->disabled = 0; + // attach flag to indicate whether the type needs to be (re)written to the + // database + $active_types->types[$type]->new_state = 1; + + // Copy subset of the types property's data to the name property + $active_types->names[$type] = $active_types->types[$type]->name; + } + } + + // Gather node types from the node_type table (and I have no clue what the + // translatable tag does) + $db_types = db_select('node_type', 'nt') ->addTag('translatable') ->addTag('node_type_access') ->fields('nt') - ->orderBy('nt.type', 'ASC'); - if (!$rebuild) { - $query->condition('disabled', 0); - } - foreach ($query->execute() as $type_object) { - $type_db = $type_object->type; - // Original disabled value. - $disabled = $type_object->disabled; - // Check for node types from disabled modules and mark their types for removal. - // Types defined by the node module in the database (rather than by a separate - // module using hook_node_info) have a base value of 'node_content'. The isset() - // check prevents errors on old (pre-Drupal 7) databases. - if (isset($type_object->base) && $type_object->base != 'node_content' && empty($_node_types->types[$type_db])) { - $type_object->disabled = TRUE; - } - if (isset($_node_types->types[$type_db])) { - $type_object->disabled = FALSE; - } - if (!isset($_node_types->types[$type_db]) || $type_object->modified) { - $_node_types->types[$type_db] = $type_object; - $_node_types->names[$type_db] = $type_object->name; - - if ($type_db != $type_object->orig_type) { - unset($_node_types->types[$type_object->orig_type]); - unset($_node_types->names[$type_object->orig_type]); + ->orderBy('nt.type', 'ASC') + ->execute(); + + // Reconcile node_type table against the node types obtained from the + // hook_node_info() hooks of enabled modules. The node type's behavior varies + // with its method of construction: + // - Constructed by node_type_save(): The node type's disabled flag is + // decoupled from the enabled state of its module. + // - Constructed by hook_node_info(): The node type's disabled flag follows + // from whether its defining module is disabled. + $enabled_modules = module_list(); + $state_change = array(); + foreach ($db_types as $db_type) { + if ($db_type->custom) { + // Attach custom nodes verbatim + $active_types->types[$db_type->type] = $db_type; + $active_types->types[$db_type->type]->is_new = 0; + $active_types->types[$db_type->type]->new_state = 0; + } elseif (isset($active_types->types[$db_type->type])) { + $active_types->types[$db_type->type]->is_new = 0; + $active_types->types[$db_type->type]->new_state = 0; + } else { + // hook_node_info() is unavailable, so disable the node type if it was + // implemented by some hook_node_info() + $active_types->types[$db_type->type] = $db_type; + $type = &$active_types->types[$db_type->type]; + if (isset($enabled_modules[$db_type->module])) { + // Save some computation (node_type_save() was necessarily used to build + // the node type in this case) + $type->is_new = 0; + $type->disabled = 0; + $type->new_state = 0; + } else { + if (drupal_load('module', $db_type->module)) { + // If the drupal_load() fools Drupal into thinking that the + // corresponding module is enabled, then a hook_node_info() + // implementing several node types could trip up the following logic. + // Using module_list() outside the loop comes with performance + // benefits and avoids the potential bug. + if (function_exists($db_type->module . '_node_info')) { + $type->is_new = 0; + drupal_set_message('disabled case'); + $type->disabled = 1; + } else { + $type->is_new = 0; + $type->disabled = 0; + } + } else { + // Missing resource or misrepresented module property + // Drupal core policy watchdog, etc.? + } } + $type->new_state = ($type->disabled != $db_type->disabled); } - $_node_types->types[$type_db]->disabled = $type_object->disabled; - $_node_types->types[$type_db]->disabled_changed = $disabled != $type_object->disabled; + // Copy data to names array + $active_types->names[$db_type->type] = + $active_types->types[$db_type->type]->name; } + // Save new node types if a rebuild was requested if ($rebuild) { - foreach ($_node_types->types as $type => $type_object) { - if (!empty($type_object->is_new) || !empty($type_object->disabled_changed)) { - node_type_save($type_object); + foreach ($active_types->types as $type) { + if ($type->is_new || $type->new_state) { + node_type_save($type); } } } - asort($_node_types->names); + asort($active_types->names); - cache_set($cid, $_node_types); + cache_set($cid, $active_types); - return $_node_types; + return $active_types; } /** -- 1.7.6.1