Should simple_access be using the node_api hook?

I've not done much with the node access system before, but in trying to get simple access to respect image thumbnail regeneration (and revision roll back) I used the Drupal api documentation as a guide.

The example module (node_access_example) does two things differently to simple access:

1. It uses a separate table to store the node access data, and that data is merged in to the node_access table as required (e.g. during node_access_rebuild, on node save etc)
2. It uses hook_nodeapi() to load access data for the node, and that data is then available across other operations (form, node_access_records etc)

This got me thinking...

The patch to fix the revisions bug repeats the code that loads grants from the access table as part of simple_access_form. So the same code appears in two places - simple_access_form and simple_access_node_access_records. If the api's are followed then this code should appear once under simple_access_nodeapi when the operation is 'load'. That is cleaner and easier to maintain?

Because simple_access doesn't store its data anywhere other than the node_access table, then when simple_access is disabled then all the existing privacy settings are lost. This now tells me why when I upgraded my site I had to manually re-instate all the privacy I had - simple_access was disabled as it was non-core. When I reactivated it it was like a new set up. If simple_access followed the same format as the example code and had its own table then it would preserve settings as the module is enabled/disabled. That might also help if debugging interaction with other modules!

The node access documentation says that the only interaction with the node_access table should be via node_access_acquire_grants, but in its current format simple access breaks this rule as it writes directly during set up.

I have been doing some experiments and have re-worked simple access on my local server to see if I can tackle any of the above (I'm trying to get to grips with Drupal and this seemed a good exercise!)

I have a version that now uses a new table (simple_access_grants) to store the access data, and it lets the core handle the necessary writing to the node_access table. It provides all the same functions as the original simple access module, but it uses nodeapi to handle load, delete, update and insert operations. It handles revisions and image regeneration correctly, and it also creates simple_access_author and "everyone can view" entries as required.

Is this a "useful" development, in which case I can post the file here for others to look at? It needs a new .install file (which I have also created) to handle a fresh installation, and the install also provides update_2 which creates/migrates an existing set up.

I didn't want to post directly as this is quite a change to the current version and wasn't sure what Drupal etiquette is!

It's been a useful learning curve for me - I now have a better understanding of how Drupal manages node access!