Details

      Description

      The attached patch fixes clustered file download with WebDAV. There may be other remaining WebDAV cluster issues, though. I'm not sure if upload is stable yet.

        Activity

        Hide
        Bertrand Dunogier added a comment -

        Fixed in 4.4.0beta2: http://github.com/ezsystems/ezpublish/commit/abb4234

        Used the way you proposed, GL, but it would still have been better to use the passthrough() method, but since it is marked as deprecated and unused, this feels a bit risky.

        Show
        Bertrand Dunogier added a comment - Fixed in 4.4.0beta2: http://github.com/ezsystems/ezpublish/commit/abb4234 Used the way you proposed, GL, but it would still have been better to use the passthrough() method, but since it is marked as deprecated and unused, this feels a bit risky.
        Hide
        (inactive) Gunnstein Lye added a comment -

        In reply to comment #052037
        @BD:
        Thanks!
        Shouldn't this be committed to all stable releases? Or is it? Please add github links if it is.

        Show
        (inactive) Gunnstein Lye added a comment - In reply to comment #052037 @BD: Thanks! Shouldn't this be committed to all stable releases? Or is it? Please add github links if it is.
        Hide
        André R added a comment -

        Reopen as patch is not good (working on trying to make unit tests for webdav working again, and came over this).
        Basically 1: You use old webdav server in new ezc based webdav server for files 2. It causes issues since ezc webdav expects return value to be content, and not a eZWebDAVServer::OK_SILENT constant.

        Something like this might (only tested it with unit tests, so haven't checked if it works on clustered set-up) be better:

            protected function getResourceContents( $target )
            {
                $result = array( 'data' => false, 'file' => false );
                $fullPath = $target;
                $target = $this->splitFirstPathElement( $fullPath, $currentSite );
         
                $data = $this->getVirtualFolderData( $result, $currentSite, $target, $fullPath );
                if ( $data['file'] )
                {
                    return $this->getClusteredContents( $data['file'] );
                }
         
                return false;
            }
         
            protected function getClusteredContents( $realPath )
            {
                $file = eZClusterFileHandler::instance( $realPath );
                $file->fetch();
                $content = file_get_contents( $realPath );
                $file->deleteLocal();
                return $content;
            }
        

        Show
        André R added a comment - Reopen as patch is not good (working on trying to make unit tests for webdav working again, and came over this). Basically 1: You use old webdav server in new ezc based webdav server for files 2. It causes issues since ezc webdav expects return value to be content, and not a eZWebDAVServer::OK_SILENT constant. Something like this might (only tested it with unit tests, so haven't checked if it works on clustered set-up) be better: protected function getResourceContents( $target ) { $result = array( 'data' => false, 'file' => false ); $fullPath = $target; $target = $this->splitFirstPathElement( $fullPath, $currentSite );   $data = $this->getVirtualFolderData( $result, $currentSite, $target, $fullPath ); if ( $data['file'] ) { return $this->getClusteredContents( $data['file'] ); }   return false; }   protected function getClusteredContents( $realPath ) { $file = eZClusterFileHandler::instance( $realPath ); $file->fetch(); $content = file_get_contents( $realPath ); $file->deleteLocal(); return $content; }
        Hide
        Bertrand Dunogier added a comment - - edited

        In reply to comment #052039

        protected function getClusteredContents( $realPath )
        {
            $file = eZClusterFileHandler::instance( $realPath );
            $file->fetch();
            $content = file_get_contents( $realPath );
            $file->deleteLocal();
            return $content;
        }
        

        fetch + file_get_contents are actually not necessary. You can use this:

        protected function getClusteredContents( $realPath )
        {
            return eZClusterFileHandler::instance( $realPath )->fetchContents();
        }
        

        The side benefit is that this method doesn't create a local file copy, and eleminates the risk of conflicts if multiple users are downloading the same large file at the same time.

        And you could actually eliminate the protected method altogether, and use that line directly, but instanciating, testing the instance (the file might not exist) and calling the method might still be cleaner since we don't have exceptions.

        Show
        Bertrand Dunogier added a comment - - edited In reply to comment #052039 protected function getClusteredContents( $realPath ) { $file = eZClusterFileHandler::instance( $realPath ); $file->fetch(); $content = file_get_contents( $realPath ); $file->deleteLocal(); return $content; } fetch + file_get_contents are actually not necessary. You can use this: protected function getClusteredContents( $realPath ) { return eZClusterFileHandler::instance( $realPath )->fetchContents(); } The side benefit is that this method doesn't create a local file copy, and eleminates the risk of conflicts if multiple users are downloading the same large file at the same time. And you could actually eliminate the protected method altogether, and use that line directly, but instanciating, testing the instance (the file might not exist) and calling the method might still be cleaner since we don't have exceptions.
        Hide
        André R added a comment -

        In reply to comment #052040
        > And you could actually eliminate the protected method altogether, and use that line directly, but instanciating, testing the instance (the file might not exist) and calling the method might still be cleaner since we don't have exceptions.

        +1 Do what you think is best, just return the file content to caller as transport classes and header handlers might want to do stuff on it before its given to client.

        Show
        André R added a comment - In reply to comment #052040 > And you could actually eliminate the protected method altogether, and use that line directly, but instanciating, testing the instance (the file might not exist) and calling the method might still be cleaner since we don't have exceptions. +1 Do what you think is best, just return the file content to caller as transport classes and header handlers might want to do stuff on it before its given to client.
        Show
        André R added a comment - New fix: http://github.com/ezsystems/ezpublish/commit/25d081de2b3dee471011050c4eca8685c4d848c2
        Hide
        ezrobot added a comment -

        This issue has been automatically closed due to the lack of activity over a long period of time. It is very likely that it is obsolete, but if you think it is still valid, do not hesitate to reopen it and mention why.

        Show
        ezrobot added a comment - This issue has been automatically closed due to the lack of activity over a long period of time. It is very likely that it is obsolete, but if you think it is still valid, do not hesitate to reopen it and mention why.

          People

          • Assignee:
            André R
            Reporter:
            (inactive) Gunnstein Lye
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: