Merge pull request #3734 from nextcloud/refactor/logger

use more contexts in logger statements

lib/Cron/GroupDeletedJob.php

@@ -36,7 +36,7 @@ class GroupDeletedJob extends QueuedJob {
 	protected function run($argument) {
 		$this->session->set(AppConstants::SESSION_KEY_CRON_JOB, true);
 		$group = $argument['group'];
-		$this->logger->info('Removing group shares for deleted group {group}', [
+		$this->logger->info('Removing group shares for deleted group', [
 			'group' => $group
 		]);
 

lib/Cron/UserDeletedJob.php

@@ -53,8 +53,8 @@ class UserDeletedJob extends QueuedJob {
 	protected function run($argument) {
 		$this->session->set(AppConstants::SESSION_KEY_CRON_JOB, true);
 		$userId = $argument['userId'];
-		$this->logger->info('Deleting polls for deleted user id {user}', [
-			'user' => $userId
+		$this->logger->info('Deleting polls for deleted user', [
+			'userId' => $userId
 		]);
 
 		$replacementName = 'deleted_' . $this->secureRandom->generate(

lib/Db/TableManager.php

@@ -61,11 +61,14 @@ class TableManager {
 	 */
 	public function purgeTables(): array {
 		$messages = [];
+
 		// drop all child tables
+		$droppedTables = [];
+
 		foreach (TableSchema::FK_CHILD_TABLES as $tableName) {
 			if ($this->connection->tableExists($tableName)) {
 				$this->connection->dropTable($tableName);
-				$this->logger->info('Dropped ' . $this->dbPrefix . $tableName);
+				$droppedTables[] = $this->dbPrefix . $tableName;
 				$messages[] = 'Dropped ' . $this->dbPrefix . $tableName;
 			}
 		}
@@ -73,7 +76,7 @@ class TableManager {
 		foreach (TableSchema::FK_OTHER_TABLES as $tableName) {
 			if ($this->connection->tableExists($tableName)) {
 				$this->connection->dropTable($tableName);
-				$this->logger->info('Dropped ' . $this->dbPrefix . $tableName);
+				$droppedTables[] = $this->dbPrefix . $tableName;
 				$messages[] = 'Dropped ' . $this->dbPrefix . $tableName;
 			}
 		}
@@ -81,9 +84,12 @@ class TableManager {
 		// drop parent table
 		if ($this->connection->tableExists(TableSchema::FK_PARENT_TABLE)) {
 			$this->connection->dropTable(TableSchema::FK_PARENT_TABLE);
-			$this->logger->info('Dropped ' . $this->dbPrefix . TableSchema::FK_PARENT_TABLE);
+			$droppedTables[] = $this->dbPrefix . TableSchema::FK_PARENT_TABLE;
 			$messages[] = 'Dropped ' . $this->dbPrefix . TableSchema::FK_PARENT_TABLE;
 		}
+		if (!$droppedTables) {
+			$this->logger->info('Dropped tables', $droppedTables);
+		}
 
 		// delete all migration records
 		$query = $this->connection->getQueryBuilder();
@@ -92,7 +98,7 @@ class TableManager {
 			->setParameter('appName', AppConstants::APP_ID)
 			->executeStatement();
 
-		$this->logger->info('Removed all migration records from ' . $this->dbPrefix . 'migrations');
+		$this->logger->info('Removed all migration records from {dbPrefix}migrations', ['dbPrefix' => $this->dbPrefix]);
 		$messages[] = 'Removed all migration records from ' . $this->dbPrefix . 'migrations';
 		
 		// delete all app configs
@@ -101,7 +107,7 @@ class TableManager {
 			->setParameter('appid', AppConstants::APP_ID)
 			->executeStatement();
 
-		$this->logger->info('Removed all app config records from ' . $this->dbPrefix . 'appconfig');
+		$this->logger->info('Removed all app config records from {dbPrefix}appconfig', ['dbPrefix' => $this->dbPrefix]);
 		$messages[] = 'Removed all app config records from ' . $this->dbPrefix . 'appconfig';
 		$messages[] = 'Done.';
 		$messages[] = '';

lib/Model/Mail/MailBase.php

@@ -90,7 +90,7 @@ abstract class MailBase {
 			$message->useTemplate($this->getEmailTemplate());
 			$this->mailer->send($message);
 		} catch (\Exception $e) {
-			$this->logger->error('Error sending Mail to ' . json_encode($this->recipient));
+			$this->logger->error('Error sending Mail to {recipient}', ['recipient' => json_encode($this->recipient)]);
 			$this->logger->alert($e->getMessage());
 			throw $e;
 		}

lib/Model/User/Contact.php

@@ -84,7 +84,7 @@ class Contact extends UserBase {
 		// workaround fur multiple found UIDs
 		// Don't throw an error, log the error and take the first entry
 		if (count($contacts) > 1) {
-			$this->logger->warning('Multiple contacts found for id ' . $this->id);
+			$this->logger->warning('Multiple contacts found for {id} ', ['id' => $this->id]);
 		}
 
 		$this->contact = $contacts[0];

lib/Service/MailService.php

@@ -161,9 +161,15 @@ class MailService {
 				$notication = new NotificationMail($subscription);
 				$notication->send();
 			} catch (InvalidEmailAddress $e) {
-				$this->logger->warning('Invalid or no email address for notification: ' . json_encode($subscription), ['exception' => $e]);
+				$this->logger->warning('Invalid or no email address for notification', [
+					'recipient' => json_encode($subscription),
+					'exception' => $e,
+				]);
 			} catch (\Exception $e) {
-				$this->logger->error('Error sending notification to ' . json_encode($subscription), ['exception' => $e]);
+				$this->logger->error('Error sending notification', [
+					'recipient' => json_encode($subscription),
+					'exception' => $e,
+				]);
 				continue;
 			}
 		}
@@ -183,16 +189,22 @@ class MailService {
 				if ($sentResult) {
 					$sentResult->AddSentMail($recipient);
 				}
+
 			} catch (InvalidEmailAddress $e) {
 				if ($sentResult) {
 					$sentResult->AddAbortedMail($recipient, SentResult::INVALID_EMAIL_ADDRESS);
 				}
-				$this->logger->warning('Invalid or no email address for invitation: ' . json_encode($recipient));
+				$this->logger->warning('Invalid or no email address for invitation', ['recipient' => json_encode($recipient)]);
+
 			} catch (\Exception $e) {
 				if ($sentResult) {
 					$sentResult->AddAbortedMail($recipient);
 				}
-				$this->logger->error('Error sending Invitation to ' . json_encode($recipient));
+
+				$this->logger->error('Error sending invitation', [
+					'recipient' => json_encode($recipient),
+					'exception' => $e,
+				]);
 			}
 		}
 
@@ -228,10 +240,16 @@ class MailService {
 				$this->sendConfirmationMail($participant, $pollId);
 				$sentResult->AddSentMail($participant);
 			} catch (InvalidEmailAddress $e) {
-				$this->logger->warning('Invalid or no email address for confirmation: ' . json_encode($participant));
+				$this->logger->warning('Invalid or no email address for confirmation', [
+					'recipient' => json_encode($participant),
+					'exception' => $e,
+				]);
 				$sentResult->AddAbortedMail($participant, SentResult::INVALID_EMAIL_ADDRESS);
 			} catch (\Exception $e) {
-				$this->logger->error('Error sending confirmation to ' . json_encode($participant));
+				$this->logger->error('Error sending confirmation', [
+					'recipient' => json_encode($participant),
+					'exception' => $e,
+				]);
 				$sentResult->AddAbortedMail($participant);
 			}
 		}
@@ -266,11 +284,20 @@ class MailService {
 
 			try {
 				$reminder->send();
-				$this->logger->info('Reminder for poll id ' . $poll->getId() . ' sent to ' . json_encode($recipient));
+				$this->logger->info('Reminder sent', [
+					'recipient' => json_encode($recipient),
+					'pollId' => $poll->getId(),
+				]);
 			} catch (InvalidEmailAddress $e) {
-				$this->logger->warning('Invalid or missing email address for sending out reminder for poll id ' . $poll->getid() . ' to share id ' . $share->getId());
+				$this->logger->warning('Invalid or missing email address for sending out reminder', [
+					'pollId' => $poll->getid(),
+					'shareId' => $share->getId()
+				]);
 			} catch (\Exception $e) {
-				$this->logger->error('Error sending reminder to ' . json_encode($share));
+				$this->logger->error('Error sending reminder', [
+					'share' => json_encode($share),
+					'exception' => $e
+				]);
 			}
 		}
 	}

lib/Service/OptionService.php

@@ -234,7 +234,10 @@ class OptionService {
 			try {
 				$this->optionMapper->insert($clonedOption);
 			} catch (Exception $e) {
-				$this->logger->warning('Skip sequence no. ' . $i . 'of option ' . $this->option->getId() . '. Option possibly already exists.');
+				$this->logger->warning('Skip sequence no. {sequence} of option {optionId}. Option possibly already exists.', [
+					'sequence' => $i,
+					'optionId' => $this->option->getId(),
+				]);
 			}
 		}
 

lib/Service/ShareService.php

@@ -342,7 +342,7 @@ class ShareService {
 	 *
 	 * added in Polls 7.2.4 (can be removed later)
 	 */
-	private function convertDependingObjects(string $userId, string $replacementId, int $pollId) {
+	private function convertDependingObjects(string $userId, string $replacementId, int $pollId): void {
 		$this->voteMapper->renameUserId($userId, $replacementId, $pollId);
 		$this->optionMapper->renameUserId($userId, $replacementId, $pollId);
 		$this->commentMapper->renameUserId($userId, $replacementId, $pollId);
@@ -399,7 +399,7 @@ class ShareService {
 				$this->mailService->sendInvitation($this->share);
 			}
 		} catch (\Exception $e) {
-			$this->logger->error('Error sending Mail to ' . $this->share->getEmailAddress());
+			$this->logger->error('Error sending Mail', ['emailAddress' => $this->share->getEmailAddress()]);
 		}
 
 		return $this->share;

lib/Service/SystemService.php

@@ -86,22 +86,23 @@ class SystemService {
 			IShare::TYPE_GROUP,
 			IShare::TYPE_EMAIL
 		];
+		$maxResults = 200;
 		if (Circle::isEnabled() && class_exists('\OCA\Circles\ShareByCircleProvider')) {
 			// Add circles to the search, if app is enabled
 			$types[] = IShare::TYPE_CIRCLE;
 		}
 
-		[$result, $more] = $this->userSearch->search($query, $types, false, 200, 0);
+		[$result, $more] = $this->userSearch->search($query, $types, false, $maxResults, 0);
 
 		if ($more) {
-			$this->logger->info('Only first 200 matches will be returned.');
+			$this->logger->info('Only first {maxResults} matches will be returned.', ['maxResults' => $maxResults]);
 		}
 
 		foreach (($result['users'] ?? []) as $item) {
 			if (isset($item['value']['shareWith'])) {
 				$items[] = $this->userMapper->getUserFromUserBase($item['value']['shareWith'])->getRichUserArray();
 			} else {
-				$this->handleFailedSearchResult($query, $item);
+				$this->handleFailedSearchResult($query, $item, 'users');
 			}
 		}
 
@@ -109,7 +110,7 @@ class SystemService {
 			if (isset($item['value']['shareWith'])) {
 				$items[] = $this->userMapper->getUserFromUserBase($item['value']['shareWith'])->getRichUserArray();
 			} else {
-				$this->handleFailedSearchResult($query, $item);
+				$this->handleFailedSearchResult($query, $item, 'exact users');
 			}
 		}
 
@@ -117,7 +118,7 @@ class SystemService {
 			if (isset($item['value']['shareWith'])) {
 				$items[] = (new Group($item['value']['shareWith']))->getRichUserArray();
 			} else {
-				$this->handleFailedSearchResult($query, $item);
+				$this->handleFailedSearchResult($query, $item, 'groups');
 			}
 		}
 
@@ -125,7 +126,7 @@ class SystemService {
 			if (isset($item['value']['shareWith'])) {
 				$items[] = (new Group($item['value']['shareWith']))->getRichUserArray();
 			} else {
-				$this->handleFailedSearchResult($query, $item);
+				$this->handleFailedSearchResult($query, $item, 'exact groups');
 			}
 		}
 
@@ -153,10 +154,11 @@ class SystemService {
 		return $items;
 	}
 
-	private function handleFailedSearchResult(string $query, mixed $item): void {
-		$this->logger->debug('Unrecognized result for query: \"{query}\". Result: {result]', [
+	private function handleFailedSearchResult(string $query, mixed $item, string $type = 'unspecified'): void {
+		$this->logger->debug('Unrecognized search result', [
 			'query' => $query,
 			'result' => json_encode($item),
+			'type' => $type,
 		]);
 	}