Merge pull request #444 from jordandukart/7.x-smarter-links

Update some derivative tests and add a filter function to smarterly filter

includes/derivatives.inc

@@ -41,20 +41,7 @@ function islandora_do_derivatives(AbstractObject $object, array $options) {
   $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object));
   uasort($hooks, 'drupal_sort_weight');
   $results = array();
-
+  $hooks = islandora_filter_derivatives($hooks, $options, $object);
-  if (array_key_exists('source_dsid', $options)) {
-    $hooks = array_filter($hooks, function($filter_hook) use($options) {
-      return array_key_exists('source_dsid', $filter_hook) &&
-      $filter_hook['source_dsid'] == $options['source_dsid'];
-    });
-  }
-
-  if (array_key_exists('destination_dsid', $options)) {
-    $hooks = array_filter($hooks, function($filter_hook) use($options) {
-      return array_key_exists('destination_dsid', $filter_hook) &&
-      $filter_hook['destination_dsid'] == $options['destination_dsid'];
-    });
-  }
 
   foreach ($hooks as $hook) {
     if (isset($hook['file'])) {
@@ -148,20 +135,7 @@ function islandora_do_batch_derivatives(AbstractObject $object, array $options)
   uasort($hooks, 'drupal_sort_weight');
   $operations = array();
 
-  if (array_key_exists('source_dsid', $options)) {
+  $hooks = islandora_filter_derivatives($hooks, $options, $object);
-    $hooks = array_filter($hooks, function($filter_hook) use($options) {
-      return array_key_exists('source_dsid', $filter_hook) &&
-      $filter_hook['source_dsid'] == $options['source_dsid'];
-    });
-  }
-
-  if (array_key_exists('destination_dsid', $options)) {
-    $hooks = array_filter($hooks, function($filter_hook) use($options) {
-      return array_key_exists('destination_dsid', $filter_hook) &&
-      $filter_hook['destination_dsid'] == $options['destination_dsid'];
-    });
-  }
-
   foreach ($hooks as $hook) {
     $file = FALSE;
     if (isset($hook['file'])) {
@@ -178,3 +152,50 @@ function islandora_do_batch_derivatives(AbstractObject $object, array $options)
   }
   return $operations;
 }
+
+/**
+ * Filter the derivative functions to only call those which are valid.
+ *
+ * @param array $hooks
+ *   An array of hooks to be filtered depending on options.
+ * @param array $options
+ *   An array of options for the derivative generation.
+ * @param AbstractObject $object
+ *   An AbstractObject representing an object within Fedora.
+ *
+ * @return array
+ *   Returns the filtered array of hooks to be ran.
+ */
+function islandora_filter_derivatives($hooks, $options, AbstractObject $object) {
+  if (array_key_exists('source_dsid', $options)) {
+    $hooks = array_filter($hooks, function ($filter_hook) use ($options) {
+      return array_key_exists('source_dsid', $filter_hook) &&
+      $filter_hook['source_dsid'] == $options['source_dsid'];
+    });
+  }
+  if (array_key_exists('destination_dsid', $options)) {
+    $hooks = array_filter($hooks, function ($filter_hook) use ($options) {
+      return array_key_exists('destination_dsid', $filter_hook) &&
+      $filter_hook['destination_dsid'] == $options['destination_dsid'];
+    });
+  }
+  // Do a final filtering to make sure that the source DSID exists on the object
+  // where needed. Using a defined function as opposed to the way above as
+  // it seems to break PHPCS as of 1.4.8.
+  $filter_function = function ($filter_hook) use ($object) {
+    $to_return = FALSE;
+    if (array_key_exists('source_dsid', $filter_hook)) {
+      if ($filter_hook['source_dsid'] != NULL) {
+        if (isset($object[$filter_hook['source_dsid']])) {
+          $to_return = TRUE;
+        }
+      }
+      else {
+        $to_return = TRUE;
+      }
+    }
+    return $to_return;
+  };
+  $hooks = array_filter($hooks, $filter_function);
+  return $hooks;
+}

includes/object_properties.form.inc

@@ -28,7 +28,10 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr
   }
   $regenerate_derivatives_access = FALSE;
   if (islandora_object_access(ISLANDORA_REGENERATE_DERIVATIVES, $object)) {
-    if (count(islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object))) > 0) {
+    module_load_include('inc', 'islandora', 'includes/derivatives');
+    $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object));
+    $hooks = islandora_filter_derivatives($hooks, array('force' => TRUE), $object);
+    if (count($hooks) > 1) {
       $regenerate_derivatives_access = TRUE;
     }
   }

islandora.module

@@ -1838,11 +1838,13 @@ function islandora_islandora_datastream_access($op, AbstractDatastream $datastre
   $result = islandora_namespace_accessible($datastream->parent->id) && user_access($op, $user);
 
   if ($result && $op == ISLANDORA_REGENERATE_DERIVATIVES) {
+    module_load_include('inc', 'islandora', 'includes/derivatives');
     $applicable_hook = FALSE;
     $object = $datastream->parent;
     $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object));
+    $hooks = islandora_filter_derivatives($hooks, array('force' => TRUE), $object);
     foreach ($hooks as $hook) {
-      if ($hook['destination_dsid'] == $datastream->id && ((isset($hook['source_dsid']) && isset($object[$hook['source_dsid']]) && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $object[$hook['source_dsid']], $user)) || (array_key_exists('source_dsid', $hook) && $hook['source_dsid'] == NULL))) {
+      if ($hook['destination_dsid'] == $datastream->id && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $object[$hook['source_dsid']], $user)) {
         $applicable_hook = TRUE;
         break;
       }

tests/derivatives.test

@@ -159,7 +159,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
   public function testDerivativeFilteringOnSourceDSID() {
     global $_islandora_derivative_test_derivative_functions;
     $_islandora_derivative_test_derivative_functions = array();
-    $this->constructBaseObject();
+    $object = $this->constructBaseObject();
+    $this->constructSOMEWEIRDDATASTREAMDatastream($object);
     $object = islandora_object_load($this->pid);
     islandora_do_derivatives($object, array(
       'source_dsid' => 'OBJ',
@@ -207,7 +208,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
   public function testNoSourceDSIDNoForce() {
     global $_islandora_derivative_test_derivative_functions;
     $_islandora_derivative_test_derivative_functions = array();
-    $this->constructBaseObject();
+    $object = $this->constructBaseObject();
+    $object = $this->constructSOMEWEIRDDATASTREAMDatastream($object);
     $object = islandora_object_load($this->pid);
     islandora_do_derivatives($object, array());
     $this->assertDatastreams($object, array(
@@ -216,6 +218,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
       'OBJ',
       'DERIV',
       'NOSOURCE',
+      'SOMEWEIRDDATASTREAM',
+      'STANLEY',
     ));
     $this->assertEqual(3, count($_islandora_derivative_test_derivative_functions), 'Expected 3 derivative functions when there is no source_dsid, got ' . count($_islandora_derivative_test_derivative_functions) . '.');
   }
@@ -226,7 +230,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
   public function testNoSourceDSIDForce() {
     global $_islandora_derivative_test_derivative_functions;
     $_islandora_derivative_test_derivative_functions = array();
-    $this->constructBaseObject();
+    $object = $this->constructBaseObject();
+    $this->constructSOMEWEIRDDATASTREAMDatastream($object);
     $object = islandora_object_load($this->pid);
     islandora_do_derivatives($object, array(
       'force' => TRUE,
@@ -236,6 +241,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
       'RELS-EXT',
       'OBJ',
       'DERIV',
+      'SOMEWEIRDDATASTREAM',
+      'STANLEY',
       'NOSOURCE',
     ));
     $this->assertEqual(3, count($_islandora_derivative_test_derivative_functions), 'Expected 3 derivative functions when there is no source_dsid, got ' . count($_islandora_derivative_test_derivative_functions) . '.');
@@ -293,4 +300,22 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase {
     $object->ingestDatastream($ds);
     return $object;
   }
+
+  /**
+   * Helper function to construct the SWD datastream without firing hooks.
+   *
+   * @param AbstractObject $object
+   *   An AbstractObject representing a FedoraObject.
+   *
+   * @return AbstractObject
+   *   The modified AbstractObject.
+   */
+  public function constructSOMEWEIRDDATASTREAMDatastream(AbstractObject $object) {
+    $dsid = 'SOMEWEIRDDATASTREAM';
+    $ds = $object->constructDatastream($dsid);
+    $ds->label = 'Test';
+    $ds->content = 'Omnomnom';
+    $object->ingestDatastream($ds);
+    return $object;
+  }
 }

tests/islandora_derivatives_test.module

@@ -23,7 +23,7 @@ function islandora_derivatives_test_some_cmodel_islandora_derivative() {
       'destination_dsid' => 'STANLEY',
       'weight' => '-1',
       'function' => array(
-        'islandora_derivatives_test_create_some_weird_datastream',
+        'islandora_derivatives_test_create_stanley_datastream',
       ),
     ),
     array(
@@ -81,17 +81,20 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje
 }
 
 /**
- * Stub function that used only for datastream filtering counts.
+ * Creates the STANLEY datastream for use in testing.
  *
  * @param AbstractObject $object
  *   An AbstractObject representing a Fedora object.
  * @param bool $force
  *   Whether the derivatives are being forcefully generated or not.
  */
-function islandora_derivatives_test_create_some_weird_datastream(AbstractObject $object, $force = FALSE) {
+function islandora_derivatives_test_create_stanley_datastream(AbstractObject $object, $force = FALSE) {
   global $_islandora_derivative_test_derivative_functions;
   // Add to the global that we got to this function.
   $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_create_some_weird_datastream';
+  if (!isset($object['STANLEY']) || (isset($object['STANLEY']) && $force === TRUE)) {
+    islandora_derivatives_test_add_datastream($object, 'STANLEY', 'yum');
+  }
 }
 
 /**