+global\RefreshImageMetadata::execute()

Complexity

Conditions	14
Paths	160

Size

Total Lines	104
Code Lines	72

Duplication

Lines	18
Ratio	17.31 %

Importance

Changes

0

<?php
	public function execute() {
		$force = $this->hasOption( 'force' );
		$brokenOnly = $this->hasOption( 'broken-only' );
		$verbose = $this->hasOption( 'verbose' );
		$start = $this->getOption( 'start', false );
		$this->setupParameters( $force, $brokenOnly );

		$upgraded = 0;
		$leftAlone = 0;
		$error = 0;

		$dbw = $this->getDB( DB_MASTER );
		if ( $this->mBatchSize <= 0 ) {
			$this->error( "Batch size is too low...", 12 );
		}

		$repo = RepoGroup::singleton()->getLocalRepo();
		$conds = $this->getConditions( $dbw );

It seems like $dbw defined by $this->getDB(DB_MASTER) on line 105 can be null; however, RefreshImageMetadata::getConditions() does not accept null, maybe add an additional type check?

		// For the WHERE img_name > 'foo' condition that comes after doing a batch
		$conds2 = [];
		if ( $start !== false ) {
			$conds2[] = 'img_name >= ' . $dbw->addQuotes( $start );
		}

		$options = [
			'LIMIT' => $this->mBatchSize,
			'ORDER BY' => 'img_name ASC',
		];

		do {
			$res = $dbw->select(
				'image',
				'*',
				array_merge( $conds, $conds2 ),
				__METHOD__,
				$options
			);

			if ( $res->numRows() > 0 ) {
				$row1 = $res->current();
				$this->output( "Processing next {$this->mBatchSize} rows starting with {$row1->img_name}.\n" );
				$res->rewind();
			} else {
				$this->error( "No images to process.", 4 );
			}

			foreach ( $res as $row ) {
				// LocalFile will upgrade immediately here if obsolete
				$file = $repo->newFileFromRow( $row );
				if ( $file->getUpgraded() ) {
					// File was upgraded.
					$upgraded++;
					$newLength = strlen( $file->getMetadata() );
					$oldLength = strlen( $row->img_metadata );
					if ( $newLength < $oldLength - 5 ) {
						// If after updating, the metadata is smaller then
						// what it was before, that's probably not a good thing
						// because we extract more data with time, not less.
						// Thus this probably indicates an error of some sort,
						// or at the very least is suspicious. Have the - 5 just
						// to weed out any inconsequential changes.
						$error++;
						$this->output( "Warning: File:{$row->img_name} used to have " .
							"$oldLength bytes of metadata but now has $newLength bytes.\n" );
					} elseif ( $verbose ) {
						$this->output( "Refreshed File:{$row->img_name}.\n" );
					}
				} else {
					$leftAlone++;
					if ( $force ) {
						$file->upgradeRow();
						$newLength = strlen( $file->getMetadata() );
						$oldLength = strlen( $row->img_metadata );
						if ( $newLength < $oldLength - 5 ) {
							$error++;
							$this->output( "Warning: File:{$row->img_name} used to have " .
								"$oldLength bytes of metadata but now has $newLength bytes. (forced)\n" );
						}
						if ( $verbose ) {
							$this->output( "Forcibly refreshed File:{$row->img_name}.\n" );
						}
					} else {
						if ( $verbose ) {
							$this->output( "Skipping File:{$row->img_name}.\n" );
						}
					}
				}
			}
			$conds2 = [ 'img_name > ' . $dbw->addQuotes( $row->img_name ) ];

The variable $row seems to be defined by a foreach iteration on line 141. Are you sure the iterator is never empty, otherwise this variable is not defined?

			wfWaitForSlaves();

The function wfWaitForSlaves() has been deprecated with message: since 1.27 Use LBFactory::waitForReplication

		} while ( $res->numRows() === $this->mBatchSize );

		$total = $upgraded + $leftAlone;
		if ( $force ) {
			$this->output( "\nFinished refreshing file metadata for $total files. "
				. "$upgraded needed to be refreshed, $leftAlone did not need to "
				. "be but were refreshed anyways, and $error refreshes were suspicious.\n" );
		} else {
			$this->output( "\nFinished refreshing file metadata for $total files. "
				. "$upgraded were refreshed, $leftAlone were already up to date, "
				. "and $error refreshes were suspicious.\n" );
		}
	}