fix $node namespace collisions

could hook_nodeapi just check isset($node->modulename) before doing the assignment? This would still penalize the novice developer, but in a way that's perhaps less likely to make their whole site break.

A change of this magnitude also makes a change of $node->comment to $node->comment_setting (or some such) seem reasonable.

Yes, we need to fix this.

In doing so, we should be thinking ahead to the possibilities that the new schema API opens up.

From http://drupal.org/node/145684:

To enable schema-aware handling of a data item array, we need to reflect the relationships somehow in the data structure. The most obvious way is an approach analagous to the Forms API. That is, when loading data (or directly creating a data array), we mark keys in a way to identify them as foreign references. A node, then, would have direct properties from its primary table--the node table. All of its other properties would be in arrays associated with their respective tables of origin. E.g., a book with a parent have, instead of a 'parent' key, a '@book' key, which is an array of properties held in the book table, in this case, the parent.

// Direct properties of a node as held in the node table.
$node['nid']
// Properties in another table.
$node['@node_revision']['body']

With this structure, eventually, we won't need node_load() or node_save(). drupal_load('node', 21) will be enough. We read what needs to be loaded (or saved) from the table relationships as mapped in the schema API.

Yes, those heady days are a bit far off now. But if we're taking the trouble to change our object structure, we may as well get it right.

this patch: http://drupal.org/node/146425#comment-253153 now puts all the book module data in $node->book, and nothing else in the top-level $node object.

This is such a great thing.

I mostly agree with nedjo, and I like his '@tablename' syntax very much.
I think it is better to stick data into arrays named after the tables the data comes from than to just have an array for each module that invokes nodeapi, because this would be a big step towards a data api. There would however also have to be a place to stick data in that is not directly coming from a table (e.g. node links).

So, what we could do is
- change the $node object to a $node array (for consistency with the sub-arrays)
- only fields of the node table get top level keys
- data coming from other tables (like terms or files) lives in a '@tablename'-array
- other data that modules want to insert into the node array are either stored in '#properties' or in a 'modulename' array (this is basically the same discussion as with the node rendering patch - what is an array, and what is a child? There could also be namespace conflicts between module arrays and fields of the node table, as they'd both be top level keys. Hmmm.)

This would be a major step forward.

A fixed syntax of data arrays makes many things possible, a unified data syntax is, now that the schema API is in, probably the most important step towards a consistent and simple CRUD or data API.

While I see the possible advantages of naming per table, how would you *enforce* this? A major advantage of the suggestion above to have the nodeapi code do this is that it is automatic.

We will enforce it by documentation and good example (ie. core). Woe upon to you if you do not keep convention. In an upcoming Drupal version your code will suddenly break...

@chx - I though that that breaking code was the whole point of new Drupal versions? Not much of a threat ;-)

This is the stuff that was hashed through on the *display* side for the node rendering patch. Is there any chance that, if we're going to the work of reshuffling everything about nodes already, we could consider using renderable formapi style array? that would allow us to attach quite a bit of metadata if we needed it, and it supports constructs like access control on elements, relatively clean nesting of sub-elements, and so on.

I would REALLY hate for us to reinvent the wheel. If we're smart about it, we could even make it possible for the results of node_load() to be directly renderable for simple node types.

I'll again argue for the module name solution. I think many modules join 2 or 3 tables to add data to a node, and it makes much more sense to me to have that data together in one array.

Also, in terms of forms, this leads pretty naturally to constructs like:

$form['mymodule']['#tree'] = TRUE;
$form['mymodule']['name'] = array('#type' => textfield, '#default_value' => $node['mymodule']['name']);
$form['mymodule']['address'] = array('#type' => textfield, '#default_value' => $node['mymodule']['address']);

And then the form values map directly back into the right parts of the node object for the preview, etc.

If we've got the {table_name} and @module_name syntax, what do we actually do about stuff like the $node->links array? This data is the result of a hook invocation, so it's neither from a specific table nor a specific module. However, it's also not a property of the node array in the regular FAPI sense of #property entities. Maybe we also have $node['%hook_name'] entries? such as:

I'm startign to feel a bit concerned about this direction. One of the real advantages of Drupal's system (thus far, at least) is that it gives modules 'equal access' to stuff that's being built. I worry that if we try to enforce a rigid standard like this, we'll just end up pushing more and more complex logic into other areas of the system.

One of the benefits of using FAPI-style arrays is that, in theory at least, #module and #db_table properties could be easily added to any element if needed. Am I missing something? Adding more and more prefix types and naming conventions seems... less than ideal.

@eaton - I'm not sure I have a clear idea of the method you're suggesting. Can you post a couple lines of example code?

I think eaton is talking of something like this:

... and I've to admit that I quite like this... I'll definitely think about this some more.

That still leaves the important question of what the *standards* for that structure should be... but goodness if it doesn't feel more familiar to someone who's worked with FAPI before.

The fact that we already have lots of utility functions for working with this style of array, that we can (if we plan things right) render it straightaway, and the fact that moving forward even something like FQuery could be used for complex manipulation... those are bonus.

Also, it's worth noting that in this scheme, things like nid and title and body would need to be #properties rather than elements. The final tricky bit is that $node['#type'] should probably be 'node' rather than 'story' or 'blog' etc, for safety's sake. But I digress.

This sounds awesome. Each step we take to become more consistent across the board, is lowering the barrier to Drupal development, reducing code, and giving us greater flexibility to do some powerful stuff. This is a logical step and gives us one more object in that familiar construct. We already have hook_nodeapi('view') adding pieces to $node->content somewhat like this, so why not construct the whole $node array like that?

Thank goodness for the extra 4 weeks. So what's the game plan now?

subscribing

I posted my first stab at unifying some of these various issues in a blog post at http://www.lullabot.com/blog/trouble_nodes ... I think if we can figure out a way to load nodes (and any other objects, if possible) as structured arrays we'd gain a lot. The real problem would be the change in syntax. $node->foo would go away, for the most part. $node['foo']['#value'] would be the syntax, instead.

I had this idea:
here is some psudo code to help
function drupal_load($type, $load_args) {
call $type_load.
in the node case, this would be drupal_load('node', 234) or drupal_load('node', array('node' => array('nid' => 1234))) (syntax: array('tablename' => array(fieldname => fieldval)))
that would return an array of specially formatted table names and field names to load. e.g.

array(
  'node' => array(
    'nid',
    'uid',
    'title',
    'vid',
    'type',
    'language',
    'created',
    'changed',
    'status',
    'moderate',
    'sticky',
    'promote',
    'language',
  ),
  'node_revisions'...
  '#primary' => 'node.nid', // What field to reference in the where clause if only a number is passed in
)

then pull in data from hook_load('node') which would add extra fields
*build query here*
*execute query*
*parse results*
*return results*
}

@dww If you and Eaton don't have time to work on the full refactoring for D6, we should consider the very simple version of $node->modulename['key'] enforced via the node module as you proposed above.

One thing to keep in mind that no one has mentioned specifically is how the FAPI rebuilds node objects using only the form tree after submission. With any existing module (I'm looking at project), there will be cases where forms are divided up into fieldsets, widgets, etc in the tree. Simply turning the #tree property on does not work in this case because your generated node structure will mimic the UI, and not the desired module namespace tree. In most current cases, the FAPI will simply generate a flat node structure because #tree is false by default. We can hack our way around this by manually setting the #parents array on each form element so the generated node has a namespace structure, but that's not automatic, and a maintenance nightmare.

per message linked in #24.

hunmonk brought this patch to my attention tonight.

a) I do think it'd be great to get this resolved in core. I'm all for increased consistency, etc.
b) It's going to break everything.
c) So I need to touch base with Dries about it.
d) However, unless this change is coupled with something that has profound improvements in what it allows Drupal to do (such as the page/node rendering stuff, or data API) I don't think it has much of a shot on its own. :\ It's really hard to justify every module, theme, block, etc. having to change its code around simply because a few contributed modules never name-spaced properly.

In the meantime, I'd suggest contrib modules use the convention proposed by dww in #1.

Just stumbled upon this old issue. Is it something still interesting for D8+ now we have entities and classed objects, or is it an obsolete issue we can close?

with new entity api there's no more collisions