mirror of
https://github.com/nextcloud/server.git
synced 2026-06-09 00:32:29 -04:00
Merge pull request #33103 from nextcloud/enhancement/add-logging-lo-federation
[Stable23] Logging, updating status for general error in federation
This commit is contained in:
blizzz
GitHub
commit
8ac46a6645
4 changed files with 39 additions and 24 deletions
|
|
@ -99,9 +99,11 @@ class SyncService {
|
|||
if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
|
||||
// remote server revoked access to the address book, remove it
|
||||
$this->backend->deleteAddressBook($addressBookId);
|
||||
$this->logger->info('Authorization failed, remove address book: ' . $url, ['app' => 'dav']);
|
||||
throw $ex;
|
||||
$this->logger->error('Authorization failed, remove address book: ' . $url, ['app' => 'dav']);
|
||||
} else {
|
||||
$this->logger->error('Client exception:', ['app' => 'dav', 'exception' => $ex]);
|
||||
}
|
||||
throw $ex;
|
||||
}
|
||||
|
||||
// 3. apply changes
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ use OC\OCS\DiscoveryService;
|
|||
use OCA\DAV\CardDAV\SyncService;
|
||||
use OCP\AppFramework\Http;
|
||||
use OCP\OCS\IDiscoveryService;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
class SyncFederationAddressBooks {
|
||||
|
||||
|
|
@ -41,18 +42,18 @@ class SyncFederationAddressBooks {
|
|||
/** @var DiscoveryService */
|
||||
private $ocsDiscoveryService;
|
||||
|
||||
/**
|
||||
* @param DbHandler $dbHandler
|
||||
* @param SyncService $syncService
|
||||
* @param IDiscoveryService $ocsDiscoveryService
|
||||
*/
|
||||
/** @var LoggerInterface */
|
||||
private $logger;
|
||||
|
||||
public function __construct(DbHandler $dbHandler,
|
||||
SyncService $syncService,
|
||||
IDiscoveryService $ocsDiscoveryService
|
||||
IDiscoveryService $ocsDiscoveryService,
|
||||
LoggerInterface $logger
|
||||
) {
|
||||
$this->syncService = $syncService;
|
||||
$this->dbHandler = $dbHandler;
|
||||
$this->ocsDiscoveryService = $ocsDiscoveryService;
|
||||
$this->logger = $logger;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -71,6 +72,7 @@ class SyncFederationAddressBooks {
|
|||
$addressBookUrl = isset($endPoints['system-address-book']) ? trim($endPoints['system-address-book'], '/') : 'remote.php/dav/addressbooks/system/system/system';
|
||||
|
||||
if (is_null($sharedSecret)) {
|
||||
$this->logger->error('Shared secret is null');
|
||||
continue;
|
||||
}
|
||||
$targetBookId = $trustedServer['url_hash'];
|
||||
|
|
@ -82,10 +84,16 @@ class SyncFederationAddressBooks {
|
|||
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
|
||||
if ($newToken !== $syncToken) {
|
||||
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
|
||||
} else {
|
||||
$this->logger->info('Token unchanged from previous sync.');
|
||||
}
|
||||
} catch (\Exception $ex) {
|
||||
if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
|
||||
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_ACCESS_REVOKED);
|
||||
$this->logger->error('Server sync failed because of revoked access.');
|
||||
} else {
|
||||
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_FAILURE);
|
||||
$this->logger->error('Server sync failed.');
|
||||
}
|
||||
$callback($url, $ex);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,21 +26,20 @@
|
|||
namespace OCA\Federation;
|
||||
|
||||
use OC\BackgroundJob\TimedJob;
|
||||
use OCP\ILogger;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
class SyncJob extends TimedJob {
|
||||
|
||||
/** @var SyncFederationAddressBooks */
|
||||
protected $syncService;
|
||||
|
||||
/** @var ILogger */
|
||||
/** @var LoggerInterface */
|
||||
protected $logger;
|
||||
|
||||
/**
|
||||
* @param SyncFederationAddressBooks $syncService
|
||||
* @param ILogger $logger
|
||||
*/
|
||||
public function __construct(SyncFederationAddressBooks $syncService, ILogger $logger) {
|
||||
public function __construct(SyncFederationAddressBooks $syncService, LoggerInterface $logger) {
|
||||
// Run once a day
|
||||
$this->setInterval(24 * 60 * 60);
|
||||
$this->syncService = $syncService;
|
||||
|
|
@ -50,11 +49,11 @@ class SyncJob extends TimedJob {
|
|||
protected function run($argument) {
|
||||
$this->syncService->syncThemAll(function ($url, $ex) {
|
||||
if ($ex instanceof \Exception) {
|
||||
$this->logger->logException($ex, [
|
||||
'message' => "Error while syncing $url.",
|
||||
'level' => ILogger::INFO,
|
||||
'app' => 'fed-sync',
|
||||
]);
|
||||
$this->logger->error("Error while syncing $url.",
|
||||
[
|
||||
'exception' => $ex
|
||||
]
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,25 +31,31 @@ namespace OCA\Federation\Tests;
|
|||
use OC\OCS\DiscoveryService;
|
||||
use OCA\Federation\DbHandler;
|
||||
use OCA\Federation\SyncFederationAddressBooks;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
class SyncFederationAddressbooksTest extends \Test\TestCase {
|
||||
|
||||
/** @var array */
|
||||
private $callBacks = [];
|
||||
|
||||
/** @var \PHPUnit\Framework\MockObject\MockObject | DiscoveryService */
|
||||
/** @var MockObject | DiscoveryService */
|
||||
private $discoveryService;
|
||||
|
||||
/** @var MockObject|LoggerInterface */
|
||||
private $logger;
|
||||
|
||||
protected function setUp(): void {
|
||||
parent::setUp();
|
||||
|
||||
$this->discoveryService = $this->getMockBuilder(DiscoveryService::class)
|
||||
->disableOriginalConstructor()->getMock();
|
||||
$this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
|
||||
$this->logger = $this->createMock(LoggerInterface::class);
|
||||
}
|
||||
|
||||
public function testSync() {
|
||||
/** @var DbHandler | \PHPUnit\Framework\MockObject\MockObject $dbHandler */
|
||||
/** @var DbHandler | MockObject $dbHandler */
|
||||
$dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')->
|
||||
disableOriginalConstructor()->
|
||||
getMock();
|
||||
|
|
@ -62,8 +68,8 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
|
|||
'sync_token' => '0'
|
||||
]
|
||||
]);
|
||||
$dbHandler->expects($this->once())->method('setServerStatus')->
|
||||
with('https://cloud.drop.box', 1, '1');
|
||||
$dbHandler->expects($this->once())->method('setServerStatus')
|
||||
->with('https://cloud.drop.box', 1, '1');
|
||||
$syncService = $this->getMockBuilder('OCA\DAV\CardDAV\SyncService')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
|
|
@ -71,7 +77,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
|
|||
->willReturn(1);
|
||||
|
||||
/** @var \OCA\DAV\CardDAV\SyncService $syncService */
|
||||
$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService);
|
||||
$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger);
|
||||
$s->syncThemAll(function ($url, $ex) {
|
||||
$this->callBacks[] = [$url, $ex];
|
||||
});
|
||||
|
|
@ -79,7 +85,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
|
|||
}
|
||||
|
||||
public function testException() {
|
||||
/** @var DbHandler | \PHPUnit\Framework\MockObject\MockObject $dbHandler */
|
||||
/** @var DbHandler | MockObject $dbHandler */
|
||||
$dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')->
|
||||
disableOriginalConstructor()->
|
||||
getMock();
|
||||
|
|
@ -99,7 +105,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
|
|||
->willThrowException(new \Exception('something did not work out'));
|
||||
|
||||
/** @var \OCA\DAV\CardDAV\SyncService $syncService */
|
||||
$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService);
|
||||
$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger);
|
||||
$s->syncThemAll(function ($url, $ex) {
|
||||
$this->callBacks[] = [$url, $ex];
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue