Jump to:
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | field system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (duplicate) |
| Issue tags: | WSCCI |
Issue Summary
entity_load loads entity based on their $ids and return basic entity object, with the basic properties, and raw data field.
If you want to pass those entities to an external program (javascript, a flash client, etc.) you can't base your results on raw data only, you need to output them formatted (public://images/image1.jpg doesn't makes sense for a flash client). But you can't call the views function because it will pre-render them as an array and values you want as simple text will be wrapped in HTML code and you can't send that to your client.
field_get_display seems useless cause it takes as a parameter the $instance and if you have the instance loaded it means that you know already your display properties. But then if you try to do it you have to call field_read_instance for every fields then call the proper methods to display your image as you wanted to do.
To pass a simple list of the 3 upcoming event to my flash client, the workflow is:
<?php
$time = date('Y-m-d', REQUEST_TIME);
$query = new EntityFieldQuery();
$entities = $query
->entityCondition('entity_type', 'node')
->entityCondition('bundle', 'event')
->propertyCondition('status', 1)
->fieldCondition('field_date', 'value', $time, '>=')
->fieldOrderBy('field_date', 'value', 'ASC')
->range(0, 3)
->execute();
$nodes = node_load_multiple(array_keys($entities['node']));
$events = array_map('_events_toflash', $nodes);
$flashvars = drupal_json_encode($events);
function _event_toflash($node) {
$result = new stdClass();
$result->title = $node->title;
$settings = &drupal_static(__FUNCTION__, array());
if (empty($settings)) {
$settings['field_image'] = field_read_instance('node', 'field_date', $node->type);
$settings['field_image'] = field_read_instance('node', 'field_image', $node->type);
// More field here (and there is...)
}
$result->path = drupal_get_path_alias("node/$node->nid");
$date = $node->field_date[$node->language][0];
$result->date = format_date(strtotime($date['value']), $settings['field_date']['display']['flash']['settings']['format_type']);
$flyer = $node->field_flyer[$node->language][0];
$scheme = file_uri_scheme($flyer['uri']);
$destination = image_style_path($settings['field_image']['display']['flash']['settings']['image_style'], $flyer['uri']);
$result->flyer = url(file_stream_wrapper_get_instance_by_scheme($scheme)->getDirectoryPath() . '/' . file_uri_target($destination));
return $result;
}
?>I really feel like reinventing the wheel, going through each fields definition to re-implement the business logic, because there is no way to get this without calling _view hooks which gives me a rendering array that I can't do anything with it.
Am I doing something wrong ? Someone encounter the same problem ? I looked up at the service module but they don't implements the field handling.
Comments
#1
I actually think this is a misconception of the field API, and i'm tagging it as a bug.
We need to introduce a "bridge" feature between the presentation layer (hook_entity_view) and the domain logic layer (how data are constructed). Right know there is not such a separation and if you want to switch presentation layer from HTML to AMF or JSON, you need to re-implement the domain logic, which is a bug (or misconception) of the API to me.
That would allows service module to handle formatters on fields for instance.
#2
To give a concrete example with image field:
<?php/**
* Returns HTML for an image <u>using a specific image style.</u>
*/
function theme_image_style($variables) {
$variables['path'] = image_style_url($variables['style_name'], $variables['path']); // DOMAIN LOGIC !
return theme('image', $variables);
}
?>
the image_style_url is the domain logic of image field, the presentation layer is the theme_image function. If we can see the theme_image_style as an helper, this is completely wrong.
This the same for theme_image_formatter($variables); which involve domain logic here:
<?phpif ($variables['image_style']) {
$image['style_name'] = $variables['image_style'];
$output = theme('image_style', $image);
}
else {
$output = theme('image', $image);
}
?>
This should be set somewhere else, easily query-able, and in a uniform way. Presentation layer should be just responsible for displaying HTML 4 compliant output. Then we can switch from theme output in case you want to diplay only HTML 5, JSON data, AMF data, and so forth in the correct image style you defined in your field display for this view mode. The corresponding function would create the styled URL and pass it to theme_image for displaying nicely in HTML if needed, and allowing other client to get the styled path instead of original path according to what you set in the display settings.
#3
The whole of drupal is currently highly targeted towards HTML output, and the question of generating alternate representations (JSON etc) out of raw data is indeed a blurry area. That's true of, but not specific to, the field API. Making drupal better on this regard is an identified priority for drupal 8 (http://buytaert.net/web-services-in-drupal-8).
As far a d7 is concerned, though, this is 'by design'. We won't be introducing new concepts around 'formatters' in core d7. Specific field types need specific manual massaging of their raw values before they can be spit in JSON, XML, what-have-you.
Also: for the specific case of image fields and 'public://images/image1.jpg' -> 'http://example.com/sites/default/files/styles/medium/public/images/image1.jpg', I do think that the file and image styles have enough API funcs to let you handle that outside the theme layer ?
#4
They have enough API funcs to let me handle that, but that is not the point, the point is code duplication of domain logic of a specific field.
I then need to reproduce the following business logic:
<?php$scheme = file_uri_scheme($flyer['uri']);
$destination = image_style_path('bubble', $flyer['uri']);
$result = url(file_stream_wrapper_get_instance_by_scheme($scheme)->getDirectoryPath() . '/' . file_uri_target($destination));
?>
It mean that you can't rely on the field API cause you will have to rewrite code for each fields you need to support. It's clearly a misconception of the API, because this should be query-able by an external module without knowing the business logic. We need to extract this field logic to a query-able function: e.g a hook. And as far I know, hook is based on Template Method which is a behavioral pattern (Define the skeleton of an algorithm in an operation, deferring some steps to subclasses. Template Method lets subclasses redefine certain steps of an algorithm without changing the algorithm's structure.). Which mean no API changes, then it can be fixed on Drupal 7.
This is very simple to me, we just move the business logic from theme function, and implements it on the hook_field_formatter_view !
#5
If this is a matter of "image fields lack a 'raw url of the image in a given image style' formatter (whereas file fields do have a 'raw url' formatter)", then this would be a topic for image.module. It's plain doable by a contrib module, but you might have a shot advocating this as a common need worth an addition in a D7 point release. Would need to be done in d8 first anyway.
Other than that, I can only repeat that D7 won't introduce a whole new concept aside formatters. In Drupal 7, a node (or an entity) has an internal code representation as a $node object containing raw values, and can be rendered in various "view modes", which are a mainly HTML-oriented concept - and that's about it. Regarding Field API, this translates to : some formatters are relevant for non-HTML output, some are not.
That's also a conceptual limitation for content indexing (core search index or Solr, or whatever), or for token generation, for that matter. But that's where drupal currently lies, and it won't change in core within the D7 cycle. A limitation, even if seen as a design flaw, is not a bug.
'massage field value into something that makes sense to the outside world for non-HTML representation' is a completely valid topic to think about in D8, and I *do* encourage you to watch out for and participate to the D8 'Services' initiative (see link in my previous post) to make this better.
Within D7, you might also be interested in how http://drupal.org/project/restws adresses (or not, I didn't take a close look so far) the topic.
Having said all this - nothing can get in D7 without getting in D8 first anyway. Thus, moving back to D8.
#6
This is not only related to image, first, its true that image lacks a 'raw url' but I proposed a fix here already #992792: Image field should implements image_url_plain view mode so this is not the point.
I'm happy that dries and you think that it should be the right direction, I didn't installed your module, how you handle a specific view mode to your ws ? And how do you suggest to fix this ?
#7
restws is not my module, I didn't even take an actual look at the code yet, I only know it plans to offer canonical representations of entities in various formats, and there are smart people behind it.
#8
Took a closer look. Restws relies on the 'Entity API' contrib module (http://drupal.org/project/entity) for the heavy lifting about how to output the various entity conponents ('properties')
More precisely, from the Entity API project page :
The 'Entity API' project is the where these topics are being explored in D7, and experimented for possible D8 inclusion. I'm therefore marking this as a duplicate of #551406: Centralize entity properties meta data - which is basically 'move Entity API contrib module in core'.
#9
FYI, I just created :
(Restws module) #1136356: Fix file access
(Entity API module) #1136362: add metadata for image styles
#10
Thanks for your support and the time you spend into that yched, I really appreciate.
Let's follow the discussion in these places and I'll try to clarify the needing of such a mechanism.
#11
Proposed solution to solve this issue
Actually I re-open this topic because its not a duplicate of any posts out there. First its not specific to image field, and the goal is to having a clear separation between presentation layer (e.g. theme_html, theme_json, theme_whatever) and domain logic of a field (e.g. for images: the style, for date: the date format, for text: trimmed, default, etc...).
Each fields should provides their proper display. I am taking the Image example as it is to me the best example to see the issue.
Actual implementation :
image_field_formatter_view return an element with the theme hook, image_style, path of the image.
<?php'#theme' => 'image_formatter',
'#item' => $item,
'#image_style' => $display['settings']['image_style']
'#path' => isset($uri) ? $uri : '',
?>
then,
theme_image_formatter which actually delegate to theme_image_stye then call the image_style_urlmethods.
<?phpfunction theme_image_style($variables) {
$variables['path'] = image_style_url($variables['style_name'], $variables['path']); // <== Clearly not at the right place.
return theme('image', $variables);
}
?>
Note that there is two theme functions, theme_image and theme_image_style, the following solution removes the latter.
We can resume to:
Proposed implementation :
Which concretely, mean removing theme_image_style, and in the image_field_formatter_view, passing an element with the theme to be implemented, and the generated path
<?php'#theme' => 'image',
'#item' => $item,
// '#image_style' => $display['settings']['image_style'] No more domain scoped information for the theme layer !
'#path' => image_style_url($view_mode['style_name'], $uri), // Instead sends the formatted output.
?>
This little thing, can change everything about services, possibilities are endless if you let each layer doing their job correctly. Because if you want to implement a JSON output, then you don't need to rewrite the logic which was before in theme function, you just believe what sends you hook_field_formatter_view. No code duplication, extensibility is guaranteed. And I believe this can be applied to Drupal 7.
#12
The main motivation here is that in a javascript or flash client, if you want to retrieve the 'teaser' output mode, it sends you the original image path, that you don't care about, because you need to redirect to the styled url.
More importantly, if you are doing serious development and creating a bunch of view modes: e.g. Flash Module Front, Javascript Popup, iPad display, and you attributes different format for each of them, if you changes in the back-end, by going to manage displays in the field UI, then it doesn't reflect, cause you hardcoded the URL mapping. This makes your software poorly evoluting, and not really maintainable. And a good software is made up of three thing: Usability, Dependability, Maintainability. For the admin who changed the display name in the backend but couldn't recompile the flash client, or for the developer who changed the directory for cleaning up the back-end, they both just made the flash client not showing images anymore.
Do you see the problem a bit better now ?
#13
As you nicely advised me to take a look here: http://buytaert.net/web-services-in-drupal-8, adding the tag 'Web Services Initiative'.
#14
Skimming over this issue, I think it is still a duplicate of #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.). (Yes, we've been wrestling with this issue for quite some time.) I'm going to mark this as a duplicate to keep the discussion in one place for now. WSCCI is still focused on the context and plugin parts of the system for the time being, but let's keep the discussion going. This is a very hard problem to solve well, and we should take the time to think through the implications.
#15
#16
This was started in 2007 and its far to be fixed, my proposal is to provide a clean separation between presentation layer and domain logic. Following the 20/80 method, where 20% of the changes will result into 80% of the benefits, and hopefully will not last 4 years to fix.
I know this is a hard problem, but you want to fix the whole system, this just address a mis separation of the function roles. I really don't agree to close it again so i'll give it a last try and if someone find that it is a duplicate then I wont re-open. But think twice before closing because this issue is a small fix (20%) based on existing, that would solve (80%) of the problem.
Applied to all field theming functions, this subtle change could be a big step in the right direction.
#17
If all agree on that task I can start working on it tomorrow.
#18
More approachable title ?
#19
IMO that point about theme_image_style() is debatable. theme('image_style') is not only used by image field formatters, but is a generic shorthand to let any piece of code say 'give me the html for this image in this style'. I'm not sure we'd want to leave it to each caller to figure out the style path themselves.
Anyway, that would be something to discuss with the image.module maintainers for D8. theme_image_style() can't be changed in D7 anyway, no API break allowed. That should probably go in a new issue, this one is quite loaded now.
As for the more general issue - what is exactly this small "20%" fix you're talking about, apart from the image formatter change ? This can't be about checking every individual formatter to ensure the render arrays they produce (before being rendered and themed) make sense as exposable data for, say, JSON. That's not part of the contract of 'being a formatter', that's not what render arrays are about, and there's no way we can enforce that for contrib field types and formatters.
Solving this is exactly the scope of the issue @Crell mentioned, and this very discussion is happening over there right now. Thus I'm setting this back to 'duplicate', we don't want to fork discussions.
#20
I think code can explain things that words cannot; About the shorthand, i didn't think about that and that's true that it should not be removed. The contract of 'being a formatter' is not broken, since it will gives a styled url. For text field it will gives an already trimmed text, for date field a date formatted string, and so on to the theme function.
We then keep the domain logic in the theme function as well (and warn developers about a deprecation). It will remain API compliant to shorthand calls to theme functions and not interfere with already formatted data from formatters.
#21
My #19 crossposted with the edits in #16 and the following #17-#18, but my reasons to "won't fix" the approach and mark the generic issue as duplicate of #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.) still stand.
#22
Ok my attempt to fix with quick proposal worked nice, but with a lot of overhead since i'm calling
<?php $nodes = node_view_multiple($nodes, 'flash'); ?>.Before the patch I had the original URL - even if I set up my 'Flash' display mode to a styled image' -
After the patch I got the styled URL - But with all the #theme stuff that is relevant for HTML only - Then its a big resource waster, and i'll follow the discussion on the issue #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.).