Skip to content

Conversation

@joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Jan 1, 2026

Summary

  • Extracts nested directory scanning and app filtering logic into dedicated helper methods, reducing nesting from ~7 levels to 2-3, greatly improving readability/maintainability.
  • Modernizes directory traversal to use scandir() instead of opendir/readdir/closedir pattern.
  • Some additional helpful comments sprinkled throughout

No functional changes - preserves exact same behavior including error handling for TableExistsException during upgrades.

Before:

		foreach (\OC::$APPSROOTS as $app_dir) {
			if ($dir = opendir($app_dir['path'])) {
				while (false !== ($filename = readdir($dir))) {
					if ($filename[0] !== '.' && is_dir($app_dir['path'] . "/$filename")) {
						if (file_exists($app_dir['path'] . "/$filename/appinfo/info.xml")) {
							if ($this->config->getAppValue($filename, 'installed_version') === '') {
								$enabled = $this->appManager->isDefaultEnabled($filename);
								if (($enabled || in_array($filename, $this->appManager->getAlwaysEnabledApps()))
									  && $this->config->getAppValue($filename, 'enabled') !== 'no') {
									if ($softErrors) {
										try {
											$this->installShippedApp($filename, $output);
										} catch (HintException $e) {
											if ($e->getPrevious() instanceof TableExistsException) {
												$errors[$filename] = $e;
												continue;
											}
											throw $e;
										}
									} else {
										$this->installShippedApp($filename, $output);
									}
									$this->config->setAppValue($filename, 'enabled', 'yes');
								}
							}
						}
					}
				}
				closedir($dir);
			}
		}

TODO

  • ...

Checklist

Extracts nested directory scanning and app filtering logic into
dedicated helper methods, reducing nesting from 5-6 levels to 2-3.
Modernizes directory traversal to use scandir() instead of
opendir/readdir/closedir pattern.

No functional changes - preserves exact same behavior including
error handling for TableExistsException during upgrades.

Relates to #8505 (app management code consolidation)

Signed-off-by: Josh <[email protected]>
@joshtrichards joshtrichards added this to the Nextcloud 33 milestone Jan 1, 2026
@joshtrichards joshtrichards requested a review from come-nc January 1, 2026 16:26
@joshtrichards joshtrichards added technical debt ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jan 1, 2026
@joshtrichards joshtrichards added the 3. to review Waiting for reviews label Jan 1, 2026
@joshtrichards joshtrichards marked this pull request as ready for review January 1, 2026 17:52
@joshtrichards joshtrichards requested a review from a team as a code owner January 1, 2026 17:52
@joshtrichards joshtrichards requested review from leftybournes, salmart-dev and yemkareems and removed request for a team January 1, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews feature: apps management feature: install and update ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants