Add circular dependency check for tickets,

 and refactor to get repositoryModel always from the page

diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
index 61e12878b1d4eb3905e115564be878e669f5df86..6b09062995760bcf6622f4a7eed776d3a2527ae5 100644 (file)
--- 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;\r
 import com.gitblit.Constants.AuthorizationControl;\r
 import com.gitblit.models.RegistrantAccessPermission;\r
+import com.gitblit.models.RepositoryModel;\r
 import com.gitblit.models.TicketModel;\r
 import com.gitblit.models.TicketModel.Change;\r
 import com.gitblit.models.TicketModel.Field;\r
@@ -152,7 +153,13 @@ public class EditTicketPage extends RepositoryPage {
 \r
                form.setOutputMarkupId(true);\r
 \r
-               form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, getRepositoryModel()));\r
+               form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, ticket.number) {\r
+                       private static final long serialVersionUID = 1L;\r
+                       @Override\r
+                       protected RepositoryModel getRepositoryModel() {\r
+                               return EditTicketPage.this.getRepositoryModel();\r
+                       }\r
+               });\r
 \r
                final IModel<String> markdownPreviewModel = Model.of(ticket.body == null ? "" : ticket.body);\r
                descriptionPreview = new Label("descriptionPreview", markdownPreviewModel);\r

diff --git a/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java b/src/main/java/com/gitblit/wicket/pages/NewTicketPage.java
index 3722be10342a5b0007d972a9596c556589d42933..011f4b68b37e74aa73cd08b2f34b2fae26eac07b 100644 (file)
--- 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;\r
 import com.gitblit.Constants.AuthorizationControl;\r
 import com.gitblit.models.RegistrantAccessPermission;\r
+import com.gitblit.models.RepositoryModel;\r
 import com.gitblit.models.TicketModel;\r
 import com.gitblit.models.TicketModel.Change;\r
 import com.gitblit.models.TicketModel.Field;\r
@@ -127,7 +128,15 @@ public class NewTicketPage extends RepositoryPage {
                descriptionEditor.setRepository(repositoryName);\r
                form.add(descriptionEditor);\r
                \r
-               form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel, getRepositoryModel()));\r
+               form.add(new TicketRelationEditorPanel("dependencies", dependenciesModel,  null) {\r
+\r
+                       private static final long serialVersionUID = 1L;\r
+                       \r
+                       @Override\r
+                       protected RepositoryModel getRepositoryModel() {\r
+                               return NewTicketPage.this.getRepositoryModel();\r
+                       }\r
+               });\r
 \r
                if (currentUser.canAdmin(null, getRepositoryModel())) {\r
                        // responsible\r

diff --git a/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java b/src/main/java/com/gitblit/wicket/panels/TicketRelationEditorPanel.java
index 59cc9096ebea982f896d6885cbf4a1f1b9c51a52..9b905c01d39b454456a75f26aadb56b0e965ea86 100644 (file)
--- 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();
 
 }