changeset 3591:15e5ca7240ab

Load action: remove duplicated code, add comment field, refactor. AttachFile action: refactor. Fix bugs.
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Mon, 12 May 2008 22:06:27 +0200
parents d24d8cbeedcc
children ff60ed56002a ece71db231f1
files MoinMoin/action/AttachFile.py MoinMoin/action/Load.py docs/CHANGES
diffstat 3 files changed, 80 insertions(+), 101 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/action/AttachFile.py	Mon May 12 17:28:13 2008 +0200
+++ b/MoinMoin/action/AttachFile.py	Mon May 12 22:06:27 2008 +0200
@@ -29,6 +29,9 @@
 
 import os, time, zipfile, mimetypes, errno
 
+from MoinMoin import log
+logging = log.getLogger(__name__)
+
 from MoinMoin import config, wikiutil, packages
 from MoinMoin.Page import Page
 from MoinMoin.util import filesys, timefuncs
@@ -192,8 +195,7 @@
     fpath = os.path.join(attach_dir, target).encode(config.charset)
     exists = os.path.exists(fpath)
     if exists and not overwrite:
-        filesize = 0
-        msg = _("Attachment '%(target)s' already exists.") % {'target': target, } # XXX unused?
+        raise AttachmentAlreadyExists
     else:
         if exists:
             try:
@@ -543,6 +545,17 @@
     request.theme.send_closing_html()
 
 
+def preprocess_filename(filename):
+    """ preprocess the filename we got from upload form,
+        strip leading drive and path (IE misbehaviour)
+    """
+    if filename and len(filename) > 1 and (filename[1] == ':' or filename[0] == '\\'): # C:.... or \path... or \\server\...
+        bsindex = filename.rfind('\\')
+        if bsindex >= 0:
+            filename = filename[bsindex+1:]
+    return filename
+
+
 def _do_upload(pagename, request):
     _ = request.getText
     # Currently we only check TextCha for upload (this is what spammers ususally do),
@@ -550,7 +563,8 @@
     if not TextCha(request).check_answer_from_form():
         return _('TextCha: Wrong answer! Go back and try again...')
 
-    overwrite = request.form.get('overwrite', ['0'])[0]
+    form = request.form
+    overwrite = form.get('overwrite', [u'0'])[0]
     try:
         overwrite = int(overwrite)
     except:
@@ -560,42 +574,29 @@
        (not overwrite or not request.user.may.write(pagename) or not request.user.may.delete(pagename)):
         return _('You are not allowed to attach a file to this page.')
 
-    if 'file' not in request.form:
+    filename = form.get('file__filename__')
+    rename = form.get('rename', [u''])[0].strip()
+    if rename:
+        target = rename
+    else:
+        target = filename
+
+    target = preprocess_filename(target)
+    target = wikiutil.clean_input(target)
+
+    if not target:
+        return _("Filename of attachment not specified!")
+
+    # get file content
+    filecontent = request.form.get('file', [None])[0]
+    if filecontent is None:
         # This might happen when trying to upload file names
         # with non-ascii characters on Safari.
         return _("No file content. Delete non ASCII characters from the file name and try again.")
 
-    # make filename
-    filename = request.form.get('file__filename__')
-    rename = request.form.get('rename', [None])[0]
-    if rename:
-        rename = rename.strip()
-
-    # if we use twisted, "rename" field is NOT optional, because we
-    # can't access the client filename
-    if rename:
-        target = rename
-        # clear rename its only once wanted
-        request.form['rename'][0] = u''
-    elif filename:
-        target = filename
-    else:
-        return _("Filename of attachment not specified!")
-
-    # get file content
-    filecontent = request.form['file'][0]
-
-    # preprocess the filename
-    # strip leading drive and path (IE misbehaviour)
-    if len(target) > 1 and (target[1] == ':' or target[0] == '\\'): # C:.... or \path... or \\server\...
-        bsindex = target.rfind('\\')
-        if bsindex >= 0:
-            target = target[bsindex+1:]
-
     # add the attachment
     try:
         target, bytes = add_attachment(request, pagename, target, filecontent, overwrite=overwrite)
-
         msg = _("Attachment '%(target)s' (remote name '%(filename)s')"
                 " with %(bytes)d bytes saved.") % {
                 'target': target, 'filename': filename, 'bytes': bytes}
--- a/MoinMoin/action/Load.py	Mon May 12 17:28:13 2008 +0200
+++ b/MoinMoin/action/Load.py	Mon May 12 22:06:27 2008 +0200
@@ -1,11 +1,14 @@
 # -*- coding: iso-8859-1 -*-
 """
-    MoinMoin - Action macro for page creation from file or attach file to current page
+    MoinMoin - Action to load page content from a file upload
 
-    @copyright: 2007-2008 MoinMoin:ReimarBauer
+    @copyright: 2007-2008 MoinMoin:ReimarBauer,
+                2008 MoinMoin:ThomasWaldmann
     @license: GNU GPL, see COPYING for details.
 """
+
 import os
+
 from MoinMoin import wikiutil, config
 from MoinMoin.action import ActionBase, AttachFile
 from MoinMoin.PageEditor import PageEditor
@@ -31,73 +34,38 @@
         status = False
         _ = self._
         form = self.form
-        author = self.request.user.name
-
-        rename = form.get('rename', [u''])[0]
-
-        filename = None
-        if 'file__filename__' in form:
-            filename = form['file__filename__']
-
-        filecontent = form['file'][0]
-
-        overwrite = False
-        if 'overwrite' in form:
-            overwrite = True
-
-        target = filename
-        if rename:
-            target = wikiutil.clean_input(rename.strip())
-
-        # preprocess the filename
-        # strip leading drive and path (IE misbehaviour)
-        if len(target) > 1 and (target[1] == ':' or target[0] == '\\'): # C:.... or \path... or \\server\...
-            bsindex = target.rfind('\\')
-            if bsindex >= 0:
-                target = target[bsindex+1:]
+        request = self.request
 
-        if 'attachment' in self.request.form and self.request.user.may.write(self.pagename):
-            attach_dir = AttachFile.getAttachDir(self.request, self.pagename, create=1)
-            fpath = os.path.join(attach_dir, target).encode(config.charset)
-            exists = os.path.exists(fpath)
-            if exists and not overwrite:
-                msg = _("Attachment '%(target)s' (remote name '%(filename)s') already exists.") % {
-                         'target': target, 'filename': filename}
-                return status, msg
+        comment = form.get('comment', [u''])[0]
+        comment = wikiutil.clean_input(comment)
 
-            if exists and overwrite:
-                if self.request.user.may.delete(self.pagename):
-                    try:
-                        os.remove(fpath)
-                    except:
-                        pass
-                else:
-                    msg = _("You are not allowed to delete attachments on this page.")
-                    return status, msg
+        filename = form.get('file__filename__')
+        rename = form.get('rename', [''])[0].strip()
+        if rename:
+            target = rename
+        else:
+            target = filename
 
-            target, bytes = AttachFile.add_attachment(self.request, self.pagename, target, filecontent)
-            msg = _("Attachment '%(target)s' (remote name '%(filename)s') with %(bytes)d bytes saved.") % {
-                   'target': target, 'filename': filename, 'bytes': bytes}
-            status = True
+        target = AttachFile.preprocess_filename(target)
+        target = wikiutil.clean_input(target)
 
-        else:
-            if isinstance(filecontent, file):
+        if target:
+            filecontent = form['file'][0]
+            if hasattr(filecontent, 'read'): # a file-like object
                 filecontent = filecontent.read() # XXX reads complete file into memory!
-            if isinstance(filecontent, str):
-                filecontent = unicode(filecontent, config.charset)
+            filecontent = wikiutil.decodeUnknownInput(filecontent)
+
             self.pagename = target
-            page = Page(self.request, self.pagename)
-            pagedir = page.getPagePath("", check_create=0)
-            rev = Page.get_current_from_pagedir(page, pagedir)
-            pg = PageEditor(self.request, self.pagename, do_editor_backup=0, uid_override=author)
+            pg = PageEditor(request, self.pagename)
             try:
-                msg = pg.saveText(filecontent, rev)
+                msg = pg.saveText(filecontent, 0, comment=comment)
                 status = True
             except pg.EditConflict, e:
                 msg = e.message
             except pg.SaveError, msg:
                 msg = unicode(msg)
-
+        else:
+            msg = _("Pagename not specified!")
         return status, msg
 
     def do_action_finish(self, success):
@@ -111,15 +79,15 @@
     def get_form_html(self, buttons_html):
         _ = self._
         return """
-%(querytext_pages)s
+<h2>%(headline)s</h2>
+<p>%(explanation)s</p>
 <dl>
 <dt>%(upload_label_file)s</dt>
 <dd><input type="file" name="file" size="50" value=""></dd>
 <dt>%(upload_label_rename)s</dt>
-<dd><input type="text" name="rename" size="50" value=""></dd>
-%(querytext_attachment)s
-<dt><input type="checkbox" name="attachment" value="off"> %(upload)s</dt>
-<dt><input type="checkbox" name="overwrite" value="off"> %(overwrite)s</dt>
+<dd><input type="text" name="rename" size="50" value="%(pagename)s"></dd>
+<dt>%(upload_label_comment)s</dt>
+<dd><input type="text" name="comment" size="80" maxlength="200"></dd>
 </dl>
 <p>
 <input type="hidden" name="action" value="%(action_name)s">
@@ -128,15 +96,16 @@
 <td class="buttons">
 %(buttons_html)s
 </td>""" % {
-    'querytext_pages': '<h2>' + _("New Page or New Attachment") + '</h2><p>' +
-_("""You can upload a file to a new page or choose to upload a file as attachment for the current page""") + '</p>',
-    'querytext_attachment': '<h2>' + _("New Attachment") + '</h2>',
+    'headline': _("Upload page content"),
+    'explanation': _("You can upload content for the page named below. "
+                     "If you change the page name, you can also upload content for another page. "
+                     "If the page name is empty, we derive the page name from the file name."),
+    'upload_label_file': _('File to load page content from'),
+    'upload_label_comment': _('Comment'),
+    'upload_label_rename': _('Page Name'),
+    'pagename': self.pagename,
     'buttons_html': buttons_html,
-    'upload': _('attachment'),
-    'overwrite': _('Overwrite existing attachment of same name'),
     'action_name': self.form_trigger,
-    'upload_label_file': _('Upload'),
-    'upload_label_rename': _('New Name'),
 }
 
 def execute(pagename, request):
--- a/docs/CHANGES	Mon May 12 17:28:13 2008 +0200
+++ b/docs/CHANGES	Mon May 12 22:06:27 2008 +0200
@@ -81,6 +81,15 @@
       * wikiserverlogging.conf - to configure logging for it (default config
         should be ok for all day use, but can easily be modified for debugging)
       * wikiconfig.py - to configure the wiki engine
+    * Duplicated file attachment upload code was removed from Load action (just
+      use AttachFile action to deal with attachments).
+    * Load action now just creates a new revision of the target page, the
+      target pagename defaults to the current page name and can be edited. If
+      the target pagename is empty, moin tries to derive the target pagename
+      from the uploaded file's name.
+      Load tries to decode the file contents first using utf-8 coding and, if
+      that fails, it forces decoding using iso-8859-1 coding (and replacing
+      invalid characters).
 
   Developer notes (these should be moved to the end in the release):
     * Page.last_edit() is DEPRECATED, please use Page.edit_info().