Use IRuleMatcher getFlows (closes #60) (#61)

* Use IRuleMatcher getFlows Filter unwanted events for users by invoking ruleMatchers getFlows method. This will apply various checks and only return a value if all checks passed. Fixes #60 * "Fix" CI with mariadb Inspired by nextcloud/spreed@e8b1456
R0Wi-DEV · Jul 20, 2021 · f900234 · f900234
1 parent 48ff87a
commit f900234
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 41 deletions.
diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml
@@ -85,7 +85,7 @@ jobs:
 
     services:
       mysql:
-        image: mariadb
+        image: mariadb:10.5
         ports:
           - 4444:3306/tcp
         env:

diff --git a/lib/Operation.php b/lib/Operation.php
@@ -89,15 +89,15 @@ public function isAvailableForScope(int $scope): bool {
 	}
 
 	public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatcher): void {
-		if ($eventName !== '\OCP\Files::postCreate' && $eventName !== '\OCP\Files::postWrite' ||
-			!$event instanceof GenericEvent) {
-			$this->logger->debug('Not processing event {eventname} with argument {event}.',
-					['eventname' => $eventName, 'event' => $event]);
+		if (!$this->checkEvent($eventName, $event) ||
+			!$this->checkRuleMatcher($ruleMatcher)) {
 			return;
 		}
 
+		/** @var GenericEvent */
+		$genericEvent = $event;
 		/** @var Node*/
-		$node = $event->getSubject();
+		$node = $genericEvent->getSubject();
 
 		if (!$node instanceof Node || $node->getType() !== FileInfo::TYPE_FILE) {
 			$this->logger->debug('Not processing event {eventname} because node is not a file.',
@@ -122,6 +122,25 @@ public function getEntityId(): string {
 		return File::class;
 	}
 
+	private function checkEvent(string $eventName, Event $event) : bool {
+		if ($eventName !== '\OCP\Files::postCreate' && $eventName !== '\OCP\Files::postWrite' ||
+			!$event instanceof GenericEvent) {
+			$this->logger->debug('Not processing event {eventname} with argument {event}.',
+					['eventname' => $eventName, 'event' => $event]);
+			return false;
+		}
+		return true;
+	}
+
+	private function checkRuleMatcher(IRuleMatcher $ruleMatcher) : bool {
+		$match = $ruleMatcher->getFlows(true);
+		if (!$match) {
+			$this->logger->debug('Not processing event because IRuleMatcher->getFlows did not return anything');
+			return false;
+		}
+		return true;
+	}
+
 	private function pathIsValid(Node $node) : bool {
 		// Check path has valid structure
 		$filePath = $node->getPath();

diff --git a/tests/Unit/OperationTest.php b/tests/Unit/OperationTest.php
@@ -42,7 +42,7 @@
 use Psr\Log\LoggerInterface;
 
 class OperationTest extends TestCase {
-	
+
 	/** @var IJobList|MockObject */
 	private $jobList;
 	/** @var IL10N|MockObject */
@@ -53,6 +53,8 @@ class OperationTest extends TestCase {
 	private $urlGenerator;
 	/** @var IProcessingFileAccessor|MockObject */
 	private $processingFileAccessor;
+	/** @var IRuleMatcher|MockObject */
+	private $ruleMatcher;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -61,6 +63,9 @@ protected function setUp(): void {
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->processingFileAccessor = $this->createMock(IProcessingFileAccessor::class);
+		$this->ruleMatcher = $this->createMock(IRuleMatcher::class);
+		$this->ruleMatcher->method('getFlows')
+			->willReturn([$this->createMock(Operation::class)]); // simulate single matching operation
 	}
 
 	/**
@@ -75,9 +80,30 @@ public function testDoesNothingOnInvalidEvent(string $eventName, Event $event) {
 			->withAnyParameters();
 
 		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $this->processingFileAccessor);
-		/** @var IRuleMatcher */
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
+	}
+
+	/**
+	 * @dataProvider dataProvider_InvalidRuleMatcherResults
+	 */
+	public function testDoesNothingIfRuleMatcherDoesNotMatch($ruleMatcherResult) {
+		$this->jobList->expects($this->never())
+			->method('add')
+			->withAnyParameters();
+		$this->logger->expects($this->once())
+			->method('debug')
+			->withAnyParameters();
+
+		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $this->processingFileAccessor);
+
+		/** @var IRuleMatcher|MockObject */
 		$ruleMatcher = $this->createMock(IRuleMatcher::class);
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$ruleMatcher->expects($this->once())
+			->method('getFlows')
+			->with(true)
+			->willReturn($ruleMatcherResult);
+
+		$operation->onEvent("\OCP\Files::postCreate", new GenericEvent(), $ruleMatcher);
 	}
 
 	public function testDoesNothingOnFolderEvent() {
@@ -87,18 +113,16 @@ public function testDoesNothingOnFolderEvent() {
 		$this->logger->expects($this->once())
 			->method('debug')
 			->withAnyParameters();
-		
+
 		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $this->processingFileAccessor);
 
 		$fileMock = $this->createMock(Node::class);
 		$fileMock->method('getType')
 			->willReturn(FileInfo::TYPE_FOLDER);
 		$event = new GenericEvent($fileMock);
-		/** @var IRuleMatcher */
-		$ruleMatcher = $this->createMock(IRuleMatcher::class);
 		$eventName = '\OCP\Files::postCreate';
 
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
 	}
 
 	public function testDoesNothingOnPostWriteTriggeredByCurrentOcrProcess() {
@@ -114,7 +138,7 @@ public function testDoesNothingOnPostWriteTriggeredByCurrentOcrProcess() {
 		$processingFileAccessorMock->expects($this->once())
 			->method('getCurrentlyProcessedFileId')
 			->willReturn(42);
-		
+
 		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $processingFileAccessorMock);
 
 		$userMock = $this->createMock(IUser::class);
@@ -130,11 +154,9 @@ public function testDoesNothingOnPostWriteTriggeredByCurrentOcrProcess() {
 		$fileMock->method('getId')
 			->willReturn(42);
 		$event = new GenericEvent($fileMock);
-		/** @var IRuleMatcher */
-		$ruleMatcher = $this->createMock(IRuleMatcher::class);
 		$eventName = '\OCP\Files::postCreate';
 
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
 	}
 
 
@@ -148,7 +170,7 @@ public function testDoesNothingOnInvalidFilePath(string $filePath) {
 		$this->logger->expects($this->once())
 			->method('debug')
 			->withAnyParameters();
-		
+
 		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $this->processingFileAccessor);
 
 		$fileMock = $this->createMock(Node::class);
@@ -157,11 +179,9 @@ public function testDoesNothingOnInvalidFilePath(string $filePath) {
 		$fileMock->method('getPath')
 			->willReturn($filePath);
 		$event = new GenericEvent($fileMock);
-		/** @var IRuleMatcher */
-		$ruleMatcher = $this->createMock(IRuleMatcher::class);
 		$eventName = '\OCP\Files::postCreate';
 
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
 	}
 
 	public function testDoesNothingOnFileWithoutOwner() {
@@ -181,13 +201,11 @@ public function testDoesNothingOnFileWithoutOwner() {
 			->willReturn('/admin/files/path/to/file.pdf');
 		$fileMock->method('getOwner')
 			->willReturn(null);
-			
+
 		$event = new GenericEvent($fileMock);
-		/** @var IRuleMatcher */
-		$ruleMatcher = $this->createMock(IRuleMatcher::class);
 		$eventName = '\OCP\Files::postCreate';
 
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
 	}
 
 	public function testAddWithCorrectFilePathAndUser() {
@@ -196,7 +214,7 @@ public function testAddWithCorrectFilePathAndUser() {
 		$this->jobList->expects($this->once())
 			->method('add')
 			->with(ProcessFileJob::class, ['filePath' => $filePath, 'uid' => $uid]);
-		
+
 		$operation = new Operation($this->jobList, $this->l, $this->logger, $this->urlGenerator, $this->processingFileAccessor);
 
 		$userMock = $this->createMock(IUser::class);
@@ -213,11 +231,9 @@ public function testAddWithCorrectFilePathAndUser() {
 		$fileMock->method('getId')
 			->willReturn(42);
 		$event = new GenericEvent($fileMock);
-		/** @var IRuleMatcher */
-		$ruleMatcher = $this->createMock(IRuleMatcher::class);
 		$eventName = '\OCP\Files::postCreate';
 
-		$operation->onEvent($eventName, $event, $ruleMatcher);
+		$operation->onEvent($eventName, $event, $this->ruleMatcher);
 	}
 
 	/**
@@ -281,22 +297,29 @@ public function testEntityIdIsFile() {
 
 	public function dataProvider_InvalidEvents() {
 		$arr = [
-			["\OCP\Files\preWrite", new GenericEvent()],
-			["\OCP\Files\preCreate", new GenericEvent()],
-			["\OCP\Files\preDelete", new GenericEvent()],
-			["\OCP\Files\postDelete", new GenericEvent()],
-			["\OCP\Files\postTouch", new GenericEvent()],
-			["\OCP\Files\preTouch", new GenericEvent()],
-			["\OCP\Files\preCopy", new GenericEvent()],
-			["\OCP\Files\postCopy", new GenericEvent()],
-			["\OCP\Files\preRename", new GenericEvent()],
-			["\OCP\Files\postRename", new GenericEvent()],
-			["\OCP\Files\postWrite", new Event()],
-			["\OCP\Files\postCreate", new Event()],
+			["\OCP\Files::preWrite", new GenericEvent()],
+			["\OCP\Files::preCreate", new GenericEvent()],
+			["\OCP\Files::preDelete", new GenericEvent()],
+			["\OCP\Files::postDelete", new GenericEvent()],
+			["\OCP\Files::postTouch", new GenericEvent()],
+			["\OCP\Files::preTouch", new GenericEvent()],
+			["\OCP\Files::preCopy", new GenericEvent()],
+			["\OCP\Files::postCopy", new GenericEvent()],
+			["\OCP\Files::preRename", new GenericEvent()],
+			["\OCP\Files::postRename", new GenericEvent()],
+			["\OCP\Files::postWrite", new Event()],
+			["\OCP\Files::postCreate", new Event()],
 		];
 		return $arr;
 	}
 
+	public function dataProvider_InvalidRuleMatcherResults() {
+		return [
+			[ [] ],
+			[ null ]
+		];
+	}
+
 	public function dataProvider_InvalidFilePaths() {
 		$arr = [
 			["/user/nofiles/somefile.pdf"],