Skip to content

Commit ac12d1e

Browse files
mejo-mejo-
committed
chore(api): Throw OCS exceptions
Signed-off-by: Jonas <jonas@freesources.org>
1 parent 9d5c98c commit ac12d1e

4 files changed

Lines changed: 36 additions & 42 deletions

File tree

‎lib/Controller/SessionController.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,17 @@ public function __construct(
3737
*
3838
* @param int $collectiveId ID of the collective
3939
*
40-
* @return DataResponse<Http::STATUS_OK, array{token: string}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error: string}, array{}>
40+
* @return DataResponse<Http::STATUS_OK, array{token: string}, array{}>
41+
* @throws OCSNotFoundException Collective not found
4142
*
4243
* 200: Session created, token returned
43-
* 404: Collective not found
4444
*/
4545
#[NoAdminRequired]
4646
public function create(int $collectiveId): DataResponse {
4747
try {
4848
$session = $this->sessionService->initSession($collectiveId, $this->userId);
4949
} catch (NotFoundException $e) {
50-
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_NOT_FOUND);
50+
throw new OCSNotFoundException($e->getMessage());
5151
}
5252
return new DataResponse(['token' => $session->getToken()]);
5353
}
@@ -58,17 +58,17 @@ public function create(int $collectiveId): DataResponse {
5858
* @param int $collectiveId ID of the collective
5959
* @param string $token Token of the session
6060
*
61-
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error: string}, array{}>
61+
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
62+
* @throws OCSNotFoundException Session not found
6263
*
6364
* 200: Session updated
64-
* 404: Session not found
6565
*/
6666
#[NoAdminRequired]
6767
public function sync(int $collectiveId, string $token): DataResponse {
6868
try {
6969
$this->sessionService->syncSession($collectiveId, $token, $this->userId);
7070
} catch (NotFoundException $e) {
71-
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_NOT_FOUND);
71+
throw new OCSNotFoundException($e->getMessage());
7272
}
7373
return new DataResponse([]);
7474
}

‎lib/Controller/SettingsController.php‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCP\AppFramework\Http;
1313
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1414
use OCP\AppFramework\Http\DataResponse;
15+
use OCP\AppFramework\OCS\OCSBadRequestException;
1516
use OCP\AppFramework\OCSController;
1617
use OCP\IConfig;
1718
use OCP\IRequest;
@@ -30,53 +31,52 @@ public function __construct(
3031
parent::__construct($appName, $request);
3132
}
3233

34+
/**
35+
* @throws OCSBadRequestException
36+
*/
3337
private function validateGetUserSetting(string $setting): void {
3438
if ($setting === 'user_folder') {
3539
return;
3640
}
3741

38-
throw new InvalidArgumentException('Unsupported setting ' . $setting);
42+
throw new OCSBadRequestException('Unsupported setting ' . $setting);
3943
}
4044

4145
/**
42-
* @throws InvalidArgumentException
46+
* @throws OCSBadRequestException
4347
*/
4448
private function validateSetUserSetting(string $setting, ?string $value): void {
4549
if ($value === null) {
46-
throw new InvalidArgumentException('Empty value for setting ' . $setting);
50+
throw new OCSBadRequestException('Empty value for setting ' . $setting);
4751
}
4852

4953
if ($setting === 'user_folder') {
5054
// No root folder, has to start with `/`, not allowed to end with `/`
5155
if ($value === '/'
5256
|| !(str_starts_with($value, '/'))
5357
|| str_ends_with($value, '/')) {
54-
throw new InvalidArgumentException('Invalid collectives folder path');
58+
throw new OCSBadRequestException('Invalid collectives folder path');
5559
}
5660

5761
return;
5862
}
5963

60-
throw new InvalidArgumentException('Unsupported setting ' . $setting);
64+
throw new OCSBadRequestException('Unsupported setting ' . $setting);
6165
}
6266

6367
/**
6468
* Get a collectives user setting by key (defaults to empty string if key is unset)
6569
*
6670
* @param string $key The key to get
6771
*
68-
* @return DataResponse<Http::STATUS_OK, array<string>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}>
72+
* @return DataResponse<Http::STATUS_OK, array<string>, array{}>
73+
* @throws OCSBadRequestException Invalid key
6974
*
7075
* 200: Valid key, value returned
71-
* 400: Invalid key
7276
*/
7377
#[NoAdminRequired]
7478
public function getUserSetting(string $key): DataResponse {
75-
try {
76-
$this->validateGetUserSetting($key);
77-
} catch (InvalidArgumentException $e) {
78-
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
79-
}
79+
$this->validateGetUserSetting($key);
8080
return new DataResponse([$key => $this->config->getUserValue($this->userId, 'collectives', $key, '')]);
8181
}
8282

@@ -86,18 +86,14 @@ public function getUserSetting(string $key): DataResponse {
8686
* @param string $key The key to set
8787
* @param string $value The value
8888
*
89-
* @return DataResponse<Http::STATUS_OK, array<string>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}>
89+
* @return DataResponse<Http::STATUS_OK, array<string>, array{}>
90+
* @throws OCSBadRequestException Invalid key
9091
*
9192
* 200: Valid key set to value
92-
* 400: Invalid key
9393
*/
9494
#[NoAdminRequired]
9595
public function setUserSetting(string $key, string $value): DataResponse {
96-
try {
97-
$this->validateSetUserSetting($key, $value);
98-
} catch (InvalidArgumentException $e) {
99-
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
100-
}
96+
$this->validateSetUserSetting($key, $value);
10197
$this->config->setUserValue($this->userId, 'collectives', $key, $value);
10298
return new DataResponse([$key => $value]);
10399
}

‎tests/Integration/features/bootstrap/FeatureContext.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,10 +704,10 @@ public function userGetsSetting(string $user, string $key, ?string $value = null
704704

705705
$jsonBody = $this->getJson();
706706
if ($value) {
707-
Assert::assertEquals($value, $jsonBody['ocs']['data']);
707+
Assert::assertEquals($value, $jsonBody['ocs']['data'][$key]);
708708
}
709709

710-
return $jsonBody['ocs']['data'];
710+
return $jsonBody['ocs']['data'][$key];
711711
}
712712

713713
/**

‎tests/Unit/Controller/SettingsControllerTest.php‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCA\Collectives\Controller\SettingsController;
1313
use OCP\AppFramework\Http;
1414
use OCP\AppFramework\Http\DataResponse;
15+
use OCP\AppFramework\OCS\OCSBadRequestException;
1516
use OCP\IConfig;
1617
use OCP\IRequest;
1718
use PHPUnit\Framework\TestCase;
@@ -38,40 +39,37 @@ protected function setUp(): void {
3839
}
3940

4041
public function testSetUserSettingInvalidSetting(): void {
41-
$response = new DataResponse('Unsupported setting nonexistent', Http::STATUS_BAD_REQUEST);
42-
self::assertEquals($response, $this->settingsController->setUserSetting('nonexistent', 'value'));
42+
$this->expectException(OCSBadRequestException::class);
43+
$this->settingsController->setUserSetting('nonexistent', 'value');
4344
}
4445

4546
public function testSetUserSettingsEmptyString(): void {
46-
$response = new DataResponse('Invalid collectives folder path', Http::STATUS_BAD_REQUEST);
47-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', ''));
47+
$this->expectException(OCSBadRequestException::class);
48+
$this->settingsController->setUserSetting('user_folder', '');
4849
}
4950

5051
public function testSetUserSettingsNoSlashStart(): void {
51-
$response = new DataResponse('Invalid collectives folder path', Http::STATUS_BAD_REQUEST);
52-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', 'test'));
53-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', 'test/'));
54-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', 'test/abc'));
52+
$this->expectException(OCSBadRequestException::class);
53+
$this->settingsController->setUserSetting('user_folder', 'test');
5554
}
5655

5756
public function testSetUserSettingsRootFolder(): void {
58-
$response = new DataResponse('Invalid collectives folder path', Http::STATUS_BAD_REQUEST);
59-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', '/'));
57+
$this->expectException(OCSBadRequestException::class);
58+
$this->settingsController->setUserSetting('user_folder', '/');
6059
}
6160

6261
public function testSetUserSettingsSlashEnd(): void {
63-
$response = new DataResponse('Invalid collectives folder path', Http::STATUS_BAD_REQUEST);
64-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', '/test/'));
65-
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', '/test/abc/'));
62+
$this->expectException(OCSBadRequestException::class);
63+
$this->settingsController->setUserSetting('user_folder', '/test/');
6664
}
6765

6866
public function testSetUserSettings(): void {
69-
$response = new DataResponse('/test', Http::STATUS_OK);
67+
$response = new DataResponse(['user_folder' => '/test'], Http::STATUS_OK);
7068
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', '/test'));
7169
}
7270

7371
public function testSetUserSettingsSubfolder(): void {
74-
$response = new DataResponse('/test/abc', Http::STATUS_OK);
72+
$response = new DataResponse(['user_folder' => '/test/abc'], Http::STATUS_OK);
7573
self::assertEquals($response, $this->settingsController->setUserSetting('user_folder', '/test/abc'));
7674
}
7775
}

0 commit comments

Comments
 (0)