From c1dde29ef696291226fab77790d94a523c200da5 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 13 Jan 2025 15:17:20 +0000 Subject: [PATCH 1/5] Add description for LegacyLoggerFactory --- src/Core/Logger/Factory/LegacyLoggerFactory.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Core/Logger/Factory/LegacyLoggerFactory.php b/src/Core/Logger/Factory/LegacyLoggerFactory.php index 485f3b09dc..5c91d11771 100644 --- a/src/Core/Logger/Factory/LegacyLoggerFactory.php +++ b/src/Core/Logger/Factory/LegacyLoggerFactory.php @@ -15,7 +15,17 @@ use Friendica\Util\Profiler; use Psr\Log\LoggerInterface; /** - * Manager for the core logging instances + * Bridge for the legacy Logger factory. + * + * This class can be removed after the following classes are replaced or + * refactored implementing the `\Friendica\Core\Logger\Factory\LoggerFactory`: + * + * - Friendica\Core\Logger\Factory\StreamLogger + * - Friendica\Core\Logger\Factory\SyslogLogger + * - monolog addon: Friendica\Addon\monolog\src\Factory\Monolog + * + * @see \Friendica\Core\Logger\Factory\StreamLogger + * @see \Friendica\Core\Logger\Factory\SyslogLogger */ final class LegacyLoggerFactory implements LoggerFactory { From 39088ab003b2af862209ba777276577d6325619c Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 13 Jan 2025 16:41:35 +0000 Subject: [PATCH 2/5] Clean static private properties of LoggerManager after test --- tests/Unit/Core/Logger/LoggerManagerTest.php | 48 ++++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/tests/Unit/Core/Logger/LoggerManagerTest.php b/tests/Unit/Core/Logger/LoggerManagerTest.php index 14db1c2676..0dae2fcc6c 100644 --- a/tests/Unit/Core/Logger/LoggerManagerTest.php +++ b/tests/Unit/Core/Logger/LoggerManagerTest.php @@ -23,30 +23,30 @@ class LoggerManagerTest extends TestCase { public function testGetLoggerReturnsPsrLogger(): void { - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $this->createStub(IManageConfigValues::class), $this->createStub(LoggerFactory::class) ); $this->assertInstanceOf(LoggerInterface::class, $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } public function testGetLoggerReturnsSameObject(): void { - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $this->createStub(IManageConfigValues::class), $this->createStub(LoggerFactory::class) ); $this->assertSame($factory->getLogger(), $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } public function testGetLoggerWithDebugDisabledReturnsNullLogger(): void @@ -56,16 +56,16 @@ class LoggerManagerTest extends TestCase ['system', 'debugging', null, false], ]); - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $config, $this->createStub(LoggerFactory::class) ); $this->assertInstanceOf(NullLogger::class, $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } public function testGetLoggerWithProfilerEnabledReturnsProfilerLogger(): void @@ -76,16 +76,16 @@ class LoggerManagerTest extends TestCase ['system', 'profiling', null, true], ]); - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $config, $this->createStub(LoggerFactory::class) ); $this->assertInstanceOf(ProfilerLogger::class, $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } public function testChangeLogChannelReturnsDifferentLogger(): void @@ -96,10 +96,6 @@ class LoggerManagerTest extends TestCase ['system', 'profiling', null, true], ]); - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $config, $this->createStub(LoggerFactory::class) @@ -110,6 +106,10 @@ class LoggerManagerTest extends TestCase $factory->changeLogChannel(LogChannel::CONSOLE); $this->assertNotSame($logger1, $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } public function testChangeLogChannelToWorkerReturnsWorkerLogger(): void @@ -120,10 +120,6 @@ class LoggerManagerTest extends TestCase ['system', 'profiling', null, true], ]); - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); - $factory = new LoggerManager( $config, $this->createStub(LoggerFactory::class) @@ -132,5 +128,9 @@ class LoggerManagerTest extends TestCase $factory->changeLogChannel(LogChannel::WORKER); $this->assertInstanceOf(WorkerLogger::class, $factory->getLogger()); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); } } From f9b7b6a41358355e4af4a8340f19cd897ca59065 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 13 Jan 2025 16:52:05 +0000 Subject: [PATCH 3/5] Clean private static properties of LoggerManager via tearDown method --- tests/Unit/Core/Logger/LoggerManagerTest.php | 41 ++++++++------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/tests/Unit/Core/Logger/LoggerManagerTest.php b/tests/Unit/Core/Logger/LoggerManagerTest.php index 0dae2fcc6c..02f9c0b2e2 100644 --- a/tests/Unit/Core/Logger/LoggerManagerTest.php +++ b/tests/Unit/Core/Logger/LoggerManagerTest.php @@ -21,6 +21,23 @@ use Psr\Log\NullLogger; class LoggerManagerTest extends TestCase { + /** + * Clean the private static properties + * + * @see LoggerManager::$logger + * @see LoggerManager::$logChannel + */ + protected function tearDown(): void + { + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, null); + + $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logChannel'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue(null, LogChannel::DEFAULT); + } + public function testGetLoggerReturnsPsrLogger(): void { $factory = new LoggerManager( @@ -29,10 +46,6 @@ class LoggerManagerTest extends TestCase ); $this->assertInstanceOf(LoggerInterface::class, $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } public function testGetLoggerReturnsSameObject(): void @@ -43,10 +56,6 @@ class LoggerManagerTest extends TestCase ); $this->assertSame($factory->getLogger(), $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } public function testGetLoggerWithDebugDisabledReturnsNullLogger(): void @@ -62,10 +71,6 @@ class LoggerManagerTest extends TestCase ); $this->assertInstanceOf(NullLogger::class, $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } public function testGetLoggerWithProfilerEnabledReturnsProfilerLogger(): void @@ -82,10 +87,6 @@ class LoggerManagerTest extends TestCase ); $this->assertInstanceOf(ProfilerLogger::class, $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } public function testChangeLogChannelReturnsDifferentLogger(): void @@ -106,10 +107,6 @@ class LoggerManagerTest extends TestCase $factory->changeLogChannel(LogChannel::CONSOLE); $this->assertNotSame($logger1, $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } public function testChangeLogChannelToWorkerReturnsWorkerLogger(): void @@ -128,9 +125,5 @@ class LoggerManagerTest extends TestCase $factory->changeLogChannel(LogChannel::WORKER); $this->assertInstanceOf(WorkerLogger::class, $factory->getLogger()); - - $reflectionProperty = new \ReflectionProperty(LoggerManager::class, 'logger'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue(null, null); } } From dc382b5288cc62bda17ac238e320fe00323993d1 Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 14 Jan 2025 06:06:28 +0000 Subject: [PATCH 4/5] Changed contact update rule / added logging --- src/Module/Photo.php | 5 +++-- src/Worker/UpdateContact.php | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 30e6d57672..87da6be55b 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -27,6 +27,7 @@ use Friendica\Network\HTTPException; use Friendica\Network\HTTPException\NotModifiedException; use Friendica\Object\Image; use Friendica\Security\OpenWebAuth; +use Friendica\Util\DateTimeFormat; use Friendica\Util\Images; use Friendica\Util\ParseUrl; use Friendica\Util\Proxy; @@ -297,7 +298,7 @@ class Photo extends BaseApi return MPhoto::createPhotoForExternalResource($link['url'], (int)DI::userSession()->getLocalUserId(), $link['mimetype'] ?? '', $link['blurhash'] ?? '', $link['width'] ?? 0, $link['height'] ?? 0); case 'contact': - $fields = ['uid', 'uri-id', 'url', 'nurl', 'avatar', 'photo', 'blurhash', 'xmpp', 'addr', 'network', 'failed', 'updated']; + $fields = ['uid', 'uri-id', 'url', 'nurl', 'avatar', 'photo', 'blurhash', 'xmpp', 'addr', 'network', 'failed', 'updated', 'next-update']; $contact = Contact::getById($id, $fields); if (empty($contact)) { return false; @@ -355,7 +356,7 @@ class Photo extends BaseApi } else { // Only update federated accounts that hadn't failed before and hadn't been updated recently $update = in_array($contact['network'], Protocol::FEDERATED) && !$contact['failed'] - && ((time() - strtotime($contact['updated']) > 86400)); + && ($contact['next-update'] < DateTimeFormat::utcNow()); if ($update) { $curlResult = DI::httpClient()->head($url, [HttpClientOptions::ACCEPT_CONTENT => HttpClientAccept::IMAGE, HttpClientOptions::REQUEST => HttpClientRequest::CONTENTTYPE]); $update = !$curlResult->isSuccess() && ($curlResult->getReturnCode() == 404); diff --git a/src/Worker/UpdateContact.php b/src/Worker/UpdateContact.php index aad6a93db2..ded4679fe6 100644 --- a/src/Worker/UpdateContact.php +++ b/src/Worker/UpdateContact.php @@ -51,6 +51,7 @@ class UpdateContact return 0; } + Logger::debug('Update contact', ['id' => $contact_id]); return Worker::add($run_parameters, 'UpdateContact', $contact_id); } } From 8345ada757764e7128fcb3319f0889c86da6b4ab Mon Sep 17 00:00:00 2001 From: Michael Date: Tue, 14 Jan 2025 06:06:39 +0000 Subject: [PATCH 5/5] Fixed codestyle --- src/Module/Photo.php | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 87da6be55b..c3582ee655 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -65,19 +65,19 @@ class Photo extends BaseApi OpenWebAuth::addVisitorCookieForHTTPSigner($this->server); - $customsize = 0; + $customsize = 0; $square_resize = true; - $scale = null; - $stamp = microtime(true); + $scale = null; + $stamp = microtime(true); // User avatar if (!empty($this->parameters['type'])) { if (!empty($this->parameters['customsize'])) { - $customsize = intval($this->parameters['customsize']); + $customsize = intval($this->parameters['customsize']); $square_resize = !in_array($this->parameters['type'], ['media', 'preview']); } if (!empty($this->parameters['guid'])) { - $guid = $this->parameters['guid']; + $guid = $this->parameters['guid']; $account = DBA::selectFirst('account-user-view', ['id'], ['guid' => $guid], ['order' => ['uid' => true]]); if (empty($account)) { throw new HTTPException\NotFoundException(); @@ -92,7 +92,7 @@ class Photo extends BaseApi if (!empty($this->parameters['nickname_ext'])) { $nickname = pathinfo($this->parameters['nickname_ext'], PATHINFO_FILENAME); - $user = User::getByNickname($nickname, ['uid']); + $user = User::getByNickname($nickname, ['uid']); if (empty($user)) { throw new HTTPException\NotFoundException(); } @@ -112,9 +112,9 @@ class Photo extends BaseApi $photo = self::getPhotoById($id, $this->parameters['type'], $customsize ?: Proxy::PIXEL_SMALL); } else { $photoid = pathinfo($this->parameters['name'], PATHINFO_FILENAME); - $scale = 0; + $scale = 0; if (substr($photoid, -2, 1) == '-') { - $scale = intval(substr($photoid, -1, 1)); + $scale = intval(substr($photoid, -1, 1)); $photoid = substr($photoid, 0, -2); } @@ -176,7 +176,7 @@ class Photo extends BaseApi Logger::warning('Invalid photo', ['id' => $photo['id']]); if (in_array($photo['backend-class'], [ExternalResource::NAME])) { $reference = json_decode($photo['backend-ref'], true); - $error = DI::l10n()->t('Invalid external resource with url %s.', $reference['url']); + $error = DI::l10n()->t('Invalid external resource with url %s.', $reference['url']); } else { $error = DI::l10n()->t('Invalid photo with id %s.', $photo['id']); } @@ -230,13 +230,13 @@ class Photo extends BaseApi $output = microtime(true) - $stamp; $total = microtime(true) - $totalstamp; - $rest = $total - ($fetch + $data + $checksum + $output); + $rest = $total - ($fetch + $data + $checksum + $output); if (!is_null($scale) && ($scale < 4)) { Logger::debug('Performance:', [ - 'scale' => $scale, 'resource' => $photo['resource-id'], - 'total' => number_format($total, 3), 'fetch' => number_format($fetch, 3), - 'data' => number_format($data, 3), 'checksum' => number_format($checksum, 3), + 'scale' => $scale, 'resource' => $photo['resource-id'], + 'total' => number_format($total, 3), 'fetch' => number_format($fetch, 3), + 'data' => number_format($data, 3), 'checksum' => number_format($checksum, 3), 'output' => number_format($output, 3), 'rest' => number_format($rest, 3) ]); } @@ -298,7 +298,7 @@ class Photo extends BaseApi return MPhoto::createPhotoForExternalResource($link['url'], (int)DI::userSession()->getLocalUserId(), $link['mimetype'] ?? '', $link['blurhash'] ?? '', $link['width'] ?? 0, $link['height'] ?? 0); case 'contact': - $fields = ['uid', 'uri-id', 'url', 'nurl', 'avatar', 'photo', 'blurhash', 'xmpp', 'addr', 'network', 'failed', 'updated', 'next-update']; + $fields = ['uid', 'uri-id', 'url', 'nurl', 'avatar', 'photo', 'blurhash', 'xmpp', 'addr', 'network', 'failed', 'updated', 'next-update']; $contact = Contact::getById($id, $fields); if (empty($contact)) { return false; @@ -359,7 +359,7 @@ class Photo extends BaseApi && ($contact['next-update'] < DateTimeFormat::utcNow()); if ($update) { $curlResult = DI::httpClient()->head($url, [HttpClientOptions::ACCEPT_CONTENT => HttpClientAccept::IMAGE, HttpClientOptions::REQUEST => HttpClientRequest::CONTENTTYPE]); - $update = !$curlResult->isSuccess() && ($curlResult->getReturnCode() == 404); + $update = !$curlResult->isSuccess() && ($curlResult->getReturnCode() == 404); Logger::debug('Got return code for avatar', ['return code' => $curlResult->getReturnCode(), 'cid' => $id, 'url' => $contact['url'], 'avatar' => $url]); } if ($update) { @@ -402,7 +402,7 @@ class Photo extends BaseApi } return MPhoto::createPhotoForExternalResource($url, 0, $mimetext, $contact['blurhash'] ?? null, $customsize, $customsize); case 'header': - $fields = ['uid', 'url', 'header', 'network', 'gsid']; + $fields = ['uid', 'url', 'header', 'network', 'gsid']; $contact = Contact::getById($id, $fields); if (empty($contact)) { return false;