From e31c42eccc0a3c4ac6613364b35824a6753a3cb0 Mon Sep 17 00:00:00 2001
From: Tim Popham <popham@uw.edu>
Date: Sat, 3 Sep 2011 20:50:30 -0700
Subject: [PATCH 1/6] 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 <popham@uw.edu>
Date: Sun, 4 Sep 2011 21:48:02 -0700
Subject: [PATCH 2/6] 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


From 2d9518ed2d8d74bb89f8b484f05f5917fac00196 Mon Sep 17 00:00:00 2001
From: Tim Popham <popham@uw.edu>
Date: Mon, 5 Sep 2011 12:08:11 -0700
Subject: [PATCH 3/6] Disabling any module disables its types if
 base!=content.

---
 modules/node/node.module |  117 ++++++++++++++++++++--------------------------
 1 files changed, 51 insertions(+), 66 deletions(-)

diff --git a/modules/node/node.module b/modules/node/node.module
index c7314f8..7e6ea3c 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -648,43 +648,39 @@ function node_type_update_nodes($old_type, $type) {
 /**
  * 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.
+ * This function provides the `node_type' table state as it would appear after a
+ * rebuild, actually rebuilding the table as well for $rebuild==TRUE. The
+ * returned node type's `disabled' property indicates if the node type's
+ * implementing module has been disabled, however, any node type with a `base'
+ * value of `node_content' always remains non-disabled (making Node Content a
+ * poor name choice for a module).
+ *
  * @param $rebuild
  *  TRUE to rebuild node types. Equivalent to calling node_types_rebuild().
  * @return
  *   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.
+ *     node types, keyed by node type. This property does not necessarily 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.
+ *     - disabled: If `base==node_content', then this property does not vary from
+ *       0. Otherwise, the $type->disabled flag takes a value of 0 for any type
+ *       in the `node_type' table whose module is enabled, or it takes a value of
+ *       1 if its module is disabled (or possibly uninstalled).
+ *     - is_new: The $type->is_new flag does not appear in the `node_type' table
+ *       The property takes a value of 1 for node types implemented by
+ *       hook_node_info() that did not appear in the `node_type' table at
+ *       _node_types_build() entry. The property takes a value of 0 otherwise.
  *     - 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
+ *       `node_type' table. This includes node types whose disabled state has
+ *       changed and new node types.
  */
 function _node_types_build($rebuild = FALSE) {
   $cid = 'node_types:' . $GLOBALS['language']->language;
@@ -732,63 +728,52 @@ function _node_types_build($rebuild = FALSE) {
     ->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.
+  // Reconcile node_type table against the enabled modules list. Any node type
+  // with base!=node_content inherits its disabled state from that of its 
+  // module.
   $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])) {
+    if (isset($active_types->types[$db_type->type])) {
+      // These nodes are implemented by an available hook_node_info() and were
+      // already in the database. If they happen to have base==node_content,
+      // they're still consistent.
       $active_types->types[$db_type->type]->is_new = 0;
+      $active_types->types[$db_type->type]->disabled = 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()
+      // A hook_node_info() was unavailable for the current iterant from the
+      // database, so enable or disable the iterant depending on its module's
+      // state
       $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->is_new = 0;
+      if ($type->base=='node_content') {
+        // Special case: always enabled
         $type->disabled = 0;
         $type->new_state = 0;
+      } elseif (isset($enabled_modules[$db_type->module])) {
+        // The module is enabled
+        $type->new_state = ($type->disabled != 0);
+        $type->disabled = 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.?
-        }
+        // The module is disabled
+        $type->new_state = ($type->disabled != 1);
+        $type->disabled = 1;
       }
-      $type->new_state = ($type->disabled != $db_type->disabled);
     }
-    // Copy data to names array
+    
+    // Copy data to names array.
+    // This was inside the hook_node_info()-unavailable-case above, since the
+    // name data from the hook_node_info() call should be fine to return, but
+    // changes in hook_node_info() may not have propagated into the database yet
+    // (and a rebuild won't fix a changed name, so the spec comes into play:
+    // return what the node type table would look like after a rebuild).
     $active_types->names[$db_type->type] =
       $active_types->types[$db_type->type]->name;
   }
 
-  // Save new node types if a rebuild was requested
+  // Save new node types if a rebuild was requested. Needs a fresh loop, since
+  // the loop above doesn't touch new hook_node_info() implementors.
   if ($rebuild) {
     foreach ($active_types->types as $type) {
       if ($type->is_new || $type->new_state) {
-- 
1.7.6.1


From 64c5345a313ed714ab7ff87f1009efedfbdce2ee Mon Sep 17 00:00:00 2001
From: Tim Popham <popham@uw.edu>
Date: Mon, 5 Sep 2011 12:34:38 -0700
Subject: [PATCH 4/6] Added comment to emphasize the database query bug from
 the previous version.

---
 modules/node/node.module |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/modules/node/node.module b/modules/node/node.module
index 7e6ea3c..93092c1 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -727,6 +727,8 @@ function _node_types_build($rebuild = FALSE) {
     ->fields('nt')
     ->orderBy('nt.type', 'ASC')
     ->execute();
+  // Could condition on base!=node_content and/or custom=0, but then a second
+  // loop would be necessary to add these results verbatim to the data structure
 
   // Reconcile node_type table against the enabled modules list. Any node type
   // with base!=node_content inherits its disabled state from that of its 
-- 
1.7.6.1


From 50d3e42997bb595a368ef7c787150a0d9501076f Mon Sep 17 00:00:00 2001
From: Tim Popham <popham@uw.edu>
Date: Mon, 5 Sep 2011 20:16:37 -0700
Subject: [PATCH 5/6] Clean tests, still need to address caching issue?

---
 modules/node/node.module |   61 ++++++++++++++++++++++++----------------------
 1 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/modules/node/node.module b/modules/node/node.module
index 93092c1..fad53ab 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -382,8 +382,7 @@ function _node_extract_type($node) {
  * @see node_type_get_type()
  */
 function node_type_get_types() {
-  return array_filter(_node_types_build()->types,
-                      create_function('$nt','return !$nt->disabled;'));
+  return array_filter(_node_types_build()->types);
 }
 
 /**
@@ -672,15 +671,10 @@ function node_type_update_nodes($old_type, $type) {
  *       0. Otherwise, the $type->disabled flag takes a value of 0 for any type
  *       in the `node_type' table whose module is enabled, or it takes a value of
  *       1 if its module is disabled (or possibly uninstalled).
- *     - is_new: The $type->is_new flag does not appear in the `node_type' table
- *       The property takes a value of 1 for node types implemented by
+ *     - is_new: The $type->is_new flag does not appear in the `node_type' table.
+ *       The property gets set to 1 for node types implemented by
  *       hook_node_info() that did not appear in the `node_type' table at
- *       _node_types_build() entry. The property takes a value of 0 otherwise.
- *     - 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. This includes node types whose disabled state has
- *       changed and new node types.
+ *       _node_types_build() entry. The property is left unset otherwise.
  */
 function _node_types_build($rebuild = FALSE) {
   $cid = 'node_types:' . $GLOBALS['language']->language;
@@ -712,7 +706,9 @@ function _node_types_build($rebuild = FALSE) {
       $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;
+      if ($rebuild) {
+         $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;
@@ -728,42 +724,46 @@ function _node_types_build($rebuild = FALSE) {
     ->orderBy('nt.type', 'ASC')
     ->execute();
   // Could condition on base!=node_content and/or custom=0, but then a second
-  // loop would be necessary to add these results verbatim to the data structure
+  // query and loop would be necessary to add these results verbatim to the
+  // data structure
 
   // Reconcile node_type table against the enabled modules list. Any node type
   // with base!=node_content inherits its disabled state from that of its 
   // module.
   $enabled_modules = module_list();
   foreach ($db_types as $db_type) {
-    if (isset($active_types->types[$db_type->type])) {
-      // These nodes are implemented by an available hook_node_info() and were
-      // already in the database. If they happen to have base==node_content,
-      // they're still consistent.
-      $active_types->types[$db_type->type]->is_new = 0;
-      $active_types->types[$db_type->type]->disabled = 0;
-      $active_types->types[$db_type->type]->new_state = 0;
+    // Clobber any type information from hook_node_info() with that from the
+    // database to preserve any modifications
+    $is_hook_implemented = isset($active_types->types[$db_type->type]);
+    $active_types->types[$db_type->type] = $db_type;
+    $type = $active_types->types[$db_type->type];
+    if ($is_hook_implemented) {
+      $new_state = ($type->disabled != 0);
+      $type->disabled = 0;
     } else {
       // A hook_node_info() was unavailable for the current iterant from the
       // database, so enable or disable the iterant depending on its module's
       // state
-      $active_types->types[$db_type->type] = $db_type;
-      $type = &$active_types->types[$db_type->type];
-      $type->is_new = 0;
-      if ($type->base=='node_content') {
+      if (isset($type->base) && $type->base=='node_content') {
         // Special case: always enabled
+        $new_state = 0;
         $type->disabled = 0;
-        $type->new_state = 0;
       } elseif (isset($enabled_modules[$db_type->module])) {
         // The module is enabled
-        $type->new_state = ($type->disabled != 0);
+        $new_state = ($type->disabled != 0);
         $type->disabled = 0;
       } else {
-        // The module is disabled
-        $type->new_state = ($type->disabled != 1);
+        // The module is disabled, uninstalled, or missing entirely
+        $new_state = ($type->disabled != 1);
         $type->disabled = 1;
       }
     }
-    
+
+    // Don't litter the return data structure with internal data
+    if ($rebuild) {
+      $active_types->types[$db_type->type]->new_state = $new_state;
+    }
+
     // Copy data to names array.
     // This was inside the hook_node_info()-unavailable-case above, since the
     // name data from the hook_node_info() call should be fine to return, but
@@ -778,8 +778,11 @@ function _node_types_build($rebuild = FALSE) {
   // the loop above doesn't touch new hook_node_info() implementors.
   if ($rebuild) {
     foreach ($active_types->types as $type) {
-      if ($type->is_new || $type->new_state) {
+      if (isset($type->is_new) || isset($type->new_state)) {
         node_type_save($type);
+        // explicitly remove or someone will start depending on unspec'ed
+        // behavior
+        unset($type->new_state);
       }
     }
   }
-- 
1.7.6.1


From f0aa1523b77781b117de6a7eef83b65652eabac7 Mon Sep 17 00:00:00 2001
From: Tim Popham <popham@uw.edu>
Date: Tue, 6 Sep 2011 12:24:44 -0700
Subject: [PATCH 6/6] Mended some antique documentation, a trivial and
 formerly misguided array_filter() call, and refined the
 _node_types_build() documentation a little more.

---
 modules/node/node.module |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/modules/node/node.module b/modules/node/node.module
index fad53ab..2ee5d8b 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -371,9 +371,12 @@ function _node_extract_type($node) {
 }
 
 /**
- * Returns a list of all the available node types.
- *
- * This list can include types that are queued for addition or deletion.
+ * Returns a list of a site's known node types.
+
+ * Any node type added by hook_node_info() or node_type_save() that has not been
+ * explicitly removed by node_type_delete or direct database interaction will
+ * appear in this list. The list can include types that are queued for addition
+ * or deletion.
  * See _node_types_build() for details.
  *
  * @return
@@ -382,7 +385,7 @@ function _node_extract_type($node) {
  * @see node_type_get_type()
  */
 function node_type_get_types() {
-  return array_filter(_node_types_build()->types);
+  return _node_types_build()->types;
 }
 
 /**
@@ -456,10 +459,8 @@ function node_type_get_name($node) {
 /**
  * Updates the database cache of node types.
  *
- * All new module-defined node types are saved to the database via a call to
- * node_type_save(), and obsolete ones are deleted via a call to
- * node_type_delete(). See _node_types_build() for an explanation of the new
- * and obsolete types.
+ * All new node types defined by hook_node_info() are saved to the database via
+ * a call to node_type_save().
  */
 function node_types_rebuild() {
   _node_types_build(TRUE);
@@ -645,17 +646,18 @@ function node_type_update_nodes($old_type, $type) {
 }
 
 /**
- * Get the list of all node types that have not been deleted.
+ * Get the list of all node types that have not been deleted by a call to
+ * node_type_delete().
  *
  * This function provides the `node_type' table state as it would appear after a
- * rebuild, actually rebuilding the table as well for $rebuild==TRUE. The
- * returned node type's `disabled' property indicates if the node type's
- * implementing module has been disabled, however, any node type with a `base'
- * value of `node_content' always remains non-disabled (making Node Content a
+ * rebuild, actually rebuilding the table for $rebuild==TRUE. The returned node
+ * type's `disabled' property indicates if the node type's implementing module
+ * has been disabled, however, any node type with a `base' value of
+ * `node_content' always remains non-disabled (making Node Content a
  * poor name choice for a module).
  *
  * @param $rebuild
- *  TRUE to rebuild node types. Equivalent to calling node_types_rebuild().
+ *  TRUE to rebuild `node_types' table of the database.
  * @return
  *   An object with two properties:
  *   - names: Associative array of the names corresponding to active or disabled
@@ -780,8 +782,8 @@ function _node_types_build($rebuild = FALSE) {
     foreach ($active_types->types as $type) {
       if (isset($type->is_new) || isset($type->new_state)) {
         node_type_save($type);
-        // explicitly remove or someone will start depending on unspec'ed
-        // behavior
+        // explicitly remove the new_state flag or someone will start depending
+        // on unspec'ed behavior
         unset($type->new_state);
       }
     }
-- 
1.7.6.1