From a61421071630d38793c26e49c9df4326849e0cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Thu, 31 May 2012 13:23:14 +0300 Subject: [PATCH 1/8] test for issue 28: mtime based cache id problems --- min_unit_tests/test_issue-28.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 min_unit_tests/test_issue-28.php diff --git a/min_unit_tests/test_issue-28.php b/min_unit_tests/test_issue-28.php new file mode 100644 index 0000000..837f4bc --- /dev/null +++ b/min_unit_tests/test_issue-28.php @@ -0,0 +1,28 @@ +lastModified = $fiveSecondsAgo; + + $file2 = new stdClass(); + $file2->lastModified = $fiveSecondsAgo; + + $m1 = Minify_HTML_Helper::getLastModified(array($file1)); + error_log("last modified: $m1"); + + $m2 = Minify_HTML_Helper::getLastModified(array($file1, $file2)); + error_log("last modified: $m2"); + + assertTrue($m1 !== $m2, "mtime of group with one and two files should not be same"); +} + +test_bug28(); -- 1.7.10 From b80ddf56aa16b12f935e355ba8f0cb69c92f86ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Thu, 31 May 2012 13:36:23 +0300 Subject: [PATCH 2/8] load test_issue-28.php into all tests --- min_unit_tests/test_all.php | 1 + 1 file changed, 1 insertion(+) diff --git a/min_unit_tests/test_all.php b/min_unit_tests/test_all.php index ff26b51..c5af04c 100644 --- a/min_unit_tests/test_all.php +++ b/min_unit_tests/test_all.php @@ -18,3 +18,4 @@ require 'test_HTTP_ConditionalGet.php'; require 'test_JSMin.php'; require 'test_environment.php'; +require 'test_issue-28.php'; -- 1.7.10 From 99b264410340c58f8be42462b16221b07da6465e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Thu, 31 May 2012 18:25:48 +0300 Subject: [PATCH 3/8] testing Minify_HTML_Helper::getUri($group) now. issue #28 --- .../_test_files/issue-28_groupsConfig.php | 9 +++ min_unit_tests/test_issue-28.php | 65 ++++++++++++++++---- 2 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 min_unit_tests/_test_files/issue-28_groupsConfig.php diff --git a/min_unit_tests/_test_files/issue-28_groupsConfig.php b/min_unit_tests/_test_files/issue-28_groupsConfig.php new file mode 100644 index 0000000..0d8a845 --- /dev/null +++ b/min_unit_tests/_test_files/issue-28_groupsConfig.php @@ -0,0 +1,9 @@ +lastModified = $fiveSecondsAgo; + $file1 = new stdClass(); + $file1->lastModified = $fiveSecondsAgo; - $file2 = new stdClass(); - $file2->lastModified = $fiveSecondsAgo; + $file2 = new stdClass(); + $file2->lastModified = $fiveSecondsAgo; - $m1 = Minify_HTML_Helper::getLastModified(array($file1)); - error_log("last modified: $m1"); + $m1 = Minify_HTML_Helper::getLastModified(array($file1)); + error_log("last modified: $m1"); - $m2 = Minify_HTML_Helper::getLastModified(array($file1, $file2)); - error_log("last modified: $m2"); + $m2 = Minify_HTML_Helper::getLastModified(array($file1, $file2)); + error_log("last modified: $m2"); - assertTrue($m1 !== $m2, "mtime of group with one and two files should not be same"); + assertTrue($m1 !== $m2, "mtime of group with one and two files should not be same"); } -test_bug28(); +function test_bug28_geturi() +{ + $opts = array( + 'groupsConfigFile' => dirname(__FILE__) . '/_test_files/issue-28_groupsConfig.php', + ); + + $mtime = $_SERVER['REQUEST_TIME']; + + $file1 = new stdClass(); + $file1->lastModified = $mtime; + $file1->filepath = 'file1'; + + $file2 = new stdClass(); + $file2->lastModified = $mtime; + $file2->filepath = 'file2'; + + global $groupsConfig; + + $groupsConfig = array( + 'l' => array( + $file1, + ) + ); + $uri1 = Minify_HTML_Helper::getUri('l', $opts); + + $groupsConfig = array( + 'l' => array( + $file1, + $file2, + ) + ); + $uri2 = Minify_HTML_Helper::getUri('l', $opts); + + error_log("u1=[$uri1]"); + error_log("u2=[$uri2]"); + + assertTrue($uri1 !== $uri2, "uri of group with one and two files should not be same"); +} + +test_bug28_mtime(); +test_bug28_geturi(); \ No newline at end of file -- 1.7.10 From 92ba959863775e9f24e17fa3f3787a03a63c9db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Wed, 6 Jun 2012 00:58:03 +0300 Subject: [PATCH 4/8] add file path checksum calculation. issue #28 --- min/lib/Minify/HTML/Helper.php | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/min/lib/Minify/HTML/Helper.php b/min/lib/Minify/HTML/Helper.php index 807fdc9..f199ac4 100644 --- a/min/lib/Minify/HTML/Helper.php +++ b/min/lib/Minify/HTML/Helper.php @@ -69,7 +69,7 @@ public function getRawUri($farExpires = true, $debug = false) if ($debug) { $path .= "&debug"; } elseif ($farExpires && $this->_lastModified) { - $path .= "&" . $this->_lastModified; + $path .= "&" . ($this->_lastModified + $this->_filePathChecksum); } return $path; } @@ -105,11 +105,39 @@ public function setGroup($key, $checkLastModified = true) $gc = (require $this->groupsConfigFile); if (isset($gc[$key])) { $this->_lastModified = self::getLastModified($gc[$key]); + $this->_filePathChecksum = self::getFilePathCheckSum($gc[$key]); } } } } - + + /** + * @param Minify_Source[] $sources + * @return float|null the checksum + */ + public static function getFilePathCheckSum($sources) + { + $paths = array(); + foreach ((array)$sources as $source) { + if (is_object($source) && isset($source->filepath)) { + $paths[] = $source->filepath; + } elseif (is_string($source)) { + if (0 === strpos($source, '//')) { + $source = $_SERVER['DOCUMENT_ROOT'] . substr($source, 1); + } + if (is_file($source)) { + $paths[] = $source; + } + } + } + + if (!empty($paths)) { + // cast to float so arithmetic would work on 32 and 64bit PHP + return (float )sprintf("%u", crc32(serialize($paths))); + } + return null; + } + public static function getLastModified($sources, $lastModified = 0) { $max = $lastModified; @@ -130,9 +158,9 @@ public static function getLastModified($sources, $lastModified = 0) protected $_groupKey = null; // if present, URI will be like g=... protected $_filePaths = array(); + protected $_filePathChecksum = null; protected $_lastModified = null; - /** * In a given array of strings, find the character they all have at * a particular index @@ -157,11 +185,11 @@ protected static function _getCommonCharAtPos($arr, $pos) { * * @param array $paths root-relative URIs of files * @param string $minRoot root-relative URI of the "min" application + * @return string */ protected static function _getShortestUri($paths, $minRoot = '/min/') { $pos = 0; $base = ''; - $c; while (true) { $c = self::_getCommonCharAtPos($paths, $pos); if ($c === '') { -- 1.7.10 From 77248c7322d7dc88dabf075ccd2fb06244389d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 22 Jun 2012 14:26:42 +0300 Subject: [PATCH 5/8] testcase for replacing older files. issue #28 --- min_unit_tests/test_issue-28.php | 71 +++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/min_unit_tests/test_issue-28.php b/min_unit_tests/test_issue-28.php index e7645a3..ea8271c 100644 --- a/min_unit_tests/test_issue-28.php +++ b/min_unit_tests/test_issue-28.php @@ -26,6 +26,10 @@ function test_bug28_mtime() assertTrue($m1 !== $m2, "mtime of group with one and two files should not be same"); } +/** + * check that geturi differs if file count remains change, but files change + * @see https://github.com/mrclay/minify/issues/28#issuecomment-6036867 + */ function test_bug28_geturi() { $opts = array( @@ -65,5 +69,70 @@ function test_bug28_geturi() assertTrue($uri1 !== $uri2, "uri of group with one and two files should not be same"); } +/** + * tests that replacing older file (that is still older than newest file) generates new checksum + * @see https://github.com/mrclay/minify/issues/28#issuecomment-6505012 + */ +function test_bug28_geturi_older_file() +{ + $opts = array( + 'groupsConfigFile' => dirname(__FILE__) . '/_test_files/issue-28_groupsConfig.php', + ); + + $mtime = $_SERVER['REQUEST_TIME']; + + // file1 is the newest file in group + $file1 = new stdClass(); + $file1->lastModified = $mtime; + $file1->filepath = 'file1'; + + // these files are older than file1 + $file2 = new stdClass(); + $file2->lastModified = $mtime - 10; + $file2->filepath = 'file2'; + + $file3 = new stdClass(); + $file3->lastModified = $mtime - 20; + $file3->filepath = 'file2'; + + $file4 = new stdClass(); + $file4->lastModified = $mtime - 5; + $file4->filepath = 'file2'; + + global $groupsConfig; + + $groupsConfig = array( + 'l' => array( + $file1, + $file2, + ) + ); + $uri1 = Minify_HTML_Helper::getUri('l', $opts); + + $groupsConfig = array( + 'l' => array( + $file1, + $file3, + ) + ); + $uri2 = Minify_HTML_Helper::getUri('l', $opts); + + $groupsConfig = array( + 'l' => array( + $file1, + $file4, + ) + ); + $uri3 = Minify_HTML_Helper::getUri('l', $opts); + + error_log("u1=[$uri1]"); + error_log("u2=[$uri2]"); + error_log("u3=[$uri3]"); + + assertTrue($uri1 !== $uri2, "older file replaced with even older file"); + assertTrue($uri1 !== $uri3, "older file replaced with newer file (but older than newest file)"); +} + test_bug28_mtime(); -test_bug28_geturi(); \ No newline at end of file +test_bug28_geturi(); +test_bug28_geturi_older_file(); \ No newline at end of file -- 1.7.10 From 578571296f9885cebf5dcc5d215479be943e88a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 22 Jun 2012 14:44:28 +0300 Subject: [PATCH 6/8] take mtime of each file into checksum, instead of calculating maximum, issue #28 --- min/lib/Minify/HTML/Helper.php | 34 ++++++++++++++++++++++++++-------- min_unit_tests/test_issue-28.php | 2 ++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/min/lib/Minify/HTML/Helper.php b/min/lib/Minify/HTML/Helper.php index f199ac4..238598b 100644 --- a/min/lib/Minify/HTML/Helper.php +++ b/min/lib/Minify/HTML/Helper.php @@ -68,8 +68,8 @@ public function getRawUri($farExpires = true, $debug = false) } if ($debug) { $path .= "&debug"; - } elseif ($farExpires && $this->_lastModified) { - $path .= "&" . ($this->_lastModified + $this->_filePathChecksum); + } elseif ($farExpires && $this->_checksum) { + $path .= "&" . $this->_checksum; } return $path; } @@ -105,35 +105,49 @@ public function setGroup($key, $checkLastModified = true) $gc = (require $this->groupsConfigFile); if (isset($gc[$key])) { $this->_lastModified = self::getLastModified($gc[$key]); - $this->_filePathChecksum = self::getFilePathCheckSum($gc[$key]); + $this->_checksum = self::getChecksum($gc[$key]); } } } } /** + * Calculate checksum for $sources + * Take into account of files path, mtimes and create checksum from that + * The checksum is rounded to be 32bit unsigned integer to be portable for 32/64bit PHP's + * * @param Minify_Source[] $sources * @return float|null the checksum */ - public static function getFilePathCheckSum($sources) + public static function getChecksum($sources) { $paths = array(); + $mtime = (float )0; + + /** @var Minify_Source $source */ foreach ((array)$sources as $source) { - if (is_object($source) && isset($source->filepath)) { - $paths[] = $source->filepath; + if (is_object($source)) { + if (isset($source->filepath)) { + $paths[] = $source->filepath; + } + if (isset($source->lastModified)) { + $mtime += $source->lastModified; + } + } elseif (is_string($source)) { if (0 === strpos($source, '//')) { $source = $_SERVER['DOCUMENT_ROOT'] . substr($source, 1); } if (is_file($source)) { $paths[] = $source; + $mtime += filemtime($source); } } } if (!empty($paths)) { // cast to float so arithmetic would work on 32 and 64bit PHP - return (float )sprintf("%u", crc32(serialize($paths))); + return ($mtime & 0xFFFF) + (float )sprintf("%u", crc32(serialize($paths))); } return null; } @@ -158,7 +172,11 @@ public static function getLastModified($sources, $lastModified = 0) protected $_groupKey = null; // if present, URI will be like g=... protected $_filePaths = array(); - protected $_filePathChecksum = null; + /** + * Checksum of mtimes and filepaths + * @var float + */ + protected $_checksum = 0; protected $_lastModified = null; /** diff --git a/min_unit_tests/test_issue-28.php b/min_unit_tests/test_issue-28.php index ea8271c..74e5f5b 100644 --- a/min_unit_tests/test_issue-28.php +++ b/min_unit_tests/test_issue-28.php @@ -72,6 +72,8 @@ function test_bug28_geturi() /** * tests that replacing older file (that is still older than newest file) generates new checksum * @see https://github.com/mrclay/minify/issues/28#issuecomment-6505012 + * @see Minify_HTML_Helper::getRawUri() + * @see Minify_HTML_Helper::getChecksum() */ function test_bug28_geturi_older_file() { -- 1.7.10 From 28fc209c43c80af6f2a175ed647fd2f01db3fd5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 22 Jun 2012 14:45:19 +0300 Subject: [PATCH 7/8] remove unneeded Minify_HTML_Helper::getLastModified() testing. issue #28 --- min_unit_tests/test_issue-28.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/min_unit_tests/test_issue-28.php b/min_unit_tests/test_issue-28.php index 74e5f5b..695933f 100644 --- a/min_unit_tests/test_issue-28.php +++ b/min_unit_tests/test_issue-28.php @@ -7,25 +7,6 @@ require_once '_inc.php'; require_once 'Minify/HTML/Helper.php'; -function test_bug28_mtime() -{ - $fiveSecondsAgo = $_SERVER['REQUEST_TIME'] - 5; - - $file1 = new stdClass(); - $file1->lastModified = $fiveSecondsAgo; - - $file2 = new stdClass(); - $file2->lastModified = $fiveSecondsAgo; - - $m1 = Minify_HTML_Helper::getLastModified(array($file1)); - error_log("last modified: $m1"); - - $m2 = Minify_HTML_Helper::getLastModified(array($file1, $file2)); - error_log("last modified: $m2"); - - assertTrue($m1 !== $m2, "mtime of group with one and two files should not be same"); -} - /** * check that geturi differs if file count remains change, but files change * @see https://github.com/mrclay/minify/issues/28#issuecomment-6036867 @@ -135,6 +116,5 @@ function test_bug28_geturi_older_file() assertTrue($uri1 !== $uri3, "older file replaced with newer file (but older than newest file)"); } -test_bug28_mtime(); test_bug28_geturi(); test_bug28_geturi_older_file(); \ No newline at end of file -- 1.7.10 From d95ed0f40cb96bc4005cf658bf8fb245d2762834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 22 Jun 2012 14:50:11 +0300 Subject: [PATCH 8/8] follow upstream formatting with spaces. issue #28 --- min/lib/Minify/HTML/Helper.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/min/lib/Minify/HTML/Helper.php b/min/lib/Minify/HTML/Helper.php index 238598b..f50d3aa 100644 --- a/min/lib/Minify/HTML/Helper.php +++ b/min/lib/Minify/HTML/Helper.php @@ -111,16 +111,17 @@ public function setGroup($key, $checkLastModified = true) } } - /** + /** * Calculate checksum for $sources * Take into account of files path, mtimes and create checksum from that * The checksum is rounded to be 32bit unsigned integer to be portable for 32/64bit PHP's * - * @param Minify_Source[] $sources - * @return float|null the checksum - */ + * @param Minify_Source[] $sources + * @return float|null the checksum + */ public static function getChecksum($sources) { + $paths = array(); $mtime = (float )0; @@ -146,7 +147,7 @@ public static function getChecksum($sources) } if (!empty($paths)) { - // cast to float so arithmetic would work on 32 and 64bit PHP + // cast to float so arithmetic would work on 32 and 64bit PHP return ($mtime & 0xFFFF) + (float )sprintf("%u", crc32(serialize($paths))); } return null; -- 1.7.10