Skip to content
Snippets Groups Projects

Issue 2919464: Add Drush command to re-scan existing files

Merged Kalle Vuorjoki requested to merge issue/clamav-2919464:2919464-rescan-command into 2.0.x
8 unresolved threads

Closes #2919464

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
35 * @param \Drupal\clamav\Scanner $scanner
36 * The ClamAV scanner service.
37 * @param \Drupal\Core\File\FileSystemInterface $fileSystem
38 * The file system service.
39 */
40 public function __construct(
41 protected EntityTypeManagerInterface $entity_type_manager,
42 protected Scanner $scanner,
43 protected FileSystemInterface $fileSystem
44 ) {
45 parent::__construct();
46 $this->fileStorage = $entity_type_manager->getStorage('file');
47 }
48
49 #[CLI\Command(name: 'clamav:scan-files', aliases: ['cav-sf'])]
50 #[CLI\Option(name: 'batch-size', description: 'Optional batch-size.')]
  • 68 $this->logger()->notice('No managed files found to scan.');
    69 return;
    70 }
    71
    72 $files = $this->fileStorage->loadMultiple($file_ids);
    73 $total = count($files);
    74
    75 $this->logger()->notice("Starting scan of {$total} managed files...");
    76
    77 // Set up the batch.
    78 $batch = [
    79 'title' => dt('Processing managed files'),
    80 'operations' => [],
    81 'finished' => [$this, 'batchFinished'],
    82 'progressive' => TRUE,
    83 'init_message' => dt('Starting scanning processing...'),
  • 58 * Scans all managed files in the system.
    59 */
    60 protected function scanManagedFiles(array $options) {
    61 // Query to get all permanent file IDs.
    62 $file_ids = $this->fileStorage->getQuery()
    63 ->condition('status', FileInterface::STATUS_PERMANENT)
    64 ->accessCheck(FALSE)
    65 ->execute();
    66
    67 if (empty($file_ids)) {
    68 $this->logger()->notice('No managed files found to scan.');
    69 return;
    70 }
    71
    72 $files = $this->fileStorage->loadMultiple($file_ids);
    73 $total = count($files);
  • 126 $files = $this->fileStorage->loadMultiple($file_ids);
    127
    128 foreach ($files as $file) {
    129 /** @var \Drupal\file\FileInterface $file */
    130 $uri = $file->getFileUri();
    131 $real_path = $this->fileSystem->realpath($uri);
    132
    133 if (!$real_path || !file_exists($real_path)) {
    134 $this->logger()->warning("Could not access file: {$uri}");
    135 continue;
    136 }
    137
    138 try {
    139 $this->scanner->scan($file);
    140 $context['results']['processed']++;
    141
  • Mostly looks good, but some changes required.

    One hard blocker: unneeded loading of all files to get count.

  • added 1 commit

    Compare with previous version

  • added 1 commit

    • d087b45f - Issue 2919464: Use \Drupal::logger in Batch callback

    Compare with previous version

  • 34 * The entity type manager.
    35 * @param \Drupal\clamav\Scanner $scanner
    36 * The ClamAV scanner service.
    37 * @param \Drupal\Core\File\FileSystemInterface $fileSystem
    38 * The file system service.
    39 */
    40 public function __construct(
    41 protected EntityTypeManagerInterface $entity_type_manager,
    42 protected Scanner $scanner,
    43 protected FileSystemInterface $fileSystem
    44 ) {
    45 parent::__construct();
    46 $this->fileStorage = $entity_type_manager->getStorage('file');
    47 }
    48
    49 #[CLI\Command(name: 'clamav:scan-files', aliases: ['cav-sf'])]
    • Comment on lines +48 to +49

      Command must have a description:

      Suggested change
      51
      52 #[CLI\Command(name: 'clamav:scan-files', aliases: ['cav-sf'])]
      51
      52 /**
      53 * Scan existing managed permanent files.
      54 */
      55 #[CLI\Command(name: 'clamav:scan-files', aliases: ['cav-sf'])]

      Otherwise, calling drush clamav:scan-files --help results in:

      $ drush clamav:scan-files -h
       [warning] Undefined array key "description" HelpCLIFormatter.php:29
      
      Examples:
       drush clamav:scan-files                Scans all managed files for viruses using ClamAV          
       drush clamav:scan-files --batch-size=5 Scans all managed files using ClamAV with batch size of 5 
      
      Options:
       --batch-size[=BATCH-SIZE] Batch size, optional. [default: 50] 
    • Please register or sign in to reply
  • 118 // Query files for this batch.
    119 $file_ids = $this->fileStorage->getQuery()
    120 ->condition('status', FileInterface::STATUS_PERMANENT)
    121 ->accessCheck(FALSE)
    122 ->range($offset, $limit)
    123 ->execute();
    124
    125 $files = $this->fileStorage->loadMultiple($file_ids);
    126
    127 foreach ($files as $file) {
    128 /** @var \Drupal\file\FileInterface $file */
    129 $uri = $file->getFileUri();
    130 $real_path = $this->fileSystem->realpath($uri);
    131
    132 if (!$real_path || !file_exists($real_path)) {
    133 \Drupal::logger('clamav')->notice("Could not access file: {$uri}");
    • In Drush commands $this->logger() is using Drush logger which outputs to the STDOUT by default.

      Batch callbacks are handled a bit differently in Drush - worker/job output is expected to be placed in the $context['message'] and it will be displayed after worker will finish.

      \Drupal::logger('clamav') will log the message using configured logger/s which is completely different to the rest of this command logging behaviour and I believe should be changed to use either the $context['message'] or the Drush::logger() approach. If the default Drupal logger is a DB logger, using it with a big number of missing/inaccessible files could lead to a lot of DB queries.

    • Kirill Roskolii changed this line in version 5 of the diff

      changed this line in version 5 of the diff

    • Please register or sign in to reply
  • 128 /** @var \Drupal\file\FileInterface $file */
    129 $uri = $file->getFileUri();
    130 $real_path = $this->fileSystem->realpath($uri);
    131
    132 if (!$real_path || !file_exists($real_path)) {
    133 \Drupal::logger('clamav')->notice("Could not access file: {$uri}");
    134 continue;
    135 }
    136
    137 try {
    138 $this->scanner->scan($file);
    139 $context['results']['processed']++;
    140 }
    141 catch (\Exception $e) {
    142 $context['results']['failed']++;
    143 }
  • 138 $this->scanner->scan($file);
    139 $context['results']['processed']++;
    140 }
    141 catch (\Exception $e) {
    142 $context['results']['failed']++;
    143 }
    144 }
    145 }
    146
    147 /**
    148 * Batch finished callback.
    149 */
    150 public function batchFinished($success, $results, $operations): void {
    151 $messenger = \Drupal::messenger();
    152 if ($success) {
    153 $messenger->addMessage(new TranslatableMarkup('@count files healthy.', ['@count' => $results['processed']]));
  • Kirill Roskolii added 3 commits

    added 3 commits

    Compare with previous version

  • All issues should be addressed now

  • added 2 commits

    • e5a92329 - Issue #3489496: Add new logo and add logo to codebase
    • 65346948 - Merge branch clamav:2.0.x into 2919464-rescan-command

    Compare with previous version

  • Vladimir Roudakov changed target branch from 2.x to 2.0.x

    changed target branch from 2.x to 2.0.x

  • Please register or sign in to reply
    Loading