Skip to content
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.

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

fix: Add psalm configuration, fix some of the errors, add stubs, add baseline #1330

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
41 changes: 41 additions & 0 deletions .github/workflows/psalm.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# This workflow is provided via the organization template repository
#
# https://github.com/nextcloud/.github
# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization

name: Static analysis

on: pull_request

concurrency:
group: psalm-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
static-analysis:
runs-on: ubuntu-latest

name: static-psalm-analysis
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Get php version
id: versions
uses: icewind1991/nextcloud-version-matrix@7d433286e92318f51ed0537b6c77374759e12f46 # v1.3.0

- name: Set up php${{ steps.versions.outputs.php-available }}
uses: shivammathur/setup-php@6d7209f44a25a59e904b1ee9f3b0c33ab2cd888d # v2
with:
php-version: ${{ steps.versions.outputs.php-available }}
extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
coverage: none
ini-file: development
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Install dependencies
run: composer i

- name: Run coding standards check
run: composer run psalm
3 changes: 2 additions & 1 deletion .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
$config
->getFinder()
->ignoreVCSIgnored(true)
->notPath('build')
->notPath('tests/stubs')
->notPath('l10n')
->notPath('src')
->notPath('vendor')
->notPath('vendor-bin')
->in(__DIR__);
return $config;
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
"scripts": {
"cs:fix": "php-cs-fixer fix",
"cs:check": "php-cs-fixer fix --dry-run --diff",
"lint": "find . -name \\*.php -not -path './vendor/*' -print0 | xargs -0 -n1 php -l",
"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './vendor-bin/*' -not -path './tests/stubs/*' -print0 | xargs -0 -n1 php -l",
"psalm": "psalm.phar --threads=1",
"psalm:update-baseline": "psalm.phar --threads=1 --update-baseline",
"psalm:clear": "psalm.phar --clear-cache && psalm.phar --clear-global-cache",
"psalm:fix": "psalm.phar --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
"test:integration": "phpunit -c tests/phpunit.integration.xml --fail-on-warning",
"test:integration:dev": "phpunit -c tests/phpunit.integration.xml --no-coverage --order-by=defects --stop-on-defect --fail-on-warning --stop-on-error --stop-on-failure",
"test:unit": "phpunit -c tests/phpunit.unit.xml --fail-on-warning",
Expand Down
5 changes: 2 additions & 3 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ public function boot(IBootContext $context): void {
$context->getAppContainer()->registerService('FileHooks', function ($c) {
return new FileHooks(
$c->query(IServerContainer::class)->getRootFolder(),
\OC::$server->query(PhotofilesService::class),
\OC::$server->query(TracksService::class),
$c->query(IServerContainer::class)->getLogger(),
\OCP\Server::get(PhotofilesService::class),
\OCP\Server::get(TracksService::class),
$c->query('AppName'),
$c->query(IServerContainer::class)->getLockingProvider()
);
Expand Down
18 changes: 10 additions & 8 deletions lib/BackgroundJob/AddPhotoJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,29 @@ class AddPhotoJob extends QueuedJob {
* @param ITimeFactory $timeFactory
* @param PhotofilesService $photofilesService
*/
public function __construct(ITimeFactory $timeFactory,
public function __construct(
ITimeFactory $timeFactory,
IRootFolder $root,
PhotofilesService $photofilesService,
ICacheFactory $cacheFactory) {
ICacheFactory $cacheFactory,
) {
parent::__construct($timeFactory);
$this->photofilesService = $photofilesService;
$this->root = $root;
$this->cacheFactory = $cacheFactory;
$this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs');
}

public function run($arguments) {
$userFolder = $this->root->getUserFolder($arguments['userId']);
$files = $userFolder->getById($arguments['photoId']);
public function run($argument) {
$userFolder = $this->root->getUserFolder($argument['userId']);
$files = $userFolder->getById($argument['photoId']);
if (empty($files)) {
return;
}
$file = array_shift($files);
$this->photofilesService->addPhotoNow($file, $arguments['userId']);
$this->photofilesService->addPhotoNow($file, $argument['userId']);

$counter = $this->backgroundJobCache->get('recentlyAdded:'.$arguments['userId']) ?? 0;
$this->backgroundJobCache->set('recentlyAdded:'.$arguments['userId'], (int)$counter + 1, 60 * 60 * 3);
$counter = $this->backgroundJobCache->get('recentlyAdded:'.$argument['userId']) ?? 0;
$this->backgroundJobCache->set('recentlyAdded:'.$argument['userId'], (int)$counter + 1, 60 * 60 * 3);
}
}
15 changes: 8 additions & 7 deletions lib/BackgroundJob/LaunchUsersInstallScanJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,27 @@
use OCP\BackgroundJob\QueuedJob;
use OCP\IUser;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;

class LaunchUsersInstallScanJob extends QueuedJob {

private $jobList;

/**
* LaunchUsersInstallScanJob constructor.
*
* A QueuedJob to launch a scan job for each user
*
* @param IJobList $jobList
*/
public function __construct(ITimeFactory $timeFactory, IJobList $jobList, IUserManager $userManager) {
public function __construct(
ITimeFactory $timeFactory,
private IJobList $jobList,
private IUserManager $userManager,
) {
parent::__construct($timeFactory);
$this->jobList = $jobList;
$this->userManager = $userManager;
}

public function run($arguments) {
\OC::$server->getLogger()->debug('Launch users install scan jobs cronjob executed');
public function run($argument) {
\OCP\Server::get(LoggerInterface::class)->debug('Launch users install scan jobs cronjob executed');
$this->userManager->callForSeenUsers(function (IUser $user) {
$this->jobList->add(UserInstallScanJob::class, ['userId' => $user->getUID()]);
});
Expand Down
22 changes: 8 additions & 14 deletions lib/BackgroundJob/LookupMissingGeoJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,25 @@
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\BackgroundJob\QueuedJob;
use Psr\Log\LoggerInterface;

class LookupMissingGeoJob extends QueuedJob {

/** @var AddressService */
private $addressService;

/** @var AddressService */
private $jobList;

/**
* LookupMissingGeoJob constructor.
*
* A QueuedJob to lookup missing geo information of addresses
*
* @param AddressService $service
* @param IJobList $jobList
*/
public function __construct(ITimeFactory $timeFactory, AddressService $service, IJobList $jobList) {
public function __construct(
ITimeFactory $timeFactory,
private AddressService $addressService,
private IJobList $jobList,
) {
parent::__construct($timeFactory);
$this->addressService = $service;
$this->jobList = $jobList;
}

public function run($arguments) {
\OC::$server->getLogger()->debug('Maps address lookup cronjob executed');
public function run($argument) {
\OCP\Server::get(LoggerInterface::class)->debug('Maps address lookup cronjob executed');
// lookup at most 200 addresses
if (!$this->addressService->lookupMissingGeo(200)) {
// if not all addresses where looked up successfully add a new job for next time
Expand Down
18 changes: 9 additions & 9 deletions lib/BackgroundJob/UpdatePhotoByFileJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,30 @@ class UpdatePhotoByFileJob extends QueuedJob {
*
* A QueuedJob to scan user storage for photos and tracks
*
* @param ITimeFactory $timeFactory
* @param PhotofilesService $photofilesService
*/
public function __construct(ITimeFactory $timeFactory,
public function __construct(
ITimeFactory $timeFactory,
IRootFolder $root,
PhotofilesService $photofilesService,
ICacheFactory $cacheFactory) {
ICacheFactory $cacheFactory,
) {
parent::__construct($timeFactory);
$this->photofilesService = $photofilesService;
$this->root = $root;
$this->cacheFactory = $cacheFactory;
$this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs');
}

public function run($arguments) {
$userFolder = $this->root->getUserFolder($arguments['userId']);
$files = $userFolder->getById($arguments['fileId']);
public function run($argument) {
$userFolder = $this->root->getUserFolder($argument['userId']);
$files = $userFolder->getById($argument['fileId']);
if (empty($files)) {
return;
}
$file = array_shift($files);
$this->photofilesService->updateByFileNow($file);

$counter = $this->backgroundJobCache->get('recentlyUpdated:'.$arguments['userId']) ?? 0;
$this->backgroundJobCache->set('recentlyUpdated:'.$arguments['userId'], (int)$counter + 1, 60 * 60 * 3);
$counter = $this->backgroundJobCache->get('recentlyUpdated:'.$argument['userId']) ?? 0;
$this->backgroundJobCache->set('recentlyUpdated:'.$argument['userId'], (int)$counter + 1, 60 * 60 * 3);
}
}
7 changes: 4 additions & 3 deletions lib/BackgroundJob/UserInstallScanJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

use OCP\IConfig;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;

class UserInstallScanJob extends QueuedJob {

Expand Down Expand Up @@ -49,9 +50,9 @@ public function __construct(ITimeFactory $timeFactory, IJobList $jobList,
$this->tracksService = $tracksService;
}

public function run($arguments) {
$userId = $arguments['userId'];
\OC::$server->getLogger()->debug('Launch user install scan job for '.$userId.' cronjob executed');
public function run($argument) {
$userId = $argument['userId'];
\OCP\Server::get(LoggerInterface::class)->debug('Launch user install scan job for '.$userId.' cronjob executed');
// scan photos and tracks for given user
$this->rescanUserPhotos($userId);
$this->rescanUserTracks($userId);
Expand Down
1 change: 0 additions & 1 deletion lib/Command/RegisterMimetypes.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ protected function configure() {
* @return int
*/
protected function execute(InputInterface $input, OutputInterface $output): int {
$this->output = $output;
$output->writeln('Register mimetypes for existing files');
$this->mimetypeService->registerForExistingFiles();
$output->writeln('Register mimetypes for new files');
Expand Down
11 changes: 3 additions & 8 deletions lib/Controller/ContactsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

namespace OCA\Maps\Controller;

use OC\Files\Node\Node;
use OCA\DAV\CardDAV\CardDavBackend;
use OCA\Maps\Service\AddressService;
use OCP\AppFramework\Controller;
Expand All @@ -21,18 +20,17 @@
use OCP\Contacts\IManager;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\IAvatarManager;
use OCP\IDBConnection;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IURLGenerator;
use Sabre\VObject\Property\Text;
use Sabre\VObject\Reader;

class ContactsController extends Controller {
private $userId;
private $logger;
private $contactsManager;
private $addressService;
private $dbconnection;
Expand All @@ -45,7 +43,6 @@ class ContactsController extends Controller {

/**
* @param $AppName
* @param ILogger $logger
* @param IRequest $request
* @param IDBConnection $dbconnection
* @param IManager $contactsManager
Expand All @@ -57,7 +54,6 @@ class ContactsController extends Controller {
*/
public function __construct(
$AppName,
ILogger $logger,
IRequest $request,
IDBConnection $dbconnection,
IManager $contactsManager,
Expand All @@ -68,7 +64,6 @@ public function __construct(
IRootFolder $root,
IURLGenerator $urlGenerator) {
parent::__construct($AppName, $request);
$this->logger = $logger;
$this->userId = $UserId;
$this->avatarManager = $avatarManager;
$this->contactsManager = $contactsManager;
Expand All @@ -83,7 +78,7 @@ public function __construct(
/**
* Converts a geo string as a float array
* @param string formatted as "lat;lon"
* @return float array containing [lat;lon]
* @return float[] array containing [lat;lon]
*/
private function geoAsFloatArray($geo) {
$res = array_map(function ($value) {return floatval($value);}, explode(';', $geo));
Expand Down Expand Up @@ -116,7 +111,7 @@ private function isNewAddress($prevGeo, $geo) {
* get distance between two geo points
* @param GPS coordinates of first point
* @param GPS coordinates of second point
* @return Distance in meters between these two points
* @return float Distance in meters between these two points
*/
private function getDistance($coordsA, $coordsB) {
if (empty($coordsA) || empty($coordsB)) {
Expand Down
9 changes: 0 additions & 9 deletions lib/Controller/DevicesApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,16 @@
namespace OCA\Maps\Controller;

use OCA\Maps\Service\DevicesService;

use OCP\App\IAppManager;
use OCP\AppFramework\ApiController;

use OCP\AppFramework\Http;


use OCP\AppFramework\Http\DataResponse;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IUserManager;

use OCP\Share\IManager;

class DevicesApiController extends ApiController {
Expand All @@ -43,7 +37,6 @@ class DevicesApiController extends ApiController {
private $dbdblquotes;
private $defaultDeviceId;
private $l;
private $logger;
private $devicesService;
protected $appName;

Expand All @@ -56,15 +49,13 @@ public function __construct($AppName,
IUserManager $userManager,
IGroupManager $groupManager,
IL10N $l,
ILogger $logger,
DevicesService $devicesService,
$UserId) {
parent::__construct($AppName, $request,
'PUT, POST, GET, DELETE, PATCH, OPTIONS',
'Authorization, Content-Type, Accept',
1728000);
$this->devicesService = $devicesService;
$this->logger = $logger;
$this->appName = $AppName;
$this->appVersion = $config->getAppValue('maps', 'installed_version');
$this->userId = $UserId;
Expand Down
Loading
Loading