diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index e853c80..f367788 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: - php-version: [ '8.1', '8.2' ] + php-version: [ '8.1', '8.2', '8.3', '8.4', '8.5' ] steps: - name: Setup PHP @@ -25,7 +25,7 @@ jobs: coverage: xdebug - name: Setup Pages - uses: actions/configure-pages@v3 + uses: actions/configure-pages@v5 - uses: actions/checkout@v3 @@ -48,8 +48,11 @@ jobs: - name: Check code style run: composer run-script lint-codestyle + # Every leg produces the same report and they all upload under the same artifact + # name, so publish from a single version. - name: Archive code coverage results - uses: actions/upload-pages-artifact@v1 + if: matrix.php-version == '8.4' + uses: actions/upload-pages-artifact@v3 with: path: report/ @@ -58,6 +61,8 @@ jobs: needs: build + if: github.ref == 'refs/heads/master' + environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} @@ -73,4 +78,4 @@ jobs: steps: - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v2 + uses: actions/deploy-pages@v4 diff --git a/.gitignore b/.gitignore index 7579f74..6230d1f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,5 @@ vendor composer.lock +.phpunit.cache +.php-cs-fixer.cache +report diff --git a/src/RuleAggregate.php b/src/RuleAggregate.php index 48395a9..45f3811 100644 --- a/src/RuleAggregate.php +++ b/src/RuleAggregate.php @@ -53,14 +53,24 @@ public function getAllRules(): array } /** + * Rules of an already known domain are added to its existing collection. Replacing the collection + * instead would drop every rule a previous call contributed for that domain. + * * @param array $collections */ public function addCollections(array $collections): void { - $this->ruleCollections = array_merge( - $this->ruleCollections, - $collections, - ); + foreach ($collections as $domainIdentifier => $collection) { + if (!isset($this->ruleCollections[$domainIdentifier])) { + $this->ruleCollections[$domainIdentifier] = $collection; + + continue; + } + + foreach ($collection->getAllRules() as $rule) { + $this->ruleCollections[$domainIdentifier]->addRule($rule); + } + } } /** diff --git a/tests/RuleAggregateFactoryTest.php b/tests/RuleAggregateFactoryTest.php index 038998c..e10ccae 100644 --- a/tests/RuleAggregateFactoryTest.php +++ b/tests/RuleAggregateFactoryTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use ProtonLabs\AdblockParser\DomainParserInterface; +use ProtonLabs\AdblockParser\DummyDomainParser; use ProtonLabs\AdblockParser\Rule; use ProtonLabs\AdblockParser\RuleAggregateFactory; use ProtonLabs\AdblockParser\RuleApplier; @@ -38,6 +39,21 @@ public function testCreateRuleAggregate(): void ->shouldBlock('http://example.com//avmws_exception.js', $ruleAggregate)); } + public function testCreateFromFilesKeepsRulesOfEveryFileForASharedDomain(): void + { + $factory = new RuleAggregateFactory(new RuleFactory(new DummyDomainParser())); + + $ruleAggregate = $factory->createFromFiles([ + __DIR__ . '/test-rules-shared-blocker.txt', + __DIR__ . '/test-rules-shared-exception.txt', + ]); + + $applier = new RuleApplier(new DummyDomainParser()); + + Assert::assertTrue($applier->shouldBlock('http://shared-example.com/blocked.gif', $ruleAggregate)); + Assert::assertFalse($applier->shouldBlock('http://shared-example.com/allowed.gif', $ruleAggregate)); + } + public function createRuleApplier(): RuleApplier { $domainParser = $this->createMock(DomainParserInterface::class); diff --git a/tests/RuleAggregateTest.php b/tests/RuleAggregateTest.php new file mode 100644 index 0000000..dc50d3e --- /dev/null +++ b/tests/RuleAggregateTest.php @@ -0,0 +1,69 @@ + $this->collectionOf(new Rule('first', false, 'example.com')), + ]); + + $ruleAggregate->addCollections([ + 'example.com' => $this->collectionOf(new Rule('second', false, 'example.com')), + ]); + + Assert::assertSame( + ['first', 'second'], + array_map( + static fn (Rule $rule): ?string => $rule->getRegex(), + $ruleAggregate->getRuleCollections()['example.com']->getBlockers(), + ), + ); + } + + public function testAddCollectionsKeepsExceptionsAndBlockersOfTheSameDomain(): void + { + $ruleAggregate = new RuleAggregate([ + 'example.com' => $this->collectionOf(new Rule('blocker', false, 'example.com')), + ]); + + $ruleAggregate->addCollections([ + 'example.com' => $this->collectionOf(new Rule('exception', true, 'example.com')), + ]); + + $collection = $ruleAggregate->getRuleCollections()['example.com']; + Assert::assertCount(1, $collection->getBlockers()); + Assert::assertCount(1, $collection->getExceptions()); + } + + public function testAddCollectionsAddsUnknownDomains(): void + { + $ruleAggregate = new RuleAggregate([ + 'example.com' => $this->collectionOf(new Rule('first', false, 'example.com')), + ]); + + $ruleAggregate->addCollections([ + 'other.com' => $this->collectionOf(new Rule('second', false, 'other.com')), + ]); + + Assert::assertSame(['example.com', 'other.com'], array_keys($ruleAggregate->getRuleCollections())); + } + + private function collectionOf(Rule $rule): RuleCollection + { + $collection = new RuleCollection(); + $collection->addRule($rule); + + return $collection; + } +} diff --git a/tests/RuleApplierTest.php b/tests/RuleApplierTest.php index e859057..ea6a4e4 100644 --- a/tests/RuleApplierTest.php +++ b/tests/RuleApplierTest.php @@ -40,9 +40,9 @@ public function testBlockByAddressParts(): void ); $this->shouldNotBlock( [ - 'http://example.com/banner/img', - 'http://example.com/banner/foo/imgraph', - 'http://example.com/banner/foo/img.gif', + 'http://example.com/banner/img', + 'http://example.com/banner/foo/imgraph', + 'http://example.com/banner/foo/img.gif', ], $ruleAggregate, ); @@ -197,7 +197,7 @@ public function testParserException(): void } /** - * @param array $url + * @param array $urls */ private function shouldBlock(array $urls, RuleAggregate $ruleAggregate): void { diff --git a/tests/test-rules-shared-blocker.txt b/tests/test-rules-shared-blocker.txt new file mode 100644 index 0000000..270cb4e --- /dev/null +++ b/tests/test-rules-shared-blocker.txt @@ -0,0 +1,2 @@ +! Blocks a path on a domain that the other file also has rules for +||shared-example.com/blocked.gif diff --git a/tests/test-rules-shared-exception.txt b/tests/test-rules-shared-exception.txt new file mode 100644 index 0000000..40fb031 --- /dev/null +++ b/tests/test-rules-shared-exception.txt @@ -0,0 +1,2 @@ +! Allows a different path on the same domain +@@||shared-example.com/allowed.gif