From cd7e4f9186f2ace4416780a7dd6341e01e23a45f Mon Sep 17 00:00:00 2001 From: Paul Martin <paul@paulsputer.com> Date: Wed, 06 Apr 2016 14:46:58 -0400 Subject: [PATCH] Fix for #962 - Delete patchset ability --- src/main/java/com/gitblit/tickets/ITicketService.java | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 258 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/gitblit/tickets/ITicketService.java b/src/main/java/com/gitblit/tickets/ITicketService.java index 9522e42..e831003 100644 --- a/src/main/java/com/gitblit/tickets/ITicketService.java +++ b/src/main/java/com/gitblit/tickets/ITicketService.java @@ -36,6 +36,7 @@ import com.gitblit.IStoredSettings; import com.gitblit.Keys; import com.gitblit.extensions.TicketHook; +import com.gitblit.manager.IManager; import com.gitblit.manager.INotificationManager; import com.gitblit.manager.IPluginManager; import com.gitblit.manager.IRepositoryManager; @@ -47,8 +48,10 @@ import com.gitblit.models.TicketModel.Change; import com.gitblit.models.TicketModel.Field; import com.gitblit.models.TicketModel.Patchset; +import com.gitblit.models.TicketModel.PatchsetType; import com.gitblit.models.TicketModel.Status; import com.gitblit.tickets.TicketIndexer.Lucene; +import com.gitblit.utils.DeepCopier; import com.gitblit.utils.DiffUtils; import com.gitblit.utils.DiffUtils.DiffStat; import com.gitblit.utils.StringUtils; @@ -62,7 +65,9 @@ * @author James Moger * */ -public abstract class ITicketService { +public abstract class ITicketService implements IManager { + + public static final String SETTING_UPDATE_DIFFSTATS = "migration.updateDiffstats"; private static final String LABEL = "label"; @@ -105,6 +110,8 @@ private final Map<String, List<TicketLabel>> labelsCache; private final Map<String, List<TicketMilestone>> milestonesCache; + + private final boolean updateDiffstats; private static class TicketKey { final String repository; @@ -163,18 +170,22 @@ this.labelsCache = new ConcurrentHashMap<String, List<TicketLabel>>(); this.milestonesCache = new ConcurrentHashMap<String, List<TicketMilestone>>(); + + this.updateDiffstats = settings.getBoolean(SETTING_UPDATE_DIFFSTATS, true); } /** * Start the service. - * + * @since 1.4.0 */ + @Override public abstract ITicketService start(); /** * Stop the service. - * + * @since 1.4.0 */ + @Override public final ITicketService stop() { indexer.close(); ticketsCache.invalidateAll(); @@ -185,7 +196,7 @@ /** * Creates a ticket notifier. The ticket notifier is not thread-safe! - * + * @since 1.4.0 */ public TicketNotifier createNotifier() { return new TicketNotifier( @@ -200,6 +211,7 @@ * Returns the ready status of the ticket service. * * @return true if the ticket service is ready + * @since 1.4.0 */ public boolean isReady() { return true; @@ -210,6 +222,7 @@ * * @param repository * @return true if patchsets are being accepted + * @since 1.4.0 */ public boolean isAcceptingNewPatchsets(RepositoryModel repository) { return isReady() @@ -224,6 +237,7 @@ * * @param repository * @return true if tickets are being accepted + * @since 1.4.0 */ public boolean isAcceptingNewTickets(RepositoryModel repository) { return isReady() @@ -237,9 +251,11 @@ * * @param repository * @return true if tickets are allowed to be updated + * @since 1.4.0 */ public boolean isAcceptingTicketUpdates(RepositoryModel repository) { return isReady() + && repository.hasCommits && repository.isBare && !repository.isFrozen && !repository.isMirror; @@ -249,6 +265,7 @@ * Returns true if the repository has any tickets * @param repository * @return true if the repository has tickets + * @since 1.4.0 */ public boolean hasTickets(RepositoryModel repository) { return indexer.hasTickets(repository); @@ -256,11 +273,13 @@ /** * Closes any open resources used by this service. + * @since 1.4.0 */ protected abstract void close(); /** * Reset all caches in the service. + * @since 1.4.0 */ public final synchronized void resetCaches() { ticketsCache.invalidateAll(); @@ -269,10 +288,15 @@ resetCachesImpl(); } + /** + * Reset all caches in the service. + * @since 1.4.0 + */ protected abstract void resetCachesImpl(); /** * Reset any caches for the repository in the service. + * @since 1.4.0 */ public final synchronized void resetCaches(RepositoryModel repository) { List<TicketKey> repoKeys = new ArrayList<TicketKey>(); @@ -287,6 +311,12 @@ resetCachesImpl(repository); } + /** + * Reset the caches for the specified repository. + * + * @param repository + * @since 1.4.0 + */ protected abstract void resetCachesImpl(RepositoryModel repository); @@ -295,6 +325,7 @@ * * @param repository * @return the list of labels + * @since 1.4.0 */ public List<TicketLabel> getLabels(RepositoryModel repository) { String key = repository.name; @@ -327,6 +358,7 @@ * @param repository * @param label * @return a TicketLabel + * @since 1.4.0 */ public TicketLabel getLabel(RepositoryModel repository, String label) { for (TicketLabel tl : getLabels(repository)) { @@ -346,6 +378,7 @@ * @param milestone * @param createdBy * @return the label + * @since 1.4.0 */ public synchronized TicketLabel createLabel(RepositoryModel repository, String label, String createdBy) { TicketLabel lb = new TicketMilestone(label); @@ -358,7 +391,9 @@ } catch (IOException e) { log.error("failed to create label " + label + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return lb; } @@ -370,6 +405,7 @@ * @param label * @param createdBy * @return true if the update was successful + * @since 1.4.0 */ public synchronized boolean updateLabel(RepositoryModel repository, TicketLabel label, String createdBy) { Repository db = null; @@ -383,7 +419,9 @@ } catch (IOException e) { log.error("failed to update label " + label + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } @@ -396,6 +434,7 @@ * @param newName * @param createdBy * @return true if the rename was successful + * @since 1.4.0 */ public synchronized boolean renameLabel(RepositoryModel repository, String oldName, String newName, String createdBy) { if (StringUtils.isEmpty(newName)) { @@ -421,7 +460,9 @@ } catch (IOException e) { log.error("failed to rename label " + oldName + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } @@ -433,6 +474,7 @@ * @param label * @param createdBy * @return true if the delete was successful + * @since 1.4.0 */ public synchronized boolean deleteLabel(RepositoryModel repository, String label, String createdBy) { if (StringUtils.isEmpty(label)) { @@ -449,7 +491,9 @@ } catch (IOException e) { log.error("failed to delete label " + label + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } @@ -459,6 +503,7 @@ * * @param repository * @return the list of milestones + * @since 1.4.0 */ public List<TicketMilestone> getMilestones(RepositoryModel repository) { String key = repository.name; @@ -500,6 +545,7 @@ * @param repository * @param status * @return the list of milestones + * @since 1.4.0 */ public List<TicketMilestone> getMilestones(RepositoryModel repository, Status status) { List<TicketMilestone> matches = new ArrayList<TicketMilestone>(); @@ -517,13 +563,15 @@ * @param repository * @param milestone * @return the milestone or null if it does not exist + * @since 1.4.0 */ public TicketMilestone getMilestone(RepositoryModel repository, String milestone) { for (TicketMilestone ms : getMilestones(repository)) { if (ms.name.equalsIgnoreCase(milestone)) { + TicketMilestone tm = DeepCopier.copy(ms); String q = QueryBuilder.q(Lucene.rid.matches(repository.getRID())).and(Lucene.milestone.matches(milestone)).build(); - ms.tickets = indexer.queryFor(q, 1, 0, Lucene.number.name(), true); - return ms; + tm.tickets = indexer.queryFor(q, 1, 0, Lucene.number.name(), true); + return tm; } } return null; @@ -536,6 +584,7 @@ * @param milestone * @param createdBy * @return the milestone + * @since 1.4.0 */ public synchronized TicketMilestone createMilestone(RepositoryModel repository, String milestone, String createdBy) { TicketMilestone ms = new TicketMilestone(milestone); @@ -551,7 +600,9 @@ } catch (IOException e) { log.error("failed to create milestone " + milestone + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return ms; } @@ -563,6 +614,7 @@ * @param milestone * @param createdBy * @return true if successful + * @since 1.4.0 */ public synchronized boolean updateMilestone(RepositoryModel repository, TicketMilestone milestone, String createdBy) { Repository db = null; @@ -582,7 +634,9 @@ } catch (IOException e) { log.error("failed to update milestone " + milestone + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } @@ -595,44 +649,71 @@ * @param newName * @param createdBy * @return true if successful + * @since 1.4.0 */ public synchronized boolean renameMilestone(RepositoryModel repository, String oldName, String newName, String createdBy) { + return renameMilestone(repository, oldName, newName, createdBy, true); + } + + /** + * Renames a milestone. + * + * @param repository + * @param oldName + * @param newName + * @param createdBy + * @param notifyOpenTickets + * @return true if successful + * @since 1.6.0 + */ + public synchronized boolean renameMilestone(RepositoryModel repository, String oldName, + String newName, String createdBy, boolean notifyOpenTickets) { if (StringUtils.isEmpty(newName)) { throw new IllegalArgumentException("new milestone can not be empty!"); } Repository db = null; try { db = repositoryManager.getRepository(repository.name); - TicketMilestone milestone = getMilestone(repository, oldName); + TicketMilestone tm = getMilestone(repository, oldName); + if (tm == null) { + return false; + } StoredConfig config = db.getConfig(); config.unsetSection(MILESTONE, oldName); - config.setString(MILESTONE, newName, STATUS, milestone.status.name()); - config.setString(MILESTONE, newName, COLOR, milestone.color); - if (milestone.due != null) { - config.setString(MILESTONE, milestone.name, DUE, - new SimpleDateFormat(DUE_DATE_PATTERN).format(milestone.due)); + config.setString(MILESTONE, newName, STATUS, tm.status.name()); + config.setString(MILESTONE, newName, COLOR, tm.color); + if (tm.due != null) { + config.setString(MILESTONE, newName, DUE, + new SimpleDateFormat(DUE_DATE_PATTERN).format(tm.due)); } config.save(); milestonesCache.remove(repository.name); TicketNotifier notifier = createNotifier(); - for (QueryResult qr : milestone.tickets) { + for (QueryResult qr : tm.tickets) { Change change = new Change(createdBy); change.setField(Field.milestone, newName); TicketModel ticket = updateTicket(repository, qr.number, change); - notifier.queueMailing(ticket); + if (notifyOpenTickets && ticket.isOpen()) { + notifier.queueMailing(ticket); + } } - notifier.sendAll(); + if (notifyOpenTickets) { + notifier.sendAll(); + } return true; } catch (IOException e) { log.error("failed to rename milestone " + oldName + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } + /** * Deletes a milestone. * @@ -640,13 +721,33 @@ * @param milestone * @param createdBy * @return true if successful + * @since 1.4.0 */ public synchronized boolean deleteMilestone(RepositoryModel repository, String milestone, String createdBy) { + return deleteMilestone(repository, milestone, createdBy, true); + } + + /** + * Deletes a milestone. + * + * @param repository + * @param milestone + * @param createdBy + * @param notifyOpenTickets + * @return true if successful + * @since 1.6.0 + */ + public synchronized boolean deleteMilestone(RepositoryModel repository, String milestone, + String createdBy, boolean notifyOpenTickets) { if (StringUtils.isEmpty(milestone)) { throw new IllegalArgumentException("milestone can not be empty!"); } Repository db = null; try { + TicketMilestone tm = getMilestone(repository, milestone); + if (tm == null) { + return false; + } db = repositoryManager.getRepository(repository.name); StoredConfig config = db.getConfig(); config.unsetSection(MILESTONE, milestone); @@ -654,20 +755,44 @@ milestonesCache.remove(repository.name); + TicketNotifier notifier = createNotifier(); + for (QueryResult qr : tm.tickets) { + Change change = new Change(createdBy); + change.setField(Field.milestone, ""); + TicketModel ticket = updateTicket(repository, qr.number, change); + if (notifyOpenTickets && ticket.isOpen()) { + notifier.queueMailing(ticket); + } + } + if (notifyOpenTickets) { + notifier.sendAll(); + } return true; } catch (IOException e) { log.error("failed to delete milestone " + milestone + " in " + repository, e); } finally { - db.close(); + if (db != null) { + db.close(); + } } return false; } + + /** + * Returns the set of assigned ticket ids in the repository. + * + * @param repository + * @return a set of assigned ticket ids in the repository + * @since 1.6.0 + */ + public abstract Set<Long> getIds(RepositoryModel repository); /** * Assigns a new ticket id. * * @param repository * @return a new ticket id + * @since 1.4.0 */ public abstract long assignNewId(RepositoryModel repository); @@ -677,6 +802,7 @@ * @param repository * @param ticketId * @return true if the ticket exists + * @since 1.4.0 */ public abstract boolean hasTicket(RepositoryModel repository, long ticketId); @@ -685,6 +811,7 @@ * * @param repository * @return all tickets + * @since 1.4.0 */ public List<TicketModel> getTickets(RepositoryModel repository) { return getTickets(repository, null); @@ -700,6 +827,7 @@ * @param filter * optional issue filter to only return matching results * @return a list of tickets + * @since 1.4.0 */ public abstract List<TicketModel> getTickets(RepositoryModel repository, TicketFilter filter); @@ -709,31 +837,35 @@ * @param repository * @param ticketId * @return a ticket, if it exists, otherwise null + * @since 1.4.0 */ public final TicketModel getTicket(RepositoryModel repository, long ticketId) { TicketKey key = new TicketKey(repository, ticketId); TicketModel ticket = ticketsCache.getIfPresent(key); + // if ticket not cached if (ticket == null) { - // load & cache ticket + //load ticket ticket = getTicketImpl(repository, ticketId); - if (ticket.hasPatchsets()) { - Repository r = repositoryManager.getRepository(repository.name); - try { - Patchset patchset = ticket.getCurrentPatchset(); - DiffStat diffStat = DiffUtils.getDiffStat(r, patchset.base, patchset.tip); - // diffstat could be null if we have ticket data without the - // commit objects. e.g. ticket replication without repo - // mirroring - if (diffStat != null) { - ticket.insertions = diffStat.getInsertions(); - ticket.deletions = diffStat.getDeletions(); - } - } finally { - r.close(); - } - } + // if ticket exists if (ticket != null) { + if (ticket.hasPatchsets() && updateDiffstats) { + Repository r = repositoryManager.getRepository(repository.name); + try { + Patchset patchset = ticket.getCurrentPatchset(); + DiffStat diffStat = DiffUtils.getDiffStat(r, patchset.base, patchset.tip); + // diffstat could be null if we have ticket data without the + // commit objects. e.g. ticket replication without repo + // mirroring + if (diffStat != null) { + ticket.insertions = diffStat.getInsertions(); + ticket.deletions = diffStat.getDeletions(); + } + } finally { + r.close(); + } + } + //cache ticket ticketsCache.put(key, ticket); } } @@ -746,14 +878,43 @@ * @param repository * @param ticketId * @return a ticket, if it exists, otherwise null + * @since 1.4.0 */ protected abstract TicketModel getTicketImpl(RepositoryModel repository, long ticketId); + + + /** + * Returns the journal used to build a ticket. + * + * @param repository + * @param ticketId + * @return the journal for the ticket, if it exists, otherwise null + * @since 1.6.0 + */ + public final List<Change> getJournal(RepositoryModel repository, long ticketId) { + if (hasTicket(repository, ticketId)) { + List<Change> journal = getJournalImpl(repository, ticketId); + return journal; + } + return null; + } + + /** + * Retrieves the ticket journal. + * + * @param repository + * @param ticketId + * @return a ticket, if it exists, otherwise null + * @since 1.6.0 + */ + protected abstract List<Change> getJournalImpl(RepositoryModel repository, long ticketId); /** * Get the ticket url * * @param ticket * @return the ticket url + * @since 1.4.0 */ public String getTicketUrl(TicketModel ticket) { final String canonicalUrl = settings.getString(Keys.web.canonicalUrl, "https://localhost:8443"); @@ -767,6 +928,7 @@ * @param base * @param tip * @return the compare url + * @since 1.4.0 */ public String getCompareUrl(TicketModel ticket, String base, String tip) { final String canonicalUrl = settings.getString(Keys.web.canonicalUrl, "https://localhost:8443"); @@ -778,6 +940,7 @@ * Returns true if attachments are supported. * * @return true if attachments are supported + * @since 1.4.0 */ public abstract boolean supportsAttachments(); @@ -788,6 +951,7 @@ * @param ticketId * @param filename * @return an attachment, if found, null otherwise + * @since 1.4.0 */ public abstract Attachment getAttachment(RepositoryModel repository, long ticketId, String filename); @@ -799,6 +963,7 @@ * @param repository * @param change * @return true if successful + * @since 1.4.0 */ public TicketModel createTicket(RepositoryModel repository, Change change) { return createTicket(repository, 0L, change); @@ -813,6 +978,7 @@ * @param ticketId (if <=0 the ticket id will be assigned) * @param change * @return true if successful + * @since 1.4.0 */ public TicketModel createTicket(RepositoryModel repository, long ticketId, Change change) { @@ -861,6 +1027,7 @@ * @param ticketId * @param change * @return the ticket model if successful + * @since 1.4.0 */ public final TicketModel updateTicket(RepositoryModel repository, long ticketId, Change change) { if (change == null) { @@ -899,6 +1066,7 @@ * Deletes all tickets in every repository. * * @return true if successful + * @since 1.4.0 */ public boolean deleteAll() { List<String> repositories = repositoryManager.getRepositoryList(); @@ -921,6 +1089,7 @@ * Deletes all tickets in the specified repository. * @param repository * @return true if succesful + * @since 1.4.0 */ public boolean deleteAll(RepositoryModel repository) { boolean success = deleteAllImpl(repository); @@ -932,6 +1101,12 @@ return success; } + /** + * Delete all tickets for the specified repository. + * @param repository + * @return true if successful + * @since 1.4.0 + */ protected abstract boolean deleteAllImpl(RepositoryModel repository); /** @@ -940,6 +1115,7 @@ * @param oldRepositoryName * @param newRepositoryName * @return true if successful + * @since 1.4.0 */ public boolean rename(RepositoryModel oldRepository, RepositoryModel newRepository) { if (renameImpl(oldRepository, newRepository)) { @@ -951,6 +1127,14 @@ return false; } + /** + * Renames a repository. + * + * @param oldRepository + * @param newRepository + * @return true if successful + * @since 1.4.0 + */ protected abstract boolean renameImpl(RepositoryModel oldRepository, RepositoryModel newRepository); /** @@ -960,6 +1144,7 @@ * @param ticketId * @param deletedBy * @return true if successful + * @since 1.4.0 */ public boolean deleteTicket(RepositoryModel repository, long ticketId, String deletedBy) { TicketModel ticket = getTicket(repository, ticketId); @@ -981,6 +1166,7 @@ * @param ticket * @param deletedBy * @return true if successful + * @since 1.4.0 */ protected abstract boolean deleteTicketImpl(RepositoryModel repository, TicketModel ticket, String deletedBy); @@ -996,6 +1182,7 @@ * @param comment * the revised comment * @return the revised ticket if the change was successful + * @since 1.4.0 */ public final TicketModel updateComment(TicketModel ticket, String commentId, String updatedBy, String comment) { @@ -1016,6 +1203,7 @@ * @param deletedBy * the user deleting the comment * @return the revised ticket if the deletion was successful + * @since 1.4.0 */ public final TicketModel deleteComment(TicketModel ticket, String commentId, String deletedBy) { Change deletion = new Change(deletedBy); @@ -1026,6 +1214,30 @@ TicketModel revisedTicket = updateTicket(repository, ticket.number, deletion); return revisedTicket; } + + /** + * Deletes a patchset from a ticket. + * + * @param ticket + * @param patchset + * the patchset to delete (should be the highest revision) + * @param userName + * the user deleting the commit + * @return the revised ticket if the deletion was successful + * @since 1.8.0 + */ + public final TicketModel deletePatchset(TicketModel ticket, Patchset patchset, String userName) { + Change deletion = new Change(userName); + deletion.patchset = new Patchset(); + deletion.patchset.number = patchset.number; + deletion.patchset.rev = patchset.rev; + deletion.patchset.type = PatchsetType.Delete; + + RepositoryModel repository = repositoryManager.getRepositoryModel(ticket.repository); + TicketModel revisedTicket = updateTicket(repository, ticket.number, deletion); + + return revisedTicket; + } /** * Commit a ticket change to the repository. @@ -1034,6 +1246,7 @@ * @param ticketId * @param change * @return true, if the change was committed + * @since 1.4.0 */ protected abstract boolean commitChangeImpl(RepositoryModel repository, long ticketId, Change change); @@ -1048,6 +1261,7 @@ * @param page * @param pageSize * @return a list of matching tickets + * @since 1.4.0 */ public List<QueryResult> searchFor(RepositoryModel repository, String text, int page, int pageSize) { return indexer.searchFor(repository, text, page, pageSize); @@ -1062,6 +1276,7 @@ * @param sortBy * @param descending * @return a list of matching tickets or an empty list + * @since 1.4.0 */ public List<QueryResult> queryFor(String query, int page, int pageSize, String sortBy, boolean descending) { return indexer.queryFor(query, page, pageSize, sortBy, descending); @@ -1070,6 +1285,7 @@ /** * Destroys an existing index and reindexes all tickets. * This operation may be expensive and time-consuming. + * @since 1.4.0 */ public void reindex() { long start = System.nanoTime(); @@ -1096,6 +1312,7 @@ /** * Destroys any existing index and reindexes all tickets. * This operation may be expensive and time-consuming. + * @since 1.4.0 */ public void reindex(RepositoryModel repository) { long start = System.nanoTime(); @@ -1113,6 +1330,7 @@ * of ticket updates, namely merging from the web ui. * * @param runnable + * @since 1.4.0 */ public synchronized void exec(Runnable runnable) { runnable.run(); -- Gitblit v1.9.1