Bug: Renaming the recipe folder causes a silent empty-folder recreation loop (with solutions)

[eals2019]

Bug: Renaming the recipe folder causes a silent empty-folder recreation loop

App: Cookbook 0.11.7
Platform: Nextcloud 34.0.0 · MariaDB · Docker · Group folder

---

Summary

When a user renames their configured recipe folder via Nextcloud Files, Cookbook's stored path in oc_preferences becomes stale. On every subsequent page load, UserFolderHelper::getOrCreateFolder() silently creates a new empty folder at the old path rather than surfacing an error. This makes all recipes invisible with no diagnostic message, and the empty folder reappears immediately after the user deletes it — creating an inescapable loop until the database is corrected manually via occ.

---

Steps to Reproduce

1. Configure Cookbook to use a folder, e.g. /Family Drive/Family_Recipes.
2. Outside of Cookbook — via Nextcloud Files or a desktop client — rename that folder to Family Recipes (with a space).
3. Return to Cookbook. Recipes are gone. No error is displayed.
4. Open Nextcloud Files: a new, empty Family_Recipes folder has been created by Cookbook.
5. Delete the empty folder. Reload Cookbook. The empty folder immediately reappears.
6. The loop cannot be broken from the UI; it requires a manual occ user:setting correction.

---

Current vs Expected Behaviour

Current: When the configured folder is not found, getOrCreateFolder() catches NotFoundException and silently calls $this->root->newFolder($path), creating an empty folder at the stale path. Cookbook then operates against this empty folder, returning zero recipes. No message is shown to the user.

Expected: When the configured folder does not exist, Cookbook should surface a clear, actionable error — e.g. "Recipe folder '/Family Drive/Family_Recipes' was not found. Please update it in Settings." — and not silently mutate the filesystem.

---

Root Cause

The path stored in oc_preferences (appid = 'cookbook', configkey = 'folder') is never updated when the folder is renamed outside of Cookbook. Nextcloud has no mechanism to notify Cookbook of filesystem renames unless Cookbook explicitly listens for them.

The secondary failure is that the app treats a missing configured folder as a first-run condition and recreates it — appropriate at initial setup, destructive when the configuration is merely stale.

lib/Helper/UserFolderHelper.php — current problematic code:

private function getOrCreateFolder(string $path): Folder {
    try {
        $node = $this->root->get($path);
    } catch (NotFoundException $ex) {
        try {
            $node = $this->root->newFolder($path);  // ← silently creates empty folder
        } catch (NotPermittedException $ex1) {
            throw new UserFolderNotValidException(
                $this->l->t('The user folder cannot be created due to missing permissions.'),
                0, $ex1
            );
        }
    }
    // ...
}

---

Proposed Fix — Option 1 (Minimal): Throw instead of recreate

Remove the silent folder creation and throw UserFolderNotValidException when the configured path is missing. This stops the recreation loop and surfaces an actionable error to the user. The user still needs to re-select their folder in Settings.

Note on first-run: If getOrCreateFolder() is relied upon to create the initial /Recipes folder on first use, that creation should be moved to a dedicated setup step triggered only when no folder has ever been configured, rather than running on every page load.

lib/Helper/UserFolderHelper.php:

 private function getOrCreateFolder(string $path): Folder {
     try {
         $node = $this->root->get($path);
     } catch (NotFoundException $ex) {
-        try {
-            $node = $this->root->newFolder($path);
-        } catch (NotPermittedException $ex1) {
-            throw new UserFolderNotValidException(
-                $this->l->t('The user folder cannot be created due to missing permissions.'),
-                0, $ex1
-            );
-        }
+        throw new UserFolderNotValidException(
+            $this->l->t(
+                'Recipe folder "%s" was not found. Please update your cookbook folder in Settings.',
+                [$path]
+            ),
+            0,
+            $ex
+        );
     }

     if ($node->getType() !== FileInfo::TYPE_FOLDER) {
         throw new UserFolderNotValidException(
             $this->l->t('The configured user folder is a file.')
         );
     }
     // ...
 }

---

Proposed Fix — Option 2 (Complete): Auto-update path on rename via Nextcloud event

Register a listener for NodeRenamedEvent. When a node is renamed and its old path matches (or is a parent of) the user's configured recipe folder, the stored path is updated automatically. The user renames their folder in Files and Cookbook follows — no Settings visit required.

This fix includes Option 1: the throw-not-create change is still required as a safety net for cases where the folder is deleted entirely rather than renamed.

Step 1 — Create lib/Listener/RecipeFolderRenamedListener.php (new file):

<?php

namespace OCA\Cookbook\Listener;

use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Files\Events\Node\NodeRenamedEvent;
use OCP\IConfig;
use OCP\IUserSession;

/**
 * When the user renames the configured recipe folder, keep
 * the stored oc_preferences path in sync automatically.
 */
class RecipeFolderRenamedListener implements IEventListener {
    private const APP_ID     = 'cookbook';
    private const CONFIG_KEY = 'folder';

    public function __construct(
        private IConfig $config,
        private IUserSession $userSession,
    ) {}

    public function handle(Event $event): void {
        if (!($event instanceof NodeRenamedEvent)) {
            return;
        }

        $user = $this->userSession->getUser();
        if ($user === null) {
            return;
        }

        $userId     = $user->getUID();
        $sourcePath = $event->getSource()->getPath();  // /userId/files/old/path
        $targetPath = $event->getTarget()->getPath();  // /userId/files/new/path

        // Strip the /userId/files prefix to get the user-relative path
        $filesRoot = '/' . $userId . '/files';
        if (!str_starts_with($sourcePath, $filesRoot)) {
            return;
        }

        $sourceRelative = substr($sourcePath, strlen($filesRoot));  // /old/path
        $storedFolder   = $this->config->getUserValue($userId, self::APP_ID, self::CONFIG_KEY);

        // Match if the stored folder IS the renamed node, or is inside it
        if ($storedFolder !== $sourceRelative
            && !str_starts_with($storedFolder, $sourceRelative . '/')
        ) {
            return;
        }

        $targetRelative = substr($targetPath, strlen($filesRoot));  // /new/path
        $newFolder      = $targetRelative . substr($storedFolder, strlen($sourceRelative));

        $this->config->setUserValue($userId, self::APP_ID, self::CONFIG_KEY, $newFolder);
    }
}

Step 2 — Register the listener in lib/AppInfo/Application.php:

 use OCP\AppFramework\Bootstrap\IBootContext;
+use OCP\Files\Events\Node\NodeRenamedEvent;
+use OCA\Cookbook\Listener\RecipeFolderRenamedListener;

 public function boot(IBootContext $context): void {
     $context->injectFn(function (
         /* existing injections ... */
+        IEventDispatcher $dispatcher,
     ) {
+        $dispatcher->addServiceListener(
+            NodeRenamedEvent::class,
+            RecipeFolderRenamedListener::class,
+        );
     });
 }

Edge cases handled:

• Parent folder renamed — if /Family Drive itself is renamed, str_starts_with($storedFolder, $sourceRelative . '/') catches it and the sub-path is preserved in the updated value.
• Unrelated renames — the guard returns early, so the listener is a no-op for all other filesystem operations.
• Folder moved (not just renamed) — Nextcloud models moves as renames; NodeRenamedEvent fires for both.
• Folder deleted entirely — not covered by the event listener; Option 1's throw-not-create is the safety net for this case.

---

Additional Code Defects (lower severity)

Inverted condition in RestParameterParser::getEncoding()

substr_compare() returns 0 (falsy) on a match, so the condition below is true when the string does not begin with charset= — the opposite of the comment. Charset extraction never triggers. No observable impact today since the UTF-8 fallback default is always correct, but the logic is wrong.

lib/Helper/RestParameterParser.php:

-if (substr_compare(trim($parts[$i]), self::CHARSET, 0, strlen(self::CHARSET))) {
+if (substr_compare(trim($parts[$i]), self::CHARSET, 0, strlen(self::CHARSET)) === 0) {
     $enc = substr(trim($parts[$i]), strlen(self::CHARSET) + 1);
     break;
 }

Inverted condition in RestParameterParser::parseUrlEncoded()

The same inversion affects []-suffixed array key detection. The body runs when the key does not end in [], stripping two characters from ordinary keys instead. Not triggered by the current JS client (which sends JSON), but would affect any future non-POST, URL-encoded caller.

lib/Helper/RestParameterParser.php:

-if (substr_compare($key, '[]', -2, 2)) {   // true when key does NOT end in []
+if (substr_compare($key, '[]', -2, 2) === 0) {  // true when key ends in []
     $key = substr($key, 0, -2);
     // ...
 }