Inspection of "Merge pull request #55628 from nextcloud/fix/dav/r..." - nextcloud/server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( f22232...4c1722 )

unknown

created 2025-10-20 11:20 UTC

Status

Indentation +114 added lines, -114 removed lines patch added patch discarded remove patch

@@ -26,118 +26,118 @@
 block discarded – undo
  * @group DB
  */
 class RemoveInvalidSharesTest extends TestCase {
-	private RemoveInvalidShares $command;
-
-	private IDBConnection $db;
-	private Principal&MockObject $principalBackend;
-	private RemoteUserPrincipalBackend&MockObject $remoteUserPrincipalBackend;
-
-	protected function setUp(): void {
-		parent::setUp();
-
-		$this->db = Server::get(IDBConnection::class);
-		$this->principalBackend = $this->createMock(Principal::class);
-		$this->remoteUserPrincipalBackend = $this->createMock(RemoteUserPrincipalBackend::class);
-
-		$this->db->insertIfNotExist('*PREFIX*dav_shares', [
-			'principaluri' => 'principal:unknown',
-			'type' => 'calendar',
-			'access' => 2,
-			'resourceid' => 666,
-		]);
-		$this->db->insertIfNotExist('*PREFIX*dav_shares', [
-			'principaluri' => 'principals/remote-users/foobar',
-			'type' => 'calendar',
-			'access' => 2,
-			'resourceid' => 666,
-		]);
-
-		$this->command = new RemoveInvalidShares(
-			$this->db,
-			$this->principalBackend,
-			$this->remoteUserPrincipalBackend,
-		);
-	}
-
-	private function selectShares(): array {
-		$query = $this->db->getQueryBuilder();
-		$query->select('*')
-			->from('dav_shares')
-			->where($query->expr()->in(
-				'principaluri',
-				$query->createNamedParameter(
-					['principal:unknown', 'principals/remote-users/foobar'],
-					IQueryBuilder::PARAM_STR_ARRAY,
-				),
-			));
-		$result = $query->executeQuery();
-		$data = $result->fetchAll();
-		$result->closeCursor();
-
-		return $data;
-	}
-
-	public function testWithoutPrincipals(): void {
-		$this->principalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principal:unknown', null],
-				['principals/remote-users/foobar', null],
-			]);
-		$this->remoteUserPrincipalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principal:unknown', null],
-				['principals/remote-users/foobar', null],
-			]);
-
-		$this->command->run(
-			$this->createMock(InputInterface::class),
-			$this->createMock(OutputInterface::class),
-		);
-
-		$data = $this->selectShares();
-		$this->assertCount(0, $data);
-	}
-
-	public function testWithLocalPrincipal(): void {
-		$this->principalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principal:unknown', ['uri' => 'principal:unknown']],
-				['principals/remote-users/foobar', null],
-			]);
-		$this->remoteUserPrincipalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principals/remote-users/foobar', null],
-			]);
-
-		$this->command->run(
-			$this->createMock(InputInterface::class),
-			$this->createMock(OutputInterface::class),
-		);
-
-		$data = $this->selectShares();
-		$this->assertCount(1, $data);
-		$this->assertEquals('principal:unknown', $data[0]['principaluri']);
-	}
-
-	public function testWithRemotePrincipal() {
-		$this->principalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principal:unknown', null],
-				['principals/remote-users/foobar', null],
-			]);
-		$this->remoteUserPrincipalBackend->method('getPrincipalByPath')
-			->willReturnMap([
-				['principal:unknown', null],
-				['principals/remote-users/foobar', ['uri' => 'principals/remote-users/foobar']],
-			]);
-
-		$this->command->run(
-			$this->createMock(InputInterface::class),
-			$this->createMock(OutputInterface::class),
-		);
-
-		$data = $this->selectShares();
-		$this->assertCount(1, $data);
-		$this->assertEquals('principals/remote-users/foobar', $data[0]['principaluri']);
-	}
+    private RemoveInvalidShares $command;
+
+    private IDBConnection $db;
+    private Principal&MockObject $principalBackend;
+    private RemoteUserPrincipalBackend&MockObject $remoteUserPrincipalBackend;
+
+    protected function setUp(): void {
+        parent::setUp();
+
+        $this->db = Server::get(IDBConnection::class);
+        $this->principalBackend = $this->createMock(Principal::class);
+        $this->remoteUserPrincipalBackend = $this->createMock(RemoteUserPrincipalBackend::class);
+
+        $this->db->insertIfNotExist('*PREFIX*dav_shares', [
+            'principaluri' => 'principal:unknown',
+            'type' => 'calendar',
+            'access' => 2,
+            'resourceid' => 666,
+        ]);
+        $this->db->insertIfNotExist('*PREFIX*dav_shares', [
+            'principaluri' => 'principals/remote-users/foobar',
+            'type' => 'calendar',
+            'access' => 2,
+            'resourceid' => 666,
+        ]);
+
+        $this->command = new RemoveInvalidShares(
+            $this->db,
+            $this->principalBackend,
+            $this->remoteUserPrincipalBackend,
+        );
+    }
+
+    private function selectShares(): array {
+        $query = $this->db->getQueryBuilder();
+        $query->select('*')
+            ->from('dav_shares')
+            ->where($query->expr()->in(
+                'principaluri',
+                $query->createNamedParameter(
+                    ['principal:unknown', 'principals/remote-users/foobar'],
+                    IQueryBuilder::PARAM_STR_ARRAY,
+                ),
+            ));
+        $result = $query->executeQuery();
+        $data = $result->fetchAll();
+        $result->closeCursor();
+
+        return $data;
+    }
+
+    public function testWithoutPrincipals(): void {
+        $this->principalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principal:unknown', null],
+                ['principals/remote-users/foobar', null],
+            ]);
+        $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principal:unknown', null],
+                ['principals/remote-users/foobar', null],
+            ]);
+
+        $this->command->run(
+            $this->createMock(InputInterface::class),
+            $this->createMock(OutputInterface::class),
+        );
+
+        $data = $this->selectShares();
+        $this->assertCount(0, $data);
+    }
+
+    public function testWithLocalPrincipal(): void {
+        $this->principalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principal:unknown', ['uri' => 'principal:unknown']],
+                ['principals/remote-users/foobar', null],
+            ]);
+        $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principals/remote-users/foobar', null],
+            ]);
+
+        $this->command->run(
+            $this->createMock(InputInterface::class),
+            $this->createMock(OutputInterface::class),
+        );
+
+        $data = $this->selectShares();
+        $this->assertCount(1, $data);
+        $this->assertEquals('principal:unknown', $data[0]['principaluri']);
+    }
+
+    public function testWithRemotePrincipal() {
+        $this->principalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principal:unknown', null],
+                ['principals/remote-users/foobar', null],
+            ]);
+        $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
+            ->willReturnMap([
+                ['principal:unknown', null],
+                ['principals/remote-users/foobar', ['uri' => 'principals/remote-users/foobar']],
+            ]);
+
+        $this->command->run(
+            $this->createMock(InputInterface::class),
+            $this->createMock(OutputInterface::class),
+        );
+
+        $data = $this->selectShares();
+        $this->assertCount(1, $data);
+        $this->assertEquals('principals/remote-users/foobar', $data[0]['principaluri']);
+    }
 }

Please login to merge, or discard this patch.

apps/dav/lib/Command/RemoveInvalidShares.php 1 patch

Indentation +37 added lines, -37 removed lines patch added patch discarded remove patch

@@ -22,46 +22,46 @@
 block discarded – undo
  * have no matching principal. Happened because of a bug in the calendar app.
  */
 class RemoveInvalidShares extends Command {
-	public function __construct(
-		private IDBConnection $connection,
-		private Principal $principalBackend,
-		private RemoteUserPrincipalBackend $remoteUserPrincipalBackend,
-	) {
-		parent::__construct();
-	}
+    public function __construct(
+        private IDBConnection $connection,
+        private Principal $principalBackend,
+        private RemoteUserPrincipalBackend $remoteUserPrincipalBackend,
+    ) {
+        parent::__construct();
+    }
 
-	protected function configure(): void {
-		$this
-			->setName('dav:remove-invalid-shares')
-			->setDescription('Remove invalid dav shares');
-	}
+    protected function configure(): void {
+        $this
+            ->setName('dav:remove-invalid-shares')
+            ->setDescription('Remove invalid dav shares');
+    }
 
-	protected function execute(InputInterface $input, OutputInterface $output): int {
-		$query = $this->connection->getQueryBuilder();
-		$result = $query->selectDistinct('principaluri')
-			->from('dav_shares')
-			->executeQuery();
+    protected function execute(InputInterface $input, OutputInterface $output): int {
+        $query = $this->connection->getQueryBuilder();
+        $result = $query->selectDistinct('principaluri')
+            ->from('dav_shares')
+            ->executeQuery();
 
-		while ($row = $result->fetch()) {
-			$principaluri = $row['principaluri'];
-			$p = $this->principalBackend->getPrincipalByPath($principaluri)
-				?? $this->remoteUserPrincipalBackend->getPrincipalByPath($principaluri);
-			if ($p === null) {
-				$this->deleteSharesForPrincipal($principaluri);
-			}
-		}
+        while ($row = $result->fetch()) {
+            $principaluri = $row['principaluri'];
+            $p = $this->principalBackend->getPrincipalByPath($principaluri)
+                ?? $this->remoteUserPrincipalBackend->getPrincipalByPath($principaluri);
+            if ($p === null) {
+                $this->deleteSharesForPrincipal($principaluri);
+            }
+        }
 
-		$result->closeCursor();
-		return self::SUCCESS;
-	}
+        $result->closeCursor();
+        return self::SUCCESS;
+    }
 
-	/**
-	 * @param string $principaluri
-	 */
-	private function deleteSharesForPrincipal($principaluri): void {
-		$delete = $this->connection->getQueryBuilder();
-		$delete->delete('dav_shares')
-			->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri)));
-		$delete->executeStatement();
-	}
+    /**
+     * @param string $principaluri
+     */
+    private function deleteSharesForPrincipal($principaluri): void {
+        $delete = $this->connection->getQueryBuilder();
+        $delete->delete('dav_shares')
+            ->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri)));
+        $delete->executeStatement();
+    }
 }

Please login to merge, or discard this patch.

		@@ -26,118 +26,118 @@
		block discarded – undo
26	26	* @group DB
27	27	*/
28	28	class RemoveInvalidSharesTest extends TestCase {
29		- private RemoveInvalidShares $command;
30		-
31		- private IDBConnection $db;
32		- private Principal&MockObject $principalBackend;
33		- private RemoteUserPrincipalBackend&MockObject $remoteUserPrincipalBackend;
34		-
35		- protected function setUp(): void {
36		- parent::setUp();
37		-
38		- $this->db = Server::get(IDBConnection::class);
39		- $this->principalBackend = $this->createMock(Principal::class);
40		- $this->remoteUserPrincipalBackend = $this->createMock(RemoteUserPrincipalBackend::class);
41		-
42		- $this->db->insertIfNotExist('PREFIXdav_shares', [
43		- 'principaluri' => 'principal:unknown',
44		- 'type' => 'calendar',
45		- 'access' => 2,
46		- 'resourceid' => 666,
47		- ]);
48		- $this->db->insertIfNotExist('PREFIXdav_shares', [
49		- 'principaluri' => 'principals/remote-users/foobar',
50		- 'type' => 'calendar',
51		- 'access' => 2,
52		- 'resourceid' => 666,
53		- ]);
54		-
55		- $this->command = new RemoveInvalidShares(
56		- $this->db,
57		- $this->principalBackend,
58		- $this->remoteUserPrincipalBackend,
59		- );
60		- }
61		-
62		- private function selectShares(): array {
63		- $query = $this->db->getQueryBuilder();
64		- $query->select('*')
65		- ->from('dav_shares')
66		- ->where($query->expr()->in(
67		- 'principaluri',
68		- $query->createNamedParameter(
69		- ['principal:unknown', 'principals/remote-users/foobar'],
70		- IQueryBuilder::PARAM_STR_ARRAY,
71		- ),
72		- ));
73		- $result = $query->executeQuery();
74		- $data = $result->fetchAll();
75		- $result->closeCursor();
76		-
77		- return $data;
78		- }
79		-
80		- public function testWithoutPrincipals(): void {
81		- $this->principalBackend->method('getPrincipalByPath')
82		- ->willReturnMap([
83		- ['principal:unknown', null],
84		- ['principals/remote-users/foobar', null],
85		- ]);
86		- $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
87		- ->willReturnMap([
88		- ['principal:unknown', null],
89		- ['principals/remote-users/foobar', null],
90		- ]);
91		-
92		- $this->command->run(
93		- $this->createMock(InputInterface::class),
94		- $this->createMock(OutputInterface::class),
95		- );
96		-
97		- $data = $this->selectShares();
98		- $this->assertCount(0, $data);
99		- }
100		-
101		- public function testWithLocalPrincipal(): void {
102		- $this->principalBackend->method('getPrincipalByPath')
103		- ->willReturnMap([
104		- ['principal:unknown', ['uri' => 'principal:unknown']],
105		- ['principals/remote-users/foobar', null],
106		- ]);
107		- $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
108		- ->willReturnMap([
109		- ['principals/remote-users/foobar', null],
110		- ]);
111		-
112		- $this->command->run(
113		- $this->createMock(InputInterface::class),
114		- $this->createMock(OutputInterface::class),
115		- );
116		-
117		- $data = $this->selectShares();
118		- $this->assertCount(1, $data);
119		- $this->assertEquals('principal:unknown', $data[0]['principaluri']);
120		- }
121		-
122		- public function testWithRemotePrincipal() {
123		- $this->principalBackend->method('getPrincipalByPath')
124		- ->willReturnMap([
125		- ['principal:unknown', null],
126		- ['principals/remote-users/foobar', null],
127		- ]);
128		- $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
129		- ->willReturnMap([
130		- ['principal:unknown', null],
131		- ['principals/remote-users/foobar', ['uri' => 'principals/remote-users/foobar']],
132		- ]);
133		-
134		- $this->command->run(
135		- $this->createMock(InputInterface::class),
136		- $this->createMock(OutputInterface::class),
137		- );
138		-
139		- $data = $this->selectShares();
140		- $this->assertCount(1, $data);
141		- $this->assertEquals('principals/remote-users/foobar', $data[0]['principaluri']);
142		- }
	29	+ private RemoveInvalidShares $command;
	30	+
	31	+ private IDBConnection $db;
	32	+ private Principal&MockObject $principalBackend;
	33	+ private RemoteUserPrincipalBackend&MockObject $remoteUserPrincipalBackend;
	34	+
	35	+ protected function setUp(): void {
	36	+ parent::setUp();
	37	+
	38	+ $this->db = Server::get(IDBConnection::class);
	39	+ $this->principalBackend = $this->createMock(Principal::class);
	40	+ $this->remoteUserPrincipalBackend = $this->createMock(RemoteUserPrincipalBackend::class);
	41	+
	42	+ $this->db->insertIfNotExist('PREFIXdav_shares', [
	43	+ 'principaluri' => 'principal:unknown',
	44	+ 'type' => 'calendar',
	45	+ 'access' => 2,
	46	+ 'resourceid' => 666,
	47	+ ]);
	48	+ $this->db->insertIfNotExist('PREFIXdav_shares', [
	49	+ 'principaluri' => 'principals/remote-users/foobar',
	50	+ 'type' => 'calendar',
	51	+ 'access' => 2,
	52	+ 'resourceid' => 666,
	53	+ ]);
	54	+
	55	+ $this->command = new RemoveInvalidShares(
	56	+ $this->db,
	57	+ $this->principalBackend,
	58	+ $this->remoteUserPrincipalBackend,
	59	+ );
	60	+ }
	61	+
	62	+ private function selectShares(): array {
	63	+ $query = $this->db->getQueryBuilder();
	64	+ $query->select('*')
	65	+ ->from('dav_shares')
	66	+ ->where($query->expr()->in(
	67	+ 'principaluri',
	68	+ $query->createNamedParameter(
	69	+ ['principal:unknown', 'principals/remote-users/foobar'],
	70	+ IQueryBuilder::PARAM_STR_ARRAY,
	71	+ ),
	72	+ ));
	73	+ $result = $query->executeQuery();
	74	+ $data = $result->fetchAll();
	75	+ $result->closeCursor();
	76	+
	77	+ return $data;
	78	+ }
	79	+
	80	+ public function testWithoutPrincipals(): void {
	81	+ $this->principalBackend->method('getPrincipalByPath')
	82	+ ->willReturnMap([
	83	+ ['principal:unknown', null],
	84	+ ['principals/remote-users/foobar', null],
	85	+ ]);
	86	+ $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
	87	+ ->willReturnMap([
	88	+ ['principal:unknown', null],
	89	+ ['principals/remote-users/foobar', null],
	90	+ ]);
	91	+
	92	+ $this->command->run(
	93	+ $this->createMock(InputInterface::class),
	94	+ $this->createMock(OutputInterface::class),
	95	+ );
	96	+
	97	+ $data = $this->selectShares();
	98	+ $this->assertCount(0, $data);
	99	+ }
	100	+
	101	+ public function testWithLocalPrincipal(): void {
	102	+ $this->principalBackend->method('getPrincipalByPath')
	103	+ ->willReturnMap([
	104	+ ['principal:unknown', ['uri' => 'principal:unknown']],
	105	+ ['principals/remote-users/foobar', null],
	106	+ ]);
	107	+ $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
	108	+ ->willReturnMap([
	109	+ ['principals/remote-users/foobar', null],
	110	+ ]);
	111	+
	112	+ $this->command->run(
	113	+ $this->createMock(InputInterface::class),
	114	+ $this->createMock(OutputInterface::class),
	115	+ );
	116	+
	117	+ $data = $this->selectShares();
	118	+ $this->assertCount(1, $data);
	119	+ $this->assertEquals('principal:unknown', $data[0]['principaluri']);
	120	+ }
	121	+
	122	+ public function testWithRemotePrincipal() {
	123	+ $this->principalBackend->method('getPrincipalByPath')
	124	+ ->willReturnMap([
	125	+ ['principal:unknown', null],
	126	+ ['principals/remote-users/foobar', null],
	127	+ ]);
	128	+ $this->remoteUserPrincipalBackend->method('getPrincipalByPath')
	129	+ ->willReturnMap([
	130	+ ['principal:unknown', null],
	131	+ ['principals/remote-users/foobar', ['uri' => 'principals/remote-users/foobar']],
	132	+ ]);
	133	+
	134	+ $this->command->run(
	135	+ $this->createMock(InputInterface::class),
	136	+ $this->createMock(OutputInterface::class),
	137	+ );
	138	+
	139	+ $data = $this->selectShares();
	140	+ $this->assertCount(1, $data);
	141	+ $this->assertEquals('principals/remote-users/foobar', $data[0]['principaluri']);
	142	+ }
143	143	}

		@@ -22,46 +22,46 @@
		block discarded – undo
22	22	* have no matching principal. Happened because of a bug in the calendar app.
23	23	*/
24	24	class RemoveInvalidShares extends Command {
25		- public function __construct(
26		- private IDBConnection $connection,
27		- private Principal $principalBackend,
28		- private RemoteUserPrincipalBackend $remoteUserPrincipalBackend,
29		- ) {
30		- parent::__construct();
31		- }
	25	+ public function __construct(
	26	+ private IDBConnection $connection,
	27	+ private Principal $principalBackend,
	28	+ private RemoteUserPrincipalBackend $remoteUserPrincipalBackend,
	29	+ ) {
	30	+ parent::__construct();
	31	+ }
32	32
33		- protected function configure(): void {
34		- $this
35		- ->setName('dav:remove-invalid-shares')
36		- ->setDescription('Remove invalid dav shares');
37		- }
	33	+ protected function configure(): void {
	34	+ $this
	35	+ ->setName('dav:remove-invalid-shares')
	36	+ ->setDescription('Remove invalid dav shares');
	37	+ }
38	38
39		- protected function execute(InputInterface $input, OutputInterface $output): int {
40		- $query = $this->connection->getQueryBuilder();
41		- $result = $query->selectDistinct('principaluri')
42		- ->from('dav_shares')
43		- ->executeQuery();
	39	+ protected function execute(InputInterface $input, OutputInterface $output): int {
	40	+ $query = $this->connection->getQueryBuilder();
	41	+ $result = $query->selectDistinct('principaluri')
	42	+ ->from('dav_shares')
	43	+ ->executeQuery();
44	44
45		- while ($row = $result->fetch()) {
46		- $principaluri = $row['principaluri'];
47		- $p = $this->principalBackend->getPrincipalByPath($principaluri)
48		- ?? $this->remoteUserPrincipalBackend->getPrincipalByPath($principaluri);
49		- if ($p === null) {
50		- $this->deleteSharesForPrincipal($principaluri);
51		- }
52		- }
	45	+ while ($row = $result->fetch()) {
	46	+ $principaluri = $row['principaluri'];
	47	+ $p = $this->principalBackend->getPrincipalByPath($principaluri)
	48	+ ?? $this->remoteUserPrincipalBackend->getPrincipalByPath($principaluri);
	49	+ if ($p === null) {
	50	+ $this->deleteSharesForPrincipal($principaluri);
	51	+ }
	52	+ }
53	53
54		- $result->closeCursor();
55		- return self::SUCCESS;
56		- }
	54	+ $result->closeCursor();
	55	+ return self::SUCCESS;
	56	+ }
57	57
58		- /**
59		- * @param string $principaluri
60		- */
61		- private function deleteSharesForPrincipal($principaluri): void {
62		- $delete = $this->connection->getQueryBuilder();
63		- $delete->delete('dav_shares')
64		- ->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri)));
65		- $delete->executeStatement();
66		- }
	58	+ /**
	59	+ * @param string $principaluri
	60	+ */
	61	+ private function deleteSharesForPrincipal($principaluri): void {
	62	+ $delete = $this->connection->getQueryBuilder();
	63	+ $delete->delete('dav_shares')
	64	+ ->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri)));
	65	+ $delete->executeStatement();
	66	+ }
67	67	}

nextcloud / server

Push — master ( f22232...4c1722 )

Status

Category

Indentation +114 added lines, -114 removed lines patch added patch discarded remove patch

Indentation +37 added lines, -37 removed lines patch added patch discarded remove patch