diff --git a/trac/admin/tests/console-tests.txt b/trac/admin/tests/console-tests.txt --- a/trac/admin/tests/console-tests.txt +++ b/trac/admin/tests/console-tests.txt @@ -120,7 +120,7 @@ TICKET_EDIT_CC, TICKET_EDIT_COMMENT, TICKET_EDIT_DESCRIPTION, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, TRAC_ADMIN, VERSIONCONTROL_ADMIN, WIKI_ADMIN, WIKI_CREATE, WIKI_DELETE, WIKI_MODIFY, - WIKI_VIEW + WIKI_RENAME, WIKI_VIEW ===== test_permission_add_one_action_ok ===== @@ -155,7 +155,7 @@ TICKET_EDIT_CC, TICKET_EDIT_COMMENT, TICKET_EDIT_DESCRIPTION, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, TRAC_ADMIN, VERSIONCONTROL_ADMIN, WIKI_ADMIN, WIKI_CREATE, WIKI_DELETE, WIKI_MODIFY, - WIKI_VIEW + WIKI_RENAME, WIKI_VIEW ===== test_permission_add_multiple_actions_ok ===== @@ -191,7 +191,7 @@ TICKET_EDIT_CC, TICKET_EDIT_COMMENT, TICKET_EDIT_DESCRIPTION, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, TRAC_ADMIN, VERSIONCONTROL_ADMIN, WIKI_ADMIN, WIKI_CREATE, WIKI_DELETE, WIKI_MODIFY, - WIKI_VIEW + WIKI_RENAME, WIKI_VIEW ===== test_permission_remove_one_action_ok ===== @@ -225,7 +225,7 @@ TICKET_EDIT_CC, TICKET_EDIT_COMMENT, TICKET_EDIT_DESCRIPTION, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, TRAC_ADMIN, VERSIONCONTROL_ADMIN, WIKI_ADMIN, WIKI_CREATE, WIKI_DELETE, WIKI_MODIFY, - WIKI_VIEW + WIKI_RENAME, WIKI_VIEW ===== test_permission_remove_multiple_actions_ok ===== @@ -259,7 +259,7 @@ TICKET_EDIT_CC, TICKET_EDIT_COMMENT, TICKET_EDIT_DESCRIPTION, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, TRAC_ADMIN, VERSIONCONTROL_ADMIN, WIKI_ADMIN, WIKI_CREATE, WIKI_DELETE, WIKI_MODIFY, - WIKI_VIEW + WIKI_RENAME, WIKI_VIEW ===== test_component_list_ok ===== diff --git a/trac/attachment.py b/trac/attachment.py --- a/trac/attachment.py +++ b/trac/attachment.py @@ -63,6 +63,9 @@ def attachment_deleted(attachment): """Called when an attachment is deleted.""" + def attachment_reparented(attachment, old_parent_realm, old_parent_id): + """Called when an attachment is reparented.""" + class IAttachmentManipulator(Interface): """Extension point interface for components that need to manipulate @@ -155,18 +158,20 @@ self.author = row[4] self.ipnr = row[5] - def _get_path(self): - path = os.path.join(self.env.path, 'attachments', self.parent_realm, - unicode_quote(self.parent_id)) - if self.filename: - path = os.path.join(path, unicode_quote(self.filename)) + def _get_path(self, parent_realm, parent_id, filename): + path = os.path.join(self.env.path, 'attachments', parent_realm, + unicode_quote(parent_id)) + if filename: + path = os.path.join(path, unicode_quote(filename)) return os.path.normpath(path) - path = property(_get_path) + + @property + def path(self): + return self._get_path(self.parent_realm, self.parent_id, self.filename) - def _get_title(self): - return '%s:%s: %s' % (self.parent_realm, - self.parent_id, self.filename) - title = property(_get_title) + @property + def title(self): + return '%s:%s: %s' % (self.parent_realm, self.parent_id, self.filename) def delete(self, db=None): assert self.filename, 'Cannot delete non-existent attachment' @@ -192,6 +197,47 @@ for listener in AttachmentModule(self.env).change_listeners: listener.attachment_deleted(self) + def reparent(self, new_realm, new_id, db=None): + assert self.filename, 'Cannot reparent non-existent attachment' + new_id = unicode(new_id) + + @with_transaction(self.env, db) + def do_reparent(db): + cursor = db.cursor() + new_path = self._get_path(new_realm, new_id, self.filename) + if os.path.exists(new_path): + raise TracError(_('Cannot reparent attachment "%(att)s" as ' + 'it already exists in %(realm)s:%(id)s', + att=self.filename, realm=new_realm, + id=new_id)) + cursor.execute(""" + UPDATE attachment SET type=%s, id=%s + WHERE type=%s AND id=%s AND filename=%s + """, (new_realm, new_id, self.parent_realm, self.parent_id, + self.filename)) + dirname = os.path.dirname(new_path) + if not os.path.exists(dirname): + os.makedirs(dirname) + if os.path.isfile(self.path): + try: + os.rename(self.path, new_path) + except OSError, e: + self.env.log.error('Failed to move attachment file %s: %s', + self.path, + exception_to_unicode(e, traceback=True)) + raise TracError(_('Could not reparent attachment %(name)s', + name=self.filename)) + + old_realm, old_id = self.parent_realm, self.parent_id + self.parent_realm, self.parent_id = new_realm, new_id + self.resource = Resource(new_realm, new_id).child('attachment', + self.filename) + + self.env.log.info('Attachment reparented: %s' % self.title) + + for listener in AttachmentModule(self.env).change_listeners: + if hasattr(listener, 'attachment_reparented'): + listener.attachment_deleted(self, old_realm, old_id) def insert(self, filename, fileobj, size, t=None, db=None): self.size = size and int(size) or 0 @@ -274,6 +320,22 @@ except OSError, e: env.log.error("Can't delete attachment directory %s: %s", attachment_dir, exception_to_unicode(e, traceback=True)) + + @classmethod + def reparent_all(cls, env, parent_realm, parent_id, new_realm, new_id, + db=None): + """Reparent all attachments of a given resource to another resource. + """ + attachment_dir = None + for attachment in list(cls.select(env, parent_realm, parent_id, db)): + attachment_dir = os.path.dirname(attachment.path) + attachment.reparent(new_realm, new_id, db) + if attachment_dir: + try: + os.rmdir(attachment_dir) + except OSError, e: + env.log.error("Can't delete attachment directory %s: %s", + attachment_dir, exception_to_unicode(e, traceback=True)) def open(self): self.env.log.debug('Trying to open attachment at %s', self.path) diff --git a/trac/htdocs/css/wiki.css b/trac/htdocs/css/wiki.css --- a/trac/htdocs/css/wiki.css +++ b/trac/htdocs/css/wiki.css @@ -42,7 +42,7 @@ #changeinfo br { clear: left } #changeinfo .options { padding: 0 0 1em 1em } #changeinfo .options, #changeinfo .buttons { clear: left } -#delete, #save { margin-left: 6em } +#delete, #rename, #save { margin-left: 3em } #preview { background: #f4f4f4 url(../draft.png); margin: 1em 0 2em; diff --git a/trac/tests/attachment.py b/trac/tests/attachment.py --- a/trac/tests/attachment.py +++ b/trac/tests/attachment.py @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- -import os +import os.path import shutil +from StringIO import StringIO import tempfile import unittest @@ -70,9 +71,9 @@ def test_insert(self): attachment = Attachment(self.env, 'ticket', 42) - attachment.insert('foo.txt', tempfile.TemporaryFile(), 0, 1) + attachment.insert('foo.txt', StringIO(''), 0, 1) attachment = Attachment(self.env, 'ticket', 42) - attachment.insert('bar.jpg', tempfile.TemporaryFile(), 0, 2) + attachment.insert('bar.jpg', StringIO(''), 0, 2) attachments = Attachment.select(self.env, 'ticket', 42) self.assertEqual('foo.txt', attachments.next().filename) @@ -81,22 +82,22 @@ def test_insert_unique(self): attachment = Attachment(self.env, 'ticket', 42) - attachment.insert('foo.txt', tempfile.TemporaryFile(), 0) + attachment.insert('foo.txt', StringIO(''), 0) self.assertEqual('foo.txt', attachment.filename) attachment = Attachment(self.env, 'ticket', 42) - attachment.insert('foo.txt', tempfile.TemporaryFile(), 0) + attachment.insert('foo.txt', StringIO(''), 0) self.assertEqual('foo.2.txt', attachment.filename) def test_insert_outside_attachments_dir(self): attachment = Attachment(self.env, '../../../../../sth/private', 42) self.assertRaises(AssertionError, attachment.insert, 'foo.txt', - tempfile.TemporaryFile(), 0) + StringIO(''), 0) def test_delete(self): attachment1 = Attachment(self.env, 'wiki', 'SomePage') - attachment1.insert('foo.txt', tempfile.TemporaryFile(), 0) + attachment1.insert('foo.txt', StringIO(''), 0) attachment2 = Attachment(self.env, 'wiki', 'SomePage') - attachment2.insert('bar.jpg', tempfile.TemporaryFile(), 0) + attachment2.insert('bar.jpg', StringIO(''), 0) attachments = Attachment.select(self.env, 'wiki', 'SomePage') self.assertEqual(2, len(list(attachments))) @@ -116,11 +117,37 @@ doesn't exist for some reason. """ attachment = Attachment(self.env, 'wiki', 'SomePage') - attachment.insert('foo.txt', tempfile.TemporaryFile(), 0) + attachment.insert('foo.txt', StringIO(''), 0) os.unlink(attachment.path) attachment.delete() + def test_reparent(self): + attachment1 = Attachment(self.env, 'wiki', 'SomePage') + attachment1.insert('foo.txt', StringIO(''), 0) + path1 = attachment1.path + attachment2 = Attachment(self.env, 'wiki', 'SomePage') + attachment2.insert('bar.jpg', StringIO(''), 0) + + attachments = Attachment.select(self.env, 'wiki', 'SomePage') + self.assertEqual(2, len(list(attachments))) + attachments = Attachment.select(self.env, 'ticket', 123) + self.assertEqual(0, len(list(attachments))) + assert os.path.exists(path1) and os.path.exists(attachment2.path) + + attachment1.reparent('ticket', 123) + self.assertEqual('ticket', attachment1.parent_realm) + self.assertEqual('ticket', attachment1.resource.parent.realm) + self.assertEqual('123', attachment1.parent_id) + self.assertEqual('123', attachment1.resource.parent.id) + + attachments = Attachment.select(self.env, 'wiki', 'SomePage') + self.assertEqual(1, len(list(attachments))) + attachments = Attachment.select(self.env, 'ticket', 123) + self.assertEqual(1, len(list(attachments))) + assert not os.path.exists(path1) and os.path.exists(attachment1.path) + assert os.path.exists(attachment2.path) + def test_legacy_permission_on_parent(self): """Ensure that legacy action tests are done on parent. As `ATTACHMENT_VIEW` maps to `TICKET_VIEW`, the `TICKET_VIEW` is tested diff --git a/trac/tests/functional/tester.py b/trac/tests/functional/tester.py --- a/trac/tests/functional/tester.py +++ b/trac/tests/functional/tester.py @@ -176,6 +176,7 @@ tc.formvalue('attachment', 'replace', True) tc.submit() tc.url(self.url + '/attachment/ticket/%s/$' % ticketid) + return tempfilename def clone_ticket(self, ticketid): """Create a clone of the given ticket id using the clone button.""" @@ -232,6 +233,7 @@ tc.formvalue('attachment', 'description', random_sentence()) tc.submit() tc.url(self.url + '/attachment/wiki/%s/$' % name) + return tempfilename def create_milestone(self, name=None, due=None): """Creates the specified milestone, with a random name if none is diff --git a/trac/wiki/api.py b/trac/wiki/api.py --- a/trac/wiki/api.py +++ b/trac/wiki/api.py @@ -45,6 +45,9 @@ def wiki_page_version_deleted(page): """Called when a version of a page has been deleted.""" + def wiki_page_renamed(page, old_name): + """Called when a page has been renamed.""" + class IWikiPageManipulator(Interface): """Extension point interface for components that need to do specific diff --git a/trac/wiki/model.py b/trac/wiki/model.py --- a/trac/wiki/model.py +++ b/trac/wiki/model.py @@ -158,6 +158,37 @@ self.old_readonly = self.readonly self.old_text = self.text + def rename(self, new_name, db=None): + """Rename wiki page in-place, keeping the history intact. + Renaming a page this way will eventually leave dangling references + to the old page - which litterally doesn't exist anymore. + """ + assert self.exists, 'Cannot rename non-existent page' + + old_name = self.name + + @with_transaction(self.env, db) + def do_rename(db): + cursor = db.cursor() + new_page = WikiPage(self.env, new_name, db=db) + if new_page.exists: + raise TracError(_("Can't rename to existing %(name)s page.", + name=new_name)) + + cursor.execute("UPDATE wiki SET name=%s WHERE name=%s", + (new_name, old_name)) + WikiSystem(self.env).pages.invalidate(db) + from trac.attachment import Attachment + Attachment.reparent_all(self.env, 'wiki', old_name, + 'wiki', new_name, db) + + self.name = new_name + self.env.log.info('Renamed page %s to %s', old_name, new_name) + + for listener in WikiSystem(self.env).change_listeners: + if hasattr(listener, 'wiki_page_renamed'): + listener.wiki_page_renamed(self, old_name) + def get_history(self, db=None): if not db: db = self.env.get_db_cnx() diff --git a/trac/wiki/templates/wiki_rename.html b/trac/wiki/templates/wiki_rename.html new file mode 100644 --- /dev/null +++ b/trac/wiki/templates/wiki_rename.html @@ -0,0 +1,35 @@ + + + + + $title + + + +
+

Rename $page.name

+
+

+ + Renaming the page will rename all existing versions of the page in place.
+ The complete history of the page will be moved to the new location. +

+
+ +
+
+ +
+
+ + +
+
+
+ + diff --git a/trac/wiki/templates/wiki_view.html b/trac/wiki/templates/wiki_view.html --- a/trac/wiki/templates/wiki_view.html +++ b/trac/wiki/templates/wiki_view.html @@ -71,7 +71,8 @@ + admin_perm = 'WIKI_ADMIN' in perm(page.resource); + rename_perm = 'WIKI_RENAME' in perm(page.resource)">
@@ -101,6 +102,12 @@ +
+
+ + +
+
diff --git a/trac/wiki/tests/functional.py b/trac/wiki/tests/functional.py --- a/trac/wiki/tests/functional.py +++ b/trac/wiki/tests/functional.py @@ -12,6 +12,64 @@ self._tester.attach_file_to_wiki(pagename) +class TestWikiRename(FunctionalTwillTestCaseSetup): + def runTest(self): + """Test for simple wiki rename""" + pagename = random_unique_camel() + self._tester.create_wiki_page(pagename) + attachment = self._tester.attach_file_to_wiki(pagename) + base_url = self._tester.url + page_url = base_url + "/wiki/" + pagename + + def click_rename(): + tc.formvalue('rename', 'action', 'rename') + tc.submit() + tc.url(page_url + r'\?action=rename') + tc.find("New name:") + + tc.go(page_url) + tc.find("Rename page") + click_rename() + # attempt to rename the page to the current page name + tc.formvalue('rename', 'new_name', pagename) + tc.submit('submit') + tc.url(page_url) + tc.find("New name must be different from old name") + # attempt to rename the page to an existing page name + tc.formvalue('rename', 'new_name', 'WikiStart') + tc.submit('submit') + tc.url(page_url) + tc.find("Trac Error") + tc.find("Can't rename to existing WikiStart page") + # correct rename to new page name (old page replaced by a redirection) + tc.go(page_url) + click_rename() + newpagename = pagename + 'Renamed' + tc.formvalue('rename', 'new_name', newpagename) + tc.formvalue('rename', 'redirect', True) + tc.submit('submit') + # check redirection page + tc.url(page_url) + tc.find("See.*/wiki/" + newpagename) + # check whether attachment exists on the new page but not on old page + tc.go(base_url + '/attachment/wiki/' + newpagename + '/' + attachment) + tc.notfind("Error: Invalid Attachment") + tc.go(base_url + '/attachment/wiki/' + pagename + '/' + attachment) + tc.find("Error: Invalid Attachment") + # rename again to another new page name (this time, no redirection) + tc.go(page_url) + click_rename() + newpagename = pagename + 'RenamedAgain' + tc.formvalue('rename', 'new_name', newpagename) + tc.formvalue('rename', 'redirect', False) + tc.submit('submit') + tc.url(base_url + "/wiki/" + newpagename) + # this time, the original page is gone + tc.go(page_url) + tc.url(page_url) + tc.find("The page %s does not exist" % pagename) + + class RegressionTestTicket4812(FunctionalTwillTestCaseSetup): def runTest(self): """Test for regression of http://trac.edgewall.org/ticket/4812""" @@ -65,6 +123,7 @@ import trac.tests.functional.testcases suite = trac.tests.functional.testcases.functionalSuite() suite.addTest(TestWiki()) + suite.addTest(TestWikiRename()) suite.addTest(RegressionTestTicket4812()) if has_docutils: import docutils diff --git a/trac/wiki/tests/model.py b/trac/wiki/tests/model.py --- a/trac/wiki/tests/model.py +++ b/trac/wiki/tests/model.py @@ -1,6 +1,13 @@ +# -*- coding: utf-8 -*- + from datetime import datetime +import os.path +import shutil +from StringIO import StringIO +import tempfile import unittest +from trac.attachment import Attachment from trac.core import * from trac.test import EnvironmentStub from trac.util.datefmt import utc, to_utimestamp @@ -14,6 +21,7 @@ self.changed = [] self.deleted = [] self.deleted_version = [] + self.renamed = [] def wiki_page_added(self, page): self.added.append(page) @@ -27,14 +35,20 @@ def wiki_page_version_deleted(self, page): self.deleted_version.append(page) + def wiki_page_renamed(self, page, old_name): + self.renamed.append((page, old_name)) + class WikiPageTestCase(unittest.TestCase): def setUp(self): self.env = EnvironmentStub() + self.env.path = os.path.join(tempfile.gettempdir(), 'trac-tempenv') + os.mkdir(self.env.path) self.db = self.env.get_db_cnx() def tearDown(self): + shutil.rmtree(self.env.path) self.env.reset_db() def test_new_page(self): @@ -194,6 +208,38 @@ listener = TestWikiChangeListener(self.env) self.assertEqual(page, listener.deleted[0]) + def test_rename_page(self): + cursor = self.db.cursor() + data = (1, 42, 'joe', '::1', 'Bla bla', 'Testing', 0) + cursor.execute("INSERT INTO wiki VALUES(%s,%s,%s,%s,%s,%s,%s,%s)", + ('TestPage',) + data) + attachment = Attachment(self.env, 'wiki', 'TestPage') + attachment.insert('foo.txt', StringIO(), 0, 1) + + page = WikiPage(self.env, 'TestPage') + page.rename('PageRenamed') + self.assertEqual('PageRenamed', page.name) + + cursor.execute("SELECT version,time,author,ipnr,text,comment," + "readonly FROM wiki WHERE name=%s", ('PageRenamed',)) + self.assertEqual(data, cursor.fetchone()) + self.assertEqual(None, cursor.fetchone()) + + attachments = Attachment.select(self.env, 'wiki', 'PageRenamed') + self.assertEqual('foo.txt', attachments.next().filename) + self.assertRaises(StopIteration, attachments.next) + Attachment.delete_all(self.env, 'wiki', 'PageRenamed', self.db) + + old_page = WikiPage(self.env, 'TestPage') + self.assertEqual(False, old_page.exists) + + cursor.execute("SELECT version,time,author,ipnr,text,comment," + "readonly FROM wiki WHERE name=%s", ('TestPage',)) + self.assertEqual(None, cursor.fetchone()) + + listener = TestWikiChangeListener(self.env) + self.assertEqual((page, 'TestPage'), listener.renamed[0]) + def suite(): return unittest.makeSuite(WikiPageTestCase, 'test') diff --git a/trac/wiki/web_ui.py b/trac/wiki/web_ui.py --- a/trac/wiki/web_ui.py +++ b/trac/wiki/web_ui.py @@ -93,7 +93,8 @@ # IPermissionRequestor methods def get_permission_actions(self): - actions = ['WIKI_CREATE', 'WIKI_DELETE', 'WIKI_MODIFY', 'WIKI_VIEW'] + actions = ['WIKI_CREATE', 'WIKI_DELETE', 'WIKI_MODIFY', 'WIKI_RENAME', + 'WIKI_VIEW'] return actions + [('WIKI_ADMIN', actions)] # IRequestHandler methods @@ -145,6 +146,8 @@ return self._render_editor(req, page, action, has_collision) elif action == 'delete': self._do_delete(req, versioned_page) + elif action == 'rename': + return self._do_rename(req, page) elif action == 'diff': style, options, diff_data = get_diff_options(req) contextall = diff_data['options']['contextall'] @@ -153,7 +156,9 @@ version=version, contextall=contextall or None)) elif action == 'delete': - return self._render_confirm(req, versioned_page) + return self._render_confirm_delete(req, versioned_page) + elif action == 'rename': + return self._render_confirm_rename(req, page) elif action == 'edit': return self._render_editor(req, versioned_page) elif action == 'diff': @@ -248,7 +253,7 @@ old_version = int(req.args.get('old_version', 0)) or version @with_transaction(self.env) - def do_transaction(db): + def do_delete(db): if version and old_version and version > old_version: # delete from `old_version` exclusive to `version` inclusive: for v in range(old_version, version): @@ -272,6 +277,42 @@ version=version, name=page.name)) req.redirect(req.href.wiki(page.name)) + def _do_rename(self, req, page): + if page.readonly: + req.perm(page.resource).require('WIKI_ADMIN') + else: + req.perm(page.resource).require('WIKI_RENAME') + + if 'cancel' in req.args: + req.redirect(get_resource_url(self.env, page.resource, req.href)) + + old_name, old_version = page.name, page.version + new_name = req.args.get('new_name', '').rstrip('/') + redirect = req.args.get('redirect') + + # verify input parameters + warn = None + if not new_name: + warn = _('New name is mandatory for a rename.') + elif new_name == old_name: + warn = _('New name must be different from old name.') + if warn: + add_warning(req, warn) + return self._render_confirm_rename(req, page) + + @with_transaction(self.env) + def do_rename(db): + page.rename(new_name, db) + if redirect: + redirection = WikiPage(self.env, old_name) + redirection.text = 'See [wiki:"%s"].' % new_name + author = get_reporter_id(req) + comment = '[wiki:"%s@%d" %s] was renamed to [wiki:"%s"].' % ( + new_name, old_version, old_name, new_name) + redirection.save(author, comment, req.remote_addr, db=db) + + req.redirect(req.href.wiki(redirect and old_name or new_name)) + def _do_save(self, req, page): if page.readonly: req.perm(page.resource).require('WIKI_ADMIN') @@ -296,7 +337,7 @@ add_warning(req, _("Page not modified, showing latest version.")) return self._render_view(req, page) - def _render_confirm(self, req, page): + def _render_confirm_delete(self, req, page): if page.readonly: req.perm(page.resource).require('WIKI_ADMIN') else: @@ -321,6 +362,16 @@ self._wiki_ctxtnav(req, page) return 'wiki_delete.html', data, None + def _render_confirm_rename(self, req, page): + if page.readonly: + req.perm(page.resource).require('WIKI_ADMIN') + else: + req.perm(page.resource).require('WIKI_RENAME') + + data = self._page_data(req, page, 'rename') + self._wiki_ctxtnav(req, page) + return 'wiki_rename.html', data, None + def _render_diff(self, req, page): if not page.exists: raise TracError(_('Version %(num)s of page "%(name)s" does not '