Create a statistics API

Posting progress so far. This does the following:

• Re-implements better_statistics_exit() in the most generic way possible,
• Introduces a hook_better_statistics_fields() allowing other modules to declare statistics fields and callbacks,
• Attempts to do the above in as sane a way as possible (only allowing schema changes on cache flush, module enable, or disable).

Still to be done:

• Get the views portion of the skeleton above working,
• Clean out any cruft left in helpers.inc (may not need any of it anymore).

Whoops. Here's the patch.

Regarding a: this won't be necessary within the field array itself, since we can get that information by looping through the results of module_implements(). Definitely worth having, though.

Regarding b: I keep going back and forth on this. Ultimately, I think it's a good idea. When it's implemented, I also think it'd be a great way to get rid of those three extra hook implementations that call the single field alteration function. This way, instead of updating db schema on either flush caches/module enable/disable, it would just be on saving the admin form.

Some other problems that came up while I was thinking about it:

• How do we handle if the module needs to update the schema of the field?
• As the patch is, it won't easily write through changes to non-schema fields (like views/callback) that should change more or less automatically

Thanks for the thoughts, mondrake.

I've pushed a huge change into the 7.x-1.x dev branch that implements the API I suggested. Mondrake, please have a look and let me know of any bugs or anything I may have overlooked. Since this could be extremely handy elsewhere, a few more eyes would be great too. If you can, please refer others.

I put a significant amount of work into documentation; for that, please see better_statistics.api.php.

Otherwise, I'll also update the issue summary above to reflect the new information.

Wow, thanks for the very detailed review, mondrake!

a) In some of the older code I posted previously, I tried using a "callback_argument," but because of the way the API is engineered, it could only be a static value, even though the way it's declared makes it seem like it could be dynamic (i.e. setting $fields['my_field']['callback_argument'] = my_function(), which would work once and then permanently "cache" the value it returned). As a result, I just decided to get rid of the callback argument and force the declared callback function to take the field name as an argument. That way, in your callback function, you could set up a very simple switch/case; or, if that was still undesirable for whatever reason, you could specify different callback functions for each field and just ignore the field argument passed to them. Hopefully it's flexible enough how it is; it would require some non-trivial reengineering otherwise.

b) I like this idea, especially having seen your mini browscap integration module. Sounds like what might be best is to take cues from Views, Rules, et al. and introduce the concept of a my_module.statistics.inc file that gets autoloaded when needed. That way, the stats API code and callback won't have to clutter up the main module and those who don't want it or don't have Better Statistics installed won't have to care. I created an issue for that here: #1786896: Allow Stats API code to live in a module.statistics.inc file

c) It might be a good addition to call _better_statistics_update_fields() in the form submit, but the cases where that would need to happen would be pretty small. I don't think it would be necessary to flush the entire cache each time that form was saved. My only apprehension is having multiple, separate variable saves, but that issue can be worked out. Issue created here: #1786900: Better Stats field update code might also need running on configuration save

d) I ran into this a couple of times myself. I thought it was appropriate to fail gracefully in the case of a schema updates (since this would be happening, at least currently, when an admin was performing seemingly unrelated actions), but I think the schema add should fail hard like you saw so that developers can catch them before general release.

e) Good idea! Issue here: #1786906: Make each module's checkboxes a collapsible fieldset

f) This probably goes hand in hand with c above, though again, I don't think a full cache flush on saving that form is necessarily a good idea. I think it's best to just clear whatever caching mechanism Views uses for the field list and call it good. Issue here: #1786910: Views can list invalid fields after adding/removing Better Stats fields

Thanks again!

Just saw, and taken care of with a recent commit.

I'm signing off for the evening now, but everything in dev should be a little more stable. I'd checkout the repo directly for the latest, or if you'd prefer, wait for the Sept 19th dev package.

All of your major points above should be addressed now, so consider what's up in the repo (or the Sept 19th dev package) to be a release candidate.

I used your browscap work above to test the new hook_statistics_api() functionality, so you might try that out. See the documentation at #1786896: Allow Stats API code to live in a module.statistics.inc file

Thanks again!

All I did was take your module, stick it in browscap.statistics.inc, and add the following code to browscap.module:

/**
  * Implements hook_statistics_api().
  */
function browscap_statistics_api() {
  return array(
    'version' => 1,
  );
}

I'll let you do the honors (once Better Statistics 7.x-1.1 is released) since you put all of the effort into making the thing.

0) Correct. You will only need ctools for the Better Statistics module. If a module (like Browscap or IP Geoloc) specifies a hook_statistics_api() and has a module.statistics.inc file, but they don't have Better Statistics (or ctools), the module.statistics.inc file will never be loaded.

1) Hmm. Very cool that they're pulling the data asynchronously that way, but definitely more difficult for integration. I think that what you described may be the only good option; I don't know the specifics of how the module works, but it might be worth running the data update on cron instead of what you described? Hard to say without diving in. I don't know how the module handles it now, but the most performant way to do the statistics side is to keep whatever geo data it returns in a cookie (once it's loaded) and in the statistics callback, just pull it from the cookie. Another, very useful integration point here would be a way for Views to take that data from accesslog and place points on a map. That level of Views API work is beyond me, though.

2) This looks to be a bug. I could've sworn this field was available by default without Better Stats installed, but I guess not. It's not a bug, you just have to go to the "Advanced" fieldset in the right of the Views UI and add a user relationship.

Edit: wrong tab...

Probably a good idea. Since the schema is available at that point anyway, it might be worth doing a runtime check for strlen of the callback return vs. length of the declared schema (if one exists). Opened #1790836: Fail gracefully if db insert in hook_exit() fails for some reason

One thing I haven't tested, but I think should be tested, is what happens in the event that someone has 7.x-1.0 installed, somehow doesn't have ctools installed, and then updates to 7.x-1.1. I'm going to guess Drupal will error out in a very bad way.

Believe most of the kinks have been ironed out at this point. If you don't have any objections, mondrake, I'd like to release 7.x-1.1 with the API. Then we can focus on those integrations; if there are any other issues, we can handle them in the usual way.

Tag pushed up; release to come shortly. Thanks again for all your help, mondrake! Let's see about getting the browscap and IP Gelocation integrations underway.

http://drupal.org/node/1792876