Integrate CTools export.inc into core for declaring instances of objects
| Project: | Drupal |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | D7DX, deployment |
One of the major holes that exists in the Fields API as it currently exists in core, is that field instances can only be defined by using the CRUD API in an imperative with no mechanism to define a declarative array.
To put this in context, one of the primary reason that views is so easy to integrate with is because developers can export a view and create a hook_views_default_views implementation with their code that views can then import.
To contrast this with how this API currently works, it would basically be the same as only being able to define or update views in hook_install or hook_update_x and you would need to write down each column, or argument as a separate command ($view->addColumn, $view->addArg etc).
This has very dire consequences for developers in that it makes it incredibly difficult to ship a module that defines some custom node types,
which DIRECTLY affects site builders that are using the proper dev -> qa -> live development methodology.
These developers now have nearly no recourse but logging into the staging and live sites and repeating the front end changes they have made (see: http://developmentseed.org/blog/2009/jul/09/development-staging-producti...) or be faced with an equally daunting development burden.
A hook like this is of _critical_ importance if you want to be able to export and import your node types.
The way that views implements it's hook_views_default_views is the gold standard for this.
My first feeling is to extend hook_node_info, so you can define these additional instances of fields.
Another route that we seriously need to consider is the the export API that is in Ctools. Views is being re-developed to use this as the backing, and it's an incredibly simple and powerful API for handling stuff like this which is already being used by panels, views and others.
I am marking this issue as critical because shipping D7 without an api like this is going to have an enormous negative impact on the developer experience of Drupal as a platform.
#1
I should mention that this does not make the CRUD api irrelevant
we _needed_ the crud api first to be able to build a hook like this.
But fields in core is not finished until this hook exists.
#2
marking normal
this has been considered, but there's no issue for it as of yet.
i still feel we need to work on this, but there are several good reasons why this is incredibly complex.
#3
[double-post, sorry]
#4
It's stating the obvious, but the CRD parts are easy, it's the U part that causes 99% of problems.. other systems like Ruby on Rails have very simple structures for handling the U, but it's still assumed the developer is going to know what they're doing and handle all changes manually. If there's a rich API for modifying the schema then it could be possible, but there's a lot to it.
#5
"the CRD parts are easy, it's the U part that causes 99% of problems".
Exactly - That's why there's currently no field_update_field() in D7. See #367013: Field update (field_update_field).
One of the design points in the preparation of the FiC sprint was: "field updates are data migration. That's for contrib to handle".
Note that the default per-field storage reduces the problem a little: data doesn't have to fly around depending on field being single or multiple, shared or not, so field update is "just" updating db column definitions. Can still timeout on large tables, though (esp. with InnoDB, I'm told). Also, that's only true for the default storage engines; alternate storage engines, local or "remote" (the mythical "couchDB storage engine") might have other issues with that.
#6
+ Roughly quoted from DamZ on IRC: there's no declarative interface for schema itself. It's hook_schema (CR) + update_N (UD)
#7
It's a pain to have to make hook_schema() and hook_update_N() changes in parallel though, not that we should try to get around that, but still easy to forget either end.
#8
Well it's not the hook_schema part that's hard with declarative node types, it's the update parts. I think a first step may be to get node types import/export in core. merlinofchaos has suggested that this would be a great use for export.inc from ctools.
#9
Here is an initial cut at transforming export.inc for core. We'll need to do some work to actually integrate it into core, so right now this is *just* export.inc, but I want to get some review before any real work is done to integrate it.
Here are the issues I see:
1) This does not do database syncing. This means that objects loaded via hook_default_* are only ever kept in memory. This means that you can't perform normal queries to get a list of these objects. For things like Views and panel pages this has proven to be okay, but I have seen some object definitions where database sync would be desired. I'm not sure that it's a requirement for putting this in core, though, and it is something we could add in as a later phase.
2) It stores enabled/disabled status in a variable. Maybe this isn't ideal as it could end up taking up a rather large amount of database space. Note that it does try to reduce how much it puts in this variable by only storing state that disagrees with the state it expects. i.e, objects in the database are expected to be enabled, so these only show up in the variable when disabled. Objects in code have a ->disabled flag, so this only shows up if the state disagrees with that variable.
mikey_p suggested that the best place to integrate this into core will be to start with node type info, and that seems like a great use. This would pave the way for integrating it into field api as a second step. Even if we don't end up with time to do that in this cycle, this is a valuable tool that is an important part of the deployment cycle for many objects, and making this API available is strong.
#10
Changing title to reflect how I think this should go.
#11
Also learning to spell.
#12
#9
1) I think for a first iteration, it's fine to limit the use cases for exportables where either one object or all are retrieved and the export API's export_load_object($table) function is not being circumvented.
Configurations in Drupal core that could be made exportable:
- Aforementioned content types
- Aforementioned field definitions (with the additional challenge of writing change management for field definition changes that affect the database)
- Image module
- Drupal variables itself (?)
- Other configurations that are stored with a string as identifier (there aren't that many in core) or can be made to store with a string as id.
I've used this code to make data module and feedapi mapper module exportable. This is really good stuff and belongs in core IMO. For those hitting this thread without having thought much about exportables before, there are major benefits to support this in core:
A) more and more modules export to code (in D6: CCK, views, panels, etc.), a library makes this task much easier
B) a common library establishes a convention. Thus we can write abstracted code that handles configuration for deployment, version control or packaging (think CTool's bulk export or Features module).
#13
Our posts crossed. Fixing title.
#14
Agreed, very important feature. I'm all for a solid universal library in core for this purpose. Wish I had more to contribute than a subscription comment at the moment.
#15
Ooooh. Oh yes.
FYI, migrate and table wizard modules are both on their way to using this export API in D6.
I hope to review this soonish.
#16
i did a rough DBTNG conversion for earl. i haven't actually tested it so i left the original code there for comparison purposes.
#17
More DBTNG stuff to review for me then.. ;)
+ $query->condition($export[key], $args, 'IN');a) I think IN is the default now, no need to specify it
b) key.. I think that should either b $key or 'key', haven't looked at the code much..
+ else if ($type == 'conditions') {+ foreach ($args as $key => $value) {
+ $query->condition($key, $value);
+ }
+ }
Is there a use case for more complex conditions than this?
#18
This is just a note that I am not going to have the time to carry this forward, so it is going to need someone to champion this. I can be around to provide advice and reviews but that is about the best I can offer at this point. In particular, I fly to Paris on Sunday and my availability will be quite limited.
#19
+++ includes/export.inc@@ -0,0 +1,628 @@
+/**
+ * @defgroup exportables Exportable objects
+ * @{
+ *
+ * These are objects that can live in code OR the database, and versions in
+ * the database will override versions that are in code.
+ *
+ * This doesn't include a write routine since drupal_write_record is
+ * more or less sufficient.
+ *
+ * Exportable objects are created by adding definition to the schema
+ * in an 'export' section. The following fields are supported in this
+ * chunk of code:
As with the ajax patch, we need a more lengthy introduction here. This time, I'm not really familiar with how this functionality works, and from the given description, I don't get it.
What I get is the first sentence. We may clarify further that it really only works with objects. But before that, it would be useful to know when one would want to use/implement exportable objects - and how the overall system is supposed to work.
The second paragraph is very technical already. Not sure whether that needs to be so prominent.
The third, I don't get. Are the following properties supposed to be defined in the database schema of a module?
After scanning the code some further, it would be highly helpful if the introduction additionally contained
1) a list that describes the logical flow, from a high-perspective
2) further technical/DX details about the low-level flow, i.e. what a developer actually has to implement to use this functionality; probably using one or more example code snippets.
+++ includes/export.inc@@ -0,0 +1,628 @@
+/**
+ * A bit flag used to let us know if an object is in the database.
+ */
+define('EXPORT_IN_DATABASE', 0x01);
+
+/**
+ * A bit flag used to let us know if an object is a 'default' in code.
+ */
+define('EXPORT_IN_CODE', 0x02);
+
I wonder whether simple string flags (without defines) wouldn't be more sane here.
+++ includes/export.inc@@ -0,0 +1,628 @@
+function export_get_default_object($table, $name) {
...
+function _export_get_defaults($table, $export) {
...
+function export_var_export($var, $prefix = '') {
...
+function export_object($table, $object, $indent = '', $identifier = NULL, $additions = array(), $additions2 = array()) {
...
+function export_get_schema($table) {
...
+function export_get_schemas($for_export = FALSE) {
...
+function _export_filter_export_tables($export) {
...
+function export_get_schemas_by_module($modules = array(), $for_export = FALSE) {
...
+function export_set_status($table, $name, $new_status = TRUE) {
...
+function export_set_object_status($object, $new_status = TRUE) {
...
+function export_form(&$form_state, $code, $title = '') {
...
+function export_new_object($table, $set_defaults = TRUE) {
...
+function export_to_hook_code(&$code, $table, $names = array(), $name = 'foo') {
Can we get docs for the arguments, please?
I'm on crack. Are you, too?
#20
This issue started off being about Field API not supporting declarative definitions and has now become about CTool's export.inc. I'm going to talk about the former.
As others have pointed out, a declarative definition for Field API will be a nightmare for updates. This is exactly the same reason Schema API doesn't support declarative updates, instead requiring procedural calls from hook_update_N(). Certainly, Field API could support a declarative structure for the initial definition of fields, just as as Schema API does; that would be easy. But updates remain a problem no one wants to tackle.
My thinking for the dev/stage/prod problem for Field API is a module like devel's macro that tracks all changes to fields and instances and exports them to a series of API calls. I see no reason this can't be in contrib; any module or profile that needs it can simply depend on the contrib module.
#21
well one place where this would be very handy is with the image styles. when we pulled imagecache in from contrib we dropped the ability to import and export presets (as image styles are known there). if can get some time today i'll look at the feasibility of using this for that purpose.
#22
these changes look really good but i'm a tiny bit concerned about using the IMAGETYPE_* constants as keys for the extensions. by keying them it seems to imply an additional constraint on the API that's not documented and that other toolkits are going to have to mimic. we can either document this, drop their use, or define the constants if GD isn't installed...sorry posted to the wrong issue.
#23
Ok. i found some bugs in the export.inc , and i started a patch for the image_styles export.
The first patch is the fixed export.inc, and the second patch adds the meta-data to the image_styles table and defines a new menu item for export (ie: admin/config/media/image-styles/export/thumbnail)
the item is not linked anywhere yet, but it is exporting the base table properly.
I'm not quite sure how to do an export/import that joins across multiple tables yet tho, i'm still determining if it's even possible with export.inc.
#24
I'd talked with Earl about exporting across tables and he assured me that it worked because Panels does exactly that. I'll try to take a look at this in the afternoon.
#25
The answer should be in here then : http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/panels/pane...
specifically :
<?php
function panels_load_displays($dids) {
$displays = array();
if (empty($dids) || !is_array($dids)) {
return $displays;
}
$result = db_query("SELECT * FROM {panels_display} WHERE did IN (" . db_placeholders($dids) . ")", $dids);
ctools_include('export');
while ($obj = db_fetch_object($result)) {
$displays[$obj->did] = ctools_export_unpack_object('panels_display', $obj);
}
$result = db_query("SELECT * FROM {panels_pane} WHERE did IN (" . db_placeholders($dids) . ") ORDER BY did, panel, position", $dids);
while ($obj = db_fetch_object($result)) {
$pane = ctools_export_unpack_object('panels_pane', $obj);
$displays[$pane->did]->panels[$pane->panel][] = $pane->pid;
$displays[$pane->did]->content[$pane->pid] = $pane;
}
return $displays;
}
?>
#26
I wonder how difficult it would be to make the load_object function recursive.
So you can add the relationships to the schema definition, and it would automatically pull all the relationships (obviously they would need to be defined in one direction only).
#27
#27: the 'export' property would need to support a declaration of what other exportable tables relate to this 'base' table. Something like this:
<?php// Relates a table 'related_table' to a table 'this_table' via 'this_field' and 'related_field'.
$schema['this_table']['export']['related']['this_field'][]['related_table']['related_field'];
?>
We may get away with not having an 'export' declaration in the related table at all.
The objects that you load from this table cluster could be keyed by their table names:
<?php$object->this_table->this_field;
$object->this_table->some_other_field;
$object->related_table[0]->related_field;
$object->related_table [0]->some_other_related_field;
$object->related_table[1]->related_field;
$object->related_table [1]->some_other_related_field;
?>
#28
yeah, that's what i was thinking.
something like
<?php'relationships' => array(
'table' => ('key' => 'id', 'foreign key' => 'sid')
)
?>
#29
We're going to need to write a proper import function too, drupal_write_record wont cut it.
but it won't be very complicated.
#30
Just as a small note. Table relationships are now codified in schema API in D7.
#31
thanks for that webchick, it should be really simple to code this.
unfortunately the foreign key is defined on the wrong side for us to be able to just use it, so we're still going to need to add pointers to the table.
#32
@adrian: Would it make sense to move them around? This patch was committed so that "in theory" it would make it easier for code like this to run. If it doesn't in fact do this, I'd consider that something we should fix.
#33
@webchick I think they are implemented in a sane way.
take for example the node table.
It's unrealistic for the node table t try and define every join which can be done on it's nid column, but to define the relationships from the tables that join on the node table is sane.
The issue we have with this is that we would be starting with (say) the node table, and then have to build a model starting from that point. This isn't the kind of thing that could be done automatically, so it will need a developer to say "these are the related tables".
#34
#35
Recategorizing - this is critical for lots of different reasons.
#36
011100110111010101100010011100110110001101110010
011010010110001001100101
http://www.livephysics.com/ptools/binary-text.php
#37
matt2000, that's a mildly cute way to subscribe but the whole link thing had me ready to report it as spam.
#38
I fear "mildy cute" might be a euphemism for "annoying." If so, I apologize for the inconvenience.
Lacking a transparent subscription mechanism, I've discovered an insatiable need to find ways to amuse myself with my subscribe posts. Henceforth, they're like snowflakes; no two are alike!
So as not to waste another comment entirely, I've rerolled the previous patch to remove the word "ass" in the documentation. An obvious typo. Seriously, I'm not joking any more.
Peace. :-)