Skip to content

refactor: Fix some typing issues #5337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
CarlSchwan wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: Fix some typing issues #5337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 29 additions & 61 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\JSONResponse;
Expand Down Expand Up @@ -254,18 +256,12 @@ public function updateWatermarkSettings($settings = []): JSONResponse {
return new JSONResponse($response);
}

/**
* @NoAdminRequired
*
* @param $key
* @param $value
* @return JSONResponse
*/
public function setPersonalSettings($templateFolder,
$zoteroAPIKeyInput,
$documentSigningCertInput,
$documentSigningKeyInput,
$documentSigningCaInput) {
#[NoAdminRequired]
public function setPersonalSettings(?string $templateFolder,
?string $zoteroAPIKeyInput,
?string $documentSigningCertInput,
?string $documentSigningKeyInput,
?string $documentSigningCaInput): JSONResponse {
$message = $this->l10n->t('Saved');
$status = 'success';

Expand Down Expand Up @@ -320,14 +316,12 @@ public function setPersonalSettings($templateFolder,
}

/**
* @NoAdminRequired
* @PublicPage
* @NoCSRFRequired
*
* @return JSONResponse|DataResponse
* @throws \OCP\Files\NotPermittedException
*/
public function getFontNames() {
#[NoAdminRequired]
#[PublicPage]
#[NoCSRFRequired]
public function getFontNames(): JSONResponse|DataResponse {
$fileNames = $this->fontService->getFontFileNames();
$etag = md5(implode('/', $fileNames));
$ifNoneMatchHeader = $this->request->getHeader('If-None-Match');
Expand All @@ -340,14 +334,12 @@ public function getFontNames() {
}

/**
* @NoAdminRequired
* @PublicPage
* @NoCSRFRequired
*
* @return JSONResponse|DataResponse
* @throws \OCP\Files\NotPermittedException
*/
public function getJsonFontList() {
#[NoAdminRequired]
#[PublicPage]
#[NoCSRFRequired]
public function getJsonFontList(): JSONResponse|DataResponse {
$files = $this->fontService->getFontFiles();
$etags = array_map(
static fn (ISimpleFile $f) => $f->getETag(),
Expand All @@ -366,15 +358,12 @@ public function getJsonFontList() {
}

/**
* @NoAdminRequired
* @PublicPage
* @NoCSRFRequired
*
* @param string $name
* @return DataDisplayResponse|DataResponse
* @throws \OCP\Files\NotPermittedException
*/
public function getFontFile(string $name) {
#[NoAdminRequired]
#[PublicPage]
#[NoCSRFRequired]
public function getFontFile(string $name): DataDisplayResponse|DataResponse {
try {
$fontFile = $this->fontService->getFontFile($name);
$etag = $fontFile->getETag();
Expand All @@ -394,14 +383,11 @@ public function getFontFile(string $name) {
}

/**
* @NoAdminRequired
* @PublicPage
* @NoCSRFRequired
*
* @param string $name
* @return DataDisplayResponse
* @throws \OCP\Files\NotPermittedException
*/
#[NoAdminRequired]
#[PublicPage]
#[NoCSRFRequired]
public function getFontFileOverview(string $name): DataDisplayResponse {
try {
$fontFileOverviewContent = $this->fontService->getFontFileOverview($name);
Expand All @@ -416,11 +402,9 @@ public function getFontFileOverview(string $name): DataDisplayResponse {
}

/**
* @NoAdminRequired
*
* @param string $type - Type is 'admin' or 'user'
* @return DataResponse
* @param 'admin'|'user' $type
*/
#[NoAdminRequired]
public function generateIframeToken(string $type): DataResponse {
try {
$response = $this->settingsService->generateIframeToken($type, $this->userId);
Expand All @@ -433,8 +417,6 @@ public function generateIframeToken(string $type): DataResponse {
}

/**
* @param string $name
* @return DataResponse
* @throws NotFoundException
* @throws \OCP\Files\NotPermittedException
*/
Expand All @@ -443,9 +425,6 @@ public function deleteFontFile(string $name): DataResponse {
return new DataResponse();
}

/**
* @return JSONResponse
*/
public function uploadFontFile(): JSONResponse {
try {
$file = $this->getUploadedFile('fontfile');
Expand Down Expand Up @@ -475,19 +454,10 @@ public function uploadFontFile(): JSONResponse {
}
}

/**
* @param string $type
* @param string $token
* @param string $category
* @param string $name
*
* @return DataDisplayResponse
*
* @NoAdminRequired
* @PublicPage
* @NoCSRFRequired
**/
public function getSettingsFile(string $type, string $token, string $category, string $name) {
#[NoAdminRequired]
#[PublicPage]
#[NoCSRFRequired]
public function getSettingsFile(string $type, string $token, string $category, string $name): DataDisplayResponse {
try {
$wopi = $this->wopiMapper->getWopiForToken($token);
$userId = $wopi->getEditorUid() ?: $wopi->getOwnerUid();
Expand Down Expand Up @@ -519,8 +489,6 @@ public function getSettingsFile(string $type, string $token, string $category, s
}

/**
* @param string $key
* @return array
* @throws UploadException
*/
private function getUploadedFile(string $key): array {
Expand Down
68 changes: 19 additions & 49 deletions lib/Controller/TemplatesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
use OCA\Richdocuments\TemplateManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCsrfRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\Files\File;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
Expand All @@ -27,19 +31,10 @@

class TemplatesController extends Controller {
/** @var int Max template size */
private $maxSize = 20 * 1024 * 1024;
private int $maxSize = 20 * 1024 * 1024;

/**
* Templates controller
*
* @param string $appName
* @param IRequest $request
* @param IL10N $l10n
* @param TemplateManager $manager
* @param IPreview $preview
*/
public function __construct(
$appName,
string $appName,
IRequest $request,
private IL10N $l10n,
private TemplateManager $manager,
Expand All @@ -48,34 +43,23 @@
private LoggerInterface $logger,
) {
parent::__construct($appName, $request);

$this->appName = $appName;
$this->request = $request;
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @PublicPage
*
* Get preview for a specific template
*
* @param int $fileId The template id
* @param int $x
* @param int $y
* @param bool $a
* @param bool $forceIcon
* @param string $mode
* @return DataResponse
* @throws NotFoundResponse
*/
public function getPreview($fileId,
$x = 150,
$y = 150,
$a = false,
$forceIcon = true,
$mode = 'fill') {
#[NoAdminRequired]
#[NoCsrfRequired]

Check failure on line 54 in lib/Controller/TemplatesController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

InvalidClass 

lib/Controller/TemplatesController.php:54:4: InvalidClass: Class, interface or enum OCP\AppFramework\Http\Attribute\NoCsrfRequired has wrong casing (see https://psalm.dev/007)
#[PublicPage]
public function getPreview(int $fileId,
int $x = 150,
int $y = 150,
bool $a = false,
bool $forceIcon = true,
string $mode = 'fill'): DataResponse {
if ($fileId === '' || $x === 0 || $y === 0) {

Check failure on line 62 in lib/Controller/TemplatesController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

TypeDoesNotContainType 

lib/Controller/TemplatesController.php:62:7: TypeDoesNotContainType: Type int for $fileId is never !=string() (see https://psalm.dev/056)

Check failure on line 62 in lib/Controller/TemplatesController.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

TypeDoesNotContainType 

lib/Controller/TemplatesController.php:62:7: TypeDoesNotContainType: '' cannot be identical to int (see https://psalm.dev/056)
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}

Expand All @@ -94,10 +78,8 @@

/**
* Add a global template
*
* @return JSONResponse
*/
public function add() {
public function add(): JSONResponse {
$files = $this->request->getUploadedFile('files');

if (!is_null($files)) {
Expand Down Expand Up @@ -149,11 +131,8 @@

/**
* Delete a global template
*
* @param int $fileId
* @return JSONResponse
*/
public function delete($fileId) {
public function delete(int $fileId): JSONResponse {
try {
$this->manager->delete($fileId);

Expand All @@ -169,22 +148,13 @@
}
}

/**
* @param Node $node
* @param int $x
* @param int $y
* @param bool $a
* @param bool $forceIcon
* @param string $mode
* @return DataResponse|FileDisplayResponse
*/
private function fetchPreview(
Node $node,
File $node,
int $x,
int $y,
bool $a = false,
bool $forceIcon = true,
string $mode = IPreview::MODE_FILL): Http\Response {
string $mode = IPreview::MODE_FILL): DataResponse|FileDisplayResponse {
if (!($node instanceof Node) || (!$forceIcon && !$this->preview->isAvailable($node))) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}
Expand Down
8 changes: 6 additions & 2 deletions lib/TemplateManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@ public function setUserId(?string $userId): void {
* @param int $fileId
* @return File
*/
public function get(int $fileId) {
public function get(int $fileId): File {
// is this a global template ?
$files = $this->getEmptyTemplateDir()->getDirectoryListing();

$files = array_filter($files, fn (Node $node): bool => $node instanceof File);

foreach ($files as $file) {
if ($file->getId() === $fileId) {
return $file;
Expand All @@ -130,6 +132,8 @@ public function get(int $fileId) {
// is this a global template ?
$files = $this->getSystemTemplateDir()->getDirectoryListing();

$files = array_filter($files, fn (Node $node): bool => $node instanceof File);

foreach ($files as $file) {
if ($file->getId() === $fileId) {
return $file;
Expand All @@ -139,7 +143,7 @@ public function get(int $fileId) {
$templateDir = $this->getUserTemplateDir();
// finally get the template file
$file = $templateDir->getFirstNodeById($fileId);
if ($file !== null) {
if ($file instanceof File) {
return $file;
}

Expand Down
Loading