Merge pull request #1980 from Automattic/update/private-files-attachm…

…ent-purge

Private Files: PURGE attachment URLs on post update

files/acl/acl.php

@@ -39,6 +39,8 @@ function maybe_load_restrictions() {
 		require_once( __DIR__ . '/restrict-unpublished-files.php' );
 
 		add_filter( 'vip_files_acl_file_visibility', __NAMESPACE__ . '\Restrict_Unpublished_Files\check_file_visibility', 10, 2 );
+		// Purge attachments for posts for better cacheability
+		add_filter( 'wpcom_vip_cache_purge_urls', __NAMESPACE__ . '\Restrict_Unpublished_Files\purge_attachments_for_post', 10, 2 );
 	}
 }
 

files/acl/restrict-unpublished-files.php

@@ -12,7 +12,6 @@
 
 const CACHE_GROUP = 'vip-files-acl';
 
-
 /**
  * Given a path determine whether the file is private or public
  *
@@ -105,3 +104,44 @@ function get_attachment_id_from_file_path( $path ) {
 
 	return $attachment_id;
 }
+
+/**
+ * PURGE all attachments for a given post.
+ *
+ * This is to allow us to propagate visibility changes ASAP to the edges
+ * and also to allow us to cache public files for much longer (which comes
+ * some performance benefits).
+ *
+ * @param array urls Array of URLs to be purged
+ * @param int post_id The ID of the post for which we're purging URLs
+ *
+ * @return array An array of URLs to be purged
+ */
+function purge_attachments_for_post( $urls, $post_id ) {
+	$post = get_post( $post_id );
+
+	if ( empty( $post ) ) {
+		return $urls;
+	}
+
+	$attachment_ids = get_posts( [
+		'post_parent'    => $post->ID,
+		'post_type'      => 'attachment',
+		'posts_per_page' => 250,            // Set a reasonably high limit (instead of -1 as default)
+		'orderby'        => 'ID',           // For performance (instead of `date` as default)
+		'order'          => 'ASC',
+		'fields'         => 'ids',
+		// phpcs:ignore WordPressVIPMinimum.Performance.WPQueryParams.SuppressFiltersTrue
+		'suppress_filters' => true,
+	] );
+
+	if ( empty( $attachment_ids ) ) {
+		return $urls;
+	}
+
+	foreach ( $attachment_ids as $attachment_id ) {
+		$urls[] = wp_get_attachment_url( $attachment_id );
+	}
+
+	return $urls;
+}

tests/files/acl/test-acl.php

@@ -40,7 +40,8 @@ public function test__maybe_load_restrictions__constant_and_restrict_all_option(
 
 		maybe_load_restrictions();
 
-		$this->assertEquals( 10, has_filter( 'vip_files_acl_file_visibility', 'Automattic\VIP\Files\Acl\Restrict_All_Files\check_file_visibility' ), 'File visibility filter has no callbacks attached' );
+		$this->assertTrue( function_exists( 'Automattic\VIP\Files\Acl\Restrict_All_Files\check_file_visibility' ), 'Restrict_All_Files\check_file_visibility() is not defined; restrict-all-files.php may not have been require-d correctly' );
+		$this->assertEquals( 10, has_filter( 'vip_files_acl_file_visibility', 'Automattic\VIP\Files\Acl\Restrict_All_Files\check_file_visibility' ), 'vip_files_acl_file_visibility filter does not have the correct callback attached' );
 	}
 
 	/**
@@ -53,7 +54,9 @@ public function test__maybe_load_restrictions__constant_and_restrict_unpublished
 
 		maybe_load_restrictions();
 
-		$this->assertEquals( 10, has_filter( 'vip_files_acl_file_visibility', 'Automattic\VIP\Files\Acl\Restrict_Unpublished_Files\check_file_visibility' ), 'File visibility filter has no callbacks attached' );
+		$this->assertTrue( function_exists( 'Automattic\VIP\Files\Acl\Restrict_Unpublished_Files\check_file_visibility' ), 'Restrict_Unpublished_Files\check_file_visibility() is not defined; restrict-unpublished-files.php may not have been require-d correctly' );
+		$this->assertEquals( 10, has_filter( 'vip_files_acl_file_visibility', 'Automattic\VIP\Files\Acl\Restrict_Unpublished_Files\check_file_visibility' ), 'vip_files_acl_file_visibility filter does not have correct callback attached' );
+		$this->assertEquals( 10, has_filter( 'wpcom_vip_cache_purge_urls', 'Automattic\VIP\Files\Acl\Restrict_Unpublished_Files\purge_attachments_for_post' ), 'wpcom_vip_cache_purge_urls filter does not have correct callback attached' );
 	}
 
 	/**

tests/files/acl/test-restrict-unpublished-files.php

@@ -222,5 +222,62 @@ public function test__get_attachment_id_from_file_path__attachment_multiple_resu
 	private function strip_wpcontent_uploads( $path ) {
 		return substr( $path, strlen( '/wp-content/uploads/' ) );
 	}
+
+	public function test__purge_attachments_for_post__invalid_post() {
+		// Input and output are the same; no change
+		$input_urls = [
+			'https://example.com',
+		];
+		$expected_urls = [
+			'https://example.com',
+		];
+
+		// Set a highly unlikely post ID
+		$test_post_id = PHP_INT_MAX;
+
+		$actual_urls = purge_attachments_for_post( $input_urls, $test_post_id );
+
+		$this->assertEquals( $expected_urls, $actual_urls );
+	}
+
+	public function test__purge_attachments_for_post__post_with_no_attachments() {
+		// Input and output are the same; no change
+		$input_urls = [
+			'https://example.com',
+		];
+		$expected_urls = [
+			'https://example.com',
+		];
+
+		// No attachments for post
+		$test_post_id = $this->factory->post->create();
+
+		$actual_urls = purge_attachments_for_post( $input_urls, $test_post_id );
+
+		$this->assertEquals( $expected_urls, $actual_urls );
+	}
+
+	public function test__purge_attachments_for_post__post_with_some_attachments() {
+		$input_urls = [
+			'https://example.com',
+		];
+
+		$test_post_id = $this->factory->post->create();
+		$attachment_id_1 = $this->factory->attachment->create_upload_object( self::TEST_IMAGE_PATH, $test_post_id );
+		$attachment_id_2 = $this->factory->attachment->create_upload_object( self::TEST_IMAGE_PATH, $test_post_id );
+		$attachment_id_3 = $this->factory->attachment->create_upload_object( self::TEST_IMAGE_PATH, $test_post_id );
+
+		// Output should include new attachment URLs
+		$expected_urls = [
+			'https://example.com',
+			wp_get_attachment_url( $attachment_id_1 ),
+			wp_get_attachment_url( $attachment_id_2 ),
+			wp_get_attachment_url( $attachment_id_3 ),
+		];
+
+		$actual_urls = purge_attachments_for_post( $input_urls, $test_post_id );
+
+		$this->assertEquals( $expected_urls, $actual_urls );
+	}
 }