From 7ba85bfa11c7fcab21ada61650fe30763aafd7b0 Mon Sep 17 00:00:00 2001 From: James Moger Date: Thu, 1 Nov 2012 09:12:55 -0400 Subject: [PATCH] Gracefully deal with missing repository in permissions ui (issue 155) --- src/com/gitblit/Constants.java | 4 +-- src/com/gitblit/GitBlit.java | 5 +++- src/com/gitblit/client/EditUserDialog.java | 26 +++++++++++++++++++ .../client/RegistrantPermissionsPanel.java | 9 +++++-- src/com/gitblit/client/UsersPanel.java | 23 +--------------- .../models/RegistrantAccessPermission.java | 4 +++ src/com/gitblit/models/UserModel.java | 4 --- .../gitblit/wicket/GitBlitWebApp.properties | 4 ++- .../gitblit/wicket/pages/EditUserPage.java | 4 +-- .../panels/RegistrantPermissionsPanel.java | 15 ++++++++--- 10 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/com/gitblit/Constants.java b/src/com/gitblit/Constants.java index 426d2df0..8a3ec989 100644 --- a/src/com/gitblit/Constants.java +++ b/src/com/gitblit/Constants.java @@ -321,7 +321,7 @@ public class Constants { * The access permissions available for a repository. */ public static enum AccessPermission { - NONE("N"), EXCLUDE("X"), VIEW("V"), CLONE("R"), PUSH("RW"), CREATE("RWC"), DELETE("RWD"), REWIND("RW+"); + NONE("N"), EXCLUDE("X"), VIEW("V"), CLONE("R"), PUSH("RW"), CREATE("RWC"), DELETE("RWD"), REWIND("RW+"), OWNER("RW+"); public static final AccessPermission [] NEWPERMISSIONS = { EXCLUDE, VIEW, CLONE, PUSH, CREATE, DELETE, REWIND }; @@ -387,7 +387,7 @@ public class Constants { } public static enum PermissionType { - EXPLICIT, OWNER, ADMINISTRATOR, TEAM, REGEX; + MISSING, EXPLICIT, TEAM, REGEX, OWNER, ADMINISTRATOR; } public static enum GCStatus { diff --git a/src/com/gitblit/GitBlit.java b/src/com/gitblit/GitBlit.java index eccd7c11..2c5545ba 100644 --- a/src/com/gitblit/GitBlit.java +++ b/src/com/gitblit/GitBlit.java @@ -79,7 +79,6 @@ import com.gitblit.Constants.AuthorizationControl; import com.gitblit.Constants.FederationRequest; import com.gitblit.Constants.FederationStrategy; import com.gitblit.Constants.FederationToken; -import com.gitblit.Constants.PermissionType; import com.gitblit.models.FederationModel; import com.gitblit.models.FederationProposal; import com.gitblit.models.FederationSet; @@ -2204,6 +2203,8 @@ public class GitBlit implements ServletContextListener { case PULL_SETTINGS: case PULL_SCRIPTS: return token.equals(all); + default: + break; } return false; } @@ -2347,6 +2348,8 @@ public class GitBlit implements ServletContextListener { url = model.origin; } break; + default: + break; } if (federationSets.containsKey(token)) { diff --git a/src/com/gitblit/client/EditUserDialog.java b/src/com/gitblit/client/EditUserDialog.java index 070926dd..e954fed6 100644 --- a/src/com/gitblit/client/EditUserDialog.java +++ b/src/com/gitblit/client/EditUserDialog.java @@ -27,8 +27,10 @@ import java.awt.event.KeyEvent; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.swing.ImageIcon; @@ -47,6 +49,7 @@ import javax.swing.KeyStroke; import com.gitblit.Constants.AccessRestrictionType; import com.gitblit.Constants.AuthorizationControl; +import com.gitblit.Constants.PermissionType; import com.gitblit.Constants.RegistrantType; import com.gitblit.Keys; import com.gitblit.models.RegistrantAccessPermission; @@ -343,6 +346,7 @@ public class EditUserDialog extends JDialog { } public void setRepositories(List repositories, List permissions) { + Map repoMap = new HashMap(); List restricted = new ArrayList(); for (RepositoryModel repo : repositories) { // exclude Owner or personal repositories @@ -352,6 +356,7 @@ public class EditUserDialog extends JDialog { restricted.add(repo.name); } } + repoMap.put(repo.name.toLowerCase(), repo); } StringUtils.sortRepositorynames(restricted); @@ -381,6 +386,27 @@ public class EditUserDialog extends JDialog { list.remove(rp.registrant.toLowerCase()); } } + + // update owner and missing permissions for editing + for (RegistrantAccessPermission permission : permissions) { + if (permission.mutable && PermissionType.EXPLICIT.equals(permission.permissionType)) { + // Ensure this is NOT an owner permission - which is non-editable + // We don't know this from within the usermodel, ownership is a + // property of a repository. + RepositoryModel rm = repoMap.get(permission.registrant.toLowerCase()); + if (rm == null) { + permission.permissionType = PermissionType.MISSING; + permission.mutable = false; + continue; + } + boolean isOwner = rm.isOwner(username); + if (isOwner) { + permission.permissionType = PermissionType.OWNER; + permission.mutable = false; + } + } + } + repositoryPalette.setObjects(list, permissions); } diff --git a/src/com/gitblit/client/RegistrantPermissionsPanel.java b/src/com/gitblit/client/RegistrantPermissionsPanel.java index c28724c7..ef04a876 100644 --- a/src/com/gitblit/client/RegistrantPermissionsPanel.java +++ b/src/com/gitblit/client/RegistrantPermissionsPanel.java @@ -198,8 +198,13 @@ public class RegistrantPermissionsPanel extends JPanel { setToolTipText(MessageFormat.format(Translation.get("gb.regexPermission"), ap.source)); break; default: - setText(""); - setToolTipText(null); + if (ap.isMissing()) { + setText(Translation.get("gb.missing")); + setToolTipText(Translation.get("gb.missingPermission")); + } else { + setText(""); + setToolTipText(null); + } break; } } diff --git a/src/com/gitblit/client/UsersPanel.java b/src/com/gitblit/client/UsersPanel.java index 2c236958..469d9536 100644 --- a/src/com/gitblit/client/UsersPanel.java +++ b/src/com/gitblit/client/UsersPanel.java @@ -25,7 +25,6 @@ import java.awt.event.KeyEvent; import java.awt.event.MouseAdapter; import java.awt.event.MouseEvent; import java.io.IOException; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.List; @@ -41,6 +40,7 @@ import javax.swing.event.ListSelectionEvent; import javax.swing.event.ListSelectionListener; import javax.swing.table.TableRowSorter; +import com.gitblit.Constants.AccessPermission; import com.gitblit.Constants.PermissionType; import com.gitblit.Constants.RpcRequest; import com.gitblit.models.RegistrantAccessPermission; @@ -313,27 +313,6 @@ public abstract class UsersPanel extends JPanel { gitblit.getSettings()); dialog.setLocationRelativeTo(UsersPanel.this); dialog.setUsers(gitblit.getUsers()); - - List permissions = user.getRepositoryPermissions(); - for (RegistrantAccessPermission permission : permissions) { - if (permission.mutable && PermissionType.EXPLICIT.equals(permission.permissionType)) { - // Ensure this is NOT an owner permission - which is non-editable - // We don't know this from within the usermodel, ownership is a - // property of a repository. - RepositoryModel rm = gitblit.getRepository(permission.registrant); - if (rm == null) { - System.out.println(MessageFormat.format("{0}: failed to find registrant repository {1}", - getClass().getSimpleName(), permission.registrant)); - continue; - } - boolean isOwner = rm.isOwner(user.username); - if (isOwner) { - permission.permissionType = PermissionType.OWNER; - permission.mutable = false; - } - } - } - dialog.setRepositories(gitblit.getRepositories(), user.getRepositoryPermissions()); dialog.setTeams(gitblit.getTeams(), user.teams == null ? null : new ArrayList( user.teams)); diff --git a/src/com/gitblit/models/RegistrantAccessPermission.java b/src/com/gitblit/models/RegistrantAccessPermission.java index 0b28d197..4bdc2da4 100644 --- a/src/com/gitblit/models/RegistrantAccessPermission.java +++ b/src/com/gitblit/models/RegistrantAccessPermission.java @@ -64,6 +64,10 @@ public class RegistrantAccessPermission implements Serializable, Comparable pType = PermissionType.REGEX; source = registrant; } - if (AccessPermission.MISSING.equals(entry.getValue())) { - // repository can not be found, permission is not editable - editable = false; - } list.add(new RegistrantAccessPermission(registrant, entry.getValue(), pType, RegistrantType.REPOSITORY, source, editable)); } Collections.sort(list); diff --git a/src/com/gitblit/wicket/GitBlitWebApp.properties b/src/com/gitblit/wicket/GitBlitWebApp.properties index 1f338267..4303b134 100644 --- a/src/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/com/gitblit/wicket/GitBlitWebApp.properties @@ -368,4 +368,6 @@ gb.ownerPermission = repository owner gb.administrator = admin gb.administratorPermission = Gitblit administrator gb.team = team -gb.teamPermission = permission set by \"{0}\" team membership \ No newline at end of file +gb.teamPermission = permission set by \"{0}\" team membership +gb.missing = missing! +gb.missingPermission = the repository for this permission is missing! \ No newline at end of file diff --git a/src/com/gitblit/wicket/pages/EditUserPage.java b/src/com/gitblit/wicket/pages/EditUserPage.java index 45de1bef..ea92293e 100644 --- a/src/com/gitblit/wicket/pages/EditUserPage.java +++ b/src/com/gitblit/wicket/pages/EditUserPage.java @@ -33,7 +33,6 @@ import org.apache.wicket.model.CompoundPropertyModel; import org.apache.wicket.model.Model; import org.apache.wicket.model.util.CollectionModel; import org.apache.wicket.model.util.ListModel; -import org.slf4j.LoggerFactory; import com.gitblit.Constants.PermissionType; import com.gitblit.Constants.RegistrantType; @@ -113,7 +112,8 @@ public class EditUserPage extends RootSubPage { // property of a repository. RepositoryModel rm = GitBlit.self().getRepositoryModel(permission.registrant); if (rm == null) { - LoggerFactory.getLogger(getClass()).error("Missing repository " + permission.registrant); + permission.permissionType = PermissionType.MISSING; + permission.mutable = false; continue; } boolean isOwner = rm.isOwner(oldName); diff --git a/src/com/gitblit/wicket/panels/RegistrantPermissionsPanel.java b/src/com/gitblit/wicket/panels/RegistrantPermissionsPanel.java index a34ac71f..9431df8f 100644 --- a/src/com/gitblit/wicket/panels/RegistrantPermissionsPanel.java +++ b/src/com/gitblit/wicket/panels/RegistrantPermissionsPanel.java @@ -89,14 +89,14 @@ public class RegistrantPermissionsPanel extends BasePanel { final RegistrantAccessPermission entry = item.getModelObject(); if (RegistrantType.REPOSITORY.equals(entry.registrantType)) { String repoName = StringUtils.stripDotGit(entry.registrant); - if (StringUtils.findInvalidCharacter(repoName) == null) { + if (!entry.isMissing() && StringUtils.findInvalidCharacter(repoName) == null) { // repository, strip .git and show swatch Label registrant = new Label("registrant", repoName); WicketUtils.setCssClass(registrant, "repositorySwatch"); WicketUtils.setCssBackground(registrant, repoName); item.add(registrant); } else { - // likely a regex + // regex or missing Label label = new Label("registrant", entry.registrant); WicketUtils.setCssStyle(label, "font-weight: bold;"); item.add(label); @@ -147,7 +147,16 @@ public class RegistrantPermissionsPanel extends BasePanel { item.add(regex); break; default: - item.add(new Label("pType", "").setVisible(false)); + if (entry.isMissing()) { + // repository is missing, this permission will be removed on save + Label missing = new Label("pType", getString("gb.missing")); + WicketUtils.setCssClass(missing, "label label-important"); + WicketUtils.setHtmlTooltip(missing, getString("gb.missingPermission")); + item.add(missing); + } else { + // standard permission + item.add(new Label("pType", "").setVisible(false)); + } break; } -- 2.39.5