Skip to content

Commit 37215b3

Browse files
miaulalalamiaulalala
authored
Merge pull request #2573 from nextcloud/refactor/type-safety-data-get
refactor(types): add native parameter/return types to Data::get and APIv2Controller
2 parents 8665c86 + d03aa88 commit 37215b3

3 files changed

Lines changed: 33 additions & 76 deletions

File tree

‎lib/Controller/APIv2Controller.php‎

Lines changed: 18 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -28,29 +28,14 @@
2828
use OCP\Notification\IManager as INotificationManager;
2929

3030
class APIv2Controller extends OCSController {
31-
/** @var string */
32-
protected $filter;
33-
34-
/** @var int */
35-
protected $since;
36-
37-
/** @var int */
38-
protected $limit;
39-
40-
/** @var string */
41-
protected $sort;
42-
43-
/** @var string */
44-
protected $objectType;
45-
46-
/** @var int */
47-
protected $objectId;
48-
49-
/** @var string */
50-
protected $user;
51-
52-
/** @var bool */
53-
protected $loadPreviews;
31+
protected string $filter = 'all';
32+
protected int $since = 0;
33+
protected int $limit = 50;
34+
protected string $sort = 'desc';
35+
protected string $objectType = '';
36+
protected int $objectId = 0;
37+
protected string $user = '';
38+
protected bool $loadPreviews = false;
5439

5540
public function __construct(
5641
$appName,
@@ -71,26 +56,19 @@ public function __construct(
7156
}
7257

7358
/**
74-
* @param string $filter
75-
* @param int $since
76-
* @param int $limit
77-
* @param bool $previews
78-
* @param string $objectType
79-
* @param int $objectId
80-
* @param string $sort
8159
* @throws InvalidFilterException when the filter is invalid
8260
* @throws \OutOfBoundsException when no user is given
8361
*/
84-
protected function validateParameters($filter, $since, $limit, $previews, $objectType, $objectId, $sort) {
85-
$this->filter = \is_string($filter) ? $filter : 'all';
62+
protected function validateParameters(string $filter, int $since, int $limit, bool $previews, string $objectType, int $objectId, string $sort): void {
63+
$this->filter = $filter;
8664
if ($this->filter !== $this->data->validateFilter($this->filter)) {
8765
throw new InvalidFilterException('Invalid filter');
8866
}
89-
$this->since = (int)$since;
90-
$this->limit = (int)$limit;
91-
$this->loadPreviews = (bool)$previews;
92-
$this->objectType = (string)$objectType;
93-
$this->objectId = (int)$objectId;
67+
$this->since = $since;
68+
$this->limit = $limit;
69+
$this->loadPreviews = $previews;
70+
$this->objectType = $objectType;
71+
$this->objectId = $objectId;
9472
$this->sort = \in_array($sort, ['asc', 'desc'], true) ? $sort : 'desc';
9573

9674
if (($this->objectType !== '' && $this->objectId === 0) || ($this->objectType === '' && $this->objectId !== 0)) {
@@ -110,32 +88,15 @@ protected function validateParameters($filter, $since, $limit, $previews, $objec
11088

11189
/**
11290
* @NoAdminRequired
113-
*
114-
* @param int $since
115-
* @param int $limit
116-
* @param bool $previews
117-
* @param string $object_type
118-
* @param int $object_id
119-
* @param string $sort
120-
* @return DataResponse
12191
*/
122-
public function getDefault($since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc'): DataResponse {
92+
public function getDefault(int $since = 0, int $limit = 50, bool $previews = false, string $object_type = '', int $object_id = 0, string $sort = 'desc'): DataResponse {
12393
return $this->get('all', $since, $limit, $previews, $object_type, $object_id, $sort);
12494
}
12595

12696
/**
12797
* @NoAdminRequired
128-
*
129-
* @param string $filter
130-
* @param int $since
131-
* @param int $limit
132-
* @param bool $previews
133-
* @param string $object_type
134-
* @param int $object_id
135-
* @param string $sort
136-
* @return DataResponse
13798
*/
138-
public function getFilter($filter, $since = 0, $limit = 50, $previews = false, $object_type = '', $object_id = 0, $sort = 'desc'): DataResponse {
99+
public function getFilter(string $filter, int $since = 0, int $limit = 50, bool $previews = false, string $object_type = '', int $object_id = 0, string $sort = 'desc'): DataResponse {
139100
return $this->get($filter, $since, $limit, $previews, $object_type, $object_id, $sort);
140101
}
141102

@@ -191,17 +152,7 @@ public function listFilters(): DataResponse {
191152
return new DataResponse($filters);
192153
}
193154

194-
/**
195-
* @param string $filter
196-
* @param int $since
197-
* @param int $limit
198-
* @param bool $previews
199-
* @param string $filterObjectType
200-
* @param int $filterObjectId
201-
* @param string $sort
202-
* @return DataResponse
203-
*/
204-
protected function get($filter, $since, $limit, $previews, $filterObjectType, $filterObjectId, $sort): DataResponse {
155+
protected function get(string $filter, int $since, int $limit, bool $previews, string $filterObjectType, int $filterObjectId, string $sort): DataResponse {
205156
try {
206157
$this->validateParameters($filter, $since, $limit, $previews, $filterObjectType, $filterObjectId, $sort);
207158
} catch (InvalidFilterException $e) {

‎lib/Data.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public function storeMail(IEvent $event, int $latestSendTime): bool {
226226
* @return array
227227
*
228228
*/
229-
public function get(GroupHelper $groupHelper, UserSettings $userSettings, $user, $since, $limit, $sort, $filter, $objectType = '', $objectId = 0, bool $returnEvents = false) {
229+
public function get(GroupHelper $groupHelper, UserSettings $userSettings, string $user, int $since, int $limit, string $sort, string $filter, string $objectType = '', int $objectId = 0, bool $returnEvents = false): array {
230230
// get current user
231231
if ($user === '') {
232232
throw new \OutOfBoundsException('Invalid user', 1);
@@ -323,15 +323,15 @@ public function get(GroupHelper $groupHelper, UserSettings $userSettings, $user,
323323
*
324324
* @throws \OutOfBoundsException If $since is not owned by $user
325325
*/
326-
protected function setOffsetFromSince(IQueryBuilder $query, $user, $since, $sort) {
326+
protected function setOffsetFromSince(IQueryBuilder $query, string $user, int $since, string $sort): array {
327327
if (!$since) {
328328
return $this->getFirstKnownActivityHeader($user, $sort);
329329
}
330330

331331
$queryBuilder = $this->connection->getQueryBuilder();
332332
$queryBuilder->select(['affecteduser', 'timestamp'])
333333
->from('activity')
334-
->where($queryBuilder->expr()->eq('activity_id', $queryBuilder->createNamedParameter((int)$since)));
334+
->where($queryBuilder->expr()->eq('activity_id', $queryBuilder->createNamedParameter($since)));
335335
$result = $queryBuilder->executeQuery();
336336
$activity = $result->fetch();
337337
$result->closeCursor();

‎tests/Controller/APIv2ControllerTest.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,11 @@ public function testValidateParametersFilter(string $param, string $filter): voi
140140
->method('validateFilter')
141141
->with($param)
142142
->willReturn($filter);
143+
$userMock = $this->createMock(IUser::class);
144+
$userMock->method('getUID')->willReturn('testuser');
143145
$this->userSession->expects($this->once())
144146
->method('getUser')
145-
->willReturn($this->createMock(IUser::class));
147+
->willReturn($userMock);
146148

147149
self::invokePrivate($this->controller, 'validateParameters', [$param, 0, 0, false, '', 0, 'desc']);
148150
$this->assertSame($filter, self::invokePrivate($this->controller, 'filter'));
@@ -173,19 +175,21 @@ public static function dataValidateParametersObject(): array {
173175
return [
174176
['type', 42, 'type', 42],
175177
['type', '42', 'type', 42],
176-
[null, '42', '', 0],
177-
['type', null, '', 0],
178+
['', 42, '', 0],
179+
['type', 0, '', 0],
178180
];
179181
}
180182

181183
#[DataProvider('dataValidateParametersObject')]
182-
public function testValidateParametersObject(?string $type, mixed $id, string $expectedType, int $expectedId): void {
184+
public function testValidateParametersObject(string $type, mixed $id, string $expectedType, int $expectedId): void {
183185
$this->data->expects($this->once())
184186
->method('validateFilter')
185187
->willReturnArgument(0);
188+
$userMock = $this->createMock(IUser::class);
189+
$userMock->method('getUID')->willReturn('testuser');
186190
$this->userSession->expects($this->once())
187191
->method('getUser')
188-
->willReturn($this->createMock(IUser::class));
192+
->willReturn($userMock);
189193

190194
self::invokePrivate($this->controller, 'validateParameters', ['all', 0, 0, false, $type, $id, 'desc']);
191195
$this->assertSame($expectedType, self::invokePrivate($this->controller, 'objectType'));
@@ -229,9 +233,11 @@ public function testValidateParameters(string $param, mixed $value, string $memb
229233
$this->data->expects($this->once())
230234
->method('validateFilter')
231235
->willReturnArgument(0);
236+
$userMock = $this->createMock(IUser::class);
237+
$userMock->method('getUID')->willReturn('testuser');
232238
$this->userSession->expects($this->once())
233239
->method('getUser')
234-
->willReturn($this->createMock(IUser::class));
240+
->willReturn($userMock);
235241

236242
self::invokePrivate($this->controller, 'validateParameters', $params);
237243
$this->assertSame($expectedValue, self::invokePrivate($this->controller, $memberName));

0 commit comments

Comments
 (0)