From b57a57f3011dc5973933600496275856617cafe2 Mon Sep 17 00:00:00 2001 From: Zsombor Gegesy Date: Mon, 8 Dec 2014 00:39:50 +0100 Subject: [PATCH] Add circular dependency check for tickets, and refactor to get repositoryModel always from the page --- .../gitblit/wicket/pages/EditTicketPage.java | 9 ++- .../gitblit/wicket/pages/NewTicketPage.java | 11 +++- .../panels/TicketRelationEditorPanel.java | 65 ++++++++++++++----- 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 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> dependenciesModel; private IModel addDependencyModel; - + private Long baseTicketId; - public TicketRelationEditorPanel(String wicketId, IModel> pdependenciesModel, final RepositoryModel repositoryModel) { + public TicketRelationEditorPanel(String wicketId, IModel> pdependenciesModel, Long baseTicketId) { super(wicketId); this.dependenciesModel = pdependenciesModel; this.addDependencyModel = Model.of(); + this.baseTicketId = baseTicketId; add(new ListView("dependencyList", dependenciesModel) { @@ -38,7 +43,7 @@ public class TicketRelationEditorPanel extends BasePanel { protected void populateItem(ListItem 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 list = (List) dependenciesModel.getObject(); - list.add(String.valueOf(ticketId)); - addDependencyModel.setObject(""); - } - } catch (NumberFormatException e) { - // TODO : not allowed - + if (checkCycle(ticketIdStr)) { + List list = (List) dependenciesModel.getObject(); + list.add(ticketIdStr.trim()); + addDependencyModel.setObject(""); } } target.addComponent(form); } }); - - } + + private boolean checkCycle(String ticketId) { + Set tickets = new HashSet(); + if (baseTicketId != null) { + tickets.add(baseTicketId); + } + return checkCycle(tickets, ticketId); + } + + private boolean checkCycle(Set 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(); } -- 2.39.5