From 69a55934079b299740fa4679fbbd9faeb8319726 Mon Sep 17 00:00:00 2001 From: James Moger Date: Sun, 15 Jan 2012 19:07:34 -0500 Subject: [PATCH] More functional issues. --- src/com/gitblit/models/IssueModel.java | 305 +++++++++-- src/com/gitblit/utils/IssueUtils.java | 614 +++++++++++++++++----- src/com/gitblit/utils/JGitUtils.java | 2 +- tests/com/gitblit/tests/GitBlitSuite.java | 2 +- tests/com/gitblit/tests/IssuesTest.java | 165 ++++-- 5 files changed, 884 insertions(+), 204 deletions(-) diff --git a/src/com/gitblit/models/IssueModel.java b/src/com/gitblit/models/IssueModel.java index 3c6d9a0b..19241c29 100644 --- a/src/com/gitblit/models/IssueModel.java +++ b/src/com/gitblit/models/IssueModel.java @@ -18,10 +18,13 @@ package com.gitblit.models; import java.io.Serializable; import java.util.ArrayList; import java.util.Date; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.StringUtils; +import com.gitblit.utils.TimeUtils; /** * The Gitblit Issue model, its component classes, and enums. @@ -31,7 +34,7 @@ import com.gitblit.utils.StringUtils; */ public class IssueModel implements Serializable, Comparable { - private static final long serialVersionUID = 1L;; + private static final long serialVersionUID = 1L; public String id; @@ -56,7 +59,7 @@ public class IssueModel implements Serializable, Comparable { public List changes; public IssueModel() { - created = new Date(); + created = new Date((System.currentTimeMillis() / 1000) * 1000); type = Type.Defect; status = Status.New; @@ -72,15 +75,16 @@ public class IssueModel implements Serializable, Comparable { return s; } + public boolean hasLabel(String label) { + return getLabels().contains(label); + } + public List getLabels() { List list = new ArrayList(); String labels = null; for (Change change : changes) { - if (change.hasFieldChanges()) { - FieldChange field = change.getField(Field.Labels); - if (field != null) { - labels = field.value.toString(); - } + if (change.hasField(Field.Labels)) { + labels = change.getString(Field.Labels); } } if (!StringUtils.isEmpty(labels)) { @@ -89,10 +93,6 @@ public class IssueModel implements Serializable, Comparable { return list; } - public boolean hasLabel(String label) { - return getLabels().contains(label); - } - public Attachment getAttachment(String name) { Attachment attachment = null; for (Change change : changes) { @@ -106,16 +106,55 @@ public class IssueModel implements Serializable, Comparable { return attachment; } - public void addChange(Change change) { - if (changes == null) { - changes = new ArrayList(); - } + public void applyChange(Change change) { changes.add(change); + + if (change.hasFieldChanges()) { + for (FieldChange fieldChange : change.fieldChanges) { + switch (fieldChange.field) { + case Id: + id = fieldChange.value.toString(); + break; + case Type: + type = IssueModel.Type.fromObject(fieldChange.value); + break; + case Status: + status = IssueModel.Status.fromObject(fieldChange.value); + break; + case Priority: + priority = IssueModel.Priority.fromObject(fieldChange.value); + break; + case Summary: + summary = fieldChange.value.toString(); + break; + case Description: + description = fieldChange.value.toString(); + break; + case Reporter: + reporter = fieldChange.value.toString(); + break; + case Owner: + owner = fieldChange.value.toString(); + break; + case Milestone: + milestone = fieldChange.value.toString(); + break; + } + } + } } @Override public String toString() { - return summary; + StringBuilder sb = new StringBuilder(); + sb.append("issue "); + sb.append(id.substring(0, 8)); + sb.append(" (" + summary + ")\n"); + for (Change change : changes) { + sb.append(change); + sb.append('\n'); + } + return sb.toString(); } @Override @@ -135,41 +174,72 @@ public class IssueModel implements Serializable, Comparable { return id.hashCode(); } - public static class Change implements Serializable { + public static class Change implements Serializable, Comparable { private static final long serialVersionUID = 1L; - public Date created; + public final Date created; + + public final String author; - public String author; + public String id; + + public char code; public Comment comment; - public List fieldChanges; + public Set fieldChanges; - public List attachments; + public Set attachments; - public void comment(String text) { - comment = new Comment(text); + public Change(String author) { + this.created = new Date((System.currentTimeMillis() / 1000) * 1000); + this.author = author; + this.id = StringUtils.getSHA1(created.toString() + author); } public boolean hasComment() { - return comment != null; + return comment != null && !comment.deleted; + } + + public void comment(String text) { + comment = new Comment(text); + comment.id = StringUtils.getSHA1(created.toString() + author + text); } public boolean hasAttachments() { return !ArrayUtils.isEmpty(attachments); } + public void addAttachment(Attachment attachment) { + if (attachments == null) { + attachments = new LinkedHashSet(); + } + attachments.add(attachment); + } + + public Attachment getAttachment(String name) { + for (Attachment attachment : attachments) { + if (attachment.name.equalsIgnoreCase(name)) { + return attachment; + } + } + return null; + } + + public boolean hasField(Field field) { + return !StringUtils.isEmpty(getString(field)); + } + public boolean hasFieldChanges() { return !ArrayUtils.isEmpty(fieldChanges); } - public FieldChange getField(Field field) { + public Object getField(Field field) { if (fieldChanges != null) { for (FieldChange fieldChange : fieldChanges) { if (fieldChange.field == field) { - return fieldChange; + return fieldChange.value; } } } @@ -177,42 +247,74 @@ public class IssueModel implements Serializable, Comparable { } public void setField(Field field, Object value) { - FieldChange fieldChange = new FieldChange(); - fieldChange.field = field; - fieldChange.value = value; + FieldChange fieldChange = new FieldChange(field, value); if (fieldChanges == null) { - fieldChanges = new ArrayList(); + fieldChanges = new LinkedHashSet(); } fieldChanges.add(fieldChange); } public String getString(Field field) { - FieldChange fieldChange = getField(field); - if (fieldChange == null) { + Object value = getField(field); + if (value == null) { return null; } - return fieldChange.value.toString(); + return value.toString(); } - public void addAttachment(Attachment attachment) { - if (attachments == null) { - attachments = new ArrayList(); - } - attachments.add(attachment); + @Override + public int compareTo(Change c) { + return created.compareTo(c.created); } - public Attachment getAttachment(String name) { - for (Attachment attachment : attachments) { - if (attachment.name.equalsIgnoreCase(name)) { - return attachment; - } + @Override + public int hashCode() { + return id.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof Change) { + return id.equals(((Change) o).id); } - return null; + return false; } @Override public String toString() { - return created.toString() + " by " + author; + StringBuilder sb = new StringBuilder(); + sb.append(TimeUtils.timeAgo(created)); + switch (code) { + case '+': + sb.append(" created by "); + break; + default: + if (hasComment()) { + sb.append(" commented on by "); + } else { + sb.append(" changed by "); + } + } + sb.append(author).append(" - "); + if (hasComment()) { + if (comment.deleted) { + sb.append("(deleted) "); + } + sb.append(comment.text).append(" "); + } + if (hasFieldChanges()) { + switch (code) { + case '+': + break; + default: + for (FieldChange fieldChange : fieldChanges) { + sb.append("\n "); + sb.append(fieldChange); + } + break; + } + } + return sb.toString(); } } @@ -221,6 +323,9 @@ public class IssueModel implements Serializable, Comparable { private static final long serialVersionUID = 1L; public String text; + + public String id; + public boolean deleted; Comment(String text) { @@ -237,9 +342,27 @@ public class IssueModel implements Serializable, Comparable { private static final long serialVersionUID = 1L; - public Field field; + public final Field field; - public Object value; + public final Object value; + + FieldChange(Field field, Object value) { + this.field = field; + this.value = value; + } + + @Override + public int hashCode() { + return field.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof FieldChange) { + return field.equals(((FieldChange) o).field); + } + return false; + } @Override public String toString() { @@ -251,7 +374,8 @@ public class IssueModel implements Serializable, Comparable { private static final long serialVersionUID = 1L; - public String name; + public final String name; + public String id; public long size; public byte[] content; public boolean deleted; @@ -260,6 +384,19 @@ public class IssueModel implements Serializable, Comparable { this.name = name; } + @Override + public int hashCode() { + return name.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof Attachment) { + return name.equalsIgnoreCase(((Attachment) o).name); + } + return false; + } + @Override public String toString() { return name; @@ -267,20 +404,86 @@ public class IssueModel implements Serializable, Comparable { } public static enum Field { - Summary, Description, Reporter, Owner, Type, Status, Priority, Milestone, Labels; + Id, Summary, Description, Reporter, Owner, Type, Status, Priority, Milestone, Component, Labels; } public static enum Type { Defect, Enhancement, Task, Review, Other; + + public static Type fromObject(Object o) { + if (o instanceof Type) { + // cast and return + return (Type) o; + } else if (o instanceof String) { + // find by name + for (Type type : values()) { + String str = o.toString(); + if (type.toString().equalsIgnoreCase(str)) { + return type; + } + } + } else if (o instanceof Number) { + // by ordinal + int id = ((Number) o).intValue(); + if (id >= 0 && id < values().length) { + return values()[id]; + } + } + return null; + } } public static enum Priority { Low, Medium, High, Critical; + + public static Priority fromObject(Object o) { + if (o instanceof Priority) { + // cast and return + return (Priority) o; + } else if (o instanceof String) { + // find by name + for (Priority priority : values()) { + String str = o.toString(); + if (priority.toString().equalsIgnoreCase(str)) { + return priority; + } + } + } else if (o instanceof Number) { + // by ordinal + int id = ((Number) o).intValue(); + if (id >= 0 && id < values().length) { + return values()[id]; + } + } + return null; + } } public static enum Status { New, Accepted, Started, Review, Queued, Testing, Done, Fixed, WontFix, Duplicate, Invalid; + public static Status fromObject(Object o) { + if (o instanceof Status) { + // cast and return + return (Status) o; + } else if (o instanceof String) { + // find by name + for (Status status : values()) { + String str = o.toString(); + if (status.toString().equalsIgnoreCase(str)) { + return status; + } + } + } else if (o instanceof Number) { + // by ordinal + int id = ((Number) o).intValue(); + if (id >= 0 && id < values().length) { + return values()[id]; + } + } + return null; + } + public boolean atLeast(Status status) { return ordinal() >= status.ordinal(); } @@ -289,6 +492,10 @@ public class IssueModel implements Serializable, Comparable { return ordinal() > status.ordinal(); } + public boolean isClosed() { + return ordinal() >= Done.ordinal(); + } + public Status next() { switch (this) { case New: diff --git a/src/com/gitblit/utils/IssueUtils.java b/src/com/gitblit/utils/IssueUtils.java index 82170703..d0a01992 100644 --- a/src/com/gitblit/utils/IssueUtils.java +++ b/src/com/gitblit/utils/IssueUtils.java @@ -18,12 +18,15 @@ package com.gitblit.utils; import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; -import java.util.Date; import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import org.eclipse.jgit.JGitText; import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException; @@ -45,15 +48,21 @@ import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.filter.AndTreeFilter; +import org.eclipse.jgit.treewalk.filter.PathFilterGroup; +import org.eclipse.jgit.treewalk.filter.TreeFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.gitblit.models.IssueModel; import com.gitblit.models.IssueModel.Attachment; import com.gitblit.models.IssueModel.Change; import com.gitblit.models.IssueModel.Field; -import com.gitblit.models.PathModel; +import com.gitblit.models.IssueModel.Status; import com.gitblit.models.RefModel; import com.gitblit.utils.JsonUtils.ExcludeField; import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; /** * Utility class for reading Gitblit issues. @@ -63,8 +72,37 @@ import com.google.gson.Gson; */ public class IssueUtils { + public static interface IssueFilter { + public abstract boolean accept(IssueModel issue); + } + public static final String GB_ISSUES = "refs/heads/gb-issues"; + static final Logger LOGGER = LoggerFactory.getLogger(JGitUtils.class); + + /** + * Log an error message and exception. + * + * @param t + * @param repository + * if repository is not null it MUST be the {0} parameter in the + * pattern. + * @param pattern + * @param objects + */ + private static void error(Throwable t, Repository repository, String pattern, Object... objects) { + List parameters = new ArrayList(); + if (objects != null && objects.length > 0) { + for (Object o : objects) { + parameters.add(o); + } + } + if (repository != null) { + parameters.add(0, repository.getDirectory().getAbsolutePath()); + } + LOGGER.error(MessageFormat.format(pattern, parameters.toArray()), t); + } + /** * Returns a RefModel for the gb-issues branch in the repository. If the * branch can not be found, null is returned. @@ -77,7 +115,10 @@ public class IssueUtils { } /** - * Returns all the issues in the repository. + * Returns all the issues in the repository. Querying issues from the + * repository requires deserializing all changes for all issues. This is an + * expensive process and not recommended. Issues should be indexed by Lucene + * and queries should be executed against that index. * * @param repository * @param filter @@ -90,12 +131,85 @@ public class IssueUtils { if (issuesBranch == null) { return list; } - List paths = JGitUtils - .getDocuments(repository, Arrays.asList("json"), GB_ISSUES); - RevTree tree = JGitUtils.getCommit(repository, GB_ISSUES).getTree(); - for (PathModel path : paths) { - String json = JGitUtils.getStringContent(repository, tree, path.path); - IssueModel issue = JsonUtils.fromJsonString(json, IssueModel.class); + + // Collect the set of all issue paths + Set issuePaths = new HashSet(); + final TreeWalk tw = new TreeWalk(repository); + try { + RevCommit head = JGitUtils.getCommit(repository, GB_ISSUES); + tw.addTree(head.getTree()); + tw.setRecursive(false); + while (tw.next()) { + if (tw.getDepth() < 2 && tw.isSubtree()) { + tw.enterSubtree(); + if (tw.getDepth() == 2) { + issuePaths.add(tw.getPathString()); + } + } + } + } catch (IOException e) { + error(e, repository, "{0} failed to query issues"); + } finally { + tw.release(); + } + + // Build each issue and optionally filter out unwanted issues + + for (String issuePath : issuePaths) { + RevWalk rw = new RevWalk(repository); + try { + RevCommit start = rw.parseCommit(repository.resolve(GB_ISSUES)); + rw.markStart(start); + } catch (Exception e) { + error(e, repository, "Failed to find {1} in {0}", GB_ISSUES); + } + TreeFilter treeFilter = AndTreeFilter.create( + PathFilterGroup.createFromStrings(issuePath), TreeFilter.ANY_DIFF); + rw.setTreeFilter(treeFilter); + Iterator revlog = rw.iterator(); + + List commits = new ArrayList(); + while (revlog.hasNext()) { + commits.add(revlog.next()); + } + + // release the revwalk + rw.release(); + + if (commits.size() == 0) { + LOGGER.warn("Failed to find changes for issue " + issuePath); + continue; + } + + // sort by commit order, first commit first + Collections.reverse(commits); + + StringBuilder sb = new StringBuilder("["); + boolean first = true; + for (RevCommit commit : commits) { + if (!first) { + sb.append(','); + } + String message = commit.getFullMessage(); + // commit message is formatted: C ISSUEID\n\nJSON + // C is an single char commit code + // ISSUEID is an SHA-1 hash + String json = message.substring(43); + sb.append(json); + first = false; + } + sb.append(']'); + + // Deserialize the JSON array as a Collection, this seems + // slightly faster than deserializing each change by itself. + Collection changes = JsonUtils.fromJsonString(sb.toString(), + new TypeToken>() { + }.getType()); + + // create an issue object form the changes + IssueModel issue = buildIssue(changes, true); + + // add the issue, conditionally, to the list if (filter == null) { list.add(issue); } else { @@ -104,19 +218,36 @@ public class IssueUtils { } } } + + // sort the issues by creation Collections.sort(list); return list; } /** - * Retrieves the specified issue from the repository with complete changes - * history. + * Retrieves the specified issue from the repository with all changes + * applied to build the effective issue. * * @param repository * @param issueId * @return an issue, if it exists, otherwise null */ public static IssueModel getIssue(Repository repository, String issueId) { + return getIssue(repository, issueId, true); + } + + /** + * Retrieves the specified issue from the repository. + * + * @param repository + * @param issueId + * @param effective + * if true, the effective issue is built by processing comment + * changes, deletions, etc. if false, the raw issue is built + * without consideration for comment changes, deletions, etc. + * @return an issue, if it exists, otherwise null + */ + public static IssueModel getIssue(Repository repository, String issueId, boolean effective) { RefModel issuesBranch = getIssuesBranch(repository); if (issuesBranch == null) { return null; @@ -126,12 +257,88 @@ public class IssueUtils { return null; } - // deserialize the issue model object - IssueModel issue = null; String issuePath = getIssuePath(issueId); - RevTree tree = JGitUtils.getCommit(repository, GB_ISSUES).getTree(); - String json = JGitUtils.getStringContent(repository, tree, issuePath + "/issue.json"); - issue = JsonUtils.fromJsonString(json, IssueModel.class); + + // Collect all changes as JSON array from commit messages + List commits = JGitUtils.getRevLog(repository, GB_ISSUES, issuePath, 0, -1); + + // sort by commit order, first commit first + Collections.reverse(commits); + + StringBuilder sb = new StringBuilder("["); + boolean first = true; + for (RevCommit commit : commits) { + if (!first) { + sb.append(','); + } + String message = commit.getFullMessage(); + // commit message is formatted: C ISSUEID\n\nJSON + // C is an single char commit code + // ISSUEID is an SHA-1 hash + String json = message.substring(43); + sb.append(json); + first = false; + } + sb.append(']'); + + // Deserialize the JSON array as a Collection, this seems + // slightly faster than deserializing each change by itself. + Collection changes = JsonUtils.fromJsonString(sb.toString(), + new TypeToken>() { + }.getType()); + + // create an issue object and apply the changes to it + IssueModel issue = buildIssue(changes, effective); + return issue; + } + + /** + * Builds an issue from a set of changes. + * + * @param changes + * @param effective + * if true, the effective issue is built which accounts for + * comment changes, comment deletions, etc. if false, the raw + * issue is built. + * @return an issue + */ + private static IssueModel buildIssue(Collection changes, boolean effective) { + IssueModel issue; + if (effective) { + List effectiveChanges = new ArrayList(); + Map comments = new HashMap(); + for (Change change : changes) { + if (change.comment != null) { + if (comments.containsKey(change.comment.id)) { + Change original = comments.get(change.comment.id); + Change clone = DeepCopier.copy(original); + clone.comment.text = change.comment.text; + clone.comment.deleted = change.comment.deleted; + int idx = effectiveChanges.indexOf(original); + effectiveChanges.remove(original); + effectiveChanges.add(idx, clone); + comments.put(clone.comment.id, clone); + } else { + effectiveChanges.add(change); + comments.put(change.comment.id, change); + } + } else { + effectiveChanges.add(change); + } + } + + // effective issue + issue = new IssueModel(); + for (Change change : effectiveChanges) { + issue.applyChange(change); + } + } else { + // raw issue + issue = new IssueModel(); + for (Change change : changes) { + issue.applyChange(change); + } + } return issue; } @@ -155,10 +362,7 @@ public class IssueUtils { } // deserialize the issue model so that we have the attachment metadata - String issuePath = getIssuePath(issueId); - RevTree tree = JGitUtils.getCommit(repository, GB_ISSUES).getTree(); - String json = JGitUtils.getStringContent(repository, tree, issuePath + "/issue.json"); - IssueModel issue = JsonUtils.fromJsonString(json, IssueModel.class); + IssueModel issue = getIssue(repository, issueId, true); Attachment attachment = issue.getAttachment(filename); // attachment not found @@ -167,15 +371,21 @@ public class IssueUtils { } // retrieve the attachment content - byte[] content = JGitUtils.getByteContent(repository, tree, issuePath + "/" + filename); + String issuePath = getIssuePath(issueId); + RevTree tree = JGitUtils.getCommit(repository, GB_ISSUES).getTree(); + byte[] content = JGitUtils + .getByteContent(repository, tree, issuePath + "/" + attachment.id); attachment.content = content; attachment.size = content.length; return attachment; } /** - * Stores an issue in the gb-issues branch of the repository. The branch is - * automatically created if it does not already exist. + * Creates an issue in the gb-issues branch of the repository. The branch is + * automatically created if it does not already exist. Your change must + * include an author, summary, and description, at a minimum. If your change + * does not have those minimum requirements a RuntimeException will be + * thrown. * * @param repository * @param change @@ -186,31 +396,27 @@ public class IssueUtils { if (issuesBranch == null) { JGitUtils.createOrphanBranch(repository, "gb-issues", null); } - change.created = new Date(); - IssueModel issue = new IssueModel(); - issue.created = change.created; - issue.summary = change.getString(Field.Summary); - issue.description = change.getString(Field.Description); - issue.reporter = change.getString(Field.Reporter); - - if (StringUtils.isEmpty(issue.summary)) { - throw new RuntimeException("Must specify an issue summary!"); + if (StringUtils.isEmpty(change.author)) { + throw new RuntimeException("Must specify a change author!"); } - if (StringUtils.isEmpty(change.getString(Field.Description))) { - throw new RuntimeException("Must specify an issue description!"); + if (!change.hasField(Field.Summary)) { + throw new RuntimeException("Must specify a summary!"); } - if (StringUtils.isEmpty(change.getString(Field.Reporter))) { - throw new RuntimeException("Must specify an issue reporter!"); + if (!change.hasField(Field.Description)) { + throw new RuntimeException("Must specify a description!"); } - issue.id = StringUtils.getSHA1(issue.created.toString() + issue.reporter + issue.summary - + issue.description); + change.setField(Field.Reporter, change.author); + + String issueId = StringUtils.getSHA1(change.created.toString() + change.author + + change.getString(Field.Summary) + change.getField(Field.Description)); + change.setField(Field.Id, issueId); + change.code = '+'; - String message = createChangelog('+', issue.id, change); - boolean success = commit(repository, issue, change, message); + boolean success = commit(repository, issueId, change); if (success) { - return issue; + return getIssue(repository, issueId, false); } return null; } @@ -236,71 +442,254 @@ public class IssueUtils { } if (StringUtils.isEmpty(change.author)) { - throw new RuntimeException("must specify change.author!"); + throw new RuntimeException("must specify a change author!"); } - IssueModel issue = getIssue(repository, issueId); - change.created = new Date(); - - String message = createChangelog('=', issueId, change); - success = commit(repository, issue, change, message); + // determine update code + // default update code is '=' for a general change + change.code = '='; + if (change.hasField(Field.Status)) { + Status status = Status.fromObject(change.getField(Field.Status)); + if (status.isClosed()) { + // someone closed the issue + change.code = 'x'; + } + } + success = commit(repository, issueId, change); return success; } - private static String createChangelog(char type, String issueId, Change change) { - return type + " " + issueId + "\n\n" + toJson(change); + /** + * Deletes an issue from the repository. + * + * @param repository + * @param issueId + * @return true if successful + */ + public static boolean deleteIssue(Repository repository, String issueId, String author) { + boolean success = false; + RefModel issuesBranch = getIssuesBranch(repository); + + if (issuesBranch == null) { + throw new RuntimeException("gb-issues branch does not exist!"); + } + + if (StringUtils.isEmpty(issueId)) { + throw new RuntimeException("must specify an issue id!"); + } + + String issuePath = getIssuePath(issueId); + + String message = "- " + issueId; + try { + ObjectId headId = repository.resolve(GB_ISSUES + "^{commit}"); + ObjectInserter odi = repository.newObjectInserter(); + try { + // Create the in-memory index of the new/updated issue + DirCache index = DirCache.newInCore(); + DirCacheBuilder dcBuilder = index.builder(); + // Traverse HEAD to add all other paths + TreeWalk treeWalk = new TreeWalk(repository); + int hIdx = -1; + if (headId != null) + hIdx = treeWalk.addTree(new RevWalk(repository).parseTree(headId)); + treeWalk.setRecursive(true); + while (treeWalk.next()) { + String path = treeWalk.getPathString(); + CanonicalTreeParser hTree = null; + if (hIdx != -1) + hTree = treeWalk.getTree(hIdx, CanonicalTreeParser.class); + if (!path.startsWith(issuePath)) { + // add entries from HEAD for all other paths + if (hTree != null) { + // create a new DirCacheEntry with data retrieved + // from HEAD + final DirCacheEntry dcEntry = new DirCacheEntry(path); + dcEntry.setObjectId(hTree.getEntryObjectId()); + dcEntry.setFileMode(hTree.getEntryFileMode()); + + // add to temporary in-core index + dcBuilder.add(dcEntry); + } + } + } + + // release the treewalk + treeWalk.release(); + + // finish temporary in-core index used for this commit + dcBuilder.finish(); + + ObjectId indexTreeId = index.writeTree(odi); + + // Create a commit object + PersonIdent ident = new PersonIdent(author, "gitblit@localhost"); + CommitBuilder commit = new CommitBuilder(); + commit.setAuthor(ident); + commit.setCommitter(ident); + commit.setEncoding(Constants.CHARACTER_ENCODING); + commit.setMessage(message); + commit.setParentId(headId); + commit.setTreeId(indexTreeId); + + // Insert the commit into the repository + ObjectId commitId = odi.insert(commit); + odi.flush(); + + RevWalk revWalk = new RevWalk(repository); + try { + RevCommit revCommit = revWalk.parseCommit(commitId); + RefUpdate ru = repository.updateRef(GB_ISSUES); + ru.setNewObjectId(commitId); + ru.setExpectedOldObjectId(headId); + ru.setRefLogMessage("commit: " + revCommit.getShortMessage(), false); + Result rc = ru.forceUpdate(); + switch (rc) { + case NEW: + case FORCED: + case FAST_FORWARD: + success = true; + break; + case REJECTED: + case LOCK_FAILURE: + throw new ConcurrentRefUpdateException(JGitText.get().couldNotLockHEAD, + ru.getRef(), rc); + default: + throw new JGitInternalException(MessageFormat.format( + JGitText.get().updatingRefFailed, GB_ISSUES, commitId.toString(), + rc)); + } + } finally { + revWalk.release(); + } + } finally { + odi.release(); + } + } catch (Throwable t) { + error(t, repository, "Failed to delete issue {1} to {0}", issueId); + } + return success; } /** + * Changes the text of an issue comment. * * @param repository * @param issue * @param change - * @param changelog - * @return + * the change with the comment to change + * @param author + * the author of the revision + * @param comment + * the revised comment + * @return true, if the change was successful */ - private static boolean commit(Repository repository, IssueModel issue, Change change, - String changelog) { - boolean success = false; - String issuePath = getIssuePath(issue.id); - try { - issue.addChange(change); + public static boolean changeComment(Repository repository, IssueModel issue, Change change, + String author, String comment) { + Change revision = new Change(author); + revision.comment(comment); + revision.comment.id = change.comment.id; + return updateIssue(repository, issue.id, revision); + } - // serialize the issue as json - String json = toJson(issue); + /** + * Deletes a comment from an issue. + * + * @param repository + * @param issue + * @param change + * the change with the comment to delete + * @param author + * @return true, if the deletion was successful + */ + public static boolean deleteComment(Repository repository, IssueModel issue, Change change, + String author) { + Change deletion = new Change(author); + deletion.comment(change.comment.text); + deletion.comment.id = change.comment.id; + deletion.comment.deleted = true; + return updateIssue(repository, issue.id, deletion); + } - // cache the issue "files" in a map - Map files = new HashMap(); - CommitFile issueFile = new CommitFile(issuePath + "/issue.json", change.created); - issueFile.content = json.getBytes(Constants.CHARACTER_ENCODING); - files.put(issueFile.path, issueFile); + /** + * Commit a change to the repository. Each issue is composed on changes. + * Issues are built from applying the changes in the order they were + * committed to the repository. The changes are actually specified in the + * commit messages and not in the RevTrees which allows for clean, + * distributed merging. + * + * @param repository + * @param issue + * @param change + * @return true, if the change was committed + */ + private static boolean commit(Repository repository, String issueId, Change change) { + boolean success = false; + try { + // assign ids to new attachments + // attachments are stored by an SHA1 id if (change.hasAttachments()) { for (Attachment attachment : change.attachments) { if (!ArrayUtils.isEmpty(attachment.content)) { - CommitFile file = new CommitFile(issuePath + "/" + attachment.name, - change.created); - file.content = attachment.content; - files.put(file.path, file); + byte[] prefix = (change.created.toString() + change.author).getBytes(); + byte[] bytes = new byte[prefix.length + attachment.content.length]; + System.arraycopy(prefix, 0, bytes, 0, prefix.length); + System.arraycopy(attachment.content, 0, bytes, prefix.length, + attachment.content.length); + attachment.id = "attachment-" + StringUtils.getSHA1(bytes); } } } - ObjectId headId = repository.resolve(GB_ISSUES + "^{commit}"); + // serialize the change as json + // exclude any attachment from json serialization + Gson gson = JsonUtils.gson(new ExcludeField( + "com.gitblit.models.IssueModel$Attachment.content")); + String json = gson.toJson(change); + + // include the json change in the commit message + String issuePath = getIssuePath(issueId); + String message = change.code + " " + issueId + "\n\n" + json; + + // Create a commit file. This is required for a proper commit and + // ensures we can retrieve the commit log of the issue path. + // + // This file is NOT serialized as part of the Change object. + switch (change.code) { + case '+': { + // New Issue. + Attachment placeholder = new Attachment("issue"); + placeholder.id = placeholder.name; + placeholder.content = "DO NOT REMOVE".getBytes(Constants.CHARACTER_ENCODING); + change.addAttachment(placeholder); + break; + } + default: { + // Update Issue. + String changeId = StringUtils.getSHA1(json); + Attachment placeholder = new Attachment("change-" + changeId); + placeholder.id = placeholder.name; + placeholder.content = "REMOVABLE".getBytes(Constants.CHARACTER_ENCODING); + change.addAttachment(placeholder); + break; + } + } + ObjectId headId = repository.resolve(GB_ISSUES + "^{commit}"); ObjectInserter odi = repository.newObjectInserter(); try { - // Create the in-memory index of the new/updated issue. - DirCache index = createIndex(repository, headId, files); + // Create the in-memory index of the new/updated issue + DirCache index = createIndex(repository, headId, issuePath, change); ObjectId indexTreeId = index.writeTree(odi); // Create a commit object - PersonIdent author = new PersonIdent(issue.reporter, issue.reporter + "@gitblit"); + PersonIdent ident = new PersonIdent(change.author, "gitblit@localhost"); CommitBuilder commit = new CommitBuilder(); - commit.setAuthor(author); - commit.setCommitter(author); + commit.setAuthor(ident); + commit.setCommitter(ident); commit.setEncoding(Constants.CHARACTER_ENCODING); - commit.setMessage(changelog); + commit.setMessage(message); commit.setParentId(headId); commit.setTreeId(indexTreeId); @@ -338,23 +727,11 @@ public class IssueUtils { odi.release(); } } catch (Throwable t) { - t.printStackTrace(); + error(t, repository, "Failed to commit issue {1} to {0}", issueId); } return success; } - private static String toJson(Object o) { - try { - // exclude the attachment content field from json serialization - Gson gson = JsonUtils.gson(new ExcludeField( - "com.gitblit.models.IssueModel$Attachment.content")); - String json = gson.toJson(o); - return json; - } catch (Throwable t) { - throw new RuntimeException(t); - } - } - /** * Returns the issue path. This follows the same scheme as Git's object * store path where the first two characters of the hash id are the root @@ -372,32 +749,38 @@ public class IssueUtils { * * @param repo * @param headId - * @param files - * @param time + * @param change * @return an in-memory index * @throws IOException */ - private static DirCache createIndex(Repository repo, ObjectId headId, - Map files) throws IOException { + private static DirCache createIndex(Repository repo, ObjectId headId, String issuePath, + Change change) throws IOException { DirCache inCoreIndex = DirCache.newInCore(); DirCacheBuilder dcBuilder = inCoreIndex.builder(); ObjectInserter inserter = repo.newObjectInserter(); + Set ignorePaths = new TreeSet(); try { - // Add the issue files to the temporary index - for (CommitFile file : files.values()) { - // create an index entry for the file - final DirCacheEntry dcEntry = new DirCacheEntry(file.path); - dcEntry.setLength(file.content.length); - dcEntry.setLastModified(file.time); - dcEntry.setFileMode(FileMode.REGULAR_FILE); - - // insert object - dcEntry.setObjectId(inserter.insert(Constants.OBJ_BLOB, file.content)); - - // add to temporary in-core index - dcBuilder.add(dcEntry); + // Add any attachments to the temporary index + if (change.hasAttachments()) { + for (Attachment attachment : change.attachments) { + // build a path name for the attachment and mark as ignored + String path = issuePath + "/" + attachment.id; + ignorePaths.add(path); + + // create an index entry for this attachment + final DirCacheEntry dcEntry = new DirCacheEntry(path); + dcEntry.setLength(attachment.content.length); + dcEntry.setLastModified(change.created.getTime()); + dcEntry.setFileMode(FileMode.REGULAR_FILE); + + // insert object + dcEntry.setObjectId(inserter.insert(Constants.OBJ_BLOB, attachment.content)); + + // add to temporary in-core index + dcBuilder.add(dcEntry); + } } // Traverse HEAD to add all other paths @@ -412,7 +795,7 @@ public class IssueUtils { CanonicalTreeParser hTree = null; if (hIdx != -1) hTree = treeWalk.getTree(hIdx, CanonicalTreeParser.class); - if (!files.containsKey(path)) { + if (!ignorePaths.contains(path)) { // add entries from HEAD for all other paths if (hTree != null) { // create a new DirCacheEntry with data retrieved from @@ -437,19 +820,4 @@ public class IssueUtils { } return inCoreIndex; } - - private static class CommitFile { - String path; - long time; - byte[] content; - - CommitFile(String path, Date date) { - this.path = path; - this.time = date.getTime(); - } - } - - public static interface IssueFilter { - public abstract boolean accept(IssueModel issue); - } -} +} \ No newline at end of file diff --git a/src/com/gitblit/utils/JGitUtils.java b/src/com/gitblit/utils/JGitUtils.java index 5d6011a2..96e6bf10 100644 --- a/src/com/gitblit/utils/JGitUtils.java +++ b/src/com/gitblit/utils/JGitUtils.java @@ -1408,7 +1408,7 @@ public class JGitUtils { // Create a tree object to reference from a commit TreeFormatter tree = new TreeFormatter(); - tree.append("NEWBRANCH", FileMode.REGULAR_FILE, blobId); + tree.append(".branch", FileMode.REGULAR_FILE, blobId); ObjectId treeId = odi.insert(tree); // Create a commit object diff --git a/tests/com/gitblit/tests/GitBlitSuite.java b/tests/com/gitblit/tests/GitBlitSuite.java index 747ce1f3..9e5caf0b 100644 --- a/tests/com/gitblit/tests/GitBlitSuite.java +++ b/tests/com/gitblit/tests/GitBlitSuite.java @@ -52,7 +52,7 @@ import com.gitblit.utils.JGitUtils; ObjectCacheTest.class, UserServiceTest.class, MarkdownUtilsTest.class, JGitUtilsTest.class, SyndicationUtilsTest.class, DiffUtilsTest.class, MetricUtilsTest.class, TicgitUtilsTest.class, GitBlitTest.class, FederationTests.class, RpcTests.class, - GitServletTest.class, GroovyScriptTest.class }) + GitServletTest.class, GroovyScriptTest.class, IssuesTest.class }) public class GitBlitSuite { public static final File REPOSITORIES = new File("git"); diff --git a/tests/com/gitblit/tests/IssuesTest.java b/tests/com/gitblit/tests/IssuesTest.java index 1522ec69..26b59956 100644 --- a/tests/com/gitblit/tests/IssuesTest.java +++ b/tests/com/gitblit/tests/IssuesTest.java @@ -16,6 +16,7 @@ package com.gitblit.tests; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -30,16 +31,23 @@ import com.gitblit.models.IssueModel.Attachment; import com.gitblit.models.IssueModel.Change; import com.gitblit.models.IssueModel.Field; import com.gitblit.models.IssueModel.Priority; +import com.gitblit.models.IssueModel.Status; import com.gitblit.utils.IssueUtils; import com.gitblit.utils.IssueUtils.IssueFilter; +/** + * Tests the mechanics of distributed issue management on the gb-issues branch. + * + * @author James Moger + * + */ public class IssuesTest { @Test - public void testInsertion() throws Exception { + public void testCreation() throws Exception { Repository repository = GitBlitSuite.getIssuesTestRepository(); // create and insert the issue - Change c1 = newChange("Test issue " + Long.toHexString(System.currentTimeMillis())); + Change c1 = newChange("testCreation() " + Long.toHexString(System.currentTimeMillis())); IssueModel issue = IssueUtils.createIssue(repository, c1); assertNotNull(issue.id); @@ -47,68 +55,165 @@ public class IssuesTest { IssueModel constructed = IssueUtils.getIssue(repository, issue.id); compare(issue, constructed); - // add a note and update - Change c2 = new Change(); - c2.author = "dave"; - c2.comment("yeah, this is working"); + assertEquals(1, constructed.changes.size()); + } + + @Test + public void testUpdates() throws Exception { + Repository repository = GitBlitSuite.getIssuesTestRepository(); + // C1: create the issue + Change c1 = newChange("testUpdates() " + Long.toHexString(System.currentTimeMillis())); + IssueModel issue = IssueUtils.createIssue(repository, c1); + assertNotNull(issue.id); + + IssueModel constructed = IssueUtils.getIssue(repository, issue.id); + compare(issue, constructed); + + // C2: set owner + Change c2 = new Change("C2"); + c2.comment("I'll fix this"); + c2.setField(Field.Owner, c2.author); assertTrue(IssueUtils.updateIssue(repository, issue.id, c2)); + constructed = IssueUtils.getIssue(repository, issue.id); + assertEquals(2, constructed.changes.size()); + assertEquals(c2.author, constructed.owner); + + // C3: add a note + Change c3 = new Change("C3"); + c3.comment("yeah, this is working"); + assertTrue(IssueUtils.updateIssue(repository, issue.id, c3)); + constructed = IssueUtils.getIssue(repository, issue.id); + assertEquals(3, constructed.changes.size()); + + // C4: add attachment + Change c4 = new Change("C4"); + Attachment a = newAttachment(); + c4.addAttachment(a); + assertTrue(IssueUtils.updateIssue(repository, issue.id, c4)); + + Attachment a1 = IssueUtils.getIssueAttachment(repository, issue.id, a.name); + assertEquals(a.content.length, a1.content.length); + assertTrue(Arrays.areEqual(a.content, a1.content)); + + // C5: close the issue + Change c5 = new Change("C5"); + c5.comment("closing issue"); + c5.setField(Field.Status, Status.Fixed); + assertTrue(IssueUtils.updateIssue(repository, issue.id, c5)); // retrieve issue again constructed = IssueUtils.getIssue(repository, issue.id); - assertEquals(2, constructed.changes.size()); + assertEquals(5, constructed.changes.size()); + assertTrue(constructed.status.isClosed()); - Attachment a = IssueUtils.getIssueAttachment(repository, issue.id, "test.txt"); repository.close(); - - assertEquals(10, a.content.length); - assertTrue(Arrays.areEqual(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }, a.content)); } @Test public void testQuery() throws Exception { Repository repository = GitBlitSuite.getIssuesTestRepository(); - List list = IssueUtils.getIssues(repository, null); - List list2 = IssueUtils.getIssues(repository, new IssueFilter() { - boolean hasFirst = false; + List allIssues = IssueUtils.getIssues(repository, null); + + List openIssues = IssueUtils.getIssues(repository, new IssueFilter() { @Override public boolean accept(IssueModel issue) { - if (!hasFirst) { - hasFirst = true; - return true; - } - return false; + return !issue.status.isClosed(); } }); + + List closedIssues = IssueUtils.getIssues(repository, new IssueFilter() { + @Override + public boolean accept(IssueModel issue) { + return issue.status.isClosed(); + } + }); + + repository.close(); + assertTrue(allIssues.size() > 0); + assertEquals(1, openIssues.size()); + assertEquals(1, closedIssues.size()); + } + + @Test + public void testDelete() throws Exception { + Repository repository = GitBlitSuite.getIssuesTestRepository(); + List allIssues = IssueUtils.getIssues(repository, null); + // delete all issues + for (IssueModel issue : allIssues) { + assertTrue(IssueUtils.deleteIssue(repository, issue.id, "D")); + } + repository.close(); + } + + @Test + public void testChangeComment() throws Exception { + Repository repository = GitBlitSuite.getIssuesTestRepository(); + // C1: create the issue + Change c1 = newChange("testChangeComment() " + Long.toHexString(System.currentTimeMillis())); + IssueModel issue = IssueUtils.createIssue(repository, c1); + assertNotNull(issue.id); + assertTrue(issue.changes.get(0).hasComment()); + + assertTrue(IssueUtils.changeComment(repository, issue, c1, "E1", "I changed the comment")); + issue = IssueUtils.getIssue(repository, issue.id); + assertTrue(issue.changes.get(0).hasComment()); + assertEquals("I changed the comment", issue.changes.get(0).comment.text); + + assertTrue(IssueUtils.deleteIssue(repository, issue.id, "D")); + + repository.close(); + } + + @Test + public void testDeleteComment() throws Exception { + Repository repository = GitBlitSuite.getIssuesTestRepository(); + // C1: create the issue + Change c1 = newChange("testDeleteComment() " + Long.toHexString(System.currentTimeMillis())); + IssueModel issue = IssueUtils.createIssue(repository, c1); + assertNotNull(issue.id); + assertTrue(issue.changes.get(0).hasComment()); + + assertTrue(IssueUtils.deleteComment(repository, issue, c1, "D1")); + issue = IssueUtils.getIssue(repository, issue.id); + assertEquals(1, issue.changes.size()); + assertFalse(issue.changes.get(0).hasComment()); + + issue = IssueUtils.getIssue(repository, issue.id, false); + assertEquals(2, issue.changes.size()); + assertTrue(issue.changes.get(0).hasComment()); + assertFalse(issue.changes.get(1).hasComment()); + + assertTrue(IssueUtils.deleteIssue(repository, issue.id, "D")); + repository.close(); - assertTrue(list.size() > 0); - assertEquals(1, list2.size()); } private Change newChange(String summary) { - Change change = new Change(); - change.setField(Field.Reporter, "james"); - change.setField(Field.Owner, "dave"); + Change change = new Change("C1"); change.setField(Field.Summary, summary); change.setField(Field.Description, "this is my description"); change.setField(Field.Priority, Priority.High); change.setField(Field.Labels, "helpdesk"); change.comment("my comment"); - - Attachment attachment = new Attachment("test.txt"); - attachment.content = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; - change.addAttachment(attachment); - return change; } + private Attachment newAttachment() { + Attachment attachment = new Attachment(Long.toHexString(System.currentTimeMillis()) + + ".txt"); + attachment.content = new byte[] { 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, + 0x4a }; + return attachment; + } + private void compare(IssueModel issue, IssueModel constructed) { assertEquals(issue.id, constructed.id); assertEquals(issue.reporter, constructed.reporter); assertEquals(issue.owner, constructed.owner); - assertEquals(issue.created.getTime() / 1000, constructed.created.getTime() / 1000); assertEquals(issue.summary, constructed.summary); assertEquals(issue.description, constructed.description); + assertEquals(issue.created, constructed.created); assertTrue(issue.hasLabel("helpdesk")); } -- 2.39.5