fix(iis): dedupe preConditions across WebP/AVIF rewrite rules

Tests/Integration/classes/WriteFile/AbstractIISDirConfFile/RewriteRulesPreConditionsTest.php

@@ -0,0 +1,189 @@
+<?php
+declare( strict_types=1 );
+
+namespace Imagify\Tests\Integration\classes\WriteFile\AbstractIISDirConfFile;
+
+use Imagify\Avif\RewriteRules\IIS as AvifRewriteIIS;
+use Imagify\Tests\Integration\TestCase;
+use Imagify\Webp\RewriteRules\IIS as WebpRewriteIIS;
+
+/**
+ * Integration tests for the <preConditions> singleton handling in IIS rewrite rules.
+ *
+ * IIS allows exactly one <preConditions> collection under
+ * /configuration/system.webServer/rewrite/outboundRules. Both the WebP and AVIF
+ * rewrite-rules writers emit their own <preCondition> (IsWebp / IsAvif); they must
+ * land in a single shared container and a remove() of one format must not wipe the
+ * other (issue #1180).
+ *
+ * @covers \Imagify\WriteFile\AbstractIISDirConfFile::insert_contents
+ * @covers \Imagify\Webp\RewriteRules\IIS::get_raw_new_contents
+ * @covers \Imagify\Avif\RewriteRules\IIS::get_raw_new_contents
+ * @group  WriteFile
+ * @group  IIS
+ */
+class RewriteRulesPreConditionsTest extends TestCase {
+	protected $useApi = false;
+
+	/**
+	 * Absolute path to the temporary web.config file under test.
+	 *
+	 * @var string
+	 */
+	private $config_path;
+
+	public function set_up() {
+		parent::set_up();
+
+		// saveDomDocument() lives in wp-admin/includes/misc.php.
+		if ( ! function_exists( 'saveDomDocument' ) ) {
+			require_once ABSPATH . 'wp-admin/includes/misc.php';
+		}
+
+		$this->config_path = wp_tempnam( 'imagify-web-config' );
+
+		add_filter( 'imagify_dir_conf_path', [ $this, 'filter_conf_path' ] );
+	}
+
+	public function tear_down() {
+		remove_filter( 'imagify_dir_conf_path', [ $this, 'filter_conf_path' ] );
+
+		if ( $this->config_path && file_exists( $this->config_path ) ) {
+			unlink( $this->config_path );
+		}
+
+		parent::tear_down();
+	}
+
+	/**
+	 * Redirect the conf writers to our temp file.
+	 *
+	 * @return string
+	 */
+	public function filter_conf_path() {
+		return $this->config_path;
+	}
+
+	/**
+	 * Seed the temp web.config with the given XML string.
+	 *
+	 * @param string $xml Raw web.config content.
+	 */
+	private function seed( string $xml ) {
+		file_put_contents( $this->config_path, $xml );
+	}
+
+	/**
+	 * Load the temp web.config into a DOMXPath for assertions.
+	 *
+	 * @return \DOMXPath
+	 */
+	private function xpath(): \DOMXPath {
+		$doc                     = new \DOMDocument();
+		$doc->preserveWhiteSpace = false;
+		$doc->load( $this->config_path );
+
+		return new \DOMXPath( $doc );
+	}
+
+	/**
+	 * Assert the number of <preConditions> collections under outboundRules.
+	 *
+	 * @param int $expected Expected count.
+	 */
+	private function assertPreConditionsCount( int $expected ) {
+		$this->assertSame(
+			$expected,
+			$this->xpath()->query( '/configuration/system.webServer/rewrite/outboundRules/preConditions' )->length,
+			'Unexpected number of <preConditions> collections.'
+		);
+	}
+
+	/**
+	 * Assert the number of <preCondition> entries for a given name.
+	 *
+	 * @param string $name     preCondition name (IsWebp / IsAvif).
+	 * @param int    $expected Expected count.
+	 */
+	private function assertPreConditionCount( string $name, int $expected ) {
+		$this->assertSame(
+			$expected,
+			$this->xpath()->query( "/configuration/system.webServer/rewrite/outboundRules/preConditions/preCondition[@name='" . $name . "']" )->length,
+			"Unexpected number of {$name} preCondition entries."
+		);
+	}
+
+	/**
+	 * Both formats added must share a single <preConditions> with one entry each.
+	 */
+	public function testBothFormatsShareSinglePreConditionsContainer() {
+		$this->seed( '<configuration><system.webServer><rewrite><rules/><outboundRules/></rewrite></system.webServer></configuration>' );
+
+		( new WebpRewriteIIS() )->add();
+		( new AvifRewriteIIS() )->add();
+
+		$this->assertPreConditionsCount( 1 );
+		$this->assertPreConditionCount( 'IsWebp', 1 );
+		$this->assertPreConditionCount( 'IsAvif', 1 );
+	}
+
+	/**
+	 * Adding the same format twice must stay idempotent (one entry, one container).
+	 */
+	public function testDoubleAddIsIdempotent() {
+		$this->seed( '<configuration><system.webServer><rewrite><rules/><outboundRules/></rewrite></system.webServer></configuration>' );
+
+		$webp = new WebpRewriteIIS();
+		$webp->add();
+		$webp->add();
+
+		$this->assertPreConditionsCount( 1 );
+		$this->assertPreConditionCount( 'IsWebp', 1 );
+	}
+
+	/**
+	 * Removing WebP must keep the AVIF preCondition and its container.
+	 */
+	public function testRemoveWebpKeepsAvifPreCondition() {
+		$this->seed( '<configuration><system.webServer><rewrite><rules/><outboundRules/></rewrite></system.webServer></configuration>' );
+
+		( new WebpRewriteIIS() )->add();
+		( new AvifRewriteIIS() )->add();
+		( new WebpRewriteIIS() )->remove();
+
+		$this->assertPreConditionsCount( 1 );
+		$this->assertPreConditionCount( 'IsWebp', 0 );
+		$this->assertPreConditionCount( 'IsAvif', 1 );
+	}
+
+	/**
+	 * Removing AVIF must keep the WebP preCondition and its container.
+	 */
+	public function testRemoveAvifKeepsWebpPreCondition() {
+		$this->seed( '<configuration><system.webServer><rewrite><rules/><outboundRules/></rewrite></system.webServer></configuration>' );
+
+		( new WebpRewriteIIS() )->add();
+		( new AvifRewriteIIS() )->add();
+		( new AvifRewriteIIS() )->remove();
+
+		$this->assertPreConditionsCount( 1 );
+		$this->assertPreConditionCount( 'IsAvif', 0 );
+		$this->assertPreConditionCount( 'IsWebp', 1 );
+	}
+
+	/**
+	 * Removing the last preCondition must drop the now-empty container entirely.
+	 */
+	public function testRemoveLastPreConditionDropsEmptyContainer() {
+		$this->seed( '<configuration><system.webServer><rewrite><rules/><outboundRules/></rewrite></system.webServer></configuration>' );
+
+		( new WebpRewriteIIS() )->add();
+		( new AvifRewriteIIS() )->add();
+		( new WebpRewriteIIS() )->remove();
+		( new AvifRewriteIIS() )->remove();
+
+		$this->assertPreConditionsCount( 0 );
+		$this->assertPreConditionCount( 'IsWebp', 0 );
+		$this->assertPreConditionCount( 'IsAvif', 0 );
+	}
+}

Tests/Integration/inc/classes/ImagifyUser/getError.php

@@ -17,6 +17,10 @@ class Test_GetError extends TestCase {
 	 * Test \Imagify\User\User->get_error() should return false when succesfully fetched user account data.
 	 */
 	public function testShouldReturnFalseWhenFetchedUserData() {
+		if ( ! $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) ) {
+			$this->markTestSkipped( 'IMAGIFY_TESTS_API_KEY not set; requires a valid live API key.' );
+		}
+
 		update_imagify_option( 'api_key', $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) );
 
 		// Verify the static $user property is null.

Tests/Integration/inc/classes/ImagifyUser/getPercentConsumedQuota.php

@@ -40,6 +40,10 @@ public function testShouldReturnZeroWhenCouldNotFetchUserData() {
 	}
 
 	public function testShouldReturnQuotaWhenFetchedUserData() {
+		if ( ! $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) ) {
+			$this->markTestSkipped( 'IMAGIFY_TESTS_API_KEY not set; requires a valid live API key.' );
+		}
+
 		update_imagify_option( 'api_key', $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) );
 
 		// Verify the static $user property is null.

Tests/Integration/inc/classes/ImagifyUser/isOverQuota.php

@@ -38,6 +38,10 @@ public function testShouldReturnFalseWhenCouldNotFetchUserData() {
 	}
 
 	public function testShouldReturnFalseWhenPaidAccount() {
+		if ( ! $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) ) {
+			$this->markTestSkipped( 'IMAGIFY_TESTS_API_KEY not set; requires a valid live API key.' );
+		}
+
 		update_imagify_option( 'api_key', $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) );
 
 		// Verify the static $user property is null.
@@ -56,6 +60,10 @@ public function testShouldReturnFalseWhenPaidAccount() {
 	}
 
 	public function testShouldReturnFalseWhenFreeNotOverQuota() {
+		if ( ! $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) ) {
+			$this->markTestSkipped( 'IMAGIFY_TESTS_API_KEY not set; requires a valid live API key.' );
+		}
+
 		update_imagify_option( 'api_key', $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) );
 
 		// Verify the static $user property is null.
@@ -73,6 +81,10 @@ public function testShouldReturnFalseWhenFreeNotOverQuota() {
 	}
 
 	public function testShouldReturnTrueWhenFreeOverQuota() {
+		if ( ! $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) ) {
+			$this->markTestSkipped( 'IMAGIFY_TESTS_API_KEY not set; requires a valid live API key.' );
+		}
+
 		update_imagify_option( 'api_key', $this->getApiCredential( 'IMAGIFY_TESTS_API_KEY' ) );
 
 		// Verify the static $user property is null.

classes/Avif/RewriteRules/IIS.php

@@ -17,6 +17,16 @@ class IIS extends AbstractIISDirConfFile {
 	 */
 	const TAG_NAME = 'Imagify: rewrite rules for avif';
 
+	/**
+	 * Names of the <preCondition> entries this class owns inside the shared
+	 * <preConditions> collection.
+	 *
+	 * @return array
+	 */
+	protected function get_owned_precondition_names(): array {
+		return [ 'IsAvif' ];
+	}
+
 	/**
 	 * Get unfiltered new contents to write into the file.
 	 *
@@ -50,11 +60,11 @@ protected function get_raw_new_contents() {
 	<match serverVariable="RESPONSE_Vary" pattern=".*" />
 	<action type="Rewrite" value="Accept"/>
 </rule>
-<preConditions name="' . esc_attr( static::TAG_NAME ) . ' 4">
-	<preCondition name="IsAvif">
-		<add input="{ACCEPTS_AVIF}" pattern="true" ignoreCase="false" />
-	</preCondition>
-</preConditions>'
+
+<!-- @parent /configuration/system.webServer/rewrite/outboundRules/preConditions -->
+<preCondition name="IsAvif">
+	<add input="{ACCEPTS_AVIF}" pattern="true" ignoreCase="false" />
+</preCondition>'
 		);
 	}
 }

classes/Webp/RewriteRules/IIS.php

@@ -20,6 +20,16 @@ class IIS extends AbstractIISDirConfFile {
 	 */
 	const TAG_NAME = 'Imagify: rewrite rules for webp';
 
+	/**
+	 * Names of the <preCondition> entries this class owns inside the shared
+	 * <preConditions> collection.
+	 *
+	 * @return array
+	 */
+	protected function get_owned_precondition_names(): array {
+		return [ 'IsWebp' ];
+	}
+
 	/**
 	 * Get unfiltered new contents to write into the file.
 	 *
@@ -54,11 +64,11 @@ protected function get_raw_new_contents() {
 	<match serverVariable="RESPONSE_Vary" pattern=".*" />
 	<action type="Rewrite" value="Accept"/>
 </rule>
-<preConditions name="' . esc_attr( static::TAG_NAME ) . ' 4">
-	<preCondition name="IsWebp">
-		<add input="{ACCEPTS_WEBP}" pattern="true" ignoreCase="false" />
-	</preCondition>
-</preConditions>'
+
+<!-- @parent /configuration/system.webServer/rewrite/outboundRules/preConditions -->
+<preCondition name="IsWebp">
+	<add input="{ACCEPTS_WEBP}" pattern="true" ignoreCase="false" />
+</preCondition>'
 		);
 	}
 }

classes/WriteFile/AbstractIISDirConfFile.php

@@ -32,7 +32,7 @@ protected function insert_contents( $new_contents ) {
 		$marker = static::TAG_NAME;
 		$xpath  = new \DOMXPath( $doc );
 
-		// Remove previous rules.
+		// Remove previous rules marked with this class' tag.
 		$old_nodes = $xpath->query( ".//*[starts-with(@name,'$marker')]" );
 
 		if ( $old_nodes->length > 0 ) {
@@ -41,8 +41,24 @@ protected function insert_contents( $new_contents ) {
 			}
 		}
 
+		// Remove the <preCondition> nodes this class owns. IIS allows only one
+		// <preConditions> collection under outboundRules, so WebP and AVIF share
+		// it. Each owns a distinct inner <preCondition> (IsWebp / IsAvif); strip
+		// ours by name before re-adding so siblings survive (issue #1180).
+		foreach ( $this->get_owned_precondition_names() as $precondition_name ) {
+			$old_preconditions = $xpath->query( ".//preConditions/preCondition[@name='$precondition_name']" );
+
+			if ( $old_preconditions && $old_preconditions->length > 0 ) {
+				foreach ( $old_preconditions as $old_precondition ) {
+					$old_precondition->parentNode->removeChild( $old_precondition );
+				}
+			}
+		}
+
 		// No new contents? Stop here.
 		if ( ! $new_contents ) {
+			$this->cleanup_empty_preconditions( $xpath );
+
 			return $this->put_file_contents( $doc );
 		}
 
@@ -64,6 +80,8 @@ protected function insert_contents( $new_contents ) {
 			$this->get_node( $doc, $xpath, $path, $fragment );
 		}
 
+		$this->cleanup_empty_preconditions( $xpath );
+
 		return $this->put_file_contents( $doc );
 	}
 
@@ -80,6 +98,39 @@ protected function get_raw_file_path() {
 		return $this->filesystem->get_site_root() . 'web.config';
 	}
 
+	/**
+	 * Get the <preCondition> names this class owns inside the shared
+	 * <preConditions> collection.
+	 *
+	 * RewriteRules subclasses that emit outbound rules override this to declare
+	 * the preCondition name they own (IsWebp / IsAvif), so insert_contents() can
+	 * strip only that entry and leave siblings intact. The MIME-type classes
+	 * inherit the empty default and are unaffected.
+	 *
+	 * @return array
+	 */
+	protected function get_owned_precondition_names(): array {
+		return [];
+	}
+
+	/**
+	 * Remove <preConditions> collections left without any <preCondition> child.
+	 *
+	 * IIS treats <preConditions> as a singleton; an empty one is harmless but
+	 * pointless, so drop it after an add or a remove pass.
+	 *
+	 * @param \DOMXPath $xpath A \DOMXPath element.
+	 */
+	protected function cleanup_empty_preconditions( $xpath ) {
+		$containers = $xpath->query( './/preConditions[not(preCondition)]' );
+
+		if ( $containers && $containers->length > 0 ) {
+			foreach ( $containers as $container ) {
+				$container->parentNode->removeChild( $container );
+			}
+		}
+	}
+
 	/** ----------------------------------------------------------------------------------------- */
 	/** OTHER TOOLS ============================================================================= */
 	/** ----------------------------------------------------------------------------------------- */