Creating a library of CRUD API functions for Drupal Entities

*subscribe*

I like this direction a lot. Anything to promote standardization will lead to fewer trips to the API docs, fewer errors, and faster development. In addition, breaking the API up into smaller chunks (i.e. ensuring that these tasks can be done independent of the interface) allows for better unit tests. If we can get it right we could even have some small corner of the Drupal API that won't change radically from one release to the next. :-)

I generally prefer the listed strategy 2. I'm not sure if it totally holds up in this case, though, and #1 might be better for this reason.

The first issue is that the arguments needed are inconsistent. An API that needs to have a function like

function user_role_crud($op, $properties) {}

is problematic if the $properties required depend too much on the $op. In the strategy as proposed, the $properties are sometimes an array, sometimes a scalar. This makes for strange code; at the very simplest level you have a plural variable name that is used for singular things. Instead we could always force an array which happens to be always one element long for 'read' and 'delete'; this brings back memories of node_load(array('nid' => 52)) which we agreed was needless typing and simplified.

Do we allow for the possibility of the ID itself changing? If so, we'd need an additional parameter for "update" for the original ID so the new one could be in the array. Some systems allow for this; we probably don't have to since it doesn't make sense for most objects.

I strongly prefer user_role_crud() to drupal_crud_user_role() given the choice between the two. Contrib modules will be using this standard if adopted, and they shouldn't be injecting anything into the drupal_ namespace in my opinion.

At the moment, my vote is for strategy 1. Strategy 2 is fine if we resolve/live with the above issues. The third option adds complication with no benefit that I can see at the moment, but maybe I'm missing something. It looks like we'd need the individual functions anyway, as hooks, if we went this route.

Does this cover the gamut in terms of what we'll need for input/output values?

I like method 2 the best, though the $properties param is going to get a little crazy at times. On the issue of scalar/array input, it's much less likely that a single scalar value will be sufficient to describe the desired task. Other than $op = 'delete', most queries take more than a single variable to perform their task. Bottom line, we're not going to be 'extra' typing like we were for node_load, since most cases we'll need to pass more than one value.

I don't like that $properties takes on different usage depending on the context of the operation. Like in an insert operation, it contains the values to be inserted. On a delete, it contains items to be matched in the WHERE clause. This seems pretty straight-forward until we get to an update operation. Now we've got values that need to be in both places (values to be updated and values that need to be matched in the WHERE clause).

I'm brainstorming solutions. Suggestions?

I ran through all the core modules and it looks like my concerns where unfounded, with one exception: the blocks module, which uses that crazy 'module'-'delta' scheme.UPDATE {blocks} SET visibility = %d, pages = '%s', custom = %d WHERE module = '%s' AND delta = '%s'Has there been any discussion on giving this table a primary key? Other than this one, we're in good shape.

Personally I'd prefer the "lots of little functions" approach in general. When doing that sort of API function in my non-Drupal code, I frequently find myself just stubbing it out with a call_user_func() call and passing on the parameters, because I hate working with long switch statements. :-) It would also follow the node system, which currently has a separate hook for every action (load, insert update, delete, view).

As for a helper function to dynamically build the insert/update statement, you may be thinking of this issue:

http://drupal.org/node/53488

While it's fine for specific cases to use some kind of auto-SQL magic, we shouldn't try to cover all the cases this way. In particular, many instances will have ramifications that need to be acted on. When you delete a role, for example, you may have to reassign roles to users that had it. There are also possible cases where the data may not be stored in a single table, even if it is a single conceptual "thing". For some gotchas when designing an ORM layer like this, see:

http://blogs.tedneward.com/PermaLink,guid,33e0e84c-1a82-4362-bb15-eb18a1...

I favor an approach where we make the common case easy, but leave the "write to the metal" approach available for cases that need it.

On a related note: do we have the API functions all work on one "thing" at a time? That certainly simplifies the API, and I think I'm in favor of that approach. But we could also extend these easily to work on multiples; read operations could return all targets that match the search criteria. We should make the expected behavior clear in this regard.

I would also like to see CRUD seperated into its own files creating an abstraction layer which will more easily fascilitate porting to other databases or other data stores.

I highly prefer strategy 1. JonBob's criticism of strategy 2 provides much of the reason. I also like functions to do just one thing whenever possible. Easier to remember, easier to optimize, easier to test.

Great topic and effort, Angie!

Yep, option 1 looks good. It may seem like many more little functions...but highly likely that each function will remain correct over time. Option 2 is really just option 1 with a wrapper...and then extra logic to figure out what you're trying to do.

This is actually very similar to some of the stuff I had in mind for FAPI 3 (which is getting ever so badly named).

Basically you'd build the 'model' of say a node, and then (using standard naming, or defined callbacks) just build the CRUD for each of the models.
You can then have a form for any of them, and remix the functionality and use it in workflows etc.

Thought of introducing a 'model_' namespace, so you can define new 'things' either through naming convention, or through a similar system as the _forms hook. (ie: function model_node($node) , function model_node_read($nid), function form_node_update($fields), or in the case of system settings, you would only have the model_modulename_settings(), and it would use the same CRUD for all settings forms.

an additional interesting feature i had in mind was conversions. Say you know you have a node object, and you want to do something to the owner of that node (like demote user), you could have function model_user_load_from_node($node).

An example of that being useful is if you have an image thing, and a file thing, but there's an action which only takes files, you could just create a conversion function and have it work.

Having a full set of crud functions will make it far easier for us to build proper unit tests too =)

how about :

read_user_role();
update_user_role();
create_user_role();
delete_user_role();

Similar to how we put things in the theme namespace now, and it also
reads as cleaner english =)

Also, for stuff like node loading , where we do the loading from arrays :

read_user_role_from_array() // do the array('nid'=>12, 'user' => 12) etc thing.

Also, i think if we have a consistent CRUD mechanism, we finally have somewhere to put in an 'item' top level type

ie: the crud functions fire to add things into an 'items' table, with an iid | type | oid table.

If we are considering renaming update to save, could we also consider renaming read to load.

load / save is what we already use in Drupal. (ie: user_load, and user_save).

And what i meant with the from_array() thing, is by have a consistent mechanism for conversions, so we can
find out exactly what an object can be loaded from.

It's all a bit premature, but I'll explain at DrupalCon why I would like to see this.

Also, i see this working in a load('user', $param); format. where the load() code identifies what the input is ,
and then passes it off to the right function.

Thinking of how nodeapi works, if we provide hooks into the load() , read() etc functions, we can have all of nodeapi's
benefits across all drupal objects (nodes, users, terms, whatever).

-1 for adding new prefixes to the namespace.

Right now every Drupal function begins with the name of the module. This means no modules can have name conflicts. The only exceptions to this rule are the theme_ prefix, which is special because it is always invoked through the theme() mechanism, and a handful of core functions like t() and format_*. I really dislike further confusion in the namespace, and wonder where we would draw the line. You can do many more things to these "objects" than the four CRUD operations. Say you can "move" a foo. Then you'd have update_foo(), but foo_move()? I think the inconsistency makes the code less legible, not more legible.

So long as we are tied to the single global function namespace due to the language features we have decided to use, I plead that we keep module prefixing intact.

That looks fine to me. The only weirdness will be for Things that are named the same as their module. We would have node_delete_node() and the like. Personally I feel that's a lesser evil.

As I stated jonbob, I feel that we should have a theme() like function for each of the CRUD's.

ie: load_[module]_[thing].

Additionally, similarly to how theme functions work, the module still features in the function name.

I'm not actually coding PHP, but reading this very interesting thread I missed some

whatever_query_by_property()

call. Maybe I don't understand the API, but wouldn't it be convenient to query let's say a user id by it's email-address, like in

user_query_by_email("xyz@abc.fg");

I have two suggestions:

1. Using objects for targets

   $user = new user();
   $user->load(1);
   $user->status = 0;
   $user->save();
   

   For such things, classes do make perfect sense to me. If the object is created without a parameter (like in the example above), an empty user is created (i.e. uid = -1). If you just fill in values and run $user->save(), a new user is created. If you load a user, the properties of the object are set according to the values for the specified user. If you now run $user->save();, the user with the uid in the object is altered. You could also load a user by writing $user = new user(1);.

2. Overloaded load method
   That means, I could run either $user = new user(1) to load the user with uid 1 or $user = new user(array('uid' => 1));. In the constructor/load method, the first type is expanded to an array, like the second type has. But here’s the clue: The array acts as a kind of “query builder”, meaning that you can specify any parameters that should act as a condition for the SQL query: array('status' => 0) retrieves all blocked users.
   That implies also that a user object can contain more than one user at a time. If you alter values, all loaded users are affected. To unblock all blocked users, you could for example write:

   $blocked_users = new user(array('status' => 0));
   $blocked_users->status = 0;
   $blocked_users->save();
   

Hate to say it, but tim is correct. This is what objects were made for.

It's the activerecord pattern that ruby is so famous for. We still sit with a namespace issue though, in that
classes would have to be 'module'_'thing'. IE: node_node.

I'm also not sure we want to get on this slippery slope right now , since we would have to take things into consideration such as

$event = new event_node();
$event->field = '123';
event->create();

and also inheritance and the like, which I'm still not too sure i right for Drupal. Especially with our new 'spawn node types' code,
and the fact that what has been coded has very little to do with what is actually in the object (all our layers of AOP)

+1 for this

(subscribe)

Drupal core has an informal/semi-formal no-classes policy, IIRC. Changing that would be a major undertaking, especially as it would also mean redoing a ton of other data structures in Drupal. I simply don't see that happening. Plus, PHP 4's object system is quite lame. :-)

I think load_, save_, makes the most sense, and is most consistent with existing code. For namespace pollution, what all do we expect to include here? Users and Nodes already have _save() and _load() routines. (Actually there's a patch in queue that's trying to gut them, which is sadness. :-( ) Roles, OK. Just how many are we going to have that we need to worry about the namespace? Or are we really planning to have every contrib module able to setup their own new entity types? I thought everything is a node. :-)

And -1 to anything that puts "crud" into a function name. That's just ugly to have to write or explain to people.

My e-mail and at least a few others on the devel list related to this topic

http://lists.drupal.org/pipermail/development/2007-January/021964.html
http://lists.drupal.org/pipermail/development/2007-January/021979.html
http://lists.drupal.org/pipermail/development/2007-January/021983.html
http://lists.drupal.org/pipermail/development/2007-January/022008.html

Seem to be fairly convergent

Throwing my 2¢ in here. Webchick, I've been diving deep into install profile creation, where I want to create nodes, menus, and other Drupal-isms programmatically. drupal_execute is your friend....but it's also not your friend. Look at creating a menu, and then trying to get its menu ID!

CRUD++ -- and maybe we want to prototype them as wrappers (or SOMETHING) that can be used to help in install profile creation for the 5.x series.

*subscribing*

I like a variant strategy 3 a lot, especially as it opens the way to the introduction of uniform callbacks: consider this variant:

• 4 calls, for the 4 CRUD ops, plus formatting
  • drupal_create('some_entity', array(<params>))
  • drupal_retrieve(('some_entity', array(<params>))
  • drupal_update('some_entity', array(<old params>, <new params>))
  • drupal_delete(('some_entity', array(<params>))
• note that a call to render is not necessary, since we happen to already have a function built on this model:
  theme('some_entity', array(<params>)) which happens to be built on the same model
• automatic callback available, PRE, and POST event
  • module implementing such a CRUD type could register like
    drupal_register_callback('some entity', $when, $event)
    where $when is PRE | POST (or BEFORE | AFTER),
    and $event is (CREATE | RETRIEVE | UPDATE | DELETE).
  • each of the 4 CRUD functions runs like:
    1. check whether a registered PRE callback exists for entity
    2. if so, invoke it
    3. do work
    4. check whether a registered POST callback exists for entity
    5. if so, invoke it
• the PRE / POST callback mechanism might be extended using a result selection mechanism : typically, I think PRE callbacks could prevent the CRUD op from running by returning a specific value, and some results (failure) in the execution of the CRUD function could prevent the POST callback from running, or the callback should be different, or invoked with a status parameter mentioning failure.

One thing I forgot to mention is that callbacks can obviously be stacked: several registrations can be chained in a call array, and the CRUD method will fire the hooks in order: first PRE to last PRE, then last POST to first POST.

The reversal for POST callbacks is to take into account the fact that each callback is likely to expect a consistent situation, while invoking them in original order for POST callbacks would actually reverse the contexts (not sure I'm being very clear here ?).

@Boris: Regarding drupal_execute() in install profiles, I'm with you there! There was a patch that got rolled back a while ago (by Moshe) that would allow any node/taxonomy/menu creation forms to return the newly created ID. This is needed if you're going to add a #submit handler after the node submit handler for example. You can't cleanly get the ID of the newly created node now.

I'm referring to http://drupal.org/node/94139 and http://drupal.org/node/68418. IMHO we should focus there first since even with a super API, we still need to have that functionality for forms.

Here's an idea on returning the id of a taxonomy, node, or menu_id(from db_next_sequence) . Have a common function handle the sequences that our pgsql support requires anyway. for example

function _nick_plays_capt_obvious(&$values, $schema = array('#db_table' => 'da_table_name', '#db_primary_key' => 'dtnid')) {
$id = db_result(db_query("SELECT id FROM {sequences} WHERE name = '%s'", $schema['#db_table'].'_'.$schema['#db_primary_key'])) + 1; 
$required_form_key = $schema['#db_primary_key']; 
$values[$required_form_key] = $id;
}

One thing that is clear to me, is that we need to abstract our install profiles even further. I'm not sure the best answer is to actually require people to write *insert*, update, load, and delete crud functions like a hook. I think our install files. insert, update, delete, and load functions should CREATE the crudapi automatically.

I don't know how closely ya'll have looken into jaza's importexportapi, but it provides a really great starting place for thinking about how we can abstract everything so that we can perform this kind of magic automatically.A better approach might be to bind folks to writing out their database schema in such a way that any drupal module can execute crud functions. I know that sounds batsh#t crazy, but I've seen such possibilities in the still-beta importexport api. Its a lot easier for us to actually, finally, its bloody time, do something about hook_load, update, insert, delete, and form. Each one of those depends on each others complete agreement to operate properly -- why not think about putting a lot of that information in one place? If you ask me, it would be a hell of a lot easier to specify my inserts, and updates (and thus deletes, and loads) in the form api, than constantly having to rewrite all three hooks each time a new field gets added. I think that solving that problem would make this problem a lot easier.

But, I ramble... especially when I drink whiskey. Carry on!

subscribing

Hi webchick,

I'm trying to write some sandbox-type code to implement my ideas (I'll post when I get something working).

One thought I had for hook naming, was naming like:

hook_objectapi_X($obj)

where X= load, update, insert, submit, prepare, delete, etc.

So an implementation of this function might look like:

function mymodule_objectapi_load($obj) {

  switch($obj->class) { 
    case 'node':
      return db_fetch_array(db_query("SELECT * FROM {mymodule_node} WHERE nid = %d", $obj->nid));

     case 'comment':
        break;
     case 'user':
        break;
    case 'term'
       break;
  }
}

Thus, the taxonomy module (for example) could create a term table for every core or contrib module-defined object type (term_node, term_user, term_comment, etc)

Assuming you map the id column name to something more generic (e.g. rename nid to node_id, uid to user_id, etc), your ability to load taxonomy for any object becomes something like:

function taxonomy_obj_get_terms($obj, $key = 'tid') {
  static $terms;

  $id_name = $obj->class .'_id';
  $id = $obj->$id_name;

  if (!isset($terms[$obj->class][$id])) {
    $result = db_query(db_rewrite_sql("SELECT t.* FROM {term_". $obj->class ."} r INNER JOIN {term_data} t ON r.tid = t.tid INNER JOIN {vocabulary} v ON t.vid = v.vid WHERE r.". $id_name ."= %d ORDER BY v.weight, t.weight, t.name", 't', 'tid'), $id);
    $terms$terms[$obj->class][$id] = array();
    while ($term = db_fetch_object($result)) {
      $terms[$obj->class][$id][$term->$key] = $term;
    }
  }
  return $terms[$obj->class][$id];
}

function taxonomy_objectapi_load($obj) {

   $extra['taxonomy'] = taxonomy_obj_get_terms($obj);
   return $extra;
}

+1, subscribing

Ok, so don't try to actually run this code yet (it's neither complete nor tested), but take a look if you're interested:

http://cvs.drupal.org/viewcvs/drupal/contributions/modules/object_driver/

It's obviously still rough, but is suggestive of using an additional abstraction layer to create the table (per http://drupal.org/node/63049), and I hope shows why it's useful to be able to put the whole table definition in one big array.

It seems like with just a little work you could replace most or all of the core load/save functions (node, user, taxonomy, comment, etc).

+1 X 1,000,000 -- better handling of loads, saves, deletes would make every drupal programmer 10 times more productive. Of course, I'm at an extreme that thinks that productivity trumps supporting php4.

Inspired from all the posts here and on the mailing list, I fiddled something together. It’s basically an “entity API”: Modules can define their own entities and extend other module’s entities. For example, comment.module can extend the node entity with its own fields and tables (!). These tables are joined in the actual first/only query for the entity as opposed to hook_load() or hook_nodeapi(). This entity API can be used to load arbitrary “objects” from Drupal’s database.

Note that it is not necessary to change any data structures in your database.

http://cvs.drupal.org/…/sandbox/timcn/entities.php

It’s partially documented in the source code.

This issue http://drupal.org/node/113435, General _load, _save, and _delete methods for all objects, is a parallel approach, but less ambitious. Rather than replacing our existing object APIs, it aims to the minimal reworking to make them consistent and provide a cross-object set of methods. The approach requires tweaks to but not fundamental rewriting of existing implementations.

I'm impressed with both pwolanin and kkaefer's approaches.

We seem to have a choice: (a) generalize and improve consistency in our existing APIs, as perhaps an interim step to later, bigger changes; (b) replace our existing APIs from scratch (this issue), or (c) do nothing.

(c) is out! It's time to address this!

(a) is not completely satisfying. It gets us somewhere, it gives us many of the benefits we're after, but it leaves much of the work undone. We have to be satisfied retaining a lot of legacy inconsistency between our different APIs (or we follow up with more patches to try to reduce this consistency).

(b) is more satisfying, but ambitious. Is it feasible for the 6.0 cycle? Looking at our existing API implementations, there's really a lot to rewrite there. And of course means a major break with existing code--comparable in scale to the Forms API. If we tried it for 6.0, this change would be big enough that it should really be one of the only major changes we attempt in this cycle. It would require sustained focus by the community.

We could, of course, aim for 7.0. And sometimes gradual steps are a timewaster--if we're going to break things, let's do it all at once!

Still, I wonder if we can try both (a) and (b) at the same time. This would look like: complete and apply (a), but structure it in such a way that it can be overridden by contrib implementations of (b). At its simplest, this could mean isolating the core drupal_save(), drupal_load(), drupal_invoke_entity(), etc. (or whatever we call them) methods in an include file then conditionally loading it as follows:

  // Allow for custom object handling methods.
  if (variable_get('drupal_entity_include', '') && file_exists(variable_get('drupal_entity_include', ''))) {
    require_once './' . variable_get('drupal_entity_include', '');
  }
  else {
    require_once './includes/entity.inc';
  }

Like e.g. drupal_mail(), this would allow contrib overrides of the core functionality. So, maybe, we could do both: get some major core improvements for 6.0 (without major breaks) while providing an incubation period where we could develop and mature one or more of the promising (b) approaches, possibly for inclusion in 7.0.

+1 for this issue, and strategy 3.

I hope this will be ready for D6. As a gimmick one could call this the SLAD API (Save, Load, Add, Delete) ;) Seriously, I believe abbreviations are confusing to users, and I think we should avoid that. An alternative name might be "Actions API"?

Actually, although the summer 2007 time frame has been planned for D6, I don't think there is any specific reason for this period over another, possibly longer period: since D6 is breaking various things anyway, it could make sense to accept the breakage, and the fact that it needs more time to be done properly than just break some parts while leaving others undone.

hook_user() and user_save() are major barriers since they pass the user object in two separate arguments and include a double pass by reference. Fixing this looks to be a necessary piece of improving or replacing our object handling methods. I've opened a new issue, http://drupal.org/node/118345. Tackling that would put is a good ways ahead toward the longer term goal of streamlining our object handling. Anyone willing/able to take it on?

FYI, I worked the code in #45 into a working module for Drupal 5.x: http://drupal.org/project/object_driver

Subscribing big time!

subscribe

I second nedjo and vote for a from-the-scratch approach aiming at D7.
Particularly I am convinced that Drupal should open up to using objects where it makes sense. By the end of 2008, PHP4 will play no big role anymore, so we can rely on the much better OOP support of PHP5.

subscribe

Additional thoughts on this front: http://groups.drupal.org/node/8001

I think there might be some lessons we could learn from RoR here.

Why not centralize all CRUD routines? It's fairly easy to create self-discovery of tables and fields - even cache the information - to dynamically build the appropriate insert, update and delete statements. We could have a few simple requirements for tables - such as integer identity fields, either auto_incrementing or standard sequence names and special "created_on" and "updated_on" timestamps.

This module would expose the following functions:

crud_save($table, $data, $type = null) (where $type is 'insert' or 'update' - leave blank for either)
crud_delete($table, $key)
crud_get($table, $key)

It could expose hooks - for security traps:
hook_crud_insert($table, $key, $data)
hook_crud_update($table, $key, $data)
hook_crod_delete($table, $key, $data)

I'm working on the beginnings of this module now and I'll be contributing it soon.

- Doug

subscribing

...to potentially have another look at that nice list of potential target APIs for D8 again. At least, text formats and user roles are already done.

Subscribe.

Subscribe.

Also look at #365899: API methods for schema-based load and delete operations which is a more recent development on the theme (thx Catch for the nid).

Subscribe.

We are working on a unified entity API to make this happen as part of the configuration management initiative. Proposed architecture forthcoming.

I guess it's safe to mark this one as duplicate of #1184944: Make entities classed objects, introduce CRUD support then.