Added a Setting to set the Model Download Folder

[a4blue]

This should fix #1160
In order to be compatible with existing users i added a repair step to move the old Files to the new Location

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI

[Copilot]

Pull request overview

Adds a configurable model download location and migrates existing models.

Changes:

• Adds model-path settings and admin UI.
• Updates model downloading and status checks.
• Adds an upgrade repair step for existing models.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/components/ViewAdmin.vue	Adds model-path controls and status.
package-lock.json	Updates package version metadata.
lib/Settings/AdminSettings.php	Provides model-path status.
lib/Service/SettingsService.php	Defines and resolves the path setting.
lib/Service/DownloadModelsService.php	Downloads models to the configured location.
lib/Migration/MoveDefaultModelFolder.php	Migrates existing model files.
appinfo/info.xml	Registers the migration step.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[marcelklehr]

Removing is probably not what we want though. We should add it to SettingsService

[a4blue]

do we really want to have separate locations for models and the archive ?
In my opinion this only increases Complexity with no Benefit right now, since the Archive already was and still will be relative to the Models folder

[marcelklehr]

Yes, you're right!

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[marcelklehr]

@a4blue Would you be amenable to fix the above shortcomings?

[marcelklehr]

Sorry, more issues:

Blocking

1. Safe\mkdir doesn't exist — fatal on every fresh install. lib/Service/DownloadModelsService.php:15 adds use function Safe\mkdir;, but thecodingmachine/safe is in neither composer.json nor composer.lock (0 hits), and it's the only Safe\ usage in lib/. The aliased call at line 38 fires exactly when models_cache doesn't exist yet — the default path on a new install — so the first occ recognize:download-models dies with Call to undefined function Safe\mkdir(). Use plain mkdir() and check the return value.

2. The repair step moves nothing. lib/Migration/MoveDefaultModelFolder.php:55-64 iterates scandir($oldModelTargetPath) and calls copy()/unlink() on each entry — but all 10 top-level entries under models/ are directories (efficientnet_lite4, musicnn, movinet-a3, the seven landmarks_*; 291 files total). copy() on a directory warns "Is a directory" and returns false; unlink() on it fails too. Net result on every existing install: warnings in the log, models left in models/, app now looking in models_cache/, full re-download. Needs a recursive copy — or rename() first and fall back to recursive copy only on EXDEV, which keeps the common same-filesystem case cheap and still satisfies the cross-partition case from Add option to customize model download location #1160.

3. Same loop: unlink() is unconditional. copy()'s return value is ignored, and mkdir() at line 58 is unchecked. If the destination can't be created or the copy fails (perms, ENOSPC), the source is deleted anyway. Guard the unlink() on a successful copy, and bail out loudly if mkdir() fails.

4. occ recognize:download-models is now silent. download() dropped its ?callable $log parameter, but lib/Command/DownloadModels.php:44 still passes the output closure. PHP silently discards the extra argument, and $log ??= static fn… now always installs the no-op — so the command prints nothing at all through the whole download and extraction. Either keep the parameter or update the command. The @PARAM callable(string): void|null $log docblock above it is stale and psalm will flag it.

5. MODEL_DIR has no fallback, and it breaks the JS test CI. tests/tests.js:53,85 and tests/landmarks-tests.js:49 invoke src/classifier_imagenet.js / classifier_landmarks.js directly via execa with no env, so path.resolve(MODEL_DIR, modelName) hits path.resolve(undefined, …) → ERR_INVALID_ARG_TYPE. That fails classifier-test.yml:49 and classifier-landmarks-test.yml:56. const MODEL_DIR = process.env.MODEL_DIR || path.resolve(__dirname, '..', 'models') fixes CI and keeps the scripts runnable by hand.

Should fix

6. The resolved path leaks into the UI and gets persisted. getAll() calls getSetting(), which for models_target_path returns the resolved value — so the admin field displays /…/apps/recognize/lib/Service/../../models_cache, and submit() writes that literal string back to appconfig on the next change to any setting. The default silently becomes a hardcoded absolute path, and your $value === '' → DEFAULTS reset can never apply again. Keep raw values for the settings API and resolve only at the consumption sites (Classifier, DownloadModelsService, AdminSettings, the repair step).

7. models_archive_file is dead. It's in LAZY_SETTINGS (SettingsService.php:99) with no DEFAULTS entry — given you two agreed the archive stays next to the models, remove it. As-is, getSetting('models_archive_file') would warn on the undefined key and then TypeError in getAppValueString().

8. isPathAbsolute() reads $path[1] unguarded (SettingsService.php:225,229) — a one-character relative path produces "Uninitialized string offset 1" warnings. Add a strlen() >= 2 check.

9. "not writable" is always shown on a fresh install. AdminSettings.php:33 calls is_writable($targetPath) on a directory that doesn't exist until the first download, so it returns false. Walk up to the nearest existing ancestor, or create the directory when resolving the setting.

10. Env var naming. Everything else uses the RECOGNIZE_ prefix (RECOGNIZE_CORES, RECOGNIZE_PUREJS). MODEL_DIR is generic enough to collide with unrelated tooling in the admin's environment — suggest RECOGNIZE_MODEL_DIR.

11. No validation on the admin-supplied path. download() recursively rmdirs $targetPath/models. A trailing space or a typo'd path quietly deletes an unrelated models directory. At minimum trim() the value; ideally reject non-absolute custom values and echo the resolved path back in the UI so the admin can see what they actually configured.