Add plugin hook to and improve get_access_sql_suffix()

[brettp]

get_access_sql_suffix() is the primary way to decide if entities are available to the user. This function is used (and abused) by metadata, annotations, and river functions. I've attached a patch that cleans up the SQL generated, allows you to customize the column names, and emits a plugin hook so plugins can add to or remove restraints. In addition to removing the horrible hacks using str_replace() for river and metadata functions, I believe this is enough to implement a decent roles plugin using only plugin hooks.

[brettp]

More docs, less var_dump()

[cash]

Copying comments from email...

Cash:

It looks like a plugin author gets to insert where subclauses to the 
access clause. A typical clause could be something like "return entities 
that are public OR are at friends access level where user is a friend 
OR is private AND user is owner". A plugin could add an OR clause: "OR
user has relationship group_admin for container guid".

Am I understanding this correctly?

Brett:

The important part is get_access_sql_suffix().  This code is very old 
and hasn't been touched in a long time.  Its purpose is to add the 
access_id and enabled = 'yes' where clauses to various getter functions 
for entities, metadata, annotations, and the river.  Because the schema 
among those tables are slightly different, there were hacks that 
str_replaced()'d out bits of what get_access_sql_suffix() returned.  
The new function doesn't require those.

The plugin I'm writing allows you to set an entity to private and then 
share explicitly with other users.  It has a hook that returns this:

  $value['ors'][] = "{$table_prefix}{$guid_column} IN (
		SELECT guid_one FROM {$db_prefix}entity_relationships
		WHERE relationship='shared' AND guid_two=$owner_guid
		)";

This solves the "I need to override the access_id restrictions" 
that arise when writing a roles plugin.

There are a few problems I haven't solved with this implementation 
yet for non-entity tables:
 * Metastrings access is all or nothing based upon the entity, 
not the metadata/annotation.  There's not a slick way to check 
if access for a bit of metadata should be overridden.
 * Advanced queries would need to know if they're looking at 
an entity, metastring, or river table.

My goal for this is to provide enough extensibility to allow a 
roles plugin without rewriting the db model and without a huge 
performance hit...

[cash]

• Any uses of this patch in production?
• Do we want to pull it in for 1.8.0?

[ewinslow]

Definitely a fan of this idea. Seems like this almost obviates the need for the access collections table, actually. Everything can be tracked by relationships, plugin hooks, and magic access_id values. 1.8.0 would be nice, but we're not in that much of a rush I don't think.

[cash]

My preference is to stay away from adding features in the 1.8.x releases so are we okay with assigning this to 1.9. I think the goal should be to not take a year to release 1.9.

[brettp]

Agreed about not adding features in bugfix releases.

Setting the milestone at 1.9 because we have our hands full with 1.8 features already, and there are still some bugs to this approach.

[ewinslow]

Most definitely. Plus, 1.9 is supposedly dealing with plugin hook, so that's an appropriate time to revisit.

Also, thank you for saying that 1.9 shouldn't take a year... would like to discuss this in more detail before 1.9 dev starts.

[jtilson]

+1 for this idea.. I've been wrestling with a problem that this will solve 100%.

[brettp]

This was set for 1.9, but somehow ended up in Near Term Future release without anyone changing it...?