Merge branch 'pu/pm/FMnodeUpdateNameChgGoesThroughMove' into '2022.11'

tweak(FM/TB FS) update node should not allow name change, json fe update will...

See merge request tine20/tine20!3650

tests/tine20/Filemanager/Frontend/JsonTests.php

@@ -1738,7 +1738,7 @@ public function testDeleteDirectoryNodeInSharedWithPersonalAdminAcl()
      * @see 0006736: Create File (Edit)InfoDialog
      * @return array
      */
-    public function testGetUpdate()
+    public function testGetUpdate($doUpdateMove = true)
     {
         $this->testCreateFileNodes();
         $filter = array(array(
@@ -1753,13 +1753,22 @@ public function testGetUpdate()
 
         $node = $this->_getUit()->getNode($initialNode['id']);
         $this->assertEquals('file', $node['type']);
+        if (!$doUpdateMove) {
+            return $node;
+        }
 
         $node['description'] = 'UNITTEST';
         $node = $this->_getUit()->saveNode($node);
 
         $this->assertEquals('UNITTEST', $node['description']);
         $this->assertEquals($initialNode['contenttype'], $node['contenttype'], 'contenttype  not preserved');
 
+        $newName = $node['name'] . 'new';
+        $node['name'] = $newName;
+        $node['description'] = 'unittest';
+        $node = $this->_getUit()->saveNode($node);
+        $this->assertSame($newName, $node['name']);
+        $this->assertSame('unittest', $node['description']);
 
         return $node;
     }
@@ -1792,7 +1801,7 @@ public function testAttachTagPreserveContentType()
      */
     public function testSetRelation()
     {
-        $node = $this->testGetUpdate();
+        $node = $this->testGetUpdate(false);
         $node['relations'] = array($this->_getRelationData($node));
         $node = $this->_getUit()->saveNode($node);
 

tests/tine20/OnlyOfficeIntegrator/ControllerTests.php

@@ -539,10 +539,12 @@ public function testUpdateStatusDocumentChangedImplicitRename()
             'limit' => 2,
         ]));
 
-        static::assertStringContainsString(' name (test.txt -> test.docx)', $notes->getFirstRecord()->note);
-        static::assertStringNotContainsString(' name (test.txt -> test.docx)', $notes->getLastRecord()->note);
+        $first = strpos($notes->getFirstRecord()->note, 'test.txt') === false ? $notes->getFirstRecord() : $notes->getLastRecord();
+        $last = strpos($notes->getFirstRecord()->note, 'test.txt') !== false ? $notes->getFirstRecord() : $notes->getLastRecord();
+        static::assertStringContainsString('ame (test.txt -> test.docx)', $last->note);
+        static::assertStringNotContainsString('ame (test.txt -> test.docx)', $first->note);
         $_ = Tinebase_Translation::getTranslation(Tinebase_Config::APP_NAME, Tinebase_Core::getLocale());
-        static::assertStringContainsString(' ' . $_->_('size') . ' (', $notes->getFirstRecord()->note);
+        static::assertStringContainsString(' ' . $_->_('size') . ' (', $first->note);
 
         static::assertSame('changesContent', file_get_contents('tine20://' .
             OnlyOfficeIntegrator_Controller::getRevisionsChangesPath() . '/' . $node->getId() . '/' . $node->revision));

tine20/EFile/Controller.php

@@ -302,7 +302,7 @@ public static function nameChildByParent($_parent, $_child, $_recursive = false,
             $_parent->{EFile_Config::TREE_NODE_FLD_TIER_REF_NUMBER} . $prefix . $tierToken;
 
         if ($_updateChild) {
-            Tinebase_FileSystem::getInstance()->update($_child);
+            Tinebase_FileSystem::getInstance()->_getTreeNodeBackend()->update($_child);
             if ($_recursive && Tinebase_Model_Tree_FileObject::TYPE_FOLDER === $_child->type) {
                 Tinebase_FileSystem::getInstance()->getFromStatCache($_child->getId())
                     ->{EFile_Config::TREE_NODE_FLD_TIER_COUNTER} = [];

tine20/Filemanager/Controller/Node.php

@@ -223,6 +223,9 @@ protected function _inspectBeforeUpdate($_record, $_oldRecord)
             }
         }
 
+        // we do not allow renames in update
+        $_record->name = $_oldRecord->name;
+
         if (!Tinebase_Core::getUser()->hasGrant($_record, Tinebase_Model_Grants::GRANT_ADMIN, 'Tinebase_Model_Tree_Node')) {
             $_record->{Tinebase_Model_Tree_Node::XPROPS_REVISION} = $_oldRecord->{Tinebase_Model_Tree_Node::XPROPS_REVISION};
             $_record->quota = $_oldRecord->quota;

tine20/Filemanager/Frontend/Json.php

@@ -223,7 +223,14 @@ public function getNode($id)
     public function saveNode($recordData)
     {
         if((isset($recordData['created_by']) || array_key_exists('created_by', $recordData))) {
-            return $this->_save($recordData, Filemanager_Controller_Node::getInstance(), 'Node');
+            $result = $this->_save($recordData, Filemanager_Controller_Node::getInstance(), 'Node');
+            if ($recordData['name'] !== $result['name']) {
+                $moveResult = $this->moveNodes($result['path'], [dirname($result['path']) . '/' . $recordData['name']], false);
+                if (isset($moveResult[0]['id'])) {
+                    return $moveResult[0];
+                }
+            }
+            return $result;
         } else {    // on upload complete
             return $recordData;
         }

tine20/OnlyOfficeIntegrator/Controller.php

@@ -573,8 +573,6 @@ protected function doSave($requestData, $token)
 
         // figure out target path ... also file ending rewrite happens here
         $srcEnding = ltrim(substr($requestData['url'], strrpos($requestData['url'], '.')), '.');
-        $oldName = null;
-        $newName = '';
         $tempFile = null;
         $node = null;
         $dataSafeRAII = null;
@@ -603,8 +601,9 @@ protected function doSave($requestData, $token)
                     });
                 }
 
-                $trgtPath = 'tine20://' . Tinebase_FileSystem::getInstance()->getPathOfNode($accessToken
-                        ->{OnlyOfficeIntegrator_Model_AccessToken::FLDS_NODE_ID}, true);
+                $pathOfNode = Tinebase_FileSystem::getInstance()->getPathOfNode($accessToken
+                    ->{OnlyOfficeIntegrator_Model_AccessToken::FLDS_NODE_ID}, true);
+                $trgtPath = 'tine20://' . $pathOfNode;
 
                 $trgtEnding = ltrim(substr($trgtPath, $pos = strrpos($trgtPath, '.')), '.');
                 if (!$saveConflict) {
@@ -641,11 +640,10 @@ protected function doSave($requestData, $token)
                             $node = Tinebase_FileSystem::getInstance()->get($accessToken
                                 ->{OnlyOfficeIntegrator_Model_AccessToken::FLDS_NODE_ID});
                         }
-                        $oldName = $node->name;
                         $array = explode('/', $trgtPath);
-                        $newName = $node->name = @end($array);
+                        $node->name = @end($array);
                         /** do not use the return of this call! revision will be increased later! */
-                        Tinebase_FileSystem::getInstance()->update($node);
+                        Tinebase_FileSystem::getInstance()->rename($pathOfNode, substr($trgtPath, 9));
                         Tinebase_FileSystem::getInstance()->clearStatCache();
                     }
                 }
@@ -694,24 +692,14 @@ protected function doSave($requestData, $token)
             ]), new Tinebase_Model_Pagination([
                 'sort' => 'seq',
                 'dir' => 'DESC',
-                'limit' => 2,
+                'limit' => 1,
             ]));
 
             $note = $notes->getFirstRecord();
 
             if (null === $note || $note->created_by !== $user->getId()) {
                 Tinebase_Core::getLogger()->warn(__METHOD__ . '::' . __LINE__ . ' did not find file modification note');
             } else {
-                if ($oldName !== null) {
-                    $newNote = ' name (' . $oldName . ' -> ' . $newName . ')';
-                    $note->note = $note->note . $newNote;
-                    if ($notes->count() === 2 && strpos($notes->getLastRecord()->note, $newNote) !== false) {
-                        $notes->removeFirst();
-                        Tinebase_Notes::getInstance()->deleteNotes($notes);
-                    } else {
-                        Tinebase_Core::getLogger()->warn(__METHOD__ . '::' . __LINE__ . ' did not find file rename note');
-                    }
-                }
                 $allUsers = join(', ', Tinebase_User::getInstance()->getMultiple($allTokens
                     ->{OnlyOfficeIntegrator_Model_AccessToken::FLDS_USER_ID}, Tinebase_Model_FullUser::class)
                     ->accountDisplayName);

tine20/Tinebase/FileSystem.php

@@ -2506,6 +2506,11 @@ public function update(Tinebase_Model_Tree_Node $_node)
 
         try {
             $currentNodeObject = $this->get($_node->getId());
+
+            if ($_node->name !== $currentNodeObject->name) {
+                throw new Tinebase_Exception_Record_Validation('name may not be changed in update');
+            }
+
             /** @var Tinebase_Model_Tree_FileObject $fileObject */
             $fileObject = $this->_fileObjectBackend->get($currentNodeObject->object_id);
 

tine20/Tinebase/Frontend/WebDAV/Container.php

@@ -286,14 +286,19 @@ public function getSize()
     public function setName($name) 
     {
         Tinebase_Frontend_WebDAV_Node::checkForbiddenFile($name);
-
-        if (!Tinebase_Core::getUser()->hasGrant($this->_getContainer(), Tinebase_Model_Grants::GRANT_EDIT)) {
+
+        $fs = Tinebase_FileSystem::getInstance();
+        if (!$fs->checkPathACL($parentPath = Tinebase_Model_Tree_Node_Path::createFromStatPath($fs->getPathOfNode(
+                $this->_getContainer()->parent_id, true)), 'add') || !$fs->checkPathACL($parentPath, 'delete')) {
             throw new Sabre\DAV\Exception\Forbidden('Forbidden to rename file: ' . $this->_path);
         }
-        
-        $this->_getContainer()->name = $name;
+
+        $oldPath = $fs->getPathOfNode($this->_getContainer(), true);
         try {
-            Tinebase_FileSystem::getInstance()->update($this->_getContainer());
+            $result = $fs->rename($oldPath, dirname($oldPath) . '/' . $name);
+            if ($result) {
+                $this->_container = $result;
+            }
         } catch (Zend_Db_Statement_Exception $zdse) {
             if (Tinebase_Exception::isDbDuplicate($zdse)) {
                 if (Tinebase_Core::isLogLevel(Zend_Log::NOTICE)) Tinebase_Core::getLogger()->notice(

tine20/Tinebase/Frontend/WebDAV/Node.php

@@ -96,8 +96,9 @@ public function setName($name)
 
         list($dirname,) = Sabre\DAV\URLUtil::splitPath($this->_path);
 
-        if (!Tinebase_FileSystem::getInstance()->checkPathACL(Tinebase_Model_Tree_Node_Path::createFromStatPath($dirname)
-                , 'update', true, false, Tinebase_Model_Tree_Node_Path::createFromStatPath($this->_path))) {
+        if (!Tinebase_FileSystem::getInstance()->checkPathACL($parentPath = Tinebase_Model_Tree_Node_Path::createFromStatPath($dirname)
+                , 'add', true, false, $path = Tinebase_Model_Tree_Node_Path::createFromStatPath($this->_path)) ||
+            !Tinebase_FileSystem::getInstance()->checkPathACL($parentPath, 'delete', true, false, $path)) {
             throw new Sabre\DAV\Exception\Forbidden('Forbidden to rename file: ' . $this->_path);
         }
 
@@ -121,14 +122,8 @@ public function rename(string $newPath)
             throw new Sabre\DAV\Exception\Forbidden('Forbidden to move file: ' . $this->_path);
         }
 
-        if (!Tinebase_FileSystem::getInstance()->checkPathACL(Tinebase_Model_Tree_Node_Path::createFromStatPath($dirname)
-                , 'update', true, false, Tinebase_Model_Tree_Node_Path::createFromStatPath($this->_path))) {
-            throw new Sabre\DAV\Exception\Forbidden('Forbidden to rename file: ' . $this->_path);
-        }
-
-
         if (!Tinebase_FileSystem::getInstance()->checkPathACL(Tinebase_Model_Tree_Node_Path::createFromStatPath(dirname($newPath))
-                , 'update', true, false, Tinebase_Model_Tree_Node_Path::createFromStatPath($newPath))) {
+                , 'add', true, false, Tinebase_Model_Tree_Node_Path::createFromStatPath($newPath))) {
             throw new Sabre\DAV\Exception\Forbidden('Forbidden to create file: ' . $newPath);
         }