Uploaded image for project: 'XWiki Commons'
  1. XWiki Commons
  2. XCOMMONS-3750

LocalExtensionStorage#removeExtension() never deletes the extension version folder

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • Major
    • 18.8.0-rc-1
    • 16.10.0
    • Extension
    • None
    • Unit
    • Unknown
    • N/A
    • N/A

    Description

      Problem

      Removing a local extension (uninstalling it, or cleaning it with the Extension Tweak application) leaves its folder behind forever in the local extension repository (<permdir>/extension/repository/ by default), and logs an error:

      ERROR o.x.e.r.i.l.DefaultLocalExtensionRepository - Failed to remove extension [...]
      java.nio.file.NoSuchFileException: .../repository/<id>/<version>/<id>-<version>.xed
      

      The extension is still correctly removed from memory, so it does disappear from the UI, but the <id>/<version>/ folder is never cleaned up on disk.

      Cause

      In LocalExtensionStorage#removeExtension(), the block meant to delete the extension version folder deletes the descriptor file a second time instead:

      // Get the path to the folder that store the version of the extension being removed
      Path extensionVersionFolderPath = extensionDescriptorFilePath.getParent();
      try {
          // Delete the extension version folder
          Files.delete(extensionDescriptorFilePath);
      
          // Try to delete the extension folder
          deleteExtensionFolderIfEmpty(extensionVersionFolderPath.getParent());
      } catch (DirectoryNotEmptyException e) {
          ...
      }
      

      The descriptor file has already been deleted a few lines above, so this throws NoSuchFileException, which is not caught (only DirectoryNotEmptyException is). As a result:

      • the version folder is never deleted;
      • deleteExtensionFolderIfEmpty() is never reached, so the extension folder is never deleted either – which means XCOMMONS-3197 never actually worked;
      • the exception propagates to DefaultLocalExtensionRepository#removeExtension(), which logs it as an error.

      Two related problems in the same method:

      • the two catch (FileNotFoundException e) blocks around Files.delete() are dead code – Files.delete() throws NoSuchFileException (a FileSystemException), never FileNotFoundException – so those warnings can never be logged;
      • the DirectoryNotEmptyException warning logs the descriptor file path instead of the folder path it is about.

      Note

      Just fixing the target of the delete is not enough. An extension descriptor is also supported directly at the root of the local repository (the repository is loaded by scanning it recursively), in which case the "version folder" is the repository root folder itself, and neither it nor its parent must ever be deleted.

      Attachments

        Activity

          People

            vmassol Vincent Massol
            vmassol Vincent Massol
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: