Custom image action: Missing argument 3 for db_query_range()

It looks like the whole imagecache_actions_node_from_filepath needs a large review - I've not checked it in the D7 port yet

subscribe

subscribe

I got below error also

* Warning: Missing argument 3 for db_query_range(), called in /var/www/drp7/sites/all/modules/___imagecache_actions/utility.inc on line 273 and defined in db_query_range() (line 2245 of /var/www/drp7/includes/database/database.inc).
* Notice: Undefined variable: count in db_query_range() (line 2250 of /var/www/drp7/includes/database/database.inc).
* PDOException: SQLSTATE[HY093]: Invalid parameter number: no parameters were bound: SELECT nid FROM {image} AS i INNER JOIN {files} AS f ON i.fid = f.fid WHERE f.filepath = :f.filepath LIMIT 1, 0; Array ( ) in imagecache_actions_node_from_filepath() (line 273 of /var/www/drp7/sites/all/modules/___imagecache_actions/utility.inc).

Any one can help. Please...

Subscribing. Also using the 7.x-1.x-dev, any custom action, even the default action of return image; breaks it. Here's what I'm getting:

Recoverable fatal error: Argument 4 passed to db_query_range() must be an array, integer given, called in /mysite/sites/all/modules/imagecache_actions/utility.inc on line 273 and defined in db_query_range() (line 2314 of /mysite/includes/database/database.inc).

subscribe

subscribe

subscribe

Inserting
return;
at line 256 of utility.inc (7.x-1.x-dev) will solve this problem if you need this effect too much and don't want to wait developers.

Dirty but works.

I have rewritten the function from scratch. The good thing is that it's now only 12 lines long, and it should be much more solid than it was on Drupal 6.
Since managed files in Drupal 7 are attached to fields, not to nodes, the action now populates a $fields variable with information about the entities that use them. The entity themselves are not fully loaded, since it could be a waste of resources if the data is not actually used. If a developer is good enough to manipulate images, I suppose she'll have no trouble calling entity_load() in the action itself.

Thanks!
As you can tell by the comments - that chunk of code was badly in need of refactoring. It was just a get-r-done hack to expose what seemed to work at the time.

I'd be interested in getting a simpletest for this function, as it still could be a fragile part of the code. I've been trying (and failing) to figure out how to test for accurate image process results (can't compare binary blobs) and there is *something* of a testsuite in the current dev code.
This particular case however can be a code-only unit test, so could be worth trying.
It's a lot of work to bootstrap a unit test (last time I looked) but is a worthwhile exercise. I'll check where I got to with that.
In the meantime ... your patch looks good, and is probably OK to just apply.
I'd like to write up a full example as a manual test-case at least though... That area has been under-documented for a while, and I get a few support requests for a HOWTO there regularly.

I agree with your assessment that it requires decent developer skills to do *much* there ... OTOH, the function in question was originally written to support 'textactions' (currently a lost module of mine) that would call the full entity data preload to make everything available as tokens. That piece of dev is/was stalled for a while, so I won;t use that excuse as a hold-up this month. Perhaps I'd add a flag to allow either a full or partial entity load to that func. I was not originally worried about performance, as imagecache generation is a run-once-and-never-again sort of operation (in theory) but it's worth considering.

Does that patch actually work for you?
On first trial I'm debugging and getting :

$fields = imagecache_actions_fields_from_filepath($image->source);

... where $image->source = 'public://nile_st-2009-097.JPG';
So that where the call to the new/replacement func

function imagecache_actions_fields_from_filepath($filepath) {
  if (!module_exists('file')) {
    return;
  }

  $files = file_load_multiple(array(), array('uri' => file_build_uri($filepath)));
  // DEBUG:
  dpm(file_build_uri($filepath));

gives me
public://public://nile_st-2009-097.JPG

Now, I know it didn't work AT ALL before, so steps in the right direction are good, but I/wee need to look at this further.
I'm suspecting we need drupal_realpath() instead of file_build_uri(). If any?

... or is your setup different from mine?

I just added a content type, then added a file field to handle image uploads. Nothing fancy. And the field storage comes out of the DB with the D& pseudo protocol attached already.
:-/

I'm attacking it now.

I'm going to add

  // Unsure if we are given a real filepath (the normal assumption) or a 
  // Drupal scheme URI. Resolve either.
  if (! file_valid_uri($filepath)) {
    $filepath = file_build_uri($filepath);
  }

to be generous in what I expect. There are probably situations where either the raw filepath or the scheme uri is a valid input. It's no trouble to overload this utility and support both.

Progress. I can see the file entry exists now, however it's *very* light on data. I see no harm in at least loading the file entity data (which should have the alt and title strings) if not the whole linked node entity. It's a pretty toothless utility otherwise.

So, I'm satisfied that this patch (plus my tweaks) will 'fix' the reported error here, but it's still not much fun to work with.
The data array returned by imagecache_actions_fields_from_filepath() is very deep and a hassle to work with. The point of doing that data lookup on behalf of custom_action programmers is to allow custom_actions to be nice three-liners if possible.
With this big nested structure, the programmer has to add a lot of work to get the useful information back out again. But I see that each of the elements in that data structure is significant, so don't know how to short-cut it.

Here is a custom action that does what is needed to just "Apply a different watermark to images depending on how the owning node was tagged"

// Add a different watermark, depending on the attribution property of the owning node.
// Attribution is managed through tags - the owner node is tagged from a vocab, 
// and the vocab has terms which have logo images attached to them.

if (! empty($fields)) {
  foreach ($fields as $fid => $field_instance) {
    foreach ($field_instance as $field_id => $entity_types) {
      foreach ($entity_types as $entity_type => $entity_instances) {
        foreach ($entity_instances as $entity_id => $entity_stub) {
          $entity = entity_load_single($entity_type, $entity_id);
          if (!empty($entity->field_attribution)) {
            foreach ($entity->field_attribution[$entity->language] as $term_info) {
              $term = taxonomy_term_load($term_info['tid']);
              // This will overwrite itself if there are more than one valid result. By design.
            }
          }
        }
      }
    }
  }
}
// Inspect the owning entity
if ( !empty($term) && !empty($term->field_logo) ) {
  $logo_uri = $term->field_logo[LANGUAGE_NONE][0]['uri'];
}
if (!empty($logo_uri)) {
  // Finally, we know how to tag this image! Set up an effect that does this.
  $effect = array (
      'effect callback' => 'canvasactions_file2canvas_image',
      'data' => array (
        'xpos' => 'right+20',
        'ypos' => 'bottom',
        'alpha' => '50',
        'path' => drupal_realpath($logo_uri),
      ),
  );
  module_load_include('inc', 'imagecache_canvasactions', 'canvasactions/canvasactions');
  image_effect_apply($image, $effect);
}

return $image;

Simple job, heaps of code.
That's because the significant info I wanted this utility to retrieve (before even doing anything) is buried inside a nested array that looks like
$fields[$fid][$field_id][$entity_type][$entity_id] = $entity;

However, the patch does at least remove the PHP notice, so I'll commit what I've got so far.

I still want to bring back the $node (or an $entity) into local scope like before so that the majority of simple use-cases have something to work with. All the implications of multiple owners/contexts are rare edge-cases. Most of the time there is only one true answer. imagecache_actions_node_from_filepath() was useful to the examples I've been using it for.

Pushed to D7 dev. Includes the full rewritten load_entity (as a new subroutine) as I didn't want to lose it, though it can be flagged off (no UI) if performance is a thing.
Includes some experimental (commented out) code as a TODO.
I can't see as much use in the $fields approach given by this patch as there was in the naive $node util, so I think it should come back. But not today.

However, if you'd like to explain the use-case that you are finding this set of data actually helpful for, I'm interested.

Looking at the comments and the current code, it looks like that this issue can be marked fixed. Certainly the original "missing argument" error won't appear anymore.

Regarding what information should be readily available for custom actions, I would like to refer to comment 5 on #1402790-5: Can't access $node fields from custom action. Together with the fact that dman envisions this usage mainly in combination with text usage (getting text from attached entities to overlay over the image) I will go forward with my suggestion to reuse code from the text effect and thus to offer the same information in the same variables all over the place.