Data API: first steps

hook_drupal() is really non-obvious (-1)

Agreed, it's probably not critical at this point to agree on final terminology but 'entity' might be better until we do.

Thanks for linking back to the CRUD issue, I hadn't seen the promising contributions there in the last few days.

I've updated that issue with a reference to this one, and a possible strategy of completing this patch as a step towards a larger API reworking.

Here's a refreshed patch with at least some of the more glaring errors fixed.

Changes in this patch version:

• drupal_invoke_drupal() changed to the more intuitive drupal_invoke_entity().
• Documentation for the arguments of drupal_load().
• Entity/object handling methods are added as a new include file, entity.inc. This can be overridden by a contrib module defining the variable drupal_entity_include. This allows custom object handling methods, comparable to the approach with drupal_mail() which allows the registration of a custom mail handler.

Since this patch changes loading, saving, updating, and deleting, as well as all object API hooks (hook_user(), hook_nodeapi(), hook_taxonomy(), etc.), there's a lot of room for problems.

I've installed this on a new site and most things seem to work--create, update, and delete nodes, terms, users. I added a node with a path and that was correctly saved, indicating that the new hook_entity() is working as a wrapper for existing hook implementations (in this case, path.module's path_nodeapi(), renamed to path_node()).

One error I found was that the taxonomy term select was broken on node edit--no terms were displayed as options. There will be more such problems.

Why do we need $a3 and $a4 when we have $args as an array of N depth?

Well, sure, we could use func_get_args() to read in these arguments, then array_shift() off the first ones as needed (when we need to pass some arguments by reference, or when there are indeed consistent arguments used across all object types).

The main issue here is that in this patch we're not - yet - making the larger changes that would be needed to get all object types to have the same set of arguments.

The biggest problem looks to be user, which passes around that extra argument--the array of user properties that's used in addition to the user object.

But, again, in this patch so far I'm not wading into reworking everything. Yes, we could and should make user handling more consistent with node and comment. Yes, we could and should allow term loading to feed in an array of parameters to match like we do with users and nodes. Yes, we could and should be consistent in our choice of array vs. object data type.

I'm hoping we can tackle those one by one in follow up work after making the minimal changes to achieve the primary aim here: a single set of object handling methods and a single object hook.

Any testing, comments, or reviews would be greatly appreaciated.

Thanks, with attachment this time :)

I still feel that we'd be btter served by breaking out the object hook into a set of hooks:

I can see the point, but I don't see the strong need that would justify introducing a plethora of new hooks (most of them very similar to each other). I think our best way forward is to make these changes in steps, doing the minimum at a time. Keep in mind that there are many $op arguments used at present for the object hooks, much more than just the basic load, update, insert, delete, so we might end up with 8 or 10 or more new hooks. It would be better to incrementally reduce the need for all these $op arguments, and then consider whether we can/should separate out to separate hooks.

In general this patch would need a lot of community review and energy. I'm not confident enough in the details of the approach to press on without some informed feedback and critical review, so I'm going to leave it where it is until if and when it gets the quality of review that would allow us to move forward--or abandon this approach for a better one.

I've opened a new issue, http://drupal.org/node/118345, focusing on user methods, that could be tackled separately to make this job much easier.

Here's a refreshed patch. It needs testing and work, but it's ready for review.

With a schema API nearly ready, http://drupal.org/node/144765, we're opening the door to a much improved Data API.

Most of that improvement, likely, will happen after Drupal 6. This patch is intended only to take some preliminary steps.

Notably:
* Introduce a single set of methods for saving/updating, deleting, and loading data objects.
* Introduce a hook_dataapi that can be used to respond to these operations for all data objects.
* Enable the substitution of a custom set of object handlers via an override of the dataapi.inc file. This is designed to enable the development of contributed solutions.

Changes from the previous patch:
* changed 'entity' to 'dataapi', more descriptive.
* removed user handling. we'd need to write http://drupal.org/node/118345 before being able to integrate users.

(I'm having diff challenges, please report if the patch doesn't apply for you.)

@kika, thanks for the typo note, I'll roll a new patch soon.

The term and vocabulary methods are needed for consistency--all object methods are in the form e.g. objectname_load(). They could be included directly in taxonomy.module, they just seemed to me to make more sense as includes.

These changes are half-steps; they increase consistency, but leave most of the variability of the current methods unaddressed. In http://drupal.org/node/145684 I've tried to sketch out some next steps. It only makes sense to take the first steps now if have an idea of where we want to go next.

Rerolling the patch. Includes a few small error fixes.

Thanks, I'll get working on these issues, and more testing.

Here's an update with a few fixes.

The vocabulary and term issue has to do with our new menu system. We feed in menu items with e.g. %taxonomy_vocabulary as an argument, which is interpreted as a load function, taxonomy_vocabulary_load(). I've replaced the occurrences of %taxonomy_vocabulary with %vocabulary in the vague hope that that might trigger vocabulary_load(), but no go in my testing. Hints anyone?

Some of the errors in comment preview came from a usage of $status that I missed in comment_save(), now fixed. But there are some remaining, suggesting that certain form values are not available in validation. I haven't determined why.

This patch certainly needs plenty of further work and even basic testing. Leaving the status at 'needs review' therefore seems inappropriate. But in some ways the remaining issues are details--important ones, yes, but solvable. What's equally important at this point is, is this a general path we want to be taking? So I'm leaving it as needs review because that question remains open.

Refreshed patch with some fixes and improvements.

The vocabulary paths are now working (vocabulary editing, term adding).

I went through the various dataapi.inc methods and eliminated the hated $a3, $a4, $a5 arguments. Turned out most of them were unneeded. The only change I had to make to this was that, for node validation, hook_nodeapi() used to pass the $form array. This wasn't used anywhere in core. I've dropped it, enabling us to have clean, unambiguous arguments in hook_dataapi().

I'm still getting some undefined index errors on testing. I'll try to test on a clean install and compare.

Not for Drupal 6.

The approaches taken in this patch were interim and designed for Drupal 6. A fresh start after code freeze, based on schema information, might learn something from this patch but should start at a much more basic rewrite.