Add circular dependency check for tickets,ticket/235

 and refactor to get repositoryModel always from the page

3 files changed, 67 insertions, 18 deletions

diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
index 61e12878..6b090629 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
@@ -42,6 +42,7 @@ import com.gitblit.Constants;
 import com.gitblit.Constants.AccessPermission;

 import com.gitblit.Constants.AuthorizationControl;

 import com.gitblit.models.RegistrantAccessPermission;

+import com.gitblit.models.RepositoryModel;

 import com.gitblit.models.TicketModel;

 import com.gitblit.models.TicketModel.Change;

 import com.gitblit.models.TicketModel.Field;

@@ -152,7 +153,13 @@ public class EditTicketPage extends RepositoryPage {
 

 		form.setOutputMarkupId(true);

 

-		form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, getRepositoryModel()));

+		form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, ticket.number) {

+			private static final long serialVersionUID = 1L;

+			@Override

+			protected RepositoryModel getRepositoryModel() {

+				return EditTicketPage.this.getRepositoryModel();

+			}

+		});

 

 		final IModel<String> markdownPreviewModel = Model.of(ticket.body == null ? "" : ticket.body);

 		descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);

diff --git a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
index 3722be10..011f4b68 100644
--- a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
@@ -39,6 +39,7 @@ import com.gitblit.Constants;
 import com.gitblit.Constants.AccessPermission;

 import com.gitblit.Constants.AuthorizationControl;

 import com.gitblit.models.RegistrantAccessPermission;

+import com.gitblit.models.RepositoryModel;

 import com.gitblit.models.TicketModel;

 import com.gitblit.models.TicketModel.Change;

 import com.gitblit.models.TicketModel.Field;

@@ -127,7 +128,15 @@ public class NewTicketPage extends RepositoryPage {
 		descriptionEditor.setRepository(repositoryName);

 		form.add(descriptionEditor);

 		

-		form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, getRepositoryModel()));

+		form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel,  null) {

+

+			private static final long serialVersionUID = 1L;

+			

+			@Override

+			protected RepositoryModel getRepositoryModel() {

+				return NewTicketPage.this.getRepositoryModel();

+			}

+		});

 

 		if (currentUser.canAdmin(null, getRepositoryModel())) {

 			// responsible

diff --git a/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java b/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java
index 59cc9096..9b905c01 100644
--- a/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java
@@ -1,6 +1,8 @@
 package com.gitblit.wicket.panels;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.wicket.PageParameters;
 import org.apache.wicket.ajax.AjaxRequestTarget;
@@ -14,21 +16,24 @@ import org.apache.wicket.model.Model;
 import org.jsoup.helper.StringUtil;
 
 import com.gitblit.models.RepositoryModel;
+import com.gitblit.models.TicketModel;
+import com.gitblit.tickets.ITicketService;
 import com.gitblit.wicket.WicketUtils;
 import com.gitblit.wicket.pages.TicketsPage;
 
-public class TicketRelationEditorPanel extends BasePanel {
+public abstract class TicketRelationEditorPanel extends BasePanel {
 
 	private static final long serialVersionUID = 1L;
 	
 	private IModel<List<String>> dependenciesModel;
 	private IModel<String> addDependencyModel;
-
+	private Long baseTicketId;
 	
-	public TicketRelationEditorPanel(String wicketId, IModel<List<String>> pdependenciesModel, final RepositoryModel repositoryModel) {
+	public TicketRelationEditorPanel(String wicketId, IModel<List<String>> pdependenciesModel, Long baseTicketId) {
 		super(wicketId);
 		this.dependenciesModel = pdependenciesModel;
 		this.addDependencyModel = Model.of();
+		this.baseTicketId = baseTicketId;
 		
 		
 		add(new ListView<String>("dependencyList", dependenciesModel) {
@@ -38,7 +43,7 @@ public class TicketRelationEditorPanel extends BasePanel {
 			protected void populateItem(ListItem<String> item) {
 				final String ticketId = item.getModelObject();
 
-				PageParameters tp = WicketUtils.newObjectParameter(repositoryModel.name, ticketId);
+				PageParameters tp = WicketUtils.newObjectParameter(getRepositoryModel().name, ticketId);
 				item.add(new LinkPanel("dependencyLink", "list subject", "#"+ticketId, TicketsPage.class, tp));
 				
 				item.add(new AjaxButton("removeDependencyLink") {
@@ -60,23 +65,51 @@ public class TicketRelationEditorPanel extends BasePanel {
 			protected void onSubmit(AjaxRequestTarget target, Form<?> form) {
 				String ticketIdStr = addDependencyModel.getObject();
 				if (!StringUtil.isBlank(ticketIdStr)) {
-					try {
-						long ticketId = Long.parseLong(ticketIdStr);
-						if (app().tickets().hasTicket(repositoryModel, ticketId)) {
-							List<String> list = (List<String>) dependenciesModel.getObject();
-							list.add(String.valueOf(ticketId));
-							addDependencyModel.setObject("");
-						}
-					} catch (NumberFormatException e) {
-						// TODO : not allowed
-						
+					if (checkCycle(ticketIdStr)) {
+						List<String> list = (List<String>) dependenciesModel.getObject();
+						list.add(ticketIdStr.trim());
+						addDependencyModel.setObject("");
 					}
 				}
 				target.addComponent(form);
 			}
 		});
-		
-		
 	}
+	
+	private boolean checkCycle(String ticketId) {
+		Set<Long> tickets = new HashSet<Long>();
+		if (baseTicketId != null) {
+			tickets.add(baseTicketId);
+		}
+		return checkCycle(tickets, ticketId);
+	}
+
+	private boolean checkCycle(Set<Long> tickets, String ticketIdStr) {
+		try {
+			long ticketId = Long.parseLong(ticketIdStr);
+			if (tickets.contains(ticketId)) {
+				return false;
+			}
+			ITicketService ticketService = app().tickets();
+			RepositoryModel r = getRepositoryModel();
+			if (ticketService.hasTicket(r, ticketId)) {
+				TicketModel ticket = ticketService.getTicket(r, ticketId);
+				tickets.add(ticketId);
+				for (String subTicketIdStr : ticket.getDependencies()) {
+					if (!checkCycle(tickets, subTicketIdStr)) {
+						return false;
+					}
+				}
+				tickets.remove(ticketId);
+				return true;
+			} else {
+				return false;
+			}
+		} catch (NumberFormatException e) {
+			return false;
+		}
+	}	
+	
+	protected abstract RepositoryModel getRepositoryModel();
 
 }