Check canAbandon and canRestore with PermissionBackend
Change-Id: I22bccfbc3238912521b8e41d83a86cab309a51ea
This commit is contained in:
		 Shawn Pearce
					Shawn Pearce
				
			
				
					committed by
					
						 David Pursehouse
						David Pursehouse
					
				
			
			
				
	
			
			
			 David Pursehouse
						David Pursehouse
					
				
			
						parent
						
							3008aac922
						
					
				
				
					commit
					b665f7d455
				
			| @@ -244,7 +244,7 @@ class ChangeApiImpl implements ChangeApi { | |||||||
|   public void abandon(AbandonInput in) throws RestApiException { |   public void abandon(AbandonInput in) throws RestApiException { | ||||||
|     try { |     try { | ||||||
|       abandon.apply(change, in); |       abandon.apply(change, in); | ||||||
|     } catch (OrmException | UpdateException e) { |     } catch (OrmException | UpdateException | PermissionBackendException e) { | ||||||
|       throw new RestApiException("Cannot abandon change", e); |       throw new RestApiException("Cannot abandon change", e); | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| @@ -258,7 +258,7 @@ class ChangeApiImpl implements ChangeApi { | |||||||
|   public void restore(RestoreInput in) throws RestApiException { |   public void restore(RestoreInput in) throws RestApiException { | ||||||
|     try { |     try { | ||||||
|       restore.apply(change, in); |       restore.apply(change, in); | ||||||
|     } catch (OrmException | UpdateException e) { |     } catch (OrmException | UpdateException | PermissionBackendException e) { | ||||||
|       throw new RestApiException("Cannot restore change", e); |       throw new RestApiException("Cannot restore change", e); | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|   | |||||||
| @@ -21,7 +21,6 @@ import com.google.gerrit.extensions.api.changes.AbandonInput; | |||||||
| import com.google.gerrit.extensions.api.changes.NotifyHandling; | import com.google.gerrit.extensions.api.changes.NotifyHandling; | ||||||
| import com.google.gerrit.extensions.api.changes.RecipientType; | import com.google.gerrit.extensions.api.changes.RecipientType; | ||||||
| import com.google.gerrit.extensions.common.ChangeInfo; | import com.google.gerrit.extensions.common.ChangeInfo; | ||||||
| import com.google.gerrit.extensions.restapi.AuthException; |  | ||||||
| import com.google.gerrit.extensions.restapi.ResourceConflictException; | import com.google.gerrit.extensions.restapi.ResourceConflictException; | ||||||
| import com.google.gerrit.extensions.restapi.RestApiException; | import com.google.gerrit.extensions.restapi.RestApiException; | ||||||
| import com.google.gerrit.extensions.restapi.RestModifyView; | import com.google.gerrit.extensions.restapi.RestModifyView; | ||||||
| @@ -32,6 +31,8 @@ import com.google.gerrit.reviewdb.client.Project; | |||||||
| import com.google.gerrit.reviewdb.server.ReviewDb; | import com.google.gerrit.reviewdb.server.ReviewDb; | ||||||
| import com.google.gerrit.server.CurrentUser; | import com.google.gerrit.server.CurrentUser; | ||||||
| import com.google.gerrit.server.git.AbandonOp; | import com.google.gerrit.server.git.AbandonOp; | ||||||
|  | import com.google.gerrit.server.permissions.ChangePermission; | ||||||
|  | import com.google.gerrit.server.permissions.PermissionBackendException; | ||||||
| import com.google.gerrit.server.project.ChangeControl; | import com.google.gerrit.server.project.ChangeControl; | ||||||
| import com.google.gerrit.server.update.BatchUpdate; | import com.google.gerrit.server.update.BatchUpdate; | ||||||
| import com.google.gerrit.server.update.UpdateException; | import com.google.gerrit.server.update.UpdateException; | ||||||
| @@ -40,14 +41,10 @@ import com.google.inject.Inject; | |||||||
| import com.google.inject.Provider; | import com.google.inject.Provider; | ||||||
| import com.google.inject.Singleton; | import com.google.inject.Singleton; | ||||||
| import java.util.Collection; | import java.util.Collection; | ||||||
| import org.slf4j.Logger; |  | ||||||
| import org.slf4j.LoggerFactory; |  | ||||||
|  |  | ||||||
| @Singleton | @Singleton | ||||||
| public class Abandon | public class Abandon | ||||||
|     implements RestModifyView<ChangeResource, AbandonInput>, UiAction<ChangeResource> { |     implements RestModifyView<ChangeResource, AbandonInput>, UiAction<ChangeResource> { | ||||||
|   private static final Logger log = LoggerFactory.getLogger(Abandon.class); |  | ||||||
|  |  | ||||||
|   private final Provider<ReviewDb> dbProvider; |   private final Provider<ReviewDb> dbProvider; | ||||||
|   private final ChangeJson.Factory json; |   private final ChangeJson.Factory json; | ||||||
|   private final BatchUpdate.Factory batchUpdateFactory; |   private final BatchUpdate.Factory batchUpdateFactory; | ||||||
| @@ -70,14 +67,15 @@ public class Abandon | |||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public ChangeInfo apply(ChangeResource req, AbandonInput input) |   public ChangeInfo apply(ChangeResource req, AbandonInput input) | ||||||
|       throws RestApiException, UpdateException, OrmException { |       throws RestApiException, UpdateException, OrmException, PermissionBackendException { | ||||||
|     ChangeControl control = req.getControl(); |     req.permissions().database(dbProvider).check(ChangePermission.ABANDON); | ||||||
|     if (!control.canAbandon(dbProvider.get())) { |  | ||||||
|       throw new AuthException("abandon not permitted"); |  | ||||||
|     } |  | ||||||
|     Change change = |     Change change = | ||||||
|         abandon( |         abandon( | ||||||
|             control, input.message, input.notify, notifyUtil.resolveAccounts(input.notifyDetails)); |             req.getControl(), | ||||||
|  |             input.message, | ||||||
|  |             input.notify, | ||||||
|  |             notifyUtil.resolveAccounts(input.notifyDetails)); | ||||||
|     return json.noOptions().format(change); |     return json.noOptions().format(change); | ||||||
|   } |   } | ||||||
|  |  | ||||||
| @@ -159,19 +157,14 @@ public class Abandon | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public UiAction.Description getDescription(ChangeResource resource) { |   public UiAction.Description getDescription(ChangeResource rsrc) { | ||||||
|     boolean canAbandon = false; |     Change change = rsrc.getChange(); | ||||||
|     try { |  | ||||||
|       canAbandon = resource.getControl().canAbandon(dbProvider.get()); |  | ||||||
|     } catch (OrmException e) { |  | ||||||
|       log.error("Cannot check canAbandon status. Assuming false.", e); |  | ||||||
|     } |  | ||||||
|     return new UiAction.Description() |     return new UiAction.Description() | ||||||
|         .setLabel("Abandon") |         .setLabel("Abandon") | ||||||
|         .setTitle("Abandon the change") |         .setTitle("Abandon the change") | ||||||
|         .setVisible( |         .setVisible( | ||||||
|             resource.getChange().getStatus().isOpen() |             change.getStatus().isOpen() | ||||||
|                 && resource.getChange().getStatus() != Change.Status.DRAFT |                 && change.getStatus() != Change.Status.DRAFT | ||||||
|                 && canAbandon); |                 && rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.ABANDON)); | ||||||
|   } |   } | ||||||
| } | } | ||||||
|   | |||||||
| @@ -18,7 +18,6 @@ import com.google.common.base.Strings; | |||||||
| import com.google.gerrit.common.TimeUtil; | import com.google.gerrit.common.TimeUtil; | ||||||
| import com.google.gerrit.extensions.api.changes.RestoreInput; | import com.google.gerrit.extensions.api.changes.RestoreInput; | ||||||
| import com.google.gerrit.extensions.common.ChangeInfo; | import com.google.gerrit.extensions.common.ChangeInfo; | ||||||
| import com.google.gerrit.extensions.restapi.AuthException; |  | ||||||
| import com.google.gerrit.extensions.restapi.ResourceConflictException; | import com.google.gerrit.extensions.restapi.ResourceConflictException; | ||||||
| import com.google.gerrit.extensions.restapi.RestApiException; | import com.google.gerrit.extensions.restapi.RestApiException; | ||||||
| import com.google.gerrit.extensions.restapi.RestModifyView; | import com.google.gerrit.extensions.restapi.RestModifyView; | ||||||
| @@ -34,6 +33,8 @@ import com.google.gerrit.server.extensions.events.ChangeRestored; | |||||||
| import com.google.gerrit.server.mail.send.ReplyToChangeSender; | import com.google.gerrit.server.mail.send.ReplyToChangeSender; | ||||||
| import com.google.gerrit.server.mail.send.RestoredSender; | import com.google.gerrit.server.mail.send.RestoredSender; | ||||||
| import com.google.gerrit.server.notedb.ChangeUpdate; | import com.google.gerrit.server.notedb.ChangeUpdate; | ||||||
|  | import com.google.gerrit.server.permissions.ChangePermission; | ||||||
|  | import com.google.gerrit.server.permissions.PermissionBackendException; | ||||||
| import com.google.gerrit.server.project.ChangeControl; | import com.google.gerrit.server.project.ChangeControl; | ||||||
| import com.google.gerrit.server.update.BatchUpdate; | import com.google.gerrit.server.update.BatchUpdate; | ||||||
| import com.google.gerrit.server.update.BatchUpdateOp; | import com.google.gerrit.server.update.BatchUpdateOp; | ||||||
| @@ -80,12 +81,10 @@ public class Restore | |||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public ChangeInfo apply(ChangeResource req, RestoreInput input) |   public ChangeInfo apply(ChangeResource req, RestoreInput input) | ||||||
|       throws RestApiException, UpdateException, OrmException { |       throws RestApiException, UpdateException, OrmException, PermissionBackendException { | ||||||
|     ChangeControl ctl = req.getControl(); |     req.permissions().database(dbProvider).check(ChangePermission.RESTORE); | ||||||
|     if (!ctl.canRestore(dbProvider.get())) { |  | ||||||
|       throw new AuthException("restore not permitted"); |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|  |     ChangeControl ctl = req.getControl(); | ||||||
|     Op op = new Op(input); |     Op op = new Op(input); | ||||||
|     try (BatchUpdate u = |     try (BatchUpdate u = | ||||||
|         batchUpdateFactory.create( |         batchUpdateFactory.create( | ||||||
| @@ -150,17 +149,13 @@ public class Restore | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public UiAction.Description getDescription(ChangeResource resource) { |   public UiAction.Description getDescription(ChangeResource rsrc) { | ||||||
|     boolean canRestore = false; |  | ||||||
|     try { |  | ||||||
|       canRestore = resource.getControl().canRestore(dbProvider.get()); |  | ||||||
|     } catch (OrmException e) { |  | ||||||
|       log.error("Cannot check canRestore status. Assuming false.", e); |  | ||||||
|     } |  | ||||||
|     return new UiAction.Description() |     return new UiAction.Description() | ||||||
|         .setLabel("Restore") |         .setLabel("Restore") | ||||||
|         .setTitle("Restore the change") |         .setTitle("Restore the change") | ||||||
|         .setVisible(resource.getChange().getStatus() == Status.ABANDONED && canRestore); |         .setVisible( | ||||||
|  |             rsrc.getChange().getStatus() == Status.ABANDONED | ||||||
|  |                 && rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.RESTORE)); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private static String status(Change change) { |   private static String status(Change change) { | ||||||
|   | |||||||
| @@ -246,7 +246,7 @@ public class ChangeControl { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   /** Can this user abandon this change? */ |   /** Can this user abandon this change? */ | ||||||
|   public boolean canAbandon(ReviewDb db) throws OrmException { |   private boolean canAbandon(ReviewDb db) throws OrmException { | ||||||
|     return (isOwner() // owner (aka creator) of the change can abandon |     return (isOwner() // owner (aka creator) of the change can abandon | ||||||
|             || getRefControl().isOwner() // branch owner can abandon |             || getRefControl().isOwner() // branch owner can abandon | ||||||
|             || getProjectControl().isOwner() // project owner can abandon |             || getProjectControl().isOwner() // project owner can abandon | ||||||
| @@ -291,7 +291,7 @@ public class ChangeControl { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   /** Can this user restore this change? */ |   /** Can this user restore this change? */ | ||||||
|   public boolean canRestore(ReviewDb db) throws OrmException { |   private boolean canRestore(ReviewDb db) throws OrmException { | ||||||
|     return canAbandon(db) // Anyone who can abandon the change can restore it back |     return canAbandon(db) // Anyone who can abandon the change can restore it back | ||||||
|         && getRefControl().canUpload(); // as long as you can upload too |         && getRefControl().canUpload(); // as long as you can upload too | ||||||
|   } |   } | ||||||
|   | |||||||
| @@ -402,7 +402,7 @@ public class RefControl { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   /** @return true if this user can abandon a change for this ref */ |   /** @return true if this user can abandon a change for this ref */ | ||||||
|   public boolean canAbandon() { |   boolean canAbandon() { | ||||||
|     return canPerform(Permission.ABANDON); |     return canPerform(Permission.ABANDON); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user