Gracefully deal with missing repository in permissions ui (issue 155)

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<RepositoryModel> repositories, List<RegistrantAccessPermission> permissions) {

+		Map<String, RepositoryModel> repoMap = new HashMap<String, RepositoryModel>();

 		List<String> restricted = new ArrayList<String>();

 		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<RegistrantAccessPermission> 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<TeamModel>(

 				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<Regi
 		return PermissionType.OWNER.equals(permissionType);
 	}
 
+	public boolean isMissing() {
+		return PermissionType.MISSING.equals(permissionType);
+	}
+	
 	@Override
 	public int compareTo(RegistrantAccessPermission p) {
 		switch (registrantType) {
diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java
index 7742d5d9..0c9b9cc1 100644
--- a/src/com/gitblit/models/UserModel.java
+++ b/src/com/gitblit/models/UserModel.java
@@ -154,10 +154,6 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
 				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;
 				}