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 61e1287..6b09062 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.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 @@
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 3722be1..011f4b6 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.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 @@
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 59cc909..9b905c0 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.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 @@
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 @@
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();
}