zorba-coders team mailing list archive

["Paul J. Lucas" <paul@xxxxxxxxxxxxx>]

Paul J. Lucas has proposed merging lp:~paul-lucas/zorba/bug-1064289 into lp:zorba.

Commit message:
Fixed bug 1064289.
Added "bool follow_symlinks = true" to most File member functions (they should have had it all along).
Added ExternalFunction::getItem() convenience function.

Requested reviews:
  Paul J. Lucas (paul-lucas)
Related bugs:
  Bug #1064289 in Zorba: "file module cannot delete broken symbolic links"
  https://bugs.launchpad.net/zorba/+bug/1064289

For more details, see:
https://code.launchpad.net/~paul-lucas/zorba/bug-1064289/+merge/159904

Fixed bug 1064289.
Added "bool follow_symlinks = true" to most File member functions (they should have had it all along).
Added ExternalFunction::getItem() convenience function.
-- 
https://code.launchpad.net/~paul-lucas/zorba/bug-1064289/+merge/159904
Your team Zorba Coders is subscribed to branch lp:zorba.

=== modified file 'ChangeLog'
--- ChangeLog	2013-04-19 04:07:25 +0000
+++ ChangeLog	2013-04-19 22:28:26 +0000
@@ -72,6 +72,7 @@
   * Fixed bug #1023168 (Non-single-char-escapes in regex character ranges not
     caught)
   * Throw XQST0048 for groupby clause, as specified by XQuery 3.0.
+  * Fixed bug #1064289 (file module cannot delete broken symbolic links)
   * Fixed bug in raising XQTY0086 (copying of namespace-sensitive values during
     node construction).
   * Fixed bug #866874 (regex "range subtraction" not supported for ICU)

=== modified file 'include/zorba/file.h'
--- include/zorba/file.h	2013-02-07 17:24:36 +0000
+++ include/zorba/file.h	2013-04-19 22:28:26 +0000
@@ -70,12 +70,12 @@
       virtual const std::string getFilePath() const = 0;
       virtual const std::string getFileUri() const = 0;
 
-      virtual bool isDirectory() const = 0;
-      virtual bool isFile() const = 0;
+      virtual bool isDirectory( bool follow_symlinks = true ) const = 0;
+      virtual bool isFile( bool follow_symlinks = true ) const = 0;
       virtual bool isLink() const = 0;
-      virtual bool isVolume() const = 0;
-      virtual bool isInvalid() const = 0;
-      virtual bool exists() const = 0;
+      virtual bool isVolume( bool follow_symlinks = true ) const = 0;
+      virtual bool isInvalid() const = 0; // deprecated
+      virtual bool exists( bool follow_symlinks = true ) const = 0;
 
       virtual void remove() = 0;
       virtual bool create() = 0;

=== modified file 'include/zorba/function.h'
--- include/zorba/function.h	2013-02-07 17:24:36 +0000
+++ include/zorba/function.h	2013-04-19 22:28:26 +0000
@@ -170,6 +170,10 @@
    */
   virtual bool
   isContextual() const = 0;
+
+protected:
+  Item
+  getItem( Arguments_t const &args, unsigned pos ) const;
 };
 
 

=== modified file 'include/zorba/util/file.h'
--- include/zorba/util/file.h	2013-03-06 02:54:03 +0000
+++ include/zorba/util/file.h	2013-04-19 22:28:26 +0000
@@ -1,12 +1,12 @@
 /*
  * Copyright 2006-2008 The FLWOR Foundation.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -57,15 +57,34 @@
 public: // common methods
   void set_path(std::string const& _path ) { *((filesystem_path *) this) = _path; }
   void set_filetype(filetype)   { /* do nothing */ }  // deprecated
-  filetype get_filetype() const { return do_stat(); }
-
-  bool is_directory() const     { return do_stat() == type_directory; }  
-  bool is_file() const          { return do_stat() == type_file; }  
-  bool is_link() const          { return do_stat( false ) == type_link; }  
-  bool is_volume() const        { return do_stat() == type_volume; }  
-
-  bool is_invalid() const       { return false; }     // deprecated
-  bool exists() const           { return do_stat() != type_non_existent; }
+
+  filetype get_filetype( bool follow_symlinks = true ) const {
+    return do_stat( follow_symlinks );
+  }
+
+  bool is_directory( bool follow_symlinks = true ) const {
+    return do_stat( follow_symlinks ) == type_directory;
+  }
+
+  bool is_file( bool follow_symlinks = true ) const {
+    return do_stat( follow_symlinks ) == type_file;
+  }
+
+  bool is_link() const {
+    return do_stat( false ) == type_link;
+  }
+
+  bool is_volume( bool follow_symlinks = true ) const {
+    return do_stat( follow_symlinks ) == type_volume;
+  }
+
+  bool is_invalid() const {             // deprecated
+    return false;
+  }
+
+  bool exists( bool follow_symlinks = true ) const {
+    return do_stat( follow_symlinks ) != type_non_existent;
+  }
 
   time_t lastModified() const;
 

=== modified file 'modules/org/expath/ns/file.xq'
--- modules/org/expath/ns/file.xq	2013-03-06 04:04:43 +0000
+++ modules/org/expath/ns/file.xq	2013-04-19 22:28:26 +0000
@@ -227,7 +227,7 @@
   $path as xs:string
 ) as empty-sequence()
 {
-  if (file:exists($path)) then
+  if (file:exists($path,false())) then
     if (not(file:is-symlink($path)) and file:is-directory($path)) then
       file:delete-directory-impl($path)
     else
@@ -276,13 +276,26 @@
  : Tests if a path/URI is already used in the file system.
  :
  : @param $path The path/URI to test.
+ : @param $follow-symlinks if <code>true</code>, follows symbolic links.
+ : @return true if <code>$path</code> points to an existing file system item.
+ :)
+declare %an:nondeterministic function file:exists(
+  $path as xs:string, $follow-symlinks as xs:boolean
+) as xs:boolean external;
+
+(:~
+ : Tests if a path/URI is already used in the file system.
+ :
+ : @param $path The path/URI to test.
  : @return true if <code>$path</code> points to an existing file system item;
- : for symbolic links, retuns true if the symbolic link itself exists and not
- : whether the linked-to item exists.
+ : for symbolic links, retuns true if the linked-to item exists.
  :)
 declare %an:nondeterministic function file:exists(
   $path as xs:string
-) as xs:boolean external;
+) as xs:boolean
+{
+  file:exists($path,true())
+};
 
 (:~
  : Tests if a path/URI points to a directory. On Unix-based systems, the root

=== modified file 'modules/org/expath/ns/file.xq.src/file.cpp'
--- modules/org/expath/ns/file.xq.src/file.cpp	2013-03-13 16:17:38 +0000
+++ modules/org/expath/ns/file.xq.src/file.cpp	2013-04-19 22:28:26 +0000
@@ -60,7 +60,7 @@
   // actual mkdir
   try {
     lFile->mkdir(true);
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not create directory";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());
@@ -94,14 +94,14 @@
   File_t lFile = File::createFile(lFileStr.c_str());
 
   // precondition
-  if (!lFile->exists()) {
+  if (!lFile->exists( false )) {
     raiseFileError("FOFL0001", "A file or directory does not exist at this path", lFile->getFilePath());
   }
 
   // actual remove
   try {
     lFile->remove();
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not delete file";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());
@@ -111,7 +111,7 @@
   }
 
   // postcondition
-  if (lFile->exists()) {
+  if (lFile->exists( false )) {
     raiseFileError("FOFL9999", "The file at this path could not be deleted", lFile->getFilePath());
   }
 
@@ -153,7 +153,7 @@
         *lInStream.release(), &FileModule::streamReleaser, true
       );
 
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not read file";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());
@@ -353,13 +353,11 @@
   const StaticContext*                          aSctxCtx,
   const DynamicContext*                         aDynCtx) const
 {
-  bool   lFileExists = false;
-  String lFileStr = getFilePathString(aArgs, 0);
+  String const lFileStr = getFilePathString(aArgs, 0);
+  bool const lFollowSymlinks = getItem(aArgs, 1).getBooleanValue();
 
   File_t lFile = File::createFile(lFileStr.c_str());
-  if (lFile->exists()) {
-    lFileExists = true;
-  }
+  bool const lFileExists = lFile->exists(lFollowSymlinks);
 
   return ItemSequence_t(new SingletonItemSequence(
       theModule->getItemFactory()->createBoolean(lFileExists)));
@@ -504,7 +502,7 @@
     lInStream.close();
     lOutStream.close();
 
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not copy file";
     raiseFileError("FOFL9999", lSs.str(), lSrcFile->getFilePath());
@@ -541,7 +539,7 @@
   try {
     DirectoryIterator_t lIter = lFile->files();
     return ItemSequence_t(new IteratorBackedItemSequence(lIter, theModule->getItemFactory()));
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not list directory";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());
@@ -641,7 +639,7 @@
                                                   lT.tm_min, 
                                                   lT.tm_sec,
                                                   gmtOffset)));
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not retrieve the last modification timestamp of";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());
@@ -696,7 +694,7 @@
   File::FileSize_t lFs = -1;
   try {
     lFs = lFile->getSize();
-  } catch (ZorbaException& ze) {
+  } catch (ZorbaException const& ze) {
     std::stringstream lSs;
     lSs << "An unknown error occured: " << ze.what() << "Can not retrieve the file size of";
     raiseFileError("FOFL9999", lSs.str(), lFile->getFilePath());

=== modified file 'src/api/fileimpl.cpp'
--- src/api/fileimpl.cpp	2013-02-07 17:24:36 +0000
+++ src/api/fileimpl.cpp	2013-04-19 22:28:26 +0000
@@ -139,24 +139,26 @@
 }
 
 bool
-FileImpl::isDirectory() const
+FileImpl::isDirectory( bool follow_symlinks ) const
 {
   bool lResult = false;
 
   ZORBA_TRY
-    lResult = theInternalFile->is_directory() || theInternalFile->is_volume() || theInternalFile->is_root();
+    lResult = theInternalFile->is_directory( follow_symlinks )
+           || theInternalFile->is_volume( follow_symlinks )
+           || theInternalFile->is_root();
   ZORBA_CATCH
 
   return lResult;
 }
 
 bool
-FileImpl::isFile() const
+FileImpl::isFile( bool follow_symlinks ) const
 {
   bool lResult = false;
 
   ZORBA_TRY
-    lResult = theInternalFile->is_file() || theInternalFile->is_link();
+    lResult = theInternalFile->is_file( follow_symlinks ) || theInternalFile->is_link();
   ZORBA_CATCH
 
   return lResult;
@@ -175,12 +177,12 @@
 }
 
 bool
-FileImpl::isVolume() const
+FileImpl::isVolume( bool follow_symlinks ) const
 {
   bool lResult = false;
 
   ZORBA_TRY
-    lResult = theInternalFile->is_volume();
+    lResult = theInternalFile->is_volume( follow_symlinks );
   ZORBA_CATCH
 
   return lResult;
@@ -199,12 +201,12 @@
 }
 
 bool
-FileImpl::exists() const
+FileImpl::exists( bool follow_symlinks ) const
 {
   bool lResult = false;
 
   ZORBA_TRY
-    lResult = theInternalFile->exists();
+    lResult = theInternalFile->exists( follow_symlinks );
   ZORBA_CATCH
 
   return lResult;

=== modified file 'src/api/fileimpl.h'
--- src/api/fileimpl.h	2013-02-07 17:24:36 +0000
+++ src/api/fileimpl.h	2013-04-19 22:28:26 +0000
@@ -63,12 +63,12 @@
   const std::string getFilePath() const;
   const std::string getFileUri() const;
 
-  bool isDirectory() const;
-  bool isFile() const;
+  bool isDirectory( bool follow_symlinks = true ) const;
+  bool isFile( bool follow_symlinks = true ) const;
   bool isLink() const;
-  bool isVolume() const;
-  bool isInvalid() const;
-  bool exists() const;
+  bool isVolume( bool follow_symlinks = true ) const;
+  bool isInvalid() const;               // deprecated
+  bool exists( bool follow_symlinks = true ) const;
 
   void remove();
   bool create();

=== modified file 'src/api/functionimpl.cpp'
--- src/api/functionimpl.cpp	2013-02-07 17:24:36 +0000
+++ src/api/functionimpl.cpp	2013-04-19 22:28:26 +0000
@@ -25,8 +25,9 @@
 #include "annotations/annotations.h"
 #include "api/annotationimpl.h"
 
-namespace zorba 
-{
+namespace zorba {
+
+///////////////////////////////////////////////////////////////////////////////
 
 bool FunctionImpl::isUpdating() const
 {
@@ -143,6 +144,18 @@
   return static_cast<function*>(theFunction)->isBuiltin();
 }
 
-
-} /* namespace zorba */
+///////////////////////////////////////////////////////////////////////////////
+
+Item ExternalFunction::getItem( Arguments_t const &args, unsigned pos ) const {
+  Iterator_t i = args[ pos ]->getIterator();
+  Item item;
+  i->open();
+  i->next( item );
+  i->close();
+  return item;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
+} // namespace zorba
 /* vim:set et sw=2 ts=2: */