From c353645a09b677644c55daa9dcff7df54529dc04 Mon Sep 17 00:00:00 2001 From: Nitzan Gur-Furman Date: Wed, 19 Jul 2023 10:37:39 +0200 Subject: [PATCH] Move footer-line parsing methods from RevCommit to FooterLine This allows extracting footers from a messages not associated with a commit. The public API of RevCommit is kept intact. Change-Id: I5809c23df7b7d49641a4be3a26d6f987d3d57c9b Bug: Google b/287891316 --- .../eclipse/jgit/revwalk/FooterLineTest.java | 191 +++++++++--------- .../org/eclipse/jgit/revwalk/FooterLine.java | 105 ++++++++++ .../org/eclipse/jgit/revwalk/RevCommit.java | 76 ++----- .../org/eclipse/jgit/util/RawParseUtils.java | 19 ++ 4 files changed, 228 insertions(+), 163 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java index 113f3bee8..01f6a3a0a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java @@ -16,76 +16,74 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import java.io.IOException; import java.util.List; import org.eclipse.jgit.junit.RepositoryTestCase; -import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.junit.Test; public class FooterLineTest extends RepositoryTestCase { @Test - public void testNoFooters_EmptyBody() throws IOException { - final RevCommit commit = parse(""); - final List footers = commit.getFooterLines(); + public void testNoFooters_EmptyBody() { + String msg = buildMessage(""); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_NewlineOnlyBody1() throws IOException { - final RevCommit commit = parse("\n"); - final List footers = commit.getFooterLines(); + public void testNoFooters_NewlineOnlyBody1() { + String msg = buildMessage("\n"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_NewlineOnlyBody5() throws IOException { - final RevCommit commit = parse("\n\n\n\n\n"); - final List footers = commit.getFooterLines(); + public void testNoFooters_NewlineOnlyBody5() { + String msg = buildMessage("\n\n\n\n\n"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_OneLineBodyNoLF() throws IOException { - final RevCommit commit = parse("this is a commit"); - final List footers = commit.getFooterLines(); + public void testNoFooters_OneLineBodyNoLF() { + String msg = buildMessage("this is a commit"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_OneLineBodyWithLF() throws IOException { - final RevCommit commit = parse("this is a commit\n"); - final List footers = commit.getFooterLines(); + public void testNoFooters_OneLineBodyWithLF() { + String msg = buildMessage("this is a commit\n"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_ShortBodyNoLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit"); - final List footers = commit.getFooterLines(); + public void testNoFooters_ShortBodyNoLF() { + String msg = buildMessage("subject\n\nbody of commit"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testNoFooters_ShortBodyWithLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n"); - final List footers = commit.getFooterLines(); + public void testNoFooters_ShortBodyWithLF() { + String msg = buildMessage("subject\n\nbody of commit\n"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(0, footers.size()); } @Test - public void testSignedOffBy_OneUserNoLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Signed-off-by: A. U. Thor "); - final List footers = commit.getFooterLines(); + public void testSignedOffBy_OneUserNoLF() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Signed-off-by: A. U. Thor "); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -98,10 +96,10 @@ public void testSignedOffBy_OneUserNoLF() throws IOException { } @Test - public void testSignedOffBy_OneUserWithLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Signed-off-by: A. U. Thor \n"); - final List footers = commit.getFooterLines(); + public void testSignedOffBy_OneUserWithLF() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Signed-off-by: A. U. Thor \n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -114,14 +112,13 @@ public void testSignedOffBy_OneUserWithLF() throws IOException { } @Test - public void testSignedOffBy_IgnoreWhitespace() throws IOException { + public void testSignedOffBy_IgnoreWhitespace() { // We only ignore leading whitespace on the value, trailing // is assumed part of the value. // - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Signed-off-by: A. U. Thor \n"); - final List footers = commit.getFooterLines(); - FooterLine f; + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Signed-off-by: A. U. Thor \n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); assertEquals(1, footers.size()); @@ -133,10 +130,10 @@ public void testSignedOffBy_IgnoreWhitespace() throws IOException { } @Test - public void testEmptyValueNoLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Signed-off-by:"); - final List footers = commit.getFooterLines(); + public void testEmptyValueNoLF() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Signed-off-by:"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -149,10 +146,10 @@ public void testEmptyValueNoLF() throws IOException { } @Test - public void testEmptyValueWithLF() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Signed-off-by:\n"); - final List footers = commit.getFooterLines(); + public void testEmptyValueWithLF() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Signed-off-by:\n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -165,10 +162,10 @@ public void testEmptyValueWithLF() throws IOException { } @Test - public void testShortKey() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "K:V\n"); - final List footers = commit.getFooterLines(); + public void testShortKey() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "K:V\n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -181,10 +178,10 @@ public void testShortKey() throws IOException { } @Test - public void testNonDelimtedEmail() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Acked-by: re@example.com\n"); - final List footers = commit.getFooterLines(); + public void testNonDelimtedEmail() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Acked-by: re@example.com\n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -197,10 +194,10 @@ public void testNonDelimtedEmail() throws IOException { } @Test - public void testNotEmail() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" + "\n" - + "Acked-by: Main Tain Er\n"); - final List footers = commit.getFooterLines(); + public void testNotEmail() { + String msg = buildMessage("subject\n\nbody of commit\n" + "\n" + + "Acked-by: Main Tain Er\n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -213,15 +210,15 @@ public void testNotEmail() throws IOException { } @Test - public void testSignedOffBy_ManyUsers() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" - + "Not-A-Footer-Line: this line must not be read as a footer\n" - + "\n" // paragraph break, now footers appear in final block - + "Signed-off-by: A. U. Thor \n" - + "CC: \n" - + "Acked-by: Some Reviewer \n" - + "Signed-off-by: Main Tain Er \n"); - final List footers = commit.getFooterLines(); + public void testSignedOffBy_ManyUsers() { + String msg = buildMessage("subject\n\nbody of commit\n" + + "Not-A-Footer-Line: this line must not be read as a footer\n" + + "\n" // paragraph break, now footers appear in final block + + "Signed-off-by: A. U. Thor \n" + + "CC: \n" + + "Acked-by: Some Reviewer \n" + + "Signed-off-by: Main Tain Er \n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -249,16 +246,16 @@ public void testSignedOffBy_ManyUsers() throws IOException { } @Test - public void testSignedOffBy_SkipNonFooter() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" - + "Not-A-Footer-Line: this line must not be read as a footer\n" - + "\n" // paragraph break, now footers appear in final block - + "Signed-off-by: A. U. Thor \n" - + "CC: \n" - + "not really a footer line but we'll skip it anyway\n" - + "Acked-by: Some Reviewer \n" - + "Signed-off-by: Main Tain Er \n"); - final List footers = commit.getFooterLines(); + public void testSignedOffBy_SkipNonFooter() { + String msg = buildMessage("subject\n\nbody of commit\n" + + "Not-A-Footer-Line: this line must not be read as a footer\n" + + "\n" // paragraph break, now footers appear in final block + + "Signed-off-by: A. U. Thor \n" + + "CC: \n" + + "not really a footer line but we'll skip it anyway\n" + + "Acked-by: Some Reviewer \n" + + "Signed-off-by: Main Tain Er \n"); + List footers = FooterLine.fromMessage(msg); FooterLine f; assertNotNull(footers); @@ -282,15 +279,16 @@ public void testSignedOffBy_SkipNonFooter() throws IOException { } @Test - public void testFilterFootersIgnoreCase() throws IOException { - final RevCommit commit = parse("subject\n\nbody of commit\n" - + "Not-A-Footer-Line: this line must not be read as a footer\n" - + "\n" // paragraph break, now footers appear in final block - + "Signed-Off-By: A. U. Thor \n" - + "CC: \n" - + "Acked-by: Some Reviewer \n" - + "signed-off-by: Main Tain Er \n"); - final List footers = commit.getFooterLines("signed-off-by"); + public void testFilterFootersIgnoreCase() { + String msg = buildMessage("subject\n\nbody of commit\n" + + "Not-A-Footer-Line: this line must not be read as a footer\n" + + "\n" // paragraph break, now footers appear in final block + + "Signed-Off-By: A. U. Thor \n" + + "CC: \n" + + "Acked-by: Some Reviewer \n" + + "signed-off-by: Main Tain Er \n"); + List footers = FooterLine.getValues( + FooterLine.fromMessage(msg), "signed-off-by"); assertNotNull(footers); assertEquals(2, footers.size()); @@ -300,38 +298,33 @@ public void testFilterFootersIgnoreCase() throws IOException { } @Test - public void testMatchesBugId() throws IOException { - final RevCommit commit = parse("this is a commit subject for test\n" - + "\n" // paragraph break, now footers appear in final block - + "Simple-Bug-Id: 42\n"); - final List footers = commit.getFooterLines(); + public void testMatchesBugId() { + String msg = buildMessage("this is a commit subject for test\n" + + "\n" // paragraph break, now footers appear in final block + + "Simple-Bug-Id: 42\n"); + List footers = FooterLine.fromMessage(msg); assertNotNull(footers); assertEquals(1, footers.size()); - final FooterLine line = footers.get(0); + FooterLine line = footers.get(0); assertNotNull(line); assertEquals("Simple-Bug-Id", line.getKey()); assertEquals("42", line.getValue()); - final FooterKey bugid = new FooterKey("Simple-Bug-Id"); + FooterKey bugid = new FooterKey("Simple-Bug-Id"); assertTrue("matches Simple-Bug-Id", line.matches(bugid)); assertFalse("not Signed-off-by", line.matches(FooterKey.SIGNED_OFF_BY)); assertFalse("not CC", line.matches(FooterKey.CC)); } - private RevCommit parse(String msg) throws IOException { - final StringBuilder buf = new StringBuilder(); + private String buildMessage(String msg) { + StringBuilder buf = new StringBuilder(); buf.append("tree " + ObjectId.zeroId().name() + "\n"); buf.append("author A. U. Thor 1 +0000\n"); buf.append("committer A. U. Thor 1 +0000\n"); buf.append("\n"); buf.append(msg); - - try (RevWalk walk = new RevWalk(db)) { - RevCommit c = new RevCommit(ObjectId.zeroId()); - c.parseCanonical(walk, Constants.encode(buf.toString())); - return c; - } + return buf.toString(); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java index 676573c2f..eeaccc6d3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java @@ -11,6 +11,9 @@ package org.eclipse.jgit.revwalk; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.eclipse.jgit.util.RawParseUtils; @@ -46,6 +49,108 @@ public final class FooterLine { valEnd = ve; } + /** + * Extract the footer lines from the given message. + * + * @param str + * the message to extract footers from. + * @return ordered list of footer lines; empty list if no footers found. + * @see RevCommit#getFooterLines() + */ + public static List fromMessage( + String str) { + return fromMessage(str.getBytes()); + } + + /** + * Extract the footer lines from the given message. + * + * @param raw + * the raw message to extract footers from. + * @return ordered list of footer lines; empty list if no footers found. + * @see RevCommit#getFooterLines() + */ + public static List fromMessage( + byte[] raw) { + int ptr = raw.length - 1; + while (raw[ptr] == '\n') // trim any trailing LFs, not interesting + ptr--; + + int msgB = RawParseUtils.commitMessage(raw, 0); + ArrayList r = new ArrayList<>(4); + Charset enc = RawParseUtils.guessEncoding(raw); + for (;;) { + ptr = RawParseUtils.prevLF(raw, ptr); + if (ptr <= msgB) + break; // Don't parse commit headers as footer lines. + + int keyStart = ptr + 2; + if (raw[keyStart] == '\n') + break; // Stop at first paragraph break, no footers above it. + + int keyEnd = RawParseUtils.endOfFooterLineKey(raw, keyStart); + if (keyEnd < 0) + continue; // Not a well formed footer line, skip it. + + // Skip over the ': *' at the end of the key before the value. + // + int valStart = keyEnd + 1; + while (valStart < raw.length && raw[valStart] == ' ') + valStart++; + + // Value ends at the LF, and does not include it. + // + int valEnd = RawParseUtils.nextLF(raw, valStart); + if (raw[valEnd - 1] == '\n') + valEnd--; + + r.add(new FooterLine(raw, enc, keyStart, keyEnd, valStart, valEnd)); + } + Collections.reverse(r); + return r; + } + + /** + * Get the values of all footer lines with the given key. + * + * @param footers + * list of footers to find the values in. + * @param keyName + * footer key to find values of, case-insensitive. + * @return values of footers with key of {@code keyName}, ordered by their + * order of appearance. Duplicates may be returned if the same + * footer appeared more than once. Empty list if no footers appear + * with the specified key, or there are no footers at all. + * @see #fromMessage + */ + public static List getValues(List footers, String keyName) { + return getValues(footers, new FooterKey(keyName)); + } + + /** + * Get the values of all footer lines with the given key. + * + * @param footers + * list of footers to find the values in. + * @param key + * footer key to find values of, case-insensitive. + * @return values of footers with key of {@code keyName}, ordered by their + * order of appearance. Duplicates may be returned if the same + * footer appeared more than once. Empty list if no footers appear + * with the specified key, or there are no footers at all. + * @see #fromMessage + */ + public static List getValues(List footers, FooterKey key) { + if (footers.isEmpty()) + return Collections.emptyList(); + ArrayList r = new ArrayList<>(footers.size()); + for (FooterLine f : footers) { + if (f.matches(key)) + r.add(f.getValue()); + } + return r; + } + /** * Whether keys match * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java index 461993814..0392ea428 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java @@ -11,15 +11,13 @@ package org.eclipse.jgit.revwalk; -import static java.nio.charset.StandardCharsets.UTF_8; +import static org.eclipse.jgit.util.RawParseUtils.guessEncoding; import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.UnsupportedCharsetException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.eclipse.jgit.annotations.Nullable; @@ -484,7 +482,8 @@ public final String getFullMessage() { if (msgB < 0) { return ""; //$NON-NLS-1$ } - return RawParseUtils.decode(guessEncoding(), raw, msgB, raw.length); + return RawParseUtils.decode(guessEncoding(buffer), raw, msgB, + raw.length); } /** @@ -510,7 +509,8 @@ public final String getShortMessage() { } int msgE = RawParseUtils.endOfParagraph(raw, msgB); - String str = RawParseUtils.decode(guessEncoding(), raw, msgB, msgE); + String str = RawParseUtils.decode(guessEncoding(buffer), raw, msgB, + msgE); if (hasLF(raw, msgB, msgE)) { str = StringUtils.replaceLineBreaksWithSpace(str); } @@ -562,14 +562,6 @@ public final Charset getEncoding() { return RawParseUtils.parseEncoding(buffer); } - private Charset guessEncoding() { - try { - return getEncoding(); - } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { - return UTF_8; - } - } - /** * Parse the footer lines (e.g. "Signed-off-by") for machine processing. *

@@ -592,50 +584,14 @@ private Charset guessEncoding() { * @return ordered list of footer lines; empty list if no footers found. */ public final List getFooterLines() { - final byte[] raw = buffer; - int ptr = raw.length - 1; - while (raw[ptr] == '\n') // trim any trailing LFs, not interesting - ptr--; - - final int msgB = RawParseUtils.commitMessage(raw, 0); - final ArrayList r = new ArrayList<>(4); - final Charset enc = guessEncoding(); - for (;;) { - ptr = RawParseUtils.prevLF(raw, ptr); - if (ptr <= msgB) - break; // Don't parse commit headers as footer lines. - - final int keyStart = ptr + 2; - if (raw[keyStart] == '\n') - break; // Stop at first paragraph break, no footers above it. - - final int keyEnd = RawParseUtils.endOfFooterLineKey(raw, keyStart); - if (keyEnd < 0) - continue; // Not a well formed footer line, skip it. - - // Skip over the ': *' at the end of the key before the value. - // - int valStart = keyEnd + 1; - while (valStart < raw.length && raw[valStart] == ' ') - valStart++; - - // Value ends at the LF, and does not include it. - // - int valEnd = RawParseUtils.nextLF(raw, valStart); - if (raw[valEnd - 1] == '\n') - valEnd--; - - r.add(new FooterLine(raw, enc, keyStart, keyEnd, valStart, valEnd)); - } - Collections.reverse(r); - return r; + return FooterLine.fromMessage(buffer); } /** * Get the values of all footer lines with the given key. * * @param keyName - * footer key to find values of, case insensitive. + * footer key to find values of, case-insensitive. * @return values of footers with key of {@code keyName}, ordered by their * order of appearance. Duplicates may be returned if the same * footer appeared more than once. Empty list if no footers appear @@ -643,30 +599,22 @@ public final List getFooterLines() { * @see #getFooterLines() */ public final List getFooterLines(String keyName) { - return getFooterLines(new FooterKey(keyName)); + return FooterLine.getValues(getFooterLines(), keyName); } /** * Get the values of all footer lines with the given key. * - * @param keyName - * footer key to find values of, case insensitive. + * @param key + * footer key to find values of, case-insensitive. * @return values of footers with key of {@code keyName}, ordered by their * order of appearance. Duplicates may be returned if the same * footer appeared more than once. Empty list if no footers appear * with the specified key, or there are no footers at all. * @see #getFooterLines() */ - public final List getFooterLines(FooterKey keyName) { - final List src = getFooterLines(); - if (src.isEmpty()) - return Collections.emptyList(); - final ArrayList r = new ArrayList<>(src.size()); - for (FooterLine f : src) { - if (f.matches(keyName)) - r.add(f.getValue()); - } - return r; + public final List getFooterLines(FooterKey key) { + return FooterLine.getValues(getFooterLines(), key); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java index 0e8e9b3d8..1c98336b9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java @@ -867,6 +867,25 @@ public static Charset parseEncoding(byte[] b) { } } + /** + * Parse the "encoding " header into a character set reference. + *

+ * If unsuccessful, return UTF-8. + * + * @param buffer + * buffer to scan. + * @return the Java character set representation. Never null. Default to + * UTF-8. + * @see #parseEncoding(byte[]) + */ + public static Charset guessEncoding(byte[] buffer) { + try { + return parseEncoding(buffer); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + return UTF_8; + } + } + /** * Parse a name string (e.g. author, committer, tagger) into a PersonIdent. *