Hooking into default Drupal feeds
| Project: | Cooliris (PicLens formerly) |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
/node already provides an RSS feed as rss.xml. I thought it would be nice to just hook into this feed for PicLens. This would require adding in in the necessary tags into the "rss item" $op of hook_nodeapi() for any given node type. Taxonomy works like this too. In this way you could have the default feeds be PicLens aware vs having two or different RSS feeds on the same page.
I tried this and had some success but maybe more API around this could help. Here is what I did as a test which just grabs the first attachment (assumed to be an image for now). This worked after I figured out the namespace thing but still the question is where to interject the JS, StartLink, etc. I'd like to submit this issue and start a discussion - this is not a bug.
<?php
function pldemo_nodeapi_rss_item(&$node, $a3, $a4) {
if (is_array($node->files)) {
$files = array();
foreach ($node->files as $file) {
if ($file->list) {
$files[] = $file;
}
}
if (count($files) > 0) {
// RSS only allows one enclosure per item
$file = array_shift($files);
return array(
array(
'key' => 'media:thumbnail',
'namespace' => array('xmlns:media="http://search.yahoo.com/mrss/"'),
'attributes' => array(
'url' => file_create_url($file->filepath)
)
)
);
}
}
return array();
}
?>
#1
Here is my first patch for this.
The basic idea is to make it easy for modules to put in thumb and content tags into any core generated feed. I'm not totally sold on my api approach yet but this works and should be a discussion point starter.
My test is to use the new api to put turn any node with image file attached with the core upload module to become piclens compatible feed item in the the default node feed such as rss.xml for the default node page.
#2
I'll take a look at it later.
The irony is that someone else release http://drupal.org/project/mediarss a few weeks ago .. sucks a in way.
#3
Hum, similar but not very generic - its hard coded for a couple of basic image recipes. I'm thinking what I'm doing here allows any module to decide how to put in media rss tags. Definitely some overlap though but I don't think it conflicts with this addition to piclens. Test and comment when you get a chance.
#4
Haven't tested it yet, just took a glance at the code. Like the idea very much, some remarks though:
<?php$op = str_replace(' ', '_', $op); //$op = delete revision, rss item, etc.
$function = "piclens_nodeapi_" . $op;
if (function_exists($function))
return $function($node, $a3, $a4);
?>
My first reaction was: why not check simply if $op == 'rss item' ? Or do we want to act on other node operations too (which I fail to see right now)
<?phpstatic $mimetypes = array(
'image/jpeg',
'image/png'
);
?>
I'm not completely sure if using static here will improve a lot of speed or help reducing memory footprint. I should check it out though.
attachment is probably better ;)
Anyway, I like it, I'm going to try this out this weekend
#5
Thanks for the code review.
<?php$op = str_replace(' ', '_', $op); //$op = delete revision, rss item, etc.
$function = "piclens_nodeapi_" . $op;
if (function_exists($function))
return $function($node, $a3, $a4);
?>
This is just a little code pattern I like to use as it makes my code cleaner in my opinion since instead of a big case statement or if/else I simply dispatch the $op to the appropriate function. I agree its not really that useful when we only care about 1 op but who knows when/if we might find other ops to handle. I use this same pattern in hook_form_alter, hook_user, etc.
<?php
function piclens_check_mimetype($mimetype) {
//TODO - add all supported mimetypes for cooliris
static $mimetypes = array(
'image/jpeg',
'image/png'
);
return in_array($mimetype, $mimetypes);
}
?>
For this I'd say that its a negligible performance increase but a good coding practice also. Conceptually the array is static, the mimetypes don't dynamically change and if php had a constant keyword I'd use that. Performance wise this is call 2 times for each node in the feed so having the array static is modest performance improvement. Regarding that I was considering just moving to a single piclens hook that the callee would then handle and return both the thumb and the content instead of a hook for each. That is, combine
<?php/**
* Test piclens_nodeapi_rss_item module hooks - TODO. Make file attachment handling an optional setting or a sub module
*/
function piclens_piclens_rss_item_media_thumbnail($node) {
return _piclens_media_attachment($node, 'thumbnail');
}
function piclens_piclens_rss_item_media_content($node) {
return _piclens_media_attachment($node, 'content');
}
?>
into a single hook like hook_piclens_rss_item_media($node) and the caller would return all the extra tags it wants to add (media:thumb, media:content, etc.). I'm not sure about this yet, what do you think?
I attached a patch with the typos fixed.
Thanks.
#6
Here is a patch that combines the rss item extra tag handling into a single hook. I think this is a little more concise and also more flexible since it can allow for an arbitrary number of tags to be added, not just media:thumb and media:content.
#7
@swentel
Once we agree on this api change we could take a look at how the various sub modules work to see if they should instead just hook into default feeds. For example for taxonomy/image all we really need to support for putting image node data into the default taxonomy RSS feeds via the new hook rather then creating an entirely new feed.