From cd69c2b0181d9deb64e545c4583908751493c7f1 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 22 Mar 2022 21:47:58 -0700 Subject: [PATCH 1/9] Wiki aliases --- announcements/resources/schemas/comm.xml | 7 ++ .../postgresql/comm-22.000-22.001.sql | 8 ++ .../sqlserver/comm-22.000-22.001.sql | 8 ++ .../announcements/AnnouncementModule.java | 2 +- .../labkey/api/announcements/CommSchema.java | 5 + .../org/labkey/search/SearchController.java | 2 +- wiki/src/org/labkey/wiki/WikiCollections.java | 51 ++++---- wiki/src/org/labkey/wiki/WikiController.java | 111 ++++++++++-------- wiki/src/org/labkey/wiki/WikiManager.java | 10 +- .../org/labkey/wiki/WikiSelectManager.java | 16 ++- wiki/src/org/labkey/wiki/model/Wiki.java | 17 ++- wiki/src/org/labkey/wiki/view/wikiManage.jsp | 44 ++++++- 12 files changed, 190 insertions(+), 91 deletions(-) create mode 100644 announcements/resources/schemas/dbscripts/postgresql/comm-22.000-22.001.sql create mode 100644 announcements/resources/schemas/dbscripts/sqlserver/comm-22.000-22.001.sql diff --git a/announcements/resources/schemas/comm.xml b/announcements/resources/schemas/comm.xml index d94d053eb08..228b9d64004 100644 --- a/announcements/resources/schemas/comm.xml +++ b/announcements/resources/schemas/comm.xml @@ -649,4 +649,11 @@ + + + + + + +
diff --git a/announcements/resources/schemas/dbscripts/postgresql/comm-22.000-22.001.sql b/announcements/resources/schemas/dbscripts/postgresql/comm-22.000-22.001.sql new file mode 100644 index 00000000000..5095b1d7be4 --- /dev/null +++ b/announcements/resources/schemas/dbscripts/postgresql/comm-22.000-22.001.sql @@ -0,0 +1,8 @@ +CREATE TABLE comm.PageAliases +( + Container ENTITYID NOT NULL, + Alias VARCHAR(255) NOT NULL, + RowId INT NOT NULL, + + CONSTRAINT PK_PageAliases PRIMARY KEY (Container, Alias) +); diff --git a/announcements/resources/schemas/dbscripts/sqlserver/comm-22.000-22.001.sql b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.000-22.001.sql new file mode 100644 index 00000000000..4b1151a629a --- /dev/null +++ b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.000-22.001.sql @@ -0,0 +1,8 @@ +CREATE TABLE comm.PageAliases +( + Container ENTITYID NOT NULL, + Alias NVARCHAR(255) NOT NULL, + RowId INT NOT NULL, + + CONSTRAINT PK_PageAliases PRIMARY KEY (Container, Alias) +); diff --git a/announcements/src/org/labkey/announcements/AnnouncementModule.java b/announcements/src/org/labkey/announcements/AnnouncementModule.java index c2e745eda9f..254a2709048 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementModule.java +++ b/announcements/src/org/labkey/announcements/AnnouncementModule.java @@ -97,7 +97,7 @@ public String getName() @Override public @Nullable Double getSchemaVersion() { - return 22.000; + return 22.001; } @Override diff --git a/internal/src/org/labkey/api/announcements/CommSchema.java b/internal/src/org/labkey/api/announcements/CommSchema.java index 95f5763d374..a471426277b 100644 --- a/internal/src/org/labkey/api/announcements/CommSchema.java +++ b/internal/src/org/labkey/api/announcements/CommSchema.java @@ -93,5 +93,10 @@ public TableInfo getTableInfoTours() { return getSchema().getTable("Tours"); } + + public TableInfo getTableInfoPageAliases() + { + return getSchema().getTable("PageAliases"); + } } diff --git a/search/src/org/labkey/search/SearchController.java b/search/src/org/labkey/search/SearchController.java index bff87f8a378..23a87c77d04 100644 --- a/search/src/org/labkey/search/SearchController.java +++ b/search/src/org/labkey/search/SearchController.java @@ -318,7 +318,7 @@ public boolean handlePost(AdminForm form, BindException errors) SearchService ss = SearchService.get(); if (null == ss) { - errors.reject("Indexing service is not running"); + errors.reject(ERROR_MSG, "Indexing service is not running"); return false; } diff --git a/wiki/src/org/labkey/wiki/WikiCollections.java b/wiki/src/org/labkey/wiki/WikiCollections.java index 50d4049bebe..73b40249c61 100644 --- a/wiki/src/org/labkey/wiki/WikiCollections.java +++ b/wiki/src/org/labkey/wiki/WikiCollections.java @@ -21,9 +21,13 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.announcements.CommSchema; +import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.data.Container; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.TableSelector; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.view.NavTree; import org.labkey.wiki.model.WikiTree; @@ -36,6 +40,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /* * User: adam @@ -51,6 +56,8 @@ public class WikiCollections private final Map _treesByName; private final Map _nameTitleMap; private final List _names; + private final Map _namesByAlias; + private final MultiValuedMap _aliasesByRowsId; private final List _adminNavTree; private final List _nonAdminNavTree; @@ -115,8 +122,13 @@ public WikiCollections(Container c) _adminNavTree = createNavTree(c, true); _nonAdminNavTree = createNavTree(c, false); - } + _aliasesByRowsId = new TableSelector(CommSchema.getInstance().getTableInfoPageAliases(), PageFlowUtil.set("Alias", "RowId"), SimpleFilter.createContainerFilter(c), null) + .mapStream() + .collect(LabKeyCollectors.toMultiValuedMap(map->(Integer)map.get("RowId"), map->(String)map.get("Alias"))); + _namesByAlias = _aliasesByRowsId.entries().stream() + .collect(Collectors.toMap(Map.Entry::getValue, e->_treesByRowId.get(e.getKey()).getName())); + } private void populateWikiTree(WikiTree parent, MultiValuedMap childMap, Map treesByRowId) { @@ -135,7 +147,6 @@ private void populateWikiTree(WikiTree parent, MultiValuedMap } } - // Create name list in depth-first order private void populateNames(WikiTree root, List names) { @@ -177,19 +188,11 @@ private List createNavTree(Container c, String rootId, WikiTree tree, b return elements; } - int getPageCount() { return getNames().size(); } - - WikiTree getWikiTree() - { - return _root; - } - - @NotNull List getNames() { return _names; @@ -218,23 +221,6 @@ List getNonAdminNavTree() return _treesByRowId.get(rowId); } - // Returns null for non-existent wiki, empty collection for existing but no children - @Nullable Collection getChildren(@Nullable String parentName) - { - WikiTree parent = getWikiTree(parentName); - - if (null == parent) - return null; - - return parent.getChildren(); - } - - // Returns null for non-existent wiki, empty collection for existing but no children - @Nullable Collection getChildren(int rowId) - { - return _treesByRowId.get(rowId).getChildren(); - } - // TODO: Change to return the root WikiTree? Map getNameTitleMap() { @@ -275,4 +261,15 @@ private Set populateWikiTrees(WikiTree root, Set trees) return trees; } + + Collection getAliases(int rowId) + { + return _aliasesByRowsId.get(rowId); + } + + // Returns null for no match + @Nullable String getNameForAlias(@Nullable String alias) + { + return _namesByAlias.get(alias); + } } diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 506453352d6..3e79c88af47 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -140,7 +140,6 @@ protected BaseWikiPermissions getPermissions() return new BaseWikiPermissions(getUser(), getContainer()); } - @Override protected @Nullable HttpView getTemplate(ViewContext context, ModelAndView mv, Controller action, PageConfig page) { @@ -180,7 +179,6 @@ protected BaseWikiPermissions getPermissions() return template; } - public static class CustomizeWikiPartView extends JspView { private List _containerList; @@ -275,19 +273,16 @@ private static void populateWikiContainerListRecursive(ViewContext context, Cont } } - private ActionURL getBeginURL(Container c) { return getPageURL(getDefaultPage(c), c); } - public static ActionURL getPageURL(Container c, @Nullable String name) { return getWikiURL(c, PageAction.class, name); } - public static ActionURL getEditWikiURL(Container c, Class actionClass, @Nullable String name, Boolean create) { ActionURL url = getWikiURL(c, actionClass, name); @@ -297,7 +292,6 @@ public static ActionURL getEditWikiURL(Container c, Class return url; } - public static ActionURL getWikiURL(Container c, Class actionClass, @Nullable String name) { ActionURL url = new ActionURL(actionClass, c); @@ -308,12 +302,11 @@ public static ActionURL getWikiURL(Container c, Class acti return url; } - /** * This method represents the point of entry into the pageflow */ @RequiresPermission(ReadPermission.class) - public class BeginAction extends SimpleViewAction + public class BeginAction extends SimpleViewAction { @SuppressWarnings("UnusedDeclaration") public BeginAction() @@ -343,7 +336,6 @@ public void addNavTrail(NavTree root) } } - public static Wiki getDefaultPage(Container c) { //look for page named "default" @@ -367,7 +359,6 @@ public static Wiki getDefaultPage(Container c) return wiki; } - @RequiresPermission(ReadPermission.class) //will test explicitly below public class DeleteAction extends ConfirmAction { @@ -480,10 +471,10 @@ public ManageAction() @Override public ModelAndView getView(WikiManageForm form, boolean reshow, BindException errors) { - String name = form.getName(); + String name = form.getNewName(); if (name == null || (errors != null && errors.getErrorCount()>0)) - name = form.getOriginalName(); + name = form.getName(); _wiki = WikiSelectManager.getWiki(getContainer(), name); @@ -501,15 +492,15 @@ public ModelAndView getView(WikiManageForm form, boolean reshow, BindException e bean.siblings = WikiSelectManager.getChildren(getContainer(), _wiki.getParent()); bean.possibleParents = WikiSelectManager.getPossibleParents(getContainer(), _wiki); bean.showChildren = SHOW_CHILD_REORDERING; + bean.aliases = WikiSelectManager.getAliases(getContainer(), _wiki.getRowId()); - JspView manageView = new JspView<>("/org/labkey/wiki/view/wikiManage.jsp", bean, errors); + JspView manageView = new JspView<>("/org/labkey/wiki/view/wikiManage.jsp", bean, errors); manageView.setTitle("Wiki Configuration"); getPageConfig().setFocusId("name"); return manageView; } - public class ManageBean { public Wiki wiki; @@ -517,22 +508,24 @@ public class ManageBean public Collection siblings; public Set possibleParents; public boolean showChildren; + public Collection aliases; } - @Override public boolean handlePost(WikiManageForm form, BindException errors) { - String originalName = form.getOriginalName(); - String newName = form.getName(); + String originalName = form.getName(); + String newName = form.getNewName(); Container c = getContainer(); - _wiki = WikiSelectManager.getWiki(c, originalName); + Wiki wiki = WikiSelectManager.getWiki(c, originalName); - if (null == _wiki) + if (null == wiki) { throw new NotFoundException(); } + _wiki = new Wiki(wiki); // Clone so we don't mutate the cached copy + BaseWikiPermissions perms = getPermissions(); if (!perms.allowUpdate(_wiki)) throw new UnauthorizedException("You do not have permissions to manage this wiki page"); @@ -540,7 +533,8 @@ public boolean handlePost(WikiManageForm form, BindException errors) // Get the latest version based on previous properties WikiVersion versionOld = _wiki.getLatestVersion(); - // Now update wiki with newly submitted properties TODO: Should clone wiki instead of changing cached copy (e.g., for concurrency and in case something goes wrong with update) + // Now update wiki with newly submitted properties + _wiki.setName(newName); _wiki.setParent(form.getParent()); _wiki.setShouldIndex(form.isShouldIndex()); @@ -558,6 +552,11 @@ public boolean handlePost(WikiManageForm form, BindException errors) _wikiVersion = null; } + // If renaming and alias is requested, add it now since updateWiki() will uncache and reindex + if (form.isAddAlias()) + { + getWikiManager().addAlias(getUser(), _wiki, originalName); // TODO: error on constraint violation + } getWikiManager().updateWiki(getUser(), _wiki, _wikiVersion, false); if (SHOW_CHILD_REORDERING) @@ -600,7 +599,7 @@ public ActionURL getSuccessURL(WikiManageForm form) } catch (IllegalArgumentException ignored) {} nextPage.deleteParameters(); - nextPage.addParameter("name", form.getName()); + nextPage.addParameter("name", _wiki.getName()); return nextPage; } @@ -620,7 +619,6 @@ public ActionURL getUrl() } } - private void updateDisplayOrder(List pages, int[] order) { if (!verifyOrder(pages, order)) @@ -640,7 +638,6 @@ private void updateDisplayOrder(List pages, int[] order) } } - private boolean verifyOrder(List wikis, int[] ids) { if (ids.length != wikis.size()) @@ -676,7 +673,7 @@ public class DownloadAction extends BaseDownloadAction } @RequiresPermission(ReadPermission.class) - public class PrintAllAction extends SimpleViewAction + public class PrintAllAction extends SimpleViewAction { @Override public ModelAndView getView(Object o, BindException errors) @@ -684,7 +681,7 @@ public ModelAndView getView(Object o, BindException errors) Container c = getContainer(); Set wikiTrees = WikiSelectManager.getWikiTrees(c); - JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintAll.jsp", new PrintAllBean(wikiTrees)); + JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintAll.jsp", new PrintAllBean(wikiTrees)); v.setFrame(WebPartView.FrameType.NONE); getPageConfig().setTemplate(Template.Print); @@ -719,7 +716,7 @@ public ModelAndView getView(WikiNameForm form, BindException errors) // build a set of all descendants of the root page Set wikiTrees = WikiSelectManager.getWikiTrees(c, _rootWiki); - JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintAll.jsp", new PrintAllBean(wikiTrees)); + JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintAll.jsp", new PrintAllBean(wikiTrees)); v.setFrame(WebPartView.FrameType.NONE); getPageConfig().setTemplate(Template.Print); @@ -763,7 +760,7 @@ public ModelAndView getView(WikiNameForm form, BindException errors) //just want to re-use same jsp Set wikis = Collections.singleton(tree); - JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintRaw.jsp", new PrintRawBean(wikis)); + JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintRaw.jsp", new PrintRawBean(wikis)); v.setFrame(WebPartView.FrameType.NONE); getPageConfig().setTemplate(Template.Print); @@ -779,14 +776,14 @@ public void addNavTrail(NavTree root) @RequiresPermission(ReadPermission.class) - public class PrintAllRawAction extends SimpleViewAction + public class PrintAllRawAction extends SimpleViewAction { @Override public ModelAndView getView(Object o, BindException errors) { Container c = getContainer(); Set wikiTrees = WikiSelectManager.getWikiTrees(c); - JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintRaw.jsp", new PrintRawBean(wikiTrees)); + JspView v = new JspView<>("/org/labkey/wiki/view/wikiPrintRaw.jsp", new PrintRawBean(wikiTrees)); v.setFrame(WebPartView.FrameType.NONE); getPageConfig().setTemplate(Template.Print); @@ -1174,8 +1171,13 @@ public ModelAndView getView(WikiNameForm form, BindException errors) _wiki = WikiSelectManager.getWiki(getContainer(), name); boolean existing = _wiki != null; - if (null == _wiki) + if (!existing) { + // Redirect if name is an alias + String realName = WikiSelectManager.getNameForAlias(getContainer(), name); + if (null != realName) + throw new RedirectException(getViewContext().getActionURL().clone().replaceParameter("name", realName)); + _wiki = new Wiki(getContainer(), name); _wikiversion = new WikiVersion(name); //set new page title to be name. @@ -1659,7 +1661,6 @@ public void addNavTrail(NavTree root) public static class WikiManageForm { - private String _originalName; private String _name; private String _title; private int _parent; @@ -1668,6 +1669,8 @@ public static class WikiManageForm private String _nextAction; private String _containerPath; private boolean _shouldIndex; + private boolean _addAlias; + private String _newName; public String getContainerPath() { @@ -1758,17 +1761,6 @@ public void setTitle(String title) _title = title; } - public String getOriginalName() - { - return _originalName; - } - - @SuppressWarnings({"UnusedDeclaration"}) - public void setOriginalName(String name) - { - _originalName = name; - } - public int getParent() { return _parent; @@ -1791,18 +1783,41 @@ public void setNextAction(String nextAction) _nextAction = nextAction; } + public String getNewName() + { + return _newName; + } + + public void setNewName(String newName) + { + _newName = newName; + } + + public boolean isAddAlias() + { + return _addAlias; + } + + @SuppressWarnings({"UnusedDeclaration"}) + public void setAddAlias(boolean addAlias) + { + _addAlias = addAlias; + } + public void validate(Errors errors) { //check name - String newName = getName(); - String oldName = getOriginalName(); - if (newName == null) + String originalName = getName(); + String newName = getNewName(); + if (originalName == null) + { errors.rejectValue("name", ERROR_MSG, "You must provide a name for this page."); - else + } + else if (newName != null && !newName.equalsIgnoreCase(originalName)) { - //check for existing wiki with this name + // rename - check for existing wiki with this name Container c = ContainerManager.getForPath(getContainerPath()); - if (!newName.equalsIgnoreCase(oldName) && WikiManager.wikiNameExists(c, newName)) + if (WikiManager.wikiNameExists(c, newName)) errors.rejectValue("name", ERROR_MSG, "A page with the name '" + newName + "' already exists in this folder. Please choose a different name."); } } diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index f33464f6b4d..e2a3f15f2f3 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -52,6 +52,7 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.Path; import org.labkey.api.util.TestContext; @@ -228,7 +229,7 @@ public boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, boole * * @param copyHistory true to propagate the user and date created from the previous wiki version, else just use the current user * and current date. - * @param createNewVersion by default we create a new wiki version for each update, if this is set to false we will update + * @param createNewVersion by default, we create a new wiki version for each update, if this is set to false we will update * the latest wiki version. */ private boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, boolean copyHistory, boolean createNewVersion) @@ -311,7 +312,12 @@ private boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, bool return true; } - + public void addAlias(User user, Wiki wiki, String alias) + { + Container c = wiki.lookupContainer(); + Map map = new HashMap<>(Map.of("Container", c, "Alias", alias, "RowId", wiki.getRowId())); + Table.insert(user, CommSchema.getInstance().getTableInfoPageAliases(), map); + } public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubtree) throws SQLException { diff --git a/wiki/src/org/labkey/wiki/WikiSelectManager.java b/wiki/src/org/labkey/wiki/WikiSelectManager.java index 4a01aa0bc86..f504886b8d2 100644 --- a/wiki/src/org/labkey/wiki/WikiSelectManager.java +++ b/wiki/src/org/labkey/wiki/WikiSelectManager.java @@ -103,7 +103,7 @@ public static Wiki getWiki(Container c, final String name) { return null; } - return WikiCache.getWiki(c, name, new WikiCacheLoader() + return WikiCache.getWiki(c, name, new WikiCacheLoader<>() { @Override public Wiki load(String key, Container c) @@ -113,10 +113,22 @@ public Wiki load(String key, Container c) }); } + // Get a single wiki name based on its alias + public static String getNameForAlias(Container c, String alias) + { + return getWikiCollections(c).getNameForAlias(alias); + } + + // Get a wiki's aliases + public static Collection getAliases(Container c, int rowId) + { + return getWikiCollections(c).getAliases(rowId); + } private static WikiCollections getWikiCollections(Container c) { - return WikiCache.getWikiCollections(c, new WikiCacheLoader(){ + return WikiCache.getWikiCollections(c, new WikiCacheLoader<>() + { @Override WikiCollections load(String key, Container c) { diff --git a/wiki/src/org/labkey/wiki/model/Wiki.java b/wiki/src/org/labkey/wiki/model/Wiki.java index 71d7c966935..27f0b5491e0 100644 --- a/wiki/src/org/labkey/wiki/model/Wiki.java +++ b/wiki/src/org/labkey/wiki/model/Wiki.java @@ -59,26 +59,34 @@ public Wiki() { } - public Wiki(Container c, String name) { setContainerId(c.getId()); _name = name; } + public Wiki(Wiki wiki) + { + wiki.copyTo(this); + _rowId = wiki._rowId; + _name = wiki._name; + _parent = wiki._parent; + _displayOrder = wiki._displayOrder; + _pageVersionId = wiki._pageVersionId; + _showAttachments = wiki._showAttachments; + _shouldIndex = wiki._shouldIndex; + } public ActionURL getWikiURL(Class actionClass, String name) { return WikiController.getWikiURL(lookupContainer(), actionClass, name); } - public ActionURL getPageURL() { return getWikiURL(PageAction.class, _name); } - public @Nullable ActionURL getVersionsURL() { if (null == _name) @@ -86,7 +94,6 @@ public ActionURL getPageURL() return getWikiURL(VersionsAction.class, _name); } - public ActionURL getManageURL() { if (null == _name) @@ -95,13 +102,11 @@ public ActionURL getManageURL() return getWikiURL(ManageAction.class, _name); } - public int getRowId() { return _rowId; } - @SuppressWarnings({"UnusedDeclaration"}) public void setRowId(int rowId) { diff --git a/wiki/src/org/labkey/wiki/view/wikiManage.jsp b/wiki/src/org/labkey/wiki/view/wikiManage.jsp index 1eb5b0bc1ba..2d3191d0e3a 100644 --- a/wiki/src/org/labkey/wiki/view/wikiManage.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiManage.jsp @@ -97,6 +97,12 @@ return false; } + + function rename() + { + document.getElementById('rename').style.display = ''; + return false; + } @@ -114,10 +120,19 @@ %> - + + + <%=button("Rename").style("width:100px").submit(true).onClick("return rename()") %> + - - WARNING: Changing a page's name will break any links to the page. + + + + + + +
>Check this to add '<%=h(wiki.getName())%>' as an alias for this page, to keep existing links and shortcuts working
+ @@ -212,11 +227,32 @@ <% } %> + + + + + + + +
+ <% + SelectBuilder aliasesBuilder = new SelectBuilder().name("aliases").id("aliases").size(5).addStyle("width:500px"); + bean.aliases.forEach(alias->aliasesBuilder.addOption(new OptionBuilder() + .value(String.valueOf(alias)) + .label(alias) + .build())); + %> + <%=aliasesBuilder%> + + <%= button("Delete Alias").style("width:100px;").submit(true).onClick("return orderModule('children', 0, 'childOrder')")%> +
+ + + - <%= button("Save").submit(true).onClick("document.manage.nextAction.value = " + q(NextAction.page.name()) + "; return true;").title("Save Changes") %> From ffb070344569e3e1d9d725c705758a18ebe60ead Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 23 Mar 2022 16:18:39 -0700 Subject: [PATCH 2/9] Fix NPE when not renaming. Constraint violation -> error message. --- wiki/src/org/labkey/wiki/WikiController.java | 139 ++++++++----------- wiki/src/org/labkey/wiki/WikiManager.java | 16 ++- 2 files changed, 73 insertions(+), 82 deletions(-) diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 3e79c88af47..77b5d935e8e 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -456,6 +456,7 @@ public class ManageAction extends FormViewAction { private Wiki _wiki = null; private WikiVersion _wikiVersion = null; + private boolean _isRename = false; @SuppressWarnings({"UnusedDeclaration"}) public ManageAction() @@ -512,10 +513,30 @@ public class ManageBean } @Override - public boolean handlePost(WikiManageForm form, BindException errors) + public void validateCommand(WikiManageForm form, Errors errors) { + //check name String originalName = form.getName(); String newName = form.getNewName(); + if (originalName == null) + { + errors.rejectValue("name", ERROR_MSG, "You must provide a name for this page."); + } + else if (newName != null && !newName.equalsIgnoreCase(originalName)) + { + // rename - check for existing wiki with this name + Container c = ContainerManager.getForPath(form.getContainerPath()); + if (WikiManager.wikiNameExists(c, newName)) + errors.rejectValue("name", ERROR_MSG, "A page with the name '" + newName + "' already exists in this folder. Please choose a different name."); + _isRename = true; + } + } + + @Override + public boolean handlePost(WikiManageForm form, BindException errors) + { + String originalName = form.getName(); + String newName = _isRename ? form.getNewName() : originalName; Container c = getContainer(); Wiki wiki = WikiSelectManager.getWiki(c, originalName); @@ -553,9 +574,11 @@ public boolean handlePost(WikiManageForm form, BindException errors) } // If renaming and alias is requested, add it now since updateWiki() will uncache and reindex - if (form.isAddAlias()) + if (_isRename && form.isAddAlias()) { - getWikiManager().addAlias(getUser(), _wiki, originalName); // TODO: error on constraint violation + getWikiManager().addAlias(getUser(), _wiki, originalName, errors); + if (errors.hasErrors()) + return false; } getWikiManager().updateWiki(getUser(), _wiki, _wikiVersion, false); @@ -577,12 +600,6 @@ public boolean handlePost(WikiManageForm form, BindException errors) return true; } - @Override - public void validateCommand(WikiManageForm wikiManageForm, Errors errors) - { - wikiManageForm.validate(errors); - } - @Override public ActionURL getSuccessURL(WikiManageForm form) { @@ -730,7 +747,6 @@ public void addNavTrail(NavTree root) } } - public class PrintAllBean { public Set wikiTrees; @@ -742,7 +758,6 @@ private PrintAllBean(Set wikis) } } - @RequiresPermission(ReadPermission.class) public class PrintRawAction extends SimpleViewAction { @@ -774,7 +789,6 @@ public void addNavTrail(NavTree root) } } - @RequiresPermission(ReadPermission.class) public class PrintAllRawAction extends SimpleViewAction { @@ -797,7 +811,6 @@ public void addNavTrail(NavTree root) } } - private static List namesToWikis(Container c, List names) { LinkedList wikis = new LinkedList<>(); @@ -808,7 +821,6 @@ private static List namesToWikis(Container c, List names) return wikis; } - public class PrintRawBean { public final Set wikis; @@ -821,7 +833,6 @@ private PrintRawBean(Set wikis) } } - public static class CopyWikiForm { private String _path; @@ -883,7 +894,6 @@ public void setIsCopyingHistory(boolean isCopyingHistory) } } - private Container getSourceContainer(String source) { Container cSource; @@ -894,7 +904,6 @@ private Container getSourceContainer(String source) return cSource; } - private Container getDestContainer(String destContainer, String path, BindException errors) { if (destContainer == null) @@ -920,7 +929,6 @@ private Container getDestContainer(String destContainer, String path, BindExcept return c; } - private void displayWikiModuleInDestContainer(Container cDest) { Set activeModules = new HashSet<>(cDest.getActiveModules()); @@ -934,7 +942,6 @@ private void displayWikiModuleInDestContainer(Container cDest) } } - @RequiresPermission(AdminPermission.class) public class CopyWikiAction extends FormHandlerAction { @@ -1015,10 +1022,8 @@ public boolean handlePost(CopyWikiForm form, BindException errors) throws Except return true; } - } - @RequiresPermission(AdminPermission.class) public class CopySinglePageAction extends FormHandlerAction { @@ -1070,7 +1075,6 @@ public URLHelper getSuccessURL(CopyWikiForm copyWikiForm) } } - @RequiresPermission(AdminPermission.class) public class CopyWikiLocationAction extends SimpleViewAction { @@ -1106,8 +1110,7 @@ public void addNavTrail(NavTree root) } } - - public class CopyBean + public static class CopyBean { public HtmlString folderList; public String destContainer; @@ -1115,7 +1118,6 @@ public class CopyBean public ActionURL cancelURL; } - private ActionURL getSourceURL(String pageName, int version) { ActionURL url = new ActionURL(SourceAction.class, getContainer()); @@ -1124,7 +1126,6 @@ private ActionURL getSourceURL(String pageName, int version) return url; } - @RequiresPermission(ReadPermission.class) public class SourceAction extends PageAction { @@ -1134,7 +1135,6 @@ public SourceAction() } } - @RequiresPermission(ReadPermission.class) public class PageAction extends SimpleViewAction { @@ -1266,14 +1266,12 @@ ActionURL getUrl() } } - public static ActionURL getPageURL(Wiki wiki, Container c) { ActionURL url = new ActionURL(PageAction.class, c); return url.addParameter("name", wiki.getName()); } - @Nullable private HttpView getDiscussionView(String objectId, ActionURL pageURL, String title) { @@ -1281,7 +1279,6 @@ private HttpView getDiscussionView(String objectId, ActionURL pageURL, String ti return service.getDiscussionArea(getViewContext(), objectId, pageURL, title, true, false); } - private ActionURL getVersionURL(String name) { ActionURL url = new ActionURL(VersionAction.class, getContainer()); @@ -1289,7 +1286,6 @@ private ActionURL getVersionURL(String name) return url; } - @RequiresPermission(ReadPermission.class) public class VersionAction extends SimpleViewAction { @@ -1595,8 +1591,6 @@ public ActionURL getUrl() } } - - private ActionURL getMakeCurrentURL(String pageName, int version) { ActionURL url = new ActionURL(MakeCurrentAction.class, getContainer()); @@ -1606,7 +1600,6 @@ private ActionURL getMakeCurrentURL(String pageName, int version) return url; } - @RequiresPermission(ReadPermission.class) //will check in code below public class MakeCurrentAction extends FormViewAction { @@ -1658,7 +1651,6 @@ public void addNavTrail(NavTree root) } } - public static class WikiManageForm { private String _name; @@ -1803,88 +1795,68 @@ public void setAddAlias(boolean addAlias) { _addAlias = addAlias; } - - public void validate(Errors errors) - { - //check name - String originalName = getName(); - String newName = getNewName(); - if (originalName == null) - { - errors.rejectValue("name", ERROR_MSG, "You must provide a name for this page."); - } - else if (newName != null && !newName.equalsIgnoreCase(originalName)) - { - // rename - check for existing wiki with this name - Container c = ContainerManager.getForPath(getContainerPath()); - if (WikiManager.wikiNameExists(c, newName)) - errors.rejectValue("name", ERROR_MSG, "A page with the name '" + newName + "' already exists in this folder. Please choose a different name."); - } - } } + public static class WikiNameForm + { + private String _name; + private String _redirect; + private int _parent; + private int _version; + private boolean _isDeletingSubtree; - public static class WikiNameForm - { - private String _name; - private String _redirect; - private int _parent; - private int _version; - private boolean _isDeletingSubtree; - - public int getVersion() + public int getVersion() { return _version; } - @SuppressWarnings({"UnusedDeclaration"}) - public void setVersion(int version) + @SuppressWarnings({"UnusedDeclaration"}) + public void setVersion(int version) { _version = version; } - public int getParent() + public int getParent() { return _parent; } - public void setParent(int parent) + public void setParent(int parent) { _parent = parent; } - public String getName() + public String getName() { return _name; } - @SuppressWarnings({"UnusedDeclaration"}) - public void setName(String name) + @SuppressWarnings({"UnusedDeclaration"}) + public void setName(String name) { _name = name; } - public String getRedirect() + public String getRedirect() { return _redirect; } - public void setRedirect(String redirect) + public void setRedirect(String redirect) { _redirect = redirect; } - public boolean getIsDeletingSubtree() + public boolean getIsDeletingSubtree() { return _isDeletingSubtree; } - public void setIsDeletingSubtree(boolean isDeletingSubtree) + public void setIsDeletingSubtree(boolean isDeletingSubtree) { _isDeletingSubtree = isDeletingSubtree; } - } - + } public static class ContainerForm { @@ -2037,7 +2009,6 @@ public ModelAndView getView(EditWikiForm form, BindException errors) { throw new NotFoundException("There is no wiki in the current folder named '" + form.getName() + "'!"); } - } //endregion @@ -2074,9 +2045,18 @@ public ModelAndView getView(EditWikiForm form, BindException errors) && null != defFormat && defFormat.length() > 0) form.setFormat(defFormat); - WikiEditModel model = new WikiEditModel(getContainer(), wiki, curVersion, - form.getRedirect(), form.getCancel(), form.getFormat(), form.getDefName(), useVisualEditor, - form.getWebPartId(), getUser()); + WikiEditModel model = new WikiEditModel( + getContainer(), + wiki, + curVersion, + form.getRedirect(), + form.getCancel(), + form.getFormat(), + form.getDefName(), + useVisualEditor, + form.getWebPartId(), + getUser() + ); //endregion //region stash the wiki so we can build the nav trail @@ -2478,6 +2458,7 @@ public String[] getToDelete() return _toDelete; } + @SuppressWarnings({"UnusedDeclaration"}) public void setToDelete(String[] toDelete) { _toDelete = toDelete; diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index e2a3f15f2f3..477a9eb648d 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -52,7 +52,6 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; import org.labkey.api.util.JunitUtil; -import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.Path; import org.labkey.api.util.TestContext; @@ -76,6 +75,7 @@ import org.labkey.wiki.model.WikiVersionsGrid; import org.labkey.wiki.model.WikiView; import org.labkey.wiki.query.WikiSchema; +import org.springframework.validation.BindException; import java.io.IOException; import java.sql.ResultSet; @@ -91,6 +91,8 @@ import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; +import static org.labkey.api.action.SpringActionController.ERROR_MSG; + /** * User: mbellew @@ -312,11 +314,19 @@ private boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, bool return true; } - public void addAlias(User user, Wiki wiki, String alias) + public void addAlias(User user, Wiki wiki, String alias, BindException errors) { Container c = wiki.lookupContainer(); Map map = new HashMap<>(Map.of("Container", c, "Alias", alias, "RowId", wiki.getRowId())); - Table.insert(user, CommSchema.getInstance().getTableInfoPageAliases(), map); + try + { + Table.insert(user, CommSchema.getInstance().getTableInfoPageAliases(), map); + } + catch (RuntimeSQLException e) + { + if (e.isConstraintException()) + errors.rejectValue("name", ERROR_MSG,"Alias \"" + alias + "\" already exists!"); + } } public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubtree) throws SQLException From 4909d8c7f0e50b4f06b77a590ec97a9a83bdf026 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 23 Mar 2022 17:30:39 -0700 Subject: [PATCH 3/9] Delete aliases on container and wiki delete. Log redirects. Focus on new name input. --- wiki/src/org/labkey/wiki/WikiCollections.java | 1 + wiki/src/org/labkey/wiki/WikiController.java | 3 ++ wiki/src/org/labkey/wiki/WikiManager.java | 28 ++++++++++++++----- wiki/src/org/labkey/wiki/view/wikiManage.jsp | 1 + 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/wiki/src/org/labkey/wiki/WikiCollections.java b/wiki/src/org/labkey/wiki/WikiCollections.java index 73b40249c61..3ebbdb0620b 100644 --- a/wiki/src/org/labkey/wiki/WikiCollections.java +++ b/wiki/src/org/labkey/wiki/WikiCollections.java @@ -127,6 +127,7 @@ public WikiCollections(Container c) .mapStream() .collect(LabKeyCollectors.toMultiValuedMap(map->(Integer)map.get("RowId"), map->(String)map.get("Alias"))); _namesByAlias = _aliasesByRowsId.entries().stream() + .filter(e->_treesByRowId.get(e.getKey()) != null) // Just in case - ignore orphaned aliases .collect(Collectors.toMap(Map.Entry::getValue, e->_treesByRowId.get(e.getKey()).getName())); } diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 77b5d935e8e..1a50cfad63a 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -1176,7 +1176,10 @@ public ModelAndView getView(WikiNameForm form, BindException errors) // Redirect if name is an alias String realName = WikiSelectManager.getNameForAlias(getContainer(), name); if (null != realName) + { + LOG.debug("PageAction: requested wiki name, \"" + name + "\", is an alias; redirecting to \"" + realName + "\". Referrer: " + getViewContext().getRequest().getHeader("Referer")); throw new RedirectException(getViewContext().getActionURL().clone().replaceParameter("name", realName)); + } _wiki = new Wiki(getContainer(), name); _wikiversion = new WikiVersion(name); diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index 477a9eb648d..b602a37b880 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -316,8 +316,8 @@ private boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, bool public void addAlias(User user, Wiki wiki, String alias, BindException errors) { - Container c = wiki.lookupContainer(); - Map map = new HashMap<>(Map.of("Container", c, "Alias", alias, "RowId", wiki.getRowId())); + assert null != wiki.getContainerId(); + Map map = new HashMap<>(Map.of("Container", wiki.getContainerId(), "Alias", alias, "RowId", wiki.getRowId())); try { Table.insert(user, CommSchema.getInstance().getTableInfoPageAliases(), map); @@ -325,10 +325,24 @@ public void addAlias(User user, Wiki wiki, String alias, BindException errors) catch (RuntimeSQLException e) { if (e.isConstraintException()) - errors.rejectValue("name", ERROR_MSG,"Alias \"" + alias + "\" already exists!"); + errors.rejectValue("name", ERROR_MSG,"Alias '" + alias + "' already exists in this folder."); } } + // null == wiki ==> delete all aliases in a container + // null != wiki && null == alias => delete all aliases associated with a wiki + // null != wiki && null != alias ==> delete a single alias associated with a wiki + private void deleteAliases(Container c, @Nullable Wiki wiki, @Nullable String alias) + { + assert null != c; + SimpleFilter filter = SimpleFilter.createContainerFilter(c); + if (null != wiki) + filter.addCondition(FieldKey.fromParts("RowId"), wiki.getRowId()); + if (null != alias) + filter.addCondition(FieldKey.fromParts("Alias"), alias); + Table.delete(CommSchema.getInstance().getTableInfoPageAliases(), filter); + } + public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubtree) throws SQLException { //shift children to new parent, or delete recursively if deleting the whole subtree @@ -345,6 +359,7 @@ public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubt new SimpleFilter(FieldKey.fromParts("pageentityId"), wiki.getEntityId())); Table.delete(comm.getTableInfoPages(), new SimpleFilter(FieldKey.fromParts("entityId"), wiki.getEntityId())); + deleteAliases(c, wiki, null); getAttachmentService().deleteAttachments(wiki.getAttachmentParent()); @@ -435,17 +450,15 @@ private void handleChildren(User user, Container c, Wiki wiki, boolean isDeletin } } - public void purgeContainer(Container c) { - WikiCache.uncache(c); - DbScope scope = comm.getSchema().getScope(); try (DbScope.Transaction transaction = scope.ensureTransaction()) { new SqlExecutor(comm.getSchema()).execute("UPDATE " + comm.getTableInfoPages() + " SET PageVersionId = NULL WHERE Container = ?", c.getId()); new SqlExecutor(comm.getSchema()).execute("DELETE FROM " + comm.getTableInfoPageVersions() + " WHERE PageEntityId IN (SELECT EntityId FROM " + comm.getTableInfoPages() + " WHERE Container = ?)", c.getId()); + deleteAliases(c, null, null); // Clear all wiki webpart properties that refer to this container. This includes wiki and wiki TOC // webparts in this and potentially other containers. #13937 @@ -455,8 +468,9 @@ public void purgeContainer(Container c) transaction.commit(); } - } + WikiCache.uncache(c); + } public FormattedHtml formatWiki(Container c, Wiki wiki, WikiVersion wikiversion) { diff --git a/wiki/src/org/labkey/wiki/view/wikiManage.jsp b/wiki/src/org/labkey/wiki/view/wikiManage.jsp index 2d3191d0e3a..f259f155721 100644 --- a/wiki/src/org/labkey/wiki/view/wikiManage.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiManage.jsp @@ -101,6 +101,7 @@ function rename() { document.getElementById('rename').style.display = ''; + document.getElementById('newName').focus(); return false; } From b7372ef2b7198776a5d4a617875e5a2bcd372980 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 23 Mar 2022 17:55:29 -0700 Subject: [PATCH 4/9] Disallow renaming on the edit wiki page --- wiki/src/org/labkey/wiki/view/wikiEdit.jsp | 5 +++-- wiki/src/org/labkey/wiki/view/wikiManage.jsp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/wiki/src/org/labkey/wiki/view/wikiEdit.jsp b/wiki/src/org/labkey/wiki/view/wikiEdit.jsp index fdba4ff9bce..63ada22f227 100644 --- a/wiki/src/org/labkey/wiki/view/wikiEdit.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiEdit.jsp @@ -42,6 +42,7 @@ <% JspView me = (JspView) HttpView.currentView(); WikiEditModel model = me.getModelBean(); + final boolean existingWiki = null != model.getEntityId(); final String ID_PREFIX = "wiki-input-"; final HtmlString H_ID_PREFIX = h("wiki-input-"); String sep; @@ -120,9 +121,9 @@ - + diff --git a/wiki/src/org/labkey/wiki/view/wikiManage.jsp b/wiki/src/org/labkey/wiki/view/wikiManage.jsp index f259f155721..1f56de5dce8 100644 --- a/wiki/src/org/labkey/wiki/view/wikiManage.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiManage.jsp @@ -122,7 +122,7 @@ From 3d632d0dcc7feb498af3fe532e93cb06e1c27a74 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 23 Mar 2022 18:41:35 -0700 Subject: [PATCH 5/9] Allow editing alias list. Render multiple errors, if present. --- wiki/src/org/labkey/wiki/WikiController.java | 34 +++++++++++++++++++- wiki/src/org/labkey/wiki/WikiManager.java | 2 +- wiki/src/org/labkey/wiki/view/wikiManage.jsp | 30 ++++++----------- 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 1a50cfad63a..f475a585b34 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -113,6 +113,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; @@ -580,6 +581,25 @@ public boolean handlePost(WikiManageForm form, BindException errors) if (errors.hasErrors()) return false; } + if (null != form.getAliases()) + { + Collection existingAliases = WikiSelectManager.getAliases(getContainer(), _wiki.getRowId()); + Collection newAliases = Arrays.stream(form.getAliases().split("\n")) + .map(StringUtils::trimToNull) + .filter(Objects::nonNull) + .collect(Collectors.toCollection(ArrayList::new)); + + if (!newAliases.equals(existingAliases)) + { + WikiManager mgr = WikiManager.get(); + mgr.deleteAliases(c, wiki, null); + + // Best effort for alias editing -- reshow with error message if duplicates are encountered, but + // complete all other edits. + newAliases.forEach(alias->mgr.addAlias(getUser(), _wiki, alias, errors)); + WikiCache.uncache(c, wiki, true); + } + } getWikiManager().updateWiki(getUser(), _wiki, _wikiVersion, false); if (SHOW_CHILD_REORDERING) @@ -597,7 +617,7 @@ public boolean handlePost(WikiManageForm form, BindException errors) updateDisplayOrder(siblings, siblingOrder); } - return true; + return !errors.hasErrors(); } @Override @@ -1666,6 +1686,7 @@ public static class WikiManageForm private boolean _shouldIndex; private boolean _addAlias; private String _newName; + private String _aliases; public String getContainerPath() { @@ -1798,6 +1819,17 @@ public void setAddAlias(boolean addAlias) { _addAlias = addAlias; } + + public String getAliases() + { + return _aliases; + } + + @SuppressWarnings({"UnusedDeclaration"}) + public void setAliases(String aliases) + { + _aliases = aliases; + } } public static class WikiNameForm diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index b602a37b880..47895d30db1 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -332,7 +332,7 @@ public void addAlias(User user, Wiki wiki, String alias, BindException errors) // null == wiki ==> delete all aliases in a container // null != wiki && null == alias => delete all aliases associated with a wiki // null != wiki && null != alias ==> delete a single alias associated with a wiki - private void deleteAliases(Container c, @Nullable Wiki wiki, @Nullable String alias) + public void deleteAliases(Container c, @Nullable Wiki wiki, @Nullable String alias) { assert null != c; SimpleFilter filter = SimpleFilter.createContainerFilter(c); diff --git a/wiki/src/org/labkey/wiki/view/wikiManage.jsp b/wiki/src/org/labkey/wiki/view/wikiManage.jsp index 1f56de5dce8..d2f1b48eccc 100644 --- a/wiki/src/org/labkey/wiki/view/wikiManage.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiManage.jsp @@ -30,6 +30,7 @@ <%@ page import="org.labkey.wiki.model.Wiki" %> <%@ page import="org.springframework.validation.Errors" %> <%@ page import="org.springframework.validation.FieldError" %> +<%@ page import="java.util.List" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> @@ -113,8 +114,7 @@
- + />
- + <%=button("Rename").style("width:100px").submit(true).onClick("return rename()") %>
<% - FieldError nameError = errors.getFieldError("name"); - if (null != nameError) + for (FieldError nameError : errors.getFieldErrors("name")) { %><% } @@ -230,24 +230,14 @@ %> -
<%=h(context.getMessage(nameError))%>
- - - - -
- <% - SelectBuilder aliasesBuilder = new SelectBuilder().name("aliases").id("aliases").size(5).addStyle("width:500px"); - bean.aliases.forEach(alias->aliasesBuilder.addOption(new OptionBuilder() - .value(String.valueOf(alias)) - .label(alias) - .build())); - %> - <%=aliasesBuilder%> - - <%= button("Delete Alias").style("width:100px;").submit(true).onClick("return orderModule('children', 0, 'childOrder')")%> -
- + 		+ + + + +
+ +
From 8436545e2f793af7ec4656584c890bc6cebd3623 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 24 Mar 2022 16:54:04 -0700 Subject: [PATCH 6/9] Make aliases case-insensitive (constraint, resolution, sorting). RowId -> PageRowId for clarity. Remove crufty, unused, and unnecessary code. --- announcements/resources/schemas/comm.xml | 2 +- .../postgresql/comm-22.001-22.002.sql | 5 ++ .../sqlserver/comm-22.001-22.002.sql | 1 + .../announcements/AnnouncementModule.java | 2 +- .../api/collections/LabKeyCollectors.java | 17 +++++++ wiki/src/org/labkey/wiki/WikiCollections.java | 13 +++-- wiki/src/org/labkey/wiki/WikiController.java | 50 +++---------------- wiki/src/org/labkey/wiki/WikiManager.java | 18 +++---- wiki/src/org/labkey/wiki/WikiModule.java | 4 +- wiki/src/org/labkey/wiki/view/wikiEdit.jsp | 2 +- wiki/src/org/labkey/wiki/view/wikiManage.jsp | 35 +------------ 11 files changed, 51 insertions(+), 98 deletions(-) create mode 100644 announcements/resources/schemas/dbscripts/postgresql/comm-22.001-22.002.sql create mode 100644 announcements/resources/schemas/dbscripts/sqlserver/comm-22.001-22.002.sql diff --git a/announcements/resources/schemas/comm.xml b/announcements/resources/schemas/comm.xml index 228b9d64004..902458f76ce 100644 --- a/announcements/resources/schemas/comm.xml +++ b/announcements/resources/schemas/comm.xml @@ -653,7 +653,7 @@ - +
diff --git a/announcements/resources/schemas/dbscripts/postgresql/comm-22.001-22.002.sql b/announcements/resources/schemas/dbscripts/postgresql/comm-22.001-22.002.sql new file mode 100644 index 00000000000..64b26571d57 --- /dev/null +++ b/announcements/resources/schemas/dbscripts/postgresql/comm-22.001-22.002.sql @@ -0,0 +1,5 @@ +ALTER TABLE comm.PageAliases RENAME COLUMN RowId TO PageRowId; + +-- Aliases should be case-insensitive +ALTER TABLE comm.PageAliases DROP CONSTRAINT PK_PageAliases; +CREATE UNIQUE INDEX UQ_PageAliases ON comm.PageAliases (Container, LOWER(Alias)); diff --git a/announcements/resources/schemas/dbscripts/sqlserver/comm-22.001-22.002.sql b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.001-22.002.sql new file mode 100644 index 00000000000..d129880aa7c --- /dev/null +++ b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.001-22.002.sql @@ -0,0 +1 @@ +EXEC sp_rename 'comm.PageAliases.RowId', 'PageRowId', 'COLUMN'; diff --git a/announcements/src/org/labkey/announcements/AnnouncementModule.java b/announcements/src/org/labkey/announcements/AnnouncementModule.java index 254a2709048..70136ab8d9b 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementModule.java +++ b/announcements/src/org/labkey/announcements/AnnouncementModule.java @@ -97,7 +97,7 @@ public String getName() @Override public @Nullable Double getSchemaVersion() { - return 22.001; + return 22.002; } @Override diff --git a/api/src/org/labkey/api/collections/LabKeyCollectors.java b/api/src/org/labkey/api/collections/LabKeyCollectors.java index cb89ad98513..417368b5fbf 100644 --- a/api/src/org/labkey/api/collections/LabKeyCollectors.java +++ b/api/src/org/labkey/api/collections/LabKeyCollectors.java @@ -47,6 +47,23 @@ public class LabKeyCollectors ); } + /** + * Returns a {@link Collector} that builds a {@link CaseInsensitiveHashMap} + */ + public static Collector> toCaseInsensitiveMap( + Function keyMapper, + Function valueMapper) + { + return toMap( + keyMapper, + valueMapper, + (u, v) -> { + throw new IllegalStateException(String.format("Duplicate key %s", u)); + }, + CaseInsensitiveHashMap::new + ); + } + /** * Returns a {@link Collector} that accumulates elements into a {@link MultiValuedMap} whose keys and values are the * result of applying the provided mapping functions to the input elements, an approach that mimics {@link Collectors#toMap(Function, Function)}. diff --git a/wiki/src/org/labkey/wiki/WikiCollections.java b/wiki/src/org/labkey/wiki/WikiCollections.java index 3ebbdb0620b..92b3988ab73 100644 --- a/wiki/src/org/labkey/wiki/WikiCollections.java +++ b/wiki/src/org/labkey/wiki/WikiCollections.java @@ -34,13 +34,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; /* * User: adam @@ -123,14 +123,18 @@ public WikiCollections(Container c) _adminNavTree = createNavTree(c, true); _nonAdminNavTree = createNavTree(c, false); - _aliasesByRowsId = new TableSelector(CommSchema.getInstance().getTableInfoPageAliases(), PageFlowUtil.set("Alias", "RowId"), SimpleFilter.createContainerFilter(c), null) + _aliasesByRowsId = new TableSelector(CommSchema.getInstance().getTableInfoPageAliases(), PageFlowUtil.set("Alias", "PageRowId"), SimpleFilter.createContainerFilter(c), null) .mapStream() - .collect(LabKeyCollectors.toMultiValuedMap(map->(Integer)map.get("RowId"), map->(String)map.get("Alias"))); + .map(map->new Alias((Integer)map.get("PageRowId"), (String)map.get("Alias"))) + .sorted(Comparator.comparing(Alias::alias, String.CASE_INSENSITIVE_ORDER)) + .collect(LabKeyCollectors.toMultiValuedMap(record->record.pageRowId, record->record.alias)); _namesByAlias = _aliasesByRowsId.entries().stream() .filter(e->_treesByRowId.get(e.getKey()) != null) // Just in case - ignore orphaned aliases - .collect(Collectors.toMap(Map.Entry::getValue, e->_treesByRowId.get(e.getKey()).getName())); + .collect(LabKeyCollectors.toCaseInsensitiveMap(Map.Entry::getValue, e->_treesByRowId.get(e.getKey()).getName())); } + public record Alias(int pageRowId, String alias) {} + private void populateWikiTree(WikiTree parent, MultiValuedMap childMap, Map treesByRowId) { Collection children = parent.getChildren(); @@ -263,6 +267,7 @@ private Set populateWikiTrees(WikiTree root, Set trees) return trees; } + // Ordered by alias (case-insensitive) Collection getAliases(int rowId) { return _aliasesByRowsId.get(rowId); diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index f475a585b34..2d2a8fb2a1b 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -123,7 +123,6 @@ public class WikiController extends SpringActionController { private static final Logger LOG = LogManager.getLogger(WikiController.class); private static final DefaultActionResolver _actionResolver = new DefaultActionResolver(WikiController.class); - private static final boolean SHOW_CHILD_REORDERING = false; public WikiController() { @@ -493,8 +492,7 @@ public ModelAndView getView(WikiManageForm form, boolean reshow, BindException e bean.pageNames = WikiSelectManager.getPageNames(getContainer()); bean.siblings = WikiSelectManager.getChildren(getContainer(), _wiki.getParent()); bean.possibleParents = WikiSelectManager.getPossibleParents(getContainer(), _wiki); - bean.showChildren = SHOW_CHILD_REORDERING; - bean.aliases = WikiSelectManager.getAliases(getContainer(), _wiki.getRowId()); + bean.aliases = new ArrayList<>(WikiSelectManager.getAliases(getContainer(), _wiki.getRowId())); JspView manageView = new JspView<>("/org/labkey/wiki/view/wikiManage.jsp", bean, errors); manageView.setTitle("Wiki Configuration"); @@ -509,7 +507,6 @@ public class ManageBean public List pageNames; public Collection siblings; public Set possibleParents; - public boolean showChildren; public Collection aliases; } @@ -526,8 +523,7 @@ public void validateCommand(WikiManageForm form, Errors errors) else if (newName != null && !newName.equalsIgnoreCase(originalName)) { // rename - check for existing wiki with this name - Container c = ContainerManager.getForPath(form.getContainerPath()); - if (WikiManager.wikiNameExists(c, newName)) + if (WikiManager.wikiNameExists(getContainer(), newName)) errors.rejectValue("name", ERROR_MSG, "A page with the name '" + newName + "' already exists in this folder. Please choose a different name."); _isRename = true; } @@ -592,7 +588,7 @@ public boolean handlePost(WikiManageForm form, BindException errors) if (!newAliases.equals(existingAliases)) { WikiManager mgr = WikiManager.get(); - mgr.deleteAliases(c, wiki, null); + mgr.deleteAliases(c, wiki); // Best effort for alias editing -- reshow with error message if duplicates are encountered, but // complete all other edits. @@ -602,13 +598,6 @@ public boolean handlePost(WikiManageForm form, BindException errors) } getWikiManager().updateWiki(getUser(), _wiki, _wikiVersion, false); - if (SHOW_CHILD_REORDERING) - { - int[] childOrder = form.getChildOrderArray(); - if (childOrder.length > 0) - updateDisplayOrder(_wiki.children(), childOrder); - } - int[] siblingOrder = form.getSiblingOrderArray(); if (siblingOrder.length > 0) @@ -1679,42 +1668,20 @@ public static class WikiManageForm private String _name; private String _title; private int _parent; - private String _childOrder; private String _siblingOrder; private String _nextAction; - private String _containerPath; private boolean _shouldIndex; private boolean _addAlias; private String _newName; private String _aliases; - public String getContainerPath() - { - return _containerPath; - } - - @SuppressWarnings({"UnusedDeclaration"}) - public void setContainerPath(String containerPath) - { - _containerPath = containerPath; - } - - public String getChildOrder() - { - return _childOrder; - } - - public void setChildOrder(String childIdList) - { - _childOrder = childIdList; - } - - public boolean isShouldIndex() + public boolean isShouldIndex() { return _shouldIndex; } - public void setShouldIndex(boolean shouldIndex) + @SuppressWarnings({"UnusedDeclaration"}) + public void setShouldIndex(boolean shouldIndex) { _shouldIndex = shouldIndex; } @@ -1750,11 +1717,6 @@ public int[] getSiblingOrderArray() return breakIdList(_siblingOrder); } - public int[] getChildOrderArray() - { - return breakIdList(_childOrder); - } - public String getName() { return _name; diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index 47895d30db1..74618fd4c29 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -93,7 +93,6 @@ import static org.labkey.api.action.SpringActionController.ERROR_MSG; - /** * User: mbellew * Date: Mar 10, 2005 @@ -317,7 +316,7 @@ private boolean updateWiki(User user, Wiki wikiNew, WikiVersion versionNew, bool public void addAlias(User user, Wiki wiki, String alias, BindException errors) { assert null != wiki.getContainerId(); - Map map = new HashMap<>(Map.of("Container", wiki.getContainerId(), "Alias", alias, "RowId", wiki.getRowId())); + Map map = new HashMap<>(Map.of("Container", wiki.getContainerId(), "Alias", alias, "PageRowId", wiki.getRowId())); try { Table.insert(user, CommSchema.getInstance().getTableInfoPageAliases(), map); @@ -325,21 +324,18 @@ public void addAlias(User user, Wiki wiki, String alias, BindException errors) catch (RuntimeSQLException e) { if (e.isConstraintException()) - errors.rejectValue("name", ERROR_MSG,"Alias '" + alias + "' already exists in this folder."); + errors.rejectValue("name", ERROR_MSG, "Alias '" + alias + "' already exists in this folder."); } } // null == wiki ==> delete all aliases in a container - // null != wiki && null == alias => delete all aliases associated with a wiki - // null != wiki && null != alias ==> delete a single alias associated with a wiki - public void deleteAliases(Container c, @Nullable Wiki wiki, @Nullable String alias) + // null != wiki ==> delete all aliases associated with a wiki + public void deleteAliases(Container c, @Nullable Wiki wiki) { assert null != c; SimpleFilter filter = SimpleFilter.createContainerFilter(c); if (null != wiki) - filter.addCondition(FieldKey.fromParts("RowId"), wiki.getRowId()); - if (null != alias) - filter.addCondition(FieldKey.fromParts("Alias"), alias); + filter.addCondition(FieldKey.fromParts("PageRowId"), wiki.getRowId()); Table.delete(CommSchema.getInstance().getTableInfoPageAliases(), filter); } @@ -359,7 +355,7 @@ public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubt new SimpleFilter(FieldKey.fromParts("pageentityId"), wiki.getEntityId())); Table.delete(comm.getTableInfoPages(), new SimpleFilter(FieldKey.fromParts("entityId"), wiki.getEntityId())); - deleteAliases(c, wiki, null); + deleteAliases(c, wiki); getAttachmentService().deleteAttachments(wiki.getAttachmentParent()); @@ -458,7 +454,7 @@ public void purgeContainer(Container c) { new SqlExecutor(comm.getSchema()).execute("UPDATE " + comm.getTableInfoPages() + " SET PageVersionId = NULL WHERE Container = ?", c.getId()); new SqlExecutor(comm.getSchema()).execute("DELETE FROM " + comm.getTableInfoPageVersions() + " WHERE PageEntityId IN (SELECT EntityId FROM " + comm.getTableInfoPages() + " WHERE Container = ?)", c.getId()); - deleteAliases(c, null, null); + deleteAliases(c, null); // Clear all wiki webpart properties that refer to this container. This includes wiki and wiki TOC // webparts in this and potentially other containers. #13937 diff --git a/wiki/src/org/labkey/wiki/WikiModule.java b/wiki/src/org/labkey/wiki/WikiModule.java index 43dfeba4797..8e2a7232f0f 100644 --- a/wiki/src/org/labkey/wiki/WikiModule.java +++ b/wiki/src/org/labkey/wiki/WikiModule.java @@ -91,9 +91,9 @@ protected void init() protected Collection createWebPartFactories() { return List.of( - new WikiWebPartFactory(), + new MenuWikiWebPartFactory(), new WikiTOCFactory(), - new MenuWikiWebPartFactory() + new WikiWebPartFactory() ); } diff --git a/wiki/src/org/labkey/wiki/view/wikiEdit.jsp b/wiki/src/org/labkey/wiki/view/wikiEdit.jsp index 63ada22f227..893e9a6ad5a 100644 --- a/wiki/src/org/labkey/wiki/view/wikiEdit.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiEdit.jsp @@ -121,7 +121,7 @@ - + diff --git a/wiki/src/org/labkey/wiki/view/wikiManage.jsp b/wiki/src/org/labkey/wiki/view/wikiManage.jsp index d2f1b48eccc..079499868e8 100644 --- a/wiki/src/org/labkey/wiki/view/wikiManage.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiManage.jsp @@ -30,7 +30,6 @@ <%@ page import="org.labkey.wiki.model.Wiki" %> <%@ page import="org.springframework.validation.Errors" %> <%@ page import="org.springframework.validation.FieldError" %> -<%@ page import="java.util.List" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> @@ -108,7 +107,6 @@ -
/>
- <% - if (bean.showChildren && wiki.hasChildren()) - { - %> - - - - - <% - } - %>
@@ -145,7 +143,7 @@ <% SelectBuilder parentBuilder = new SelectBuilder() .name("parent") - .id("id") + .id("parent") .addStyle("width:420px") .onChange("document.manage.nextAction.value = " + q(NextAction.manage.name()) + "; submit();"); parentBuilder.addOption(new OptionBuilder().value("-1").label("[none]").selected(wiki.getParent() == -1).build()); @@ -197,37 +195,6 @@
- - - - -
- <% - SelectBuilder childrenBuilder = new SelectBuilder().name("children").id("children").size(10).addStyle("width:500px"); - wiki.children().forEach(child->childrenBuilder.addOption(new OptionBuilder() - .value(String.valueOf(child.getRowId())) - .label(child.getLatestVersion().getTitle() + " (" + child.getName() + ")") - .build())); - %> - <%=childrenBuilder%> - - <%= button("Move Up").style("width:100px;").submit(true).onClick("return orderModule('children', 0, 'childOrder')")%> -
- <%= button("Move Down").style("width:100px;").submit(true).onClick("return orderModule('children', 1, 'childOrder')")%> -
- -
From 6ccafb409ab6a4629f7ec6a5fba9182785042b02 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 25 Mar 2022 10:55:48 -0700 Subject: [PATCH 7/9] Use UNIQUE INDEX on SQL Server as well; not required, but more consistent with PG. Introduce PermanentRedirectException and use it. Futz with aliases saving: be more tolerant of duplicates (just warn). Remove unused rename handling on edit page. --- .../sqlserver/comm-22.002-22.003.sql | 3 ++ .../announcements/AnnouncementModule.java | 2 +- .../org/labkey/api/util/ExceptionUtil.java | 18 +++++---- .../api/view/PermanentRedirectException.java | 20 ++++++++++ .../labkey/api/view/RedirectException.java | 13 +++++-- wiki/resources/web/wiki/internal/wikiEdit.js | 26 +------------ wiki/src/org/labkey/wiki/WikiController.java | 38 ++++++++++--------- wiki/src/org/labkey/wiki/WikiManager.java | 2 +- 8 files changed, 68 insertions(+), 54 deletions(-) create mode 100644 announcements/resources/schemas/dbscripts/sqlserver/comm-22.002-22.003.sql create mode 100644 api/src/org/labkey/api/view/PermanentRedirectException.java diff --git a/announcements/resources/schemas/dbscripts/sqlserver/comm-22.002-22.003.sql b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.002-22.003.sql new file mode 100644 index 00000000000..1866f84ce11 --- /dev/null +++ b/announcements/resources/schemas/dbscripts/sqlserver/comm-22.002-22.003.sql @@ -0,0 +1,3 @@ +-- Switch from PK to UNIQUE INDEX to match PostgreSQL +ALTER TABLE comm.PageAliases DROP CONSTRAINT PK_PageAliases; +CREATE UNIQUE INDEX UQ_PageAliases ON comm.PageAliases (Container, Alias); \ No newline at end of file diff --git a/announcements/src/org/labkey/announcements/AnnouncementModule.java b/announcements/src/org/labkey/announcements/AnnouncementModule.java index 70136ab8d9b..ca22276163f 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementModule.java +++ b/announcements/src/org/labkey/announcements/AnnouncementModule.java @@ -97,7 +97,7 @@ public String getName() @Override public @Nullable Double getSchemaVersion() { - return 22.002; + return 22.003; } @Override diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 349c90c3780..fade8238d53 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -632,10 +632,10 @@ static ActionURL handleException(@NotNull HttpServletRequest request, @NotNull H } // Do redirects before response.reset() otherwise we'll lose cookies (e.g., login page) - if (ex instanceof RedirectException) + if (ex instanceof RedirectException rex) { - String url = ((RedirectException) ex).getURL(); - doErrorRedirect(response, url); + String url = rex.getURL(); + doErrorRedirect(response, url, rex.getHttpStatusCode()); return null; } @@ -1000,10 +1000,16 @@ private static void addDependenciesAndRender(int responseStatus, PageConfig page errorView.getView().render(errorView.getModel(), request, response); } - + // Temporary redirect public static void doErrorRedirect(HttpServletResponse response, String url) { - response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY); + doErrorRedirect(response, url, HttpServletResponse.SC_MOVED_TEMPORARILY); + } + + // Pass in HTTP status code to designate temporary vs. permanent redirect + private static void doErrorRedirect(HttpServletResponse response, String url, int httpStatusCode) + { + response.setStatus(httpStatusCode); response.setDateHeader("Expires", 0); response.setHeader("Location", url); response.setContentType("text/html; charset=UTF-8"); @@ -1025,8 +1031,6 @@ public static void doErrorRedirect(HttpServletResponse response, String url) } } - - public enum ExceptionInfo { ResolveURL, // suggestion for where to fix this e.g. sourceQuery.view diff --git a/api/src/org/labkey/api/view/PermanentRedirectException.java b/api/src/org/labkey/api/view/PermanentRedirectException.java new file mode 100644 index 00000000000..48b830da933 --- /dev/null +++ b/api/src/org/labkey/api/view/PermanentRedirectException.java @@ -0,0 +1,20 @@ +package org.labkey.api.view; + +import org.jetbrains.annotations.NotNull; +import org.labkey.api.util.URLHelper; + +import javax.servlet.http.HttpServletResponse; + +public class PermanentRedirectException extends RedirectException +{ + public PermanentRedirectException(@NotNull URLHelper url) + { + super(url); + } + + @Override + public int getHttpStatusCode() + { + return HttpServletResponse.SC_MOVED_PERMANENTLY; + } +} diff --git a/api/src/org/labkey/api/view/RedirectException.java b/api/src/org/labkey/api/view/RedirectException.java index 36098d09d62..139be336642 100644 --- a/api/src/org/labkey/api/view/RedirectException.java +++ b/api/src/org/labkey/api/view/RedirectException.java @@ -19,13 +19,15 @@ import org.labkey.api.util.SkipMothershipLogging; import org.labkey.api.util.URLHelper; +import javax.servlet.http.HttpServletResponse; + /** - * When thrown in the context of an HTTP request, sends the client a redirect in the HTTP response. Not treated - * as a loggable error. + * When thrown in the context of an HTTP request, sends the client a *temporary* redirect in the HTTP response. Not + * treated as a loggable error. See {@link PermanentRedirectException} if a permanent redirect is desired. */ public class RedirectException extends RuntimeException implements SkipMothershipLogging { - String _url; + private final String _url; public RedirectException(@NotNull URLHelper url) { @@ -41,4 +43,9 @@ public String getURL() { return _url; } + + public int getHttpStatusCode() + { + return HttpServletResponse.SC_MOVED_TEMPORARILY; + } } diff --git a/wiki/resources/web/wiki/internal/wikiEdit.js b/wiki/resources/web/wiki/internal/wikiEdit.js index 688bf5adba6..1bbf38bde88 100644 --- a/wiki/resources/web/wiki/internal/wikiEdit.js +++ b/wiki/resources/web/wiki/internal/wikiEdit.js @@ -363,31 +363,9 @@ function tinyMceHandleEvent(evt) { window.location = _cancelUrl ? _cancelUrl : getRedirUrl(); }; + // Note: can't change an existing wiki's name var onChangeName = function() { - //if this is an existing page, warn the user about changing the name - if (_wikiProps.entityId) { - getExt4(function() { - Ext4.Msg.show({ - title: 'Warning', - msg: "Changing the name of this page will break any links to this page embedded in other pages. Are you sure you want to change the name?", - buttons: Ext4.MessageBox.YESNO, - icon: Ext4.MessageBox.WARNING, - fn: function(btnId) { - if (btnId == "yes") { - LABKEY.setDirty(true); - _redirUrl = ''; // clear the redir URL since it will be referring to the old name - onSave(); - } - else { - updateControl("name", _wikiProps.name); - } - } - }); - }); - } - else { - LABKEY.setDirty(true); - } + LABKEY.setDirty(true); }; var onConvertSuccess = function(response) { diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 2d2a8fb2a1b..22c6491e9b8 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -75,6 +75,7 @@ import org.labkey.api.view.NavTree; import org.labkey.api.view.NavTreeManager; import org.labkey.api.view.NotFoundException; +import org.labkey.api.view.PermanentRedirectException; import org.labkey.api.view.Portal; import org.labkey.api.view.RedirectException; import org.labkey.api.view.UnauthorizedException; @@ -570,32 +571,33 @@ public boolean handlePost(WikiManageForm form, BindException errors) _wikiVersion = null; } - // If renaming and alias is requested, add it now since updateWiki() will uncache and reindex + List newAliases = new ArrayList<>(); + + // Update aliases if (_isRename && form.isAddAlias()) { - getWikiManager().addAlias(getUser(), _wiki, originalName, errors); - if (errors.hasErrors()) - return false; + newAliases.add(originalName); } if (null != form.getAliases()) { - Collection existingAliases = WikiSelectManager.getAliases(getContainer(), _wiki.getRowId()); - Collection newAliases = Arrays.stream(form.getAliases().split("\n")) + Arrays.stream(form.getAliases().split("\n")) .map(StringUtils::trimToNull) .filter(Objects::nonNull) - .collect(Collectors.toCollection(ArrayList::new)); - - if (!newAliases.equals(existingAliases)) - { - WikiManager mgr = WikiManager.get(); - mgr.deleteAliases(c, wiki); + .forEach(newAliases::add); + newAliases.sort(String.CASE_INSENSITIVE_ORDER); + } + Collection existingAliases = WikiSelectManager.getAliases(getContainer(), _wiki.getRowId()); + if (!newAliases.equals(existingAliases)) + { + WikiManager mgr = WikiManager.get(); + mgr.deleteAliases(c, wiki); - // Best effort for alias editing -- reshow with error message if duplicates are encountered, but - // complete all other edits. - newAliases.forEach(alias->mgr.addAlias(getUser(), _wiki, alias, errors)); - WikiCache.uncache(c, wiki, true); - } + // Best effort for alias editing -- reshow with error message if duplicates are encountered, but + // regardless of errors, complete all other edits. + newAliases.forEach(alias->mgr.addAlias(getUser(), _wiki, alias, errors)); + WikiCache.uncache(c, wiki, true); } + getWikiManager().updateWiki(getUser(), _wiki, _wikiVersion, false); int[] siblingOrder = form.getSiblingOrderArray(); @@ -1187,7 +1189,7 @@ public ModelAndView getView(WikiNameForm form, BindException errors) if (null != realName) { LOG.debug("PageAction: requested wiki name, \"" + name + "\", is an alias; redirecting to \"" + realName + "\". Referrer: " + getViewContext().getRequest().getHeader("Referer")); - throw new RedirectException(getViewContext().getActionURL().clone().replaceParameter("name", realName)); + throw new PermanentRedirectException(getViewContext().getActionURL().clone().replaceParameter("name", realName)); } _wiki = new Wiki(getContainer(), name); diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index 74618fd4c29..11340273d37 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -324,7 +324,7 @@ public void addAlias(User user, Wiki wiki, String alias, BindException errors) catch (RuntimeSQLException e) { if (e.isConstraintException()) - errors.rejectValue("name", ERROR_MSG, "Alias '" + alias + "' already exists in this folder."); + errors.rejectValue("name", ERROR_MSG, "Warning: Alias '" + alias + "' already exists in this folder."); } } From 3bbd9b13bd2f7942507941ef78af485ea8cee61f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 28 Mar 2022 11:57:26 -0700 Subject: [PATCH 8/9] Update api/src/org/labkey/api/view/PermanentRedirectException.java Co-authored-by: Josh Eckels --- api/src/org/labkey/api/view/PermanentRedirectException.java | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/org/labkey/api/view/PermanentRedirectException.java b/api/src/org/labkey/api/view/PermanentRedirectException.java index 48b830da933..8452f3d9f36 100644 --- a/api/src/org/labkey/api/view/PermanentRedirectException.java +++ b/api/src/org/labkey/api/view/PermanentRedirectException.java @@ -5,6 +5,7 @@ import javax.servlet.http.HttpServletResponse; +/** Use when we want search engines, browsers, etc to assume that the redirecting URL is defunct and the target URL should be used going forward */ public class PermanentRedirectException extends RedirectException { public PermanentRedirectException(@NotNull URLHelper url) From 43a3a8a6d4d2cac4e347e979d8e80c947e76cb59 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 28 Mar 2022 13:24:48 -0700 Subject: [PATCH 9/9] Simplify name change handling --- wiki/resources/web/wiki/internal/wikiEdit.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/wiki/resources/web/wiki/internal/wikiEdit.js b/wiki/resources/web/wiki/internal/wikiEdit.js index 1bbf38bde88..e8ee92ce34f 100644 --- a/wiki/resources/web/wiki/internal/wikiEdit.js +++ b/wiki/resources/web/wiki/internal/wikiEdit.js @@ -172,7 +172,7 @@ function tinyMceHandleEvent(evt) { var bindControls = function(props) { // form controls var setDirty = function(){LABKEY.setDirty(true)}; - $(_idSel + 'name').keypress(setDirty).change(onChangeName); + $(_idSel + 'name').keypress(setDirty).change(setDirty); $(_idSel + 'title').keypress(setDirty).change(setDirty); $(_idSel + 'parent').keypress(setDirty).change(setDirty); $(_idSel + 'body').keypress(setDirty).change(setDirty); @@ -363,11 +363,6 @@ function tinyMceHandleEvent(evt) { window.location = _cancelUrl ? _cancelUrl : getRedirUrl(); }; - // Note: can't change an existing wiki's name - var onChangeName = function() { - LABKEY.setDirty(true); - }; - var onConvertSuccess = function(response) { var respJson = LABKEY.Utils.decode(response.responseText);