sminnee /
silverstripe-framework
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
| 1 | <?php |
||
| 19 | class RequirementsTest extends SapphireTest { |
||
| 20 | |||
| 21 | static $html_template = '<html><head></head><body></body></html>'; |
||
| 22 | |||
| 23 | public function setUp() { |
||
| 24 | parent::setUp(); |
||
| 25 | AssetStoreTest_SpyStore::activate('RequirementsTest'); // Set backend root to /RequirementsTest |
||
| 26 | } |
||
| 27 | |||
| 28 | public function tearDown() { |
||
| 32 | |||
| 33 | public function testExternalUrls() { |
||
| 34 | /** @var Requirements_Backend $backend */ |
||
| 35 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 36 | $backend->setCombinedFilesEnabled(true); |
||
| 37 | |||
| 38 | $backend->javascript('http://www.mydomain.com/test.js'); |
||
| 39 | $backend->javascript('https://www.mysecuredomain.com/test.js'); |
||
| 40 | $backend->javascript('//scheme-relative.example.com/test.js'); |
||
| 41 | $backend->css('http://www.mydomain.com/test.css'); |
||
| 42 | $backend->css('https://www.mysecuredomain.com/test.css'); |
||
| 43 | $backend->css('//scheme-relative.example.com/test.css'); |
||
| 44 | |||
| 45 | $html = $backend->includeInHTML(self::$html_template); |
||
| 46 | |||
| 47 | $this->assertTrue( |
||
| 48 | (strpos($html, 'http://www.mydomain.com/test.js') !== false), |
||
| 49 | 'Load external javascript URL' |
||
| 50 | ); |
||
| 51 | $this->assertTrue( |
||
| 52 | (strpos($html, 'https://www.mysecuredomain.com/test.js') !== false), |
||
| 53 | 'Load external secure javascript URL' |
||
| 54 | ); |
||
| 55 | $this->assertTrue( |
||
| 56 | (strpos($html, '//scheme-relative.example.com/test.js') !== false), |
||
| 57 | 'Load external scheme-relative javascript URL' |
||
| 58 | ); |
||
| 59 | $this->assertTrue( |
||
| 60 | (strpos($html, 'http://www.mydomain.com/test.css') !== false), |
||
| 61 | 'Load external CSS URL' |
||
| 62 | ); |
||
| 63 | $this->assertTrue( |
||
| 64 | (strpos($html, 'https://www.mysecuredomain.com/test.css') !== false), |
||
| 65 | 'Load external secure CSS URL' |
||
| 66 | ); |
||
| 67 | $this->assertTrue( |
||
| 68 | (strpos($html, '//scheme-relative.example.com/test.css') !== false), |
||
| 69 | 'Load scheme-relative CSS URL' |
||
| 70 | ); |
||
| 71 | } |
||
| 72 | |||
| 73 | /** |
||
| 74 | * Setup new backend |
||
| 75 | * |
||
| 76 | * @param Requirements_Backend $backend |
||
| 77 | */ |
||
| 78 | protected function setupRequirements($backend) { |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Setup combined and non-combined js with the backend |
||
| 89 | * |
||
| 90 | * @param Requirements_Backend $backend |
||
| 91 | */ |
||
| 92 | protected function setupCombinedRequirements($backend) { |
||
| 110 | |||
| 111 | /** |
||
| 112 | * Setup combined files with the backend |
||
| 113 | * |
||
| 114 | * @param Requirements_Backend $backend |
||
| 115 | */ |
||
| 116 | protected function setupCombinedNonrequiredRequirements($backend) { |
||
| 129 | |||
| 130 | protected function setupCombinedRequirementsJavascriptAsyncDefer($backend, $async, $defer) { |
||
| 131 | $basePath = $this->getCurrentRelativePath(); |
||
| 132 | $this->setupRequirements($backend); |
||
| 133 | |||
| 134 | // require files normally (e.g. called from a FormField instance) |
||
| 135 | $backend->javascript($basePath . '/RequirementsTest_a.js'); |
||
| 136 | $backend->javascript($basePath . '/RequirementsTest_b.js'); |
||
| 137 | $backend->javascript($basePath . '/RequirementsTest_c.js'); |
||
| 138 | |||
| 139 | // require two of those files as combined includes |
||
| 140 | $backend->combineFiles( |
||
| 141 | 'RequirementsTest_bc.js', |
||
| 142 | array( |
||
| 143 | $basePath . '/RequirementsTest_b.js', |
||
| 144 | $basePath . '/RequirementsTest_c.js' |
||
| 145 | ), |
||
| 146 | array( |
||
| 147 | 'async' => $async, |
||
| 148 | 'defer' => $defer, |
||
| 149 | ) |
||
| 150 | ); |
||
| 151 | } |
||
| 152 | |||
| 153 | View Code Duplication | public function testCustomType() { |
|
| 174 | |||
| 175 | public function testCombinedJavascript() { |
||
| 176 | /** @var Requirements_Backend $backend */ |
||
| 177 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 178 | $this->setupCombinedRequirements($backend); |
||
| 179 | |||
| 180 | $combinedFileName = '/_combinedfiles/RequirementsTest_bc-2a55d56.js'; |
||
| 181 | $combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName; |
||
| 182 | |||
| 183 | $html = $backend->includeInHTML(self::$html_template); |
||
| 184 | |||
| 185 | /* COMBINED JAVASCRIPT FILE IS INCLUDED IN HTML HEADER */ |
||
| 186 | $this->assertRegExp( |
||
| 187 | '/src=".*' . preg_quote($combinedFileName, '/') . '/', |
||
| 188 | $html, |
||
| 189 | 'combined javascript file is included in html header' |
||
| 190 | ); |
||
| 191 | |||
| 192 | /* COMBINED JAVASCRIPT FILE EXISTS */ |
||
| 193 | $this->assertTrue( |
||
| 194 | file_exists($combinedFilePath), |
||
| 195 | 'combined javascript file exists' |
||
| 196 | ); |
||
| 197 | |||
| 198 | /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ |
||
| 199 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), |
||
| 200 | 'combined javascript has correct content'); |
||
| 201 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), |
||
| 202 | 'combined javascript has correct content'); |
||
| 203 | |||
| 204 | /* COMBINED FILES ARE NOT INCLUDED TWICE */ |
||
| 205 | $this->assertNotRegExp( |
||
| 206 | '/src=".*\/RequirementsTest_b\.js/', |
||
| 207 | $html, |
||
| 208 | 'combined files are not included twice' |
||
| 209 | ); |
||
| 210 | $this->assertNotRegExp( |
||
| 211 | '/src=".*\/RequirementsTest_c\.js/', |
||
| 212 | $html, |
||
| 213 | 'combined files are not included twice' |
||
| 214 | ); |
||
| 215 | |||
| 216 | /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ |
||
| 217 | $this->assertRegExp( |
||
| 218 | '/src=".*\/RequirementsTest_a\.js/', |
||
| 219 | $html, |
||
| 220 | 'normal requirements are still included' |
||
| 221 | ); |
||
| 222 | |||
| 223 | // Then do it again, this time not requiring the files beforehand |
||
| 224 | unlink($combinedFilePath); |
||
| 225 | /** @var Requirements_Backend $backend */ |
||
| 226 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 227 | $this->setupCombinedNonrequiredRequirements($backend); |
||
| 228 | $html = $backend->includeInHTML(self::$html_template); |
||
| 229 | |||
| 230 | /* COMBINED JAVASCRIPT FILE IS INCLUDED IN HTML HEADER */ |
||
| 231 | $this->assertRegExp( |
||
| 232 | '/src=".*' . preg_quote($combinedFileName, '/') . '/', |
||
| 233 | $html, |
||
| 234 | 'combined javascript file is included in html header' |
||
| 235 | ); |
||
| 236 | |||
| 237 | /* COMBINED JAVASCRIPT FILE EXISTS */ |
||
| 238 | $this->assertTrue( |
||
| 239 | file_exists($combinedFilePath), |
||
| 240 | 'combined javascript file exists' |
||
| 241 | ); |
||
| 242 | |||
| 243 | /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ |
||
| 244 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), |
||
| 245 | 'combined javascript has correct content'); |
||
| 246 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), |
||
| 247 | 'combined javascript has correct content'); |
||
| 248 | |||
| 249 | /* COMBINED FILES ARE NOT INCLUDED TWICE */ |
||
| 250 | $this->assertNotRegExp( |
||
| 251 | '/src=".*\/RequirementsTest_b\.js/', |
||
| 252 | $html, |
||
| 253 | 'combined files are not included twice' |
||
| 254 | ); |
||
| 255 | $this->assertNotRegExp( |
||
| 256 | '/src=".*\/RequirementsTest_c\.js/', |
||
| 257 | $html, |
||
| 258 | 'combined files are not included twice' |
||
| 259 | ); |
||
| 260 | } |
||
| 261 | |||
| 262 | public function testCombinedJavascriptAsyncDefer() { |
||
| 263 | /** @var Requirements_Backend $backend */ |
||
| 264 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 265 | |||
| 266 | $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, false); |
||
| 267 | |||
| 268 | $combinedFileName = '/_combinedfiles/RequirementsTest_bc-2a55d56.js'; |
||
| 269 | $combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName; |
||
| 270 | |||
| 271 | $html = $backend->includeInHTML(false, self::$html_template); |
||
| 272 | |||
| 273 | /* ASYNC IS INCLUDED IN SCRIPT TAG */ |
||
| 274 | $this->assertRegExp( |
||
| 275 | '/src=".*' . preg_quote($combinedFileName, '/') . '" async/', |
||
| 276 | $html, |
||
| 277 | 'async is included in script tag' |
||
| 278 | ); |
||
| 279 | |||
| 280 | /* DEFER IS NOT INCLUDED IN SCRIPT TAG */ |
||
| 281 | $this->assertNotContains('defer', $html, 'defer is not included'); |
||
| 282 | |||
| 283 | /* COMBINED JAVASCRIPT FILE EXISTS */ |
||
| 284 | clearstatcache(); // needed to get accurate file_exists() results |
||
| 285 | $this->assertFileExists($combinedFilePath, |
||
| 286 | 'combined javascript file exists'); |
||
| 287 | |||
| 288 | /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ |
||
| 289 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), |
||
| 290 | 'combined javascript has correct content'); |
||
| 291 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), |
||
| 292 | 'combined javascript has correct content'); |
||
| 293 | |||
| 294 | /* COMBINED FILES ARE NOT INCLUDED TWICE */ |
||
| 295 | $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, |
||
| 296 | 'combined files are not included twice'); |
||
| 297 | $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, |
||
| 298 | 'combined files are not included twice'); |
||
| 299 | |||
| 300 | /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ |
||
| 301 | $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, |
||
| 302 | 'normal requirements are still included'); |
||
| 303 | |||
| 304 | /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ |
||
| 305 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, |
||
| 306 | 'normal requirements don\'t have async'); |
||
| 307 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" defer/', $html, |
||
| 308 | 'normal requirements don\'t have defer'); |
||
| 309 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, |
||
| 310 | 'normal requirements don\'t have async/defer'); |
||
| 311 | |||
| 312 | // setup again for testing defer |
||
| 313 | unlink($combinedFilePath); |
||
| 314 | /** @var Requirements_Backend $backend */ |
||
| 315 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 316 | |||
| 317 | $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, false, true); |
||
| 318 | |||
| 319 | $html = $backend->includeInHTML(self::$html_template); |
||
| 320 | |||
| 321 | /* DEFER IS INCLUDED IN SCRIPT TAG */ |
||
| 322 | $this->assertRegExp( |
||
| 323 | '/src=".*' . preg_quote($combinedFileName, '/') . '" defer/', |
||
| 324 | $html, |
||
| 325 | 'defer is included in script tag' |
||
| 326 | ); |
||
| 327 | |||
| 328 | /* ASYNC IS NOT INCLUDED IN SCRIPT TAG */ |
||
| 329 | $this->assertNotContains('async', $html, 'async is not included'); |
||
| 330 | |||
| 331 | /* COMBINED JAVASCRIPT FILE EXISTS */ |
||
| 332 | clearstatcache(); // needed to get accurate file_exists() results |
||
| 333 | $this->assertFileExists($combinedFilePath, |
||
| 334 | 'combined javascript file exists'); |
||
| 335 | |||
| 336 | /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ |
||
| 337 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), |
||
| 338 | 'combined javascript has correct content'); |
||
| 339 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), |
||
| 340 | 'combined javascript has correct content'); |
||
| 341 | |||
| 342 | /* COMBINED FILES ARE NOT INCLUDED TWICE */ |
||
| 343 | $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, |
||
| 344 | 'combined files are not included twice'); |
||
| 345 | $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, |
||
| 346 | 'combined files are not included twice'); |
||
| 347 | |||
| 348 | /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ |
||
| 349 | $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, |
||
| 350 | 'normal requirements are still included'); |
||
| 351 | |||
| 352 | /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ |
||
| 353 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, |
||
| 354 | 'normal requirements don\'t have async'); |
||
| 355 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" defer/', $html, |
||
| 356 | 'normal requirements don\'t have defer'); |
||
| 357 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, |
||
| 358 | 'normal requirements don\'t have async/defer'); |
||
| 359 | |||
| 360 | // setup again for testing async and defer |
||
| 361 | unlink($combinedFilePath); |
||
| 362 | /** @var Requirements_Backend $backend */ |
||
| 363 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 364 | |||
| 365 | $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, true); |
||
| 366 | |||
| 367 | $html = $backend->includeInHTML(self::$html_template); |
||
| 368 | |||
| 369 | /* ASYNC/DEFER IS INCLUDED IN SCRIPT TAG */ |
||
| 370 | $this->assertRegExp( |
||
| 371 | '/src=".*' . preg_quote($combinedFileName, '/') . '" async defer/', |
||
| 372 | $html, |
||
| 373 | 'async and defer are included in script tag' |
||
| 374 | ); |
||
| 375 | |||
| 376 | /* COMBINED JAVASCRIPT FILE EXISTS */ |
||
| 377 | clearstatcache(); // needed to get accurate file_exists() results |
||
| 378 | $this->assertFileExists($combinedFilePath, |
||
| 379 | 'combined javascript file exists'); |
||
| 380 | |||
| 381 | /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ |
||
| 382 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), |
||
| 383 | 'combined javascript has correct content'); |
||
| 384 | $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), |
||
| 385 | 'combined javascript has correct content'); |
||
| 386 | |||
| 387 | /* COMBINED FILES ARE NOT INCLUDED TWICE */ |
||
| 388 | $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, |
||
| 389 | 'combined files are not included twice'); |
||
| 390 | $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, |
||
| 391 | 'combined files are not included twice'); |
||
| 392 | |||
| 393 | /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ |
||
| 394 | $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, |
||
| 395 | 'normal requirements are still included'); |
||
| 396 | |||
| 397 | /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ |
||
| 398 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, |
||
| 399 | 'normal requirements don\'t have async'); |
||
| 400 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" defer/', $html, |
||
| 401 | 'normal requirements don\'t have defer'); |
||
| 402 | $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, |
||
| 403 | 'normal requirements don\'t have async/defer'); |
||
| 404 | |||
| 405 | unlink($combinedFilePath); |
||
| 406 | } |
||
| 407 | |||
| 408 | public function testCombinedCss() { |
||
| 409 | $basePath = $this->getCurrentRelativePath(); |
||
| 410 | /** @var Requirements_Backend $backend */ |
||
| 411 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 412 | $this->setupRequirements($backend); |
||
| 413 | |||
| 414 | $backend->combineFiles( |
||
| 415 | 'print.css', |
||
| 416 | array( |
||
| 417 | $basePath . '/RequirementsTest_print_a.css', |
||
| 418 | $basePath . '/RequirementsTest_print_b.css' |
||
| 419 | ), |
||
| 420 | array( |
||
| 421 | 'media' => 'print' |
||
| 422 | ) |
||
| 423 | ); |
||
| 424 | |||
| 425 | $html = $backend->includeInHTML(self::$html_template); |
||
| 426 | |||
| 427 | $this->assertRegExp( |
||
| 428 | '/href=".*\/print\-94e723d\.css/', |
||
| 429 | $html, |
||
| 430 | 'Print stylesheets have been combined.' |
||
| 431 | ); |
||
| 432 | $this->assertRegExp( |
||
| 433 | '/media="print/', |
||
| 434 | $html, |
||
| 435 | 'Combined print stylesheet retains the media parameter' |
||
| 436 | ); |
||
| 437 | |||
| 438 | // Test that combining a file multiple times doesn't trigger an error |
||
| 439 | /** @var Requirements_Backend $backend */ |
||
| 440 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 441 | $this->setupRequirements($backend); |
||
| 442 | $backend->combineFiles( |
||
| 443 | 'style.css', |
||
| 444 | array( |
||
| 445 | $basePath . '/RequirementsTest_b.css', |
||
| 446 | $basePath . '/RequirementsTest_c.css' |
||
| 447 | ) |
||
| 448 | ); |
||
| 449 | $backend->combineFiles( |
||
| 450 | 'style.css', |
||
| 451 | array( |
||
| 452 | $basePath . '/RequirementsTest_b.css', |
||
| 453 | $basePath . '/RequirementsTest_c.css' |
||
| 454 | ) |
||
| 455 | ); |
||
| 456 | |||
| 457 | $html = $backend->includeInHTML(self::$html_template); |
||
| 458 | $this->assertRegExp( |
||
| 459 | '/href=".*\/style\-bcd90f5\.css/', |
||
| 460 | $html, |
||
| 461 | 'Stylesheets have been combined.' |
||
| 462 | ); |
||
| 463 | } |
||
| 464 | |||
| 465 | public function testBlockedCombinedJavascript() { |
||
| 521 | |||
| 522 | View Code Duplication | public function testArgsInUrls() { |
|
| 547 | |||
| 548 | public function testRequirementsBackend() { |
||
| 583 | |||
| 584 | public function testConditionalTemplateRequire() { |
||
| 585 | $testPath = ltrim(preg_replace('#^' . BASE_PATH . '#', '', dirname(__DIR__)), '/'); |
||
| 586 | |||
| 587 | /** @var Requirements_Backend $backend */ |
||
| 588 | $backend = Injector::inst()->create('SilverStripe\\View\\Requirements_Backend'); |
||
| 589 | $this->setupRequirements($backend); |
||
| 631 | |||
| 632 | public function testJsWriteToBody() { |
||
| 650 | |||
| 651 | public function testIncludedJsIsNotCommentedOut() { |
||
| 662 | |||
| 663 | public function testCommentedOutScriptTagIsIgnored() { |
||
| 677 | |||
| 678 | public function testForceJsToBottom() { |
||
| 739 | |||
| 740 | public function testSuffix() { |
||
| 768 | |||
| 769 | /** |
||
| 770 | * Tests that provided files work |
||
| 771 | */ |
||
| 772 | public function testProvidedFiles() { |
||
| 818 | |||
| 819 | /** |
||
| 820 | * Verify that the given backend includes the given files |
||
| 821 | * |
||
| 822 | * @param Requirements_Backend $backend |
||
| 823 | * @param string $type js or css |
||
| 824 | * @param array|string $files Files or list of files to check |
||
| 825 | */ |
||
| 826 | View Code Duplication | public function assertFileIncluded($backend, $type, $files) { |
|
| 851 | |||
| 852 | View Code Duplication | public function assertFileNotIncluded($backend, $type, $files) { |
|
| 876 | |||
| 877 | |||
| 878 | /** |
||
| 879 | * Get files of the given type from the backend |
||
| 880 | * |
||
| 881 | * @param Requirements_Backend $backend |
||
| 882 | * @param string $type js or css |
||
| 883 | * @return array |
||
| 884 | */ |
||
| 885 | protected function getBackendFiles($backend, $type) { |
||
| 897 | } |
||
| 898 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.