Skip to content

Commit 1de5a7a

Browse files
dartcafedartcafe
authored
Merge pull request #3996 from GeorgeApos/improve-duplication
Refactor BasePublicController to reduce duplicated code
2 parents ecfd945 + a4dc019 commit 1de5a7a

15 files changed

Lines changed: 86 additions & 256 deletions

‎lib/Controller/AdminController.php‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -95,24 +95,4 @@ public function runJanitorJob(): JSONResponse {
9595
public function runNotificationJob(): JSONResponse {
9696
return $this->response(fn () => $this->notificationCron->manuallyRun());
9797
}
98-
99-
/**
100-
* Switch archived status (move to archived polls)
101-
* @param int $pollId poll id
102-
* @deprecated 8.0.0 Not used anymore (use PUT /poll/{pollId}/toggleArchive)
103-
*/
104-
#[FrontpageRoute(verb: 'PUT', url: '/administration/poll/{pollId}/toggleArchive')]
105-
public function toggleArchive(int $pollId): JSONResponse {
106-
return $this->response(fn () => $this->pollService->toggleArchive($pollId));
107-
}
108-
109-
/**
110-
* Delete poll
111-
* @param int $pollId poll id
112-
* @deprecated 8.0.0 Not used anymore (use DELETE /poll/{pollId})
113-
*/
114-
#[FrontpageRoute(verb: 'DELETE', url: '/administration/poll/{pollId}')]
115-
public function delete(int $pollId): JSONResponse {
116-
return $this->responseDeleteTolerant(fn () => $this->pollService->delete($pollId));
117-
}
11898
}

‎lib/Controller/BaseApiV1Controller.php‎

Lines changed: 0 additions & 72 deletions
This file was deleted.

‎lib/Controller/BaseApiV2Controller.php‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
use Closure;
1212
use OCA\Polls\Exceptions\Exception;
13-
use OCA\Polls\Exceptions\NoUpdatesException;
1413
use OCP\AppFramework\Db\DoesNotExistException;
1514
use OCP\AppFramework\Http;
1615
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -39,40 +38,19 @@ public function __construct(
3938
* @param Closure $callback Callback function
4039
*/
4140
#[NoAdminRequired]
42-
protected function response(Closure $callback): DataResponse {
41+
protected function response(Closure $callback, int $successStatus = Http::STATUS_OK): DataResponse {
4342
try {
44-
return new DataResponse($callback());
45-
} catch (DoesNotExistException $e) {
46-
throw new OCSNotFoundException($e->getMessage());
47-
} catch (Exception $e) {
48-
throw new OCSBadRequestException($e->getMessage());
49-
}
50-
}
43+
return new DataResponse($callback(), $successStatus);
5144

52-
/**
53-
* response
54-
* @param Closure $callback Callback function
55-
*/
56-
#[NoAdminRequired]
57-
protected function responseLong(Closure $callback): DataResponse {
58-
try {
59-
return new DataResponse($callback());
6045
} catch (DoesNotExistException $e) {
6146
throw new OCSNotFoundException($e->getMessage());
62-
} catch (NoUpdatesException $e) {
63-
return new DataResponse([], Http::STATUS_NOT_MODIFIED);
64-
}
65-
}
6647

67-
/**
68-
* responseCreate
69-
* @param Closure $callback Callback function
70-
*/
71-
#[NoAdminRequired]
72-
protected function responseCreate(Closure $callback): DataResponse {
73-
try {
74-
return new DataResponse($callback(), Http::STATUS_CREATED);
7548
} catch (Exception $e) {
49+
50+
if ($e->getStatus() === Http::STATUS_NOT_MODIFIED) {
51+
return new DataResponse(statusCode: $e->getStatus());
52+
}
53+
7654
throw new OCSBadRequestException($e->getMessage());
7755
}
7856
}

‎lib/Controller/BaseController.php‎

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@
1010

1111
use Closure;
1212
use OCA\Polls\Exceptions\Exception;
13-
use OCA\Polls\Exceptions\NoUpdatesException;
1413
use OCP\AppFramework\Controller;
15-
use OCP\AppFramework\Db\DoesNotExistException;
1614
use OCP\AppFramework\Http;
1715
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1816
use OCP\AppFramework\Http\JSONResponse;
@@ -32,54 +30,23 @@ public function __construct(
3230
/**
3331
* response
3432
* @param Closure $callback Callback function
33+
* @param int $successStatus HTTP status code for success
3534
*/
3635
#[NoAdminRequired]
37-
protected function response(Closure $callback): JSONResponse {
36+
protected function response(
37+
Closure $callback,
38+
int $successStatus = Http::STATUS_OK,
39+
): JSONResponse {
3840
try {
39-
return new JSONResponse($callback());
41+
return new JSONResponse($callback(), $successStatus);
4042
} catch (Exception $e) {
41-
return new JSONResponse(['message' => $e->getMessage()], $e->getStatus());
42-
}
43-
}
4443

45-
/**
46-
* response
47-
* @param Closure $callback Callback function
48-
*/
49-
#[NoAdminRequired]
50-
protected function responseLong(Closure $callback): JSONResponse {
51-
try {
52-
return new JSONResponse($callback());
53-
} catch (NoUpdatesException $e) {
54-
return new JSONResponse([], Http::STATUS_NOT_MODIFIED);
55-
}
56-
}
44+
if ($e->getStatus() === Http::STATUS_NOT_MODIFIED) {
45+
return new JSONResponse(statusCode: $e->getStatus());
46+
}
5747

58-
/**
59-
* responseCreate
60-
* @param Closure $callback Callback function
61-
*/
62-
#[NoAdminRequired]
63-
protected function responseCreate(Closure $callback): JSONResponse {
64-
try {
65-
return new JSONResponse($callback(), Http::STATUS_CREATED);
66-
} catch (Exception $e) {
6748
return new JSONResponse(['message' => $e->getMessage()], $e->getStatus());
6849
}
6950
}
7051

71-
/**
72-
* responseDeleteTolerant
73-
* @param Closure $callback Callback function
74-
*/
75-
#[NoAdminRequired]
76-
protected function responseDeleteTolerant(Closure $callback): JSONResponse {
77-
try {
78-
return new JSONResponse($callback());
79-
} catch (DoesNotExistException $e) {
80-
return new JSONResponse(['message' => 'Not found, assume already deleted']);
81-
} catch (Exception $e) {
82-
return new JSONResponse(['message' => $e->getMessage()], $e->getStatus());
83-
}
84-
}
8552
}

‎lib/Controller/BasePublicController.php‎

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎lib/Controller/OptionApiController.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCA\Polls\Model\SimpleOption;
1313
use OCA\Polls\Service\OptionService;
1414
use OCA\Polls\Service\VoteService;
15+
use OCP\AppFramework\Http;
1516
use OCP\AppFramework\Http\Attribute\ApiRoute;
1617
use OCP\AppFramework\Http\Attribute\CORS;
1718
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -61,7 +62,7 @@ public function add(
6162
bool $voteYes = false,
6263
?array $sequence = null,
6364
): DataResponse {
64-
return $this->responseCreate(fn () => array_merge(
65+
return $this->response(fn () => array_merge(
6566
$this->optionService->addWithSequenceAndAutoVote(
6667
$pollId,
6768
SimpleOption::fromArray($option),
@@ -70,7 +71,7 @@ public function add(
7071
),
7172
['options' => $this->optionService->list($pollId)],
7273
['votes' => $this->voteService->list($pollId)],
73-
));
74+
), Http::STATUS_CREATED);
7475
}
7576

7677

@@ -84,7 +85,7 @@ public function add(
8485
#[NoCSRFRequired]
8586
#[ApiRoute(verb: 'POST', url: '/api/v1.0/poll/{pollId}/options', requirements: ['apiVersion' => '(v2)'])]
8687
public function addBulk(int $pollId, string $text = ''): DataResponse {
87-
return $this->responseCreate(fn () => ['options' => $this->optionService->addBulk($pollId, $text)]);
88+
return $this->response(fn () => ['options' => $this->optionService->addBulk($pollId, $text)], Http::STATUS_CREATED);
8889
}
8990

9091
/**

‎lib/Controller/OptionController.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCA\Polls\Service\CalendarService;
1414
use OCA\Polls\Service\OptionService;
1515
use OCA\Polls\Service\VoteService;
16+
use OCP\AppFramework\Http;
1617
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
1718
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1819
use OCP\AppFramework\Http\JSONResponse;
@@ -60,7 +61,7 @@ public function add(
6061
bool $voteYes = false,
6162
?array $sequence = null,
6263
): JSONResponse {
63-
return $this->responseCreate(fn () => array_merge(
64+
return $this->response(fn () => array_merge(
6465
$this->optionService->addWithSequenceAndAutoVote(
6566
$pollId,
6667
SimpleOption::fromArray($option),
@@ -69,7 +70,8 @@ public function add(
6970
),
7071
['options' => $this->optionService->list($pollId)],
7172
['votes' => $this->voteService->list($pollId)],
72-
));
73+
),
74+
Http::STATUS_CREATED);
7375
}
7476

7577
/**
@@ -80,7 +82,7 @@ public function add(
8082
#[NoAdminRequired]
8183
#[FrontpageRoute(verb: 'POST', url: '/option/bulk')]
8284
public function addBulk(int $pollId, string $text = ''): JSONResponse {
83-
return $this->responseCreate(fn () => ['options' => $this->optionService->addBulk($pollId, $text)]);
85+
return $this->response(fn () => ['options' => $this->optionService->addBulk($pollId, $text)], Http::STATUS_CREATED);
8486
}
8587

8688
/**

‎lib/Controller/PollApiController.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCA\Polls\Service\ShareService;
1616
use OCA\Polls\Service\SubscriptionService;
1717
use OCA\Polls\Service\VoteService;
18+
use OCP\AppFramework\Http;
1819
use OCP\AppFramework\Http\Attribute\ApiRoute;
1920
use OCP\AppFramework\Http\Attribute\CORS;
2021
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -80,7 +81,7 @@ public function get(int $pollId): DataResponse {
8081
#[NoCSRFRequired]
8182
#[ApiRoute(verb: 'POST', url: '/api/v1.0/poll', requirements: ['apiVersion' => '(v2)'])]
8283
public function add(string $type, string $title, string $votingVariant = Poll::VARIANT_SIMPLE): DataResponse {
83-
return $this->responseCreate(fn () => ['poll' => $this->pollService->add($type, $title, $votingVariant)]);
84+
return $this->response(fn () => ['poll' => $this->pollService->add($type, $title, $votingVariant)], Http::STATUS_CREATED);
8485
}
8586

8687
/**
@@ -153,7 +154,7 @@ public function delete(int $pollId): DataResponse {
153154
#[NoCSRFRequired]
154155
#[ApiRoute(verb: 'POST', url: '/api/v1.0/poll/{pollId}/clone', requirements: ['apiVersion' => '(v2)'])]
155156
public function clone(int $pollId): DataResponse {
156-
return $this->responseCreate(fn () => ['poll' => $this->pollService->clone($pollId)]);
157+
return $this->response(fn () => ['poll' => $this->pollService->clone($pollId)], Http::STATUS_CREATED);
157158
}
158159

159160
/**

0 commit comments

Comments
 (0)