Merge branch 'issue/762' into release/2.4.2

composer.json

@@ -4,7 +4,7 @@
     },
     "require-dev": {
         "jdgrimes/wp-filesystem-mock": "^0.1.2",
-        "jdgrimes/wpppb": "^0.3.0",
+        "jdgrimes/wpppb": "^0.3.5",
         "xrstf/composer-php52": "^1.0",
         "jdgrimes/wp-http-testcase": "^1.3"
     },

src/classes/hook/extension/conditions.php

@@ -175,6 +175,7 @@ protected function validate_condition( $settings ) {
 			return false;
 		}
 
+		/** @var WordPoints_Hook_ConditionI $condition */
 		$condition = $this->conditions->get( $data_type, $settings['type'] );
 
 		if ( ! $condition ) {

src/classes/hook/extension/periods.php

@@ -344,11 +344,15 @@ protected function get_period_by_reaction(
 		$cache_key = wp_json_encode( $reaction_guid ) . "-{$signature}-{$this->action_type}";
 
 		// Before we run the query, we try to lookup the ID in the cache.
-		$period_id = wp_cache_get( $cache_key, 'wordpoints_hook_period_ids_by_reaction' );
+		$period_id = wp_cache_get( $cache_key, 'wordpoints_hook_period_ids_by_reaction', false, $found );
 
 		// If we found it, we can retrieve the period by ID instead.
 		if ( $period_id ) {
 			return $this->get_period( $period_id );
+		} elseif ( $found ) {
+			// If the cache was set to false, then we have already checked if there
+			// are any hits for this period, and haven't found any.
+			return false;
 		}
 
 		global $wpdb;
@@ -357,7 +361,7 @@ protected function get_period_by_reaction(
 		$period = $wpdb->get_row(
 			$wpdb->prepare(
 				"
-					SELECT *, `period`.`id` AS `id`
+					SELECT `period`.`id`, `hit`.`date`, `hit`.`id` AS `hit_id`
 					FROM `{$wpdb->wordpoints_hook_periods}` AS `period`
 					INNER JOIN `{$wpdb->wordpoints_hook_hits}` AS `hit`
 						ON `hit`.`id` = period.`hit_id`
@@ -380,9 +384,21 @@ protected function get_period_by_reaction(
 		);
 
 		if ( ! $period ) {
+
+			// Cache the result anyway, so we know that no periods have been created
+			// matching this yet, and can avoid re-running this query.
+			wp_cache_set( $cache_key, false, 'wordpoints_hook_period_ids_by_reaction' );
+
 			return false;
 		}
 
+		$period->signature           = $signature;
+		$period->reaction_mode       = $reaction_guid['mode'];
+		$period->reaction_store      = $reaction_guid['store'];
+		$period->reaction_context_id = wp_json_encode( $reaction_guid['context_id'] );
+		$period->reaction_id         = $reaction_guid['id'];
+		$period->action_type         = $this->action_type;
+
 		wp_cache_set( $cache_key, $period->id, 'wordpoints_hook_period_ids_by_reaction' );
 		wp_cache_set( $period->id, $period, 'wordpoints_hook_periods' );
 

src/classes/hook/reaction/validator.php

@@ -174,6 +174,7 @@ public function validate() {
 			$this->event_args = new WordPoints_Hook_Event_Args( $event_args );
 			$this->event_args->set_validator( $this );
 
+			/** @var WordPoints_Hook_ReactorI $reactor */
 			$reactor = $reactors->get( $this->reactor_slug );
 
 			$this->settings = $reactor->validate_settings( $this->settings, $this, $this->event_args );

src/classes/hook/reactor/target/validatori.php

@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * Target validator hook reactor interface..
+ *
+ * @package WordPoints
+ * @since   2.4.2
+ */
+
+/**
+ * Interface for hook reactors that need to validate the target.
+ *
+ * Sometimes a reactor cannot hit just any target. Implementing this interface gives
+ * the option of validating the target before a hit is attempted. Note that this may
+ * happen before it is determined whether or not a hit should actually occur.
+ *
+ * @since 2.4.2
+ */
+interface WordPoints_Hook_Reactor_Target_ValidatorI {
+
+	/**
+	 * Checks if the reactor can hit the given target.
+	 *
+	 * @since 2.4.2
+	 *
+	 * @param WordPoints_EntityishI $target The target to be hit.
+	 * @param WordPoints_Hook_Fire  $fire   The fire the target would be hit by.
+	 *
+	 * @return bool Whether the target can be hit by the reactor or not.
+	 */
+	public function can_hit( WordPoints_EntityishI $target, WordPoints_Hook_Fire $fire );
+}
+
+// EOF

src/classes/hooks.php

@@ -225,6 +225,17 @@ protected function fire_reaction( $fire ) {
 
 		unset( $validator );
 
+		if ( $reactor instanceof WordPoints_Hook_Reactor_Target_ValidatorI ) {
+
+			$target = $fire->event_args->get_from_hierarchy(
+				$fire->reaction->get_meta( 'target' )
+			);
+
+			if ( ! $reactor->can_hit( $target, $fire ) ) {
+				return;
+			}
+		}
+
 		/** @var WordPoints_Hook_ExtensionI[] $extensions */
 		$extensions = $this->get_sub_app( 'extensions' )->get_all();
 

src/classes/index.php

@@ -19,6 +19,7 @@
 	'wordpoints_hook_ui_script_data_provideri' => 'hook/ui/script/data/provideri.php',
 	'wordpoints_hook_settingsi' => 'hook/settingsi.php',
 	'wordpoints_hook_reactori' => 'hook/reactori.php',
+	'wordpoints_hook_reactor_target_validatori' => 'hook/reactor/target/validatori.php',
 	'wordpoints_hook_reactioni' => 'hook/reactioni.php',
 	'wordpoints_hook_reaction_storei' => 'hook/reaction/storei.php',
 	'wordpoints_hook_extensioni' => 'hook/extensioni.php',

src/components/points/classes/hook/reactor.php

@@ -12,7 +12,9 @@
  *
  * @since 2.1.0
  */
-class WordPoints_Points_Hook_Reactor extends WordPoints_Hook_Reactor {
+class WordPoints_Points_Hook_Reactor
+	extends WordPoints_Hook_Reactor
+	implements WordPoints_Hook_Reactor_Target_ValidatorI {
 
 	/**
 	 * @since 2.1.0
@@ -122,6 +124,25 @@ public function update_settings(
 		$reaction->update_meta( 'log_text', $settings['log_text'] );
 	}
 
+	/**
+	 * @since 2.4.2
+	 */
+	public function can_hit(
+		WordPoints_EntityishI $target,
+		WordPoints_Hook_Fire $fire
+	) {
+
+		if ( 'toggle_off' === $fire->action_type ) {
+			return true;
+		}
+
+		if ( ! $target instanceof WordPoints_Entity || ! $target->get_the_id() ) {
+			return false;
+		}
+
+		return true;
+	}
+
 	/**
 	 * @since 2.1.0
 	 */

tests/phpunit/tests/classes/hook/extension/periods.php

@@ -1571,6 +1571,98 @@ public function test_should_hit_uncached() {
 		$this->assertCount( 1, $test_reactor->hits );
 	}
 
+	/**
+	 * Test caching when there are no matching periods in the DB.
+	 *
+	 * @since 2.4.2
+	 */
+	public function test_should_hit_cache_no_hit() {
+
+		$this->mock_apps();
+
+		$hooks = wordpoints_hooks();
+
+		$extensions = $hooks->get_sub_app( 'extensions' );
+		$extensions->register( $this->extension_slug, $this->extension_class );
+		$extensions->register( 'blocker', 'WordPoints_PHPUnit_Mock_Hook_Extension' );
+
+		// Get this first so that it will run before the blocker extension.
+		$extensions->get( $this->extension_slug );
+
+		// Set up the blocker extension.
+		/** @var WordPoints_PHPUnit_Mock_Hook_Extension $blocker */
+		$blocker = $extensions->get( 'blocker' );
+
+		$blocker->should_hit = false;
+
+		$settings = array(
+			$this->extension_slug => array( 'test_fire' => array( array( 'length' => DAY_IN_SECONDS ) ) ),
+			'target'              => array( 'test_entity' ),
+		);
+
+		$reaction = $this->factory->wordpoints->hook_reaction->create( $settings );
+
+		$this->assertIsReaction( $reaction );
+
+		$event_args = new WordPoints_Hook_Event_Args(
+			array( new WordPoints_Hook_Arg( 'test_entity' ) )
+		);
+
+		$get_period_queries = new WordPoints_PHPUnit_Mock_Filter();
+		$get_period_queries->count_callback = array( $this, 'is_get_period_query' );
+		add_filter( 'query', array( $get_period_queries, 'filter' ) );
+
+		$get_by_reaction_queries = new WordPoints_PHPUnit_Mock_Filter();
+		$get_by_reaction_queries->count_callback = array( $this, 'is_get_period_by_reaction_query' );
+		add_filter( 'query', array( $get_by_reaction_queries, 'filter' ) );
+
+		// Run once, to show normal behavior.
+		$hooks->fire( 'test_event', $event_args, 'test_fire' );
+
+		$this->assertSame( 0, $get_period_queries->call_count );
+		$this->assertSame( 1, $get_by_reaction_queries->call_count );
+
+		$test_reactor = $hooks->get_sub_app( 'reactors' )->get( 'test_reactor' );
+
+		$this->assertCount( 0, $test_reactor->hits );
+
+		// Flush the cache and run again, to demonstrate un-cached behavior.
+		$this->flush_cache();
+
+		$hooks->fire( 'test_event', $event_args, 'test_fire' );
+
+		$this->assertSame( 0, $get_period_queries->call_count );
+		$this->assertSame( 2, $get_by_reaction_queries->call_count );
+
+		$this->assertCount( 0, $test_reactor->hits );
+
+		// Now run again, to show the cache's effect.
+		$hooks->fire( 'test_event', $event_args, 'test_fire' );
+
+		$this->assertSame( 0, $get_period_queries->call_count );
+		$this->assertSame( 2, $get_by_reaction_queries->call_count );
+
+		$this->assertCount( 0, $test_reactor->hits );
+
+		// Turn the blocker off, to show what happens when a hit eventually occurs.
+		$blocker->should_hit = true;
+
+		$hooks->fire( 'test_event', $event_args, 'test_fire' );
+
+		$this->assertSame( 0, $get_period_queries->call_count );
+		$this->assertSame( 2, $get_by_reaction_queries->call_count );
+
+		$this->assertCount( 1, $test_reactor->hits );
+
+		// Finally, run again, to show that the cache was updated correctly.
+		$hooks->fire( 'test_event', $event_args, 'test_fire' );
+
+		$this->assertSame( 1, $get_period_queries->call_count );
+		$this->assertSame( 2, $get_by_reaction_queries->call_count );
+
+		$this->assertCount( 1, $test_reactor->hits );
+	}
+
 	/**
 	 * Travel forward in time by modifying the hit time of a period.
 	 *
@@ -1640,7 +1732,7 @@ public function is_get_period_by_reaction_query( $sql ) {
 
 		return false !== strpos(
 			$sql
-			, "SELECT *, `period`.`id` AS `id`
+			, "SELECT `period`.`id`, `hit`.`date`, `hit`.`id` AS `hit_id`
 					FROM `{$wpdb->wordpoints_hook_periods}` AS `period`
 					INNER JOIN `{$wpdb->wordpoints_hook_hits}` AS `hit`
 						ON `hit`.`id` = period.`hit_id`