Skip to content

Commit 17607a6

Browse files
samin-zsamin-z
committed
fix: when federation is disabled, local table APIs dont fail
Signed-off-by: samin-z <samin.zavarkesh@gmail.com>
1 parent e6485cc commit 17607a6

5 files changed

Lines changed: 60 additions & 52 deletions

File tree

‎lib/Controller/Api1Controller.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCA\Tables\Model\ViewUpdateInput;
2525
use OCA\Tables\ResponseDefinitions;
2626
use OCA\Tables\Service\ColumnService;
27+
use OCA\Tables\Service\ConfigService;
2728
use OCA\Tables\Service\FederationService;
2829
use OCA\Tables\Service\ImportService;
2930
use OCA\Tables\Service\RelationService;
@@ -90,6 +91,7 @@ public function __construct(
9091
IL10N $l10N,
9192
?string $userId,
9293
private FederationService $federationService,
94+
private ConfigService $configService,
9395
) {
9496
parent::__construct(Application::APP_ID, $request);
9597
$this->tableService = $service;
@@ -772,12 +774,12 @@ public function updateShareDisplayMode(int $shareId, int $displayMode, string $t
772774
#[CORS]
773775
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
774776
public function indexTableColumns(int $tableId, ?int $viewId): DataResponse {
775-
if ($this->federationService->isNodeFederated($tableId, 'table')) {
776-
$table = $this->tableService->find($tableId, true);
777-
return new DataResponse($this->federationService->getColumns($table));
778-
}
779-
780777
try {
778+
if ($this->configService->isFederationEnabled() && $this->federationService->isNodeFederated($tableId, 'table')) {
779+
$table = $this->tableService->find($tableId, true);
780+
return new DataResponse($this->federationService->getColumns($table));
781+
}
782+
781783
if ($viewId) {
782784
$view = $this->viewService->find($viewId, false, $this->userId);
783785
if ($tableId !== $view->getTableId()) {
@@ -820,12 +822,12 @@ public function indexTableColumns(int $tableId, ?int $viewId): DataResponse {
820822
#[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_VIEW, idParam: 'viewId')]
821823
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
822824
public function indexViewColumns(int $viewId): DataResponse {
823-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
824-
$view = $this->viewService->find($viewId, true);
825-
return new DataResponse($this->federationService->getColumns($view));
826-
}
827-
828825
try {
826+
if ($this->configService->isFederationEnabled() && $this->federationService->isNodeFederated($viewId, 'view')) {
827+
$view = $this->viewService->find($viewId, true);
828+
return new DataResponse($this->federationService->getColumns($view));
829+
}
830+
829831
return new DataResponse($this->columnService->formatColumns($this->columnService->findAllByView($viewId)));
830832
} catch (PermissionError $e) {
831833
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);

‎lib/Controller/RowController.php‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use OCA\Tables\AppInfo\Application;
1111
use OCA\Tables\Middleware\Attribute\RequirePermission;
12+
use OCA\Tables\Service\ConfigService;
1213
use OCA\Tables\Service\FederationService;
1314
use OCA\Tables\Service\RowService;
1415
use OCA\Tables\Service\TableService;
@@ -30,30 +31,31 @@ public function __construct(
3031
private TableService $tableService,
3132
private ViewService $viewService,
3233
private FederationService $federationService,
34+
private ConfigService $configService,
3335
) {
3436
parent::__construct(Application::APP_ID, $request);
3537
}
3638

3739
#[NoAdminRequired]
3840
#[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')]
3941
public function index(int $tableId): DataResponse {
40-
if ($this->federationService->isNodeFederated($tableId, 'table')) {
41-
$table = $this->tableService->find($tableId, true);
42-
return new DataResponse($this->federationService->getRows($table));
43-
}
4442
return $this->handleError(function () use ($tableId) {
43+
if ($this->configService->isFederationEnabled() && $this->federationService->isNodeFederated($tableId, 'table')) {
44+
$table = $this->tableService->find($tableId, true);
45+
return $this->federationService->getRows($table);
46+
}
4547
return $this->service->findAllByTable($tableId, $this->userId);
4648
});
4749
}
4850

4951
#[NoAdminRequired]
5052
#[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_VIEW, idParam: 'viewId')]
5153
public function indexView(int $viewId): DataResponse {
52-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
53-
$view = $this->viewService->find($viewId, false, $this->userId);
54-
return new DataResponse($this->federationService->getRows($view));
55-
}
5654
return $this->handleError(function () use ($viewId) {
55+
if ($this->configService->isFederationEnabled() && $this->federationService->isNodeFederated($viewId, 'view')) {
56+
$view = $this->viewService->find($viewId, false, $this->userId);
57+
return $this->federationService->getRows($view);
58+
}
5759
return $this->service->findAllByView($viewId, $this->userId);
5860
});
5961
}
@@ -112,10 +114,10 @@ public function destroyByView(int $id, int $viewId): DataResponse {
112114

113115
#[NoAdminRequired]
114116
public function presentInView(int $id, int $viewId): DataResponse {
115-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
116-
return new DataResponse(['present' => true]);
117-
}
118117
return $this->handleError(function () use ($id, $viewId) {
118+
if ($this->configService->isFederationEnabled() && $this->federationService->isNodeFederated($viewId, 'view')) {
119+
return ['present' => true];
120+
}
119121
$present = $this->service->isRowInViewPresent($id, $viewId, $this->userId);
120122
return ['present' => $present];
121123
});

‎lib/Controller/RowOCSController.php‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCA\Tables\Middleware\Attribute\RequirePermission;
1717
use OCA\Tables\Model\RowDataInput;
1818
use OCA\Tables\ResponseDefinitions;
19+
use OCA\Tables\Service\ConfigService;
1920
use OCA\Tables\Service\FederationService;
2021
use OCA\Tables\Service\RowService;
2122
use OCA\Tables\Service\TableService;
@@ -42,6 +43,7 @@ public function __construct(
4243
private TableService $tableService,
4344
private ViewService $viewService,
4445
private FederationService $federationService,
46+
private ConfigService $configService,
4547
) {
4648
parent::__construct($request, $logger, $n, $userId);
4749
}
@@ -75,16 +77,8 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat
7577
$tableId = $viewId = null;
7678
if ($iNodeType === Application::NODE_TYPE_TABLE) {
7779
$tableId = $nodeId;
78-
if ($this->federationService->isNodeFederated($tableId, 'table')) {
79-
$table = $this->tableService->find($nodeId, true);
80-
return new DataResponse($this->federationService->createRow($table, $data));
81-
}
8280
} elseif ($iNodeType === Application::NODE_TYPE_VIEW) {
8381
$viewId = $nodeId;
84-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
85-
$view = $this->viewService->find($nodeId, false, $this->userId);
86-
return new DataResponse($this->federationService->createRow($view, $data));
87-
}
8882
}
8983

9084
$newRowData = new RowDataInput();
@@ -93,6 +87,16 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat
9387
}
9488

9589
try {
90+
if ($this->configService->isFederationEnabled()) {
91+
if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) {
92+
$table = $this->tableService->find($nodeId, true);
93+
return new DataResponse($this->federationService->createRow($table, $data));
94+
}
95+
if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) {
96+
$view = $this->viewService->find($nodeId, false, $this->userId);
97+
return new DataResponse($this->federationService->createRow($view, $data));
98+
}
99+
}
96100
return new DataResponse($this->rowService->create($tableId, $viewId, $newRowData)->jsonSerialize());
97101
} catch (BadRequestError $e) {
98102
return $this->handleBadRequestError($e);
@@ -134,18 +138,20 @@ public function updateRow(string $nodeCollection, int $nodeId, int $rowId, mixed
134138
$tableId = $viewId = null;
135139
if ($iNodeType === Application::NODE_TYPE_TABLE) {
136140
$tableId = $nodeId;
137-
if ($this->federationService->isNodeFederated($tableId, 'table')) {
138-
$table = $this->tableService->find($nodeId, true);
139-
return new DataResponse($this->federationService->updateRow($table, $rowId, $data));
140-
}
141141
} elseif ($iNodeType === Application::NODE_TYPE_VIEW) {
142142
$viewId = $nodeId;
143-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
144-
$view = $this->viewService->find($nodeId, false, $this->userId);
145-
return new DataResponse($this->federationService->updateRow($view, $rowId, $data));
146-
}
147143
}
148144
try {
145+
if ($this->configService->isFederationEnabled()) {
146+
if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) {
147+
$table = $this->tableService->find($nodeId, true);
148+
return new DataResponse($this->federationService->updateRow($table, $rowId, $data));
149+
}
150+
if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) {
151+
$view = $this->viewService->find($nodeId, false, $this->userId);
152+
return new DataResponse($this->federationService->updateRow($view, $rowId, $data));
153+
}
154+
}
149155
return new DataResponse($this->rowService->updateSet($rowId, $viewId, $data, $this->userId, $tableId)->jsonSerialize());
150156
} catch (NotFoundError $e) {
151157
return $this->handleNotFoundError($e);
@@ -177,18 +183,20 @@ public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): Data
177183
$tableId = $viewId = null;
178184
if ($iNodeType === Application::NODE_TYPE_TABLE) {
179185
$tableId = $nodeId;
180-
if ($this->federationService->isNodeFederated($tableId, 'table')) {
181-
$table = $this->tableService->find($nodeId, true);
182-
return new DataResponse($this->federationService->deleteRow($table, $rowId));
183-
}
184186
} elseif ($iNodeType === Application::NODE_TYPE_VIEW) {
185187
$viewId = $nodeId;
186-
if ($this->federationService->isNodeFederated($viewId, 'view')) {
187-
$view = $this->viewService->find($nodeId, false, $this->userId);
188-
return new DataResponse($this->federationService->deleteRow($view, $rowId));
189-
}
190188
}
191189
try {
190+
if ($this->configService->isFederationEnabled()) {
191+
if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) {
192+
$table = $this->tableService->find($nodeId, true);
193+
return new DataResponse($this->federationService->deleteRow($table, $rowId));
194+
}
195+
if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) {
196+
$view = $this->viewService->find($nodeId, false, $this->userId);
197+
return new DataResponse($this->federationService->deleteRow($view, $rowId));
198+
}
199+
}
192200
return new DataResponse($this->rowService->delete($rowId, $viewId, $this->userId, $tableId)->jsonSerialize());
193201
} catch (NotFoundError $e) {
194202
return $this->handleNotFoundError($e);

‎lib/Service/FederationService.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,9 @@ public function notifyShareDelete(Share $share): void {
223223
);
224224
}
225225

226-
/**
227-
* @throws FederationDisabledError
228-
*/
229226
public function isNodeFederated(int $id, string $nodeType): bool {
230227
if (!$this->configService->isFederationEnabled()) {
231-
throw new FederationDisabledError('Federation is disabled');
228+
return false;
232229
}
233230

234231
return match($nodeType) {

‎tests/unit/Service/FederationServiceTest.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,9 @@ protected function setUp(): void {
6464
);
6565
}
6666

67-
public function testIsNodeFederatedThrowsWhenFederationDisabled(): void {
67+
public function testIsNodeFederatedReturnsFalseWhenFederationDisabled(): void {
6868
$this->configService->method('isFederationEnabled')->willReturn(false);
69-
$this->expectException(FederationDisabledError::class);
70-
$this->federationService->isNodeFederated(1, 'table');
69+
$this->assertFalse($this->federationService->isNodeFederated(1, 'table'));
7170
}
7271

7372
public function testIsNodeFederatedReturnsTrueForFederatedTable(): void {

0 commit comments

Comments
 (0)