Allow analytics to aggregate by branch When extract-branches is set, the analytics plugin aggregates commits also by branch name within the aggregation strategy established by the `aggregation` argument. The aggregated branch is also exposed as a value via the `branches` field of the response payload. Also, Improve extraction of branches performance by computing a lookup table only once rather than for every objectId. This code is now dependent on gerrit-api version 2.16-rc1. Feature: Issue 9863 Feature: Issue 9864 Bug: Issue 9924 Bug: Issue 9950 Change-Id: Ib2d58f8dda397af476a7307e90dc7466a7edff2c
diff --git a/README.md b/README.md index 29bc565..1b01f3e 100644 --- a/README.md +++ b/README.md
@@ -69,7 +69,7 @@ - --since -b Starting timestamp to consider - --until -e Ending timestamp (excluded) to consider - --aggregate -granularity -g one of email, email_year, email_month, email_day, email_hour defaulting to aggregation by email -- --extract-branches -r enables branches extraction for each commit +- --extract-branches -r enables splitting of aggregation by branch name and expose branch name in the payload - --extract-issues -i enables the extraction of issues from commentLink NOTE: Timestamp format is consistent with Gerrit's query syntax, see /Documentation/user-search.html for details. @@ -79,7 +79,7 @@ ``` $ curl http://gerrit.mycompany.com/projects/myproject/analytics~contributors {"name":"John Doe","email":"john.doe@mycompany.com","num_commits":1, "num_files":4,"added_lines":9,"deleted_lines":1, "commits":[{"sha1":"6a1f73738071e299f600017d99f7252d41b96b4b","date":"Apr 28, 2011 5:13:14 AM","merge":false}]} - {"name":"Matt Smith","email":"matt.smith@mycompany.com","num_commits":1, "num_files":1,"added_lines":90,"deleted_lines":10,"commits":[{"sha1":"54527e7e3086758a23e3b069f183db6415aca304","date":"Sep 8, 2015 3:11:23 AM","merge":true}],"branches":["master","branch1"]} + {"name":"Matt Smith","email":"matt.smith@mycompany.com","num_commits":1, "num_files":1,"added_lines":90,"deleted_lines":10,"commits":[{"sha1":"54527e7e3086758a23e3b069f183db6415aca304","date":"Sep 8, 2015 3:11:23 AM","merge":true}],"branches":["master"]} ``` SSH Example: @@ -87,6 +87,6 @@ ``` $ ssh -p 29418 admin@gerrit.mycompany.com analytics contributors myproject --since 2017-08-01 --until 2017-12-31 --extract-issues {"name":"John Doe","email":"john.doe@mycompany.com","num_commits":1, "num_files":4,"added_lines":9,"deleted_lines":1, "commits":[{"sha1":"6a1f73738071e299f600017d99f7252d41b96b4b","date":"Apr 28, 2011 5:13:14 AM","merge":false}], "issues_codes":["PRJ-001"],"issues_links":["https://jira.company.org/PRJ-001"]} - {"name":"Matt Smith","email":"matt.smith@mycompany.com","num_commits":1, "num_files":1,"added_lines":90,"deleted_lines":10,"commits":[{"sha1":"54527e7e3086758a23e3b069f183db6415aca304","date":"Sep 8, 2015 3:11:23 AM","merge":true}],"branches":["master","branch1"],"issues_codes":["PRJ-002","PRJ-003"],"issues_links":["https://jira.company.org/PRJ-002","https://jira.company.org/PRJ-003"]} + {"name":"Matt Smith","email":"matt.smith@mycompany.com","num_commits":1, "num_files":1,"added_lines":90,"deleted_lines":10,"commits":[{"sha1":"54527e7e3086758a23e3b069f183db6415aca304","date":"Sep 8, 2015 3:11:23 AM","merge":true}],"branches":["branch1"],"issues_codes":["PRJ-002","PRJ-003"],"issues_links":["https://jira.company.org/PRJ-002","https://jira.company.org/PRJ-003"]} ```
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala index 3c1253a..4b6c5e7 100644 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
@@ -137,6 +137,7 @@ histogram: UserActivityHistogram, gsonFmt: GsonFormatter) { import RichBoolean._ + import scala.collection.JavaConverters._ def get(projectRes: ProjectResource, startDate: Option[Long], stopDate: Option[Long], @@ -147,15 +148,15 @@ projectCache.get(nameKey).getCommentLinks.asScala }.toList.flatten + + ManagedResource.use(repoManager.openRepository(projectRes.getNameKey)) { repo => val stats = new Statistics(repo, commentLinks.asJava) - val commitsBranchesOptionalEnricher = extractBranches.option( - new CommitsBranches(repo, startDate, stopDate) - ) - histogram.get(repo, new AggregatedHistogramFilterByDates(startDate, stopDate, - aggregationStrategy)) + val branchesExtractor = extractBranches.option(new BranchesExtractor(repo)) + + histogram.get(repo, new AggregatedHistogramFilterByDates(startDate, stopDate, branchesExtractor, aggregationStrategy)) .par - .flatMap(UserActivitySummary.apply(stats, commitsBranchesOptionalEnricher)) + .flatMap(aggregatedCommitActivity => UserActivitySummary.apply(stats)(aggregatedCommitActivity)) .toStream } } @@ -185,24 +186,21 @@ ) object UserActivitySummary { - def apply(statisticsHandler: Statistics, - branchesLabeler: Option[CommitsBranches]) - (uca: AggregatedUserCommitActivity) + def apply(statisticsHandler: Statistics)(uca: AggregatedUserCommitActivity) : Iterable[UserActivitySummary] = { - val INCLUDESEMPTY = -1 implicit def stringToIntOrNull(x: String): Integer = if (x.isEmpty) null else new Integer(x) - uca.key.split("/", INCLUDESEMPTY) match { - case Array(email, year, month, day, hour) => - val branches = branchesLabeler.fold(Set.empty[String]) { - labeler => labeler.forCommits(uca.getIds) - } + uca.key.split("/", AggregationStrategy.MAX_MAPPING_TOKENS) match { + case Array(email, year, month, day, hour, branch) => statisticsHandler.forCommits(uca.getIds: _*).map { stat => + val maybeBranches = + Option(branch).filter(_.nonEmpty).map(b => Array(b)).getOrElse(Array.empty) + UserActivitySummary( - year, month, day, hour, uca.getName, uca.getEmail, uca.getCount, + year, month, day, hour, uca.getName, email, stat.commits.size, stat.numFiles, stat.numDistinctFiles, stat.addedLines, stat.deletedLines, - stat.commits.toArray, branches.toArray, stat.issues.map(_.code) + stat.commits.toArray, maybeBranches, stat.issues.map(_.code) .toArray, stat.issues.map(_.link).toArray, uca.getLatest, stat .isForMergeCommits )
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala index 5420a28..74abf08 100644 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala
@@ -16,7 +16,7 @@ import java.util.Date -import com.googlesource.gerrit.plugins.analytics.common.AggregatedCommitHistogram.AggregationStrategyMapping +import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.BY_BRANCH import org.eclipse.jgit.lib.PersonIdent import org.eclipse.jgit.revwalk.RevCommit import org.gitective.core.stat.{CommitHistogram, CommitHistogramFilter, UserCommitActivity} @@ -24,11 +24,20 @@ class AggregatedUserCommitActivity(val key: String, val name: String, val email: String) extends UserCommitActivity(name, email) -class AggregatedCommitHistogram(val aggregationStrategyForUser: AggregationStrategyMapping) +class AggregatedCommitHistogram(var aggregationStrategy: AggregationStrategy) extends CommitHistogram { + def includeWithBranches(commit: RevCommit, user: PersonIdent, branches: Set[String]): Unit = { + for ( branch <- branches ) { + val originalStrategy = aggregationStrategy + this.aggregationStrategy = BY_BRANCH(branch, aggregationStrategy) + this.include(commit, user) + this.aggregationStrategy = originalStrategy + } + } + override def include(commit: RevCommit, user: PersonIdent): AggregatedCommitHistogram = { - val key = aggregationStrategyForUser(user, commit.getAuthorIdent.getWhen) + val key = aggregationStrategy.mapping(user, commit.getAuthorIdent.getWhen) val activity = Option(users.get(key)) match { case None => val newActivity = new AggregatedUserCommitActivity(key, @@ -48,14 +57,11 @@ object AggregatedCommitHistogram { type AggregationStrategyMapping = (PersonIdent, Date) => String - - def apply(aggregationStrategy: AggregationStrategyMapping) = - new AggregatedCommitHistogram(aggregationStrategy) } -abstract class AbstractCommitHistogramFilter(aggregationStrategyMapping: AggregationStrategyMapping) +abstract class AbstractCommitHistogramFilter(aggregationStrategy: AggregationStrategy) extends CommitHistogramFilter { - val AbstractHistogram = new AggregatedCommitHistogram(aggregationStrategyMapping) + val AbstractHistogram = new AggregatedCommitHistogram(aggregationStrategy) override def getHistogram = AbstractHistogram }
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedHistogramFilterByDates.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedHistogramFilterByDates.scala index 9f92e3c..bc1315e 100644 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedHistogramFilterByDates.scala +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedHistogramFilterByDates.scala
@@ -21,19 +21,26 @@ * starting from and to excluded */ class AggregatedHistogramFilterByDates(val from: Option[Long] = None, val to: Option[Long] = None, + val branchesExtractor: Option[BranchesExtractor] = None, val aggregationStrategy: AggregationStrategy = AggregationStrategy.EMAIL) - extends AbstractCommitHistogramFilter(aggregationStrategy.mapping) { + extends AbstractCommitHistogramFilter(aggregationStrategy) { override def include(walker: RevWalk, commit: RevCommit) = { val commitDate = commit.getCommitterIdent.getWhen.getTime val author = commit.getAuthorIdent + if (from.fold(true)(commitDate >=) && to.fold(true)(commitDate <)) { - getHistogram.include(commit, author) + if(branchesExtractor.isDefined) { + val branches = branchesExtractor.get.branchesOfCommit(commit.getId) + getHistogram.includeWithBranches(commit, author, branches) + } else { + getHistogram.include(commit, author) + } true } else { false } } - override def clone = new AggregatedHistogramFilterByDates(from, to, aggregationStrategy) + override def clone = new AggregatedHistogramFilterByDates(from, to, branchesExtractor, aggregationStrategy) } \ No newline at end of file
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala index a7e559f..079c307 100644 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala
@@ -19,35 +19,55 @@ import java.util.Date import com.googlesource.gerrit.plugins.analytics.common.AggregatedCommitHistogram.AggregationStrategyMapping +import org.eclipse.jgit.lib.PersonIdent -sealed case class AggregationStrategy(name: String, mapping: AggregationStrategyMapping) +sealed trait AggregationStrategy { + def name: String + def mapping: AggregationStrategyMapping +} object AggregationStrategy { - val values = List(EMAIL, EMAIL_HOUR, EMAIL_DAY, EMAIL_MONTH, EMAIL_YEAR) + val baseValues = List(EMAIL, EMAIL_HOUR, EMAIL_DAY, EMAIL_MONTH, EMAIL_YEAR) + val MAX_MAPPING_TOKENS = 6 def apply(name: String): AggregationStrategy = - values.find(_.name == name.toUpperCase) match { + baseValues.find(_.name == name.toUpperCase) match { case Some(g) => g case None => throw new InvalidParameterException( - s"Must be one of: ${values.map(_.name).mkString(",")}") + s"Must be one of: ${baseValues.map(_.name).mkString(",")}") } implicit class PimpedDate(val d: Date) extends AnyVal { def utc: LocalDateTime = d.toInstant.atZone(ZoneOffset.UTC).toLocalDateTime } - object EMAIL extends AggregationStrategy("EMAIL", - (p, _) => s"${p.getEmailAddress}////") + object EMAIL extends AggregationStrategy { + val name: String = "EMAIL" + val mapping: (PersonIdent, Date) => String = (p, _) => s"${p.getEmailAddress}/////" + } - object EMAIL_YEAR extends AggregationStrategy("EMAIL_YEAR", - (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}///") + object EMAIL_YEAR extends AggregationStrategy { + val name: String = "EMAIL_YEAR" + val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}////" + } - object EMAIL_MONTH extends AggregationStrategy("EMAIL_MONTH", - (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}//") + object EMAIL_MONTH extends AggregationStrategy { + val name: String = "EMAIL_MONTH" + val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}///" + } - object EMAIL_DAY extends AggregationStrategy("EMAIL_DAY", - (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}/") + object EMAIL_DAY extends AggregationStrategy { + val name: String = "EMAIL_DAY" + val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}//" + } - object EMAIL_HOUR extends AggregationStrategy("EMAIL_HOUR", - (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}/${d.utc.getHour}") + object EMAIL_HOUR extends AggregationStrategy { + val name: String = "EMAIL_HOUR" + val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}/${d.utc.getHour}/" + } + + case class BY_BRANCH(branch: String, baseAggregationStrategy: AggregationStrategy) extends AggregationStrategy { + val name: String = s"BY_BRANCH($branch)" + val mapping: (PersonIdent, Date) => String = (p, d) => s"${baseAggregationStrategy.mapping(p, d)}$branch" + } }
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractor.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractor.scala new file mode 100644 index 0000000..9703da1 --- /dev/null +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractor.scala
@@ -0,0 +1,44 @@ +// Copyright (C) 2017 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.analytics.common + +import com.googlesource.gerrit.plugins.analytics.common.ManagedResource.use +import org.eclipse.jgit.api.Git +import org.eclipse.jgit.lib.{Constants, ObjectId, Repository} +import org.eclipse.jgit.revwalk.RevWalk + +import scala.collection.JavaConversions._ + +case class BranchesExtractor(repo: Repository) { + lazy val branchesOfCommit: Map[ObjectId, Set[String]] = { + + use(new Git(repo)) { git => + git.branchList.call.foldLeft(Map.empty[ObjectId, Set[String]]) { (branchesAcc, ref) => + val branchName = ref.getName.drop(Constants.R_HEADS.length) + + use(new RevWalk(repo)) { rw: RevWalk => + rw.markStart(rw.parseCommit(ref.getObjectId)) + rw.foldLeft(branchesAcc) { (thisBranchAcc, rev) => + val sha1 = rev.getId + thisBranchAcc.get(sha1) match { + case Some(set) => thisBranchAcc + (sha1 -> (set + branchName)) + case None => thisBranchAcc + (sha1 -> Set(branchName)) + } + } + } + } + } + } +}
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranches.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranches.scala deleted file mode 100644 index 46cca17..0000000 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranches.scala +++ /dev/null
@@ -1,53 +0,0 @@ -// Copyright (C) 2017 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.analytics.common - -import com.googlesource.gerrit.plugins.analytics.common.ManagedResource.use -import org.eclipse.jgit.api.Git -import org.eclipse.jgit.lib.{Constants, ObjectId, Repository} -import org.eclipse.jgit.revwalk.RevWalk -import org.eclipse.jgit.revwalk.filter.CommitTimeRevFilter - -import scala.collection.JavaConversions._ -import scala.collection.mutable - -class CommitsBranches(repo: Repository, from: Option[Long] = None, - to: Option[Long] = None) { - - def forCommits(objectIds: TraversableOnce[ObjectId]): Set[String] = { - val commitToBranchesMap = new mutable.HashMap[String, mutable.Set[String]] - with mutable.MultiMap[String, String] - use(new Git(repo)) { git => - val refs = git.branchList.call - for (ref <- refs) { - val branchName = ref.getName.drop(Constants.R_HEADS.length) - use(new RevWalk(repo)) { rw: RevWalk => - from.foreach(d1 => rw.setRevFilter(CommitTimeRevFilter.after(d1))) - to.foreach(d2 => rw.setRevFilter(CommitTimeRevFilter.before(d2))) - rw.markStart(rw.parseCommit(ref.getObjectId)) - rw.foreach { rev => - val sha1 = rev.getName - commitToBranchesMap.addBinding(sha1, branchName) - } - } - } - objectIds.foldLeft(Set.empty[String]) { - (branches, objectId) => { - branches ++ commitToBranchesMap(objectId.getName) - } - }.filter(_.nonEmpty) - } - } -}
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala index 2dc7515..bb5c0e2 100644 --- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala +++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsStatistics.scala
@@ -95,8 +95,10 @@ val stats = commits.map(forSingleCommit) - val nonMergeStats = stats.filterNot(_.isForMergeCommits).foldLeft(CommitsStatistics.Empty)(_ + _) - val mergeStats = stats.filter(_.isForMergeCommits).foldLeft(CommitsStatistics.EmptyMerge)(_ + _) + val (mergeStatsSeq, nonMergeStatsSeq) = stats.partition(_.isForMergeCommits) + + val nonMergeStats = nonMergeStatsSeq.foldLeft(CommitsStatistics.Empty)(_ + _) + val mergeStats = mergeStatsSeq.foldLeft(CommitsStatistics.EmptyMerge)(_ + _) List(nonMergeStats, mergeStats).filterNot(_.isEmpty) }
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractorSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractorSpec.scala new file mode 100644 index 0000000..7a15b22 --- /dev/null +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/BranchesExtractorSpec.scala
@@ -0,0 +1,29 @@ +package com.googlesource.gerrit.plugins.analytics.common + +import com.googlesource.gerrit.plugins.analytics.test.GitTestCase +import org.eclipse.jgit.internal.storage.file.FileRepository +import org.scalatest.{FlatSpec, Matchers} + +class BranchesExtractorSpec extends FlatSpec with Matchers with GitTestCase { + def commitsBranches = new BranchesExtractor(new FileRepository(testRepo)) + + behavior of "branchesOfCommit" + + it should "extract one branch for a commit existing only in one branch" in { + add("file", "content") + branch("feature/branch") + val commit = add("fileOnBranch", "content2") + + commitsBranches.branchesOfCommit(commit.getId) shouldBe Set("feature/branch") + + } + + it should "extract two branches for a commit existing in two different branches" in { + val commit = add("file", "content") + branch("feature/branch") + add("fileOnBranch", "content2") + + commitsBranches.branchesOfCommit(commit.getId) shouldBe Set("feature/branch", "master") + + } +}
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranchesTest.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranchesTest.scala deleted file mode 100644 index ad9f712..0000000 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/CommitsBranchesTest.scala +++ /dev/null
@@ -1,27 +0,0 @@ -package com.googlesource.gerrit.plugins.analytics.common - -import com.googlesource.gerrit.plugins.analytics.test.GitTestCase -import org.eclipse.jgit.internal.storage.file.FileRepository -import org.scalatest.{FlatSpec, Matchers} - -class CommitsBranchesTest extends FlatSpec with Matchers with GitTestCase { - def commitsBranches = new CommitsBranches(new FileRepository(testRepo)) - - "getAllCommitsLabeledWithBranches" should "label correctly a set of " + - "commits" in { - val c1 = add("file", "content") - val c2 = add("file2", "content") - val c3 = add("file3", "content") - val c4 = add("file4", "content") - branch("feature/branch") - val c5 = add("fileOnBranch", "content2") - val c6 = add("fileOnBranch2", "content2") - val c7 = add("fileOnBranch3", "content2") - val c8 = add("fileOnBranch4", "content2") - - commitsBranches.forCommits(Seq(c1, c2, c3, c4)) should be( - Set("master", "feature/branch")) - - commitsBranches.forCommits(Seq(c7, c8)) should be(Set("feature/branch")) - } -}
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/TestUtils.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/TestUtils.scala new file mode 100644 index 0000000..cef628e --- /dev/null +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/common/TestUtils.scala
@@ -0,0 +1,15 @@ +package com.googlesource.gerrit.plugins.analytics.common + +import com.googlesource.gerrit.plugins.analytics.test.GitTestCase +import org.gitective.core.CommitFinder + +trait TestUtils { + self: GitTestCase => + + def aggregateBy(strategy: AggregationStrategy): Array[AggregatedUserCommitActivity] = { + val filter = new AggregatedHistogramFilterByDates(aggregationStrategy = strategy) + new CommitFinder(testRepo).setFilter(filter).find + filter.getHistogram.getAggregatedUserActivity + } + +}
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregatedHistogramFilterByDatesSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregatedHistogramFilterByDatesSpec.scala index b525c7a..4e0895e 100644 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregatedHistogramFilterByDatesSpec.scala +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregatedHistogramFilterByDatesSpec.scala
@@ -16,7 +16,9 @@ import java.util.Date -import com.googlesource.gerrit.plugins.analytics.common.{AggregationStrategy, AggregatedHistogramFilterByDates} +import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.EMAIL +import com.googlesource.gerrit.plugins.analytics.common.{AggregatedHistogramFilterByDates, BranchesExtractor} +import org.eclipse.jgit.internal.storage.file.FileRepository import org.eclipse.jgit.lib.PersonIdent import org.gitective.core.CommitFinder import org.scalatest.{BeforeAndAfterEach, FlatSpec, Matchers} @@ -28,7 +30,7 @@ "select one commit without intervals restriction" in { add("file.txt", "some content") - val filter = new AggregatedHistogramFilterByDates + val filter = new AggregatedHistogramFilterByDates() new CommitFinder(testRepo).setFilter(filter).find val userActivity = filter.getHistogram.getUserActivity @@ -104,4 +106,20 @@ activity.getName should be(person.getName) activity.getEmail should be(person.getEmailAddress) } + + it should "aggregate commits of the same user separately when they are in different branches and branchesExtractor is set" in { + val repo = new FileRepository(testRepo) + add("file1.txt", "add file1.txt to master branch") + branch("another/branch") + add("file2.txt", "add file2.txt to another/branch") + val filter = new AggregatedHistogramFilterByDates( + aggregationStrategy=EMAIL, + branchesExtractor = Some(new BranchesExtractor(repo)) + ) + + new CommitFinder(testRepo).setFilter(filter).find + val userActivity = filter.getHistogram.getUserActivity + + userActivity should have size 2 + } }
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregationSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregationSpec.scala index bbb691d..6725b7e 100644 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregationSpec.scala +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/AggregationSpec.scala
@@ -16,31 +16,20 @@ import java.util.Date -import com.googlesource.gerrit.plugins.analytics.common.{AggregatedHistogramFilterByDates, AggregatedUserCommitActivity, AggregationStrategy} +import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy._ +import com.googlesource.gerrit.plugins.analytics.common.DateConversions.isoStringToLongDate +import com.googlesource.gerrit.plugins.analytics.common.TestUtils import org.eclipse.jgit.revwalk.RevCommit -import org.gitective.core.CommitFinder import org.scalatest.{FlatSpec, Inspectors, Matchers} -import AggregationStrategy._ -class AggregationSpec extends FlatSpec with Matchers with GitTestCase with Inspectors { +class AggregationSpec extends FlatSpec with Matchers with GitTestCase with TestUtils with Inspectors { - import com.googlesource.gerrit.plugins.analytics.common.DateConversions._ - - def commit(committer: String, when: String, content: String): RevCommit = { - val date = new Date(isoStringToLongDate(when)) - val person = newPersonIdent(committer, committer, date) - add("afile.txt", content, author = person, committer = author) - } - - def aggregateBy(strategy: AggregationStrategy) = { - val filter = new AggregatedHistogramFilterByDates(aggregationStrategy = strategy) - new CommitFinder(testRepo).setFilter(filter).find - filter.getHistogram.getAggregatedUserActivity - } + def commitAtDate(committer: String, when: String, content: String): RevCommit = + commit(committer, "afile.txt", content, new Date(isoStringToLongDate(when))) "AggregatedHistogramFilter by email and year" should "aggregate two commits from the same author the same year" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2017-10-05", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2017-10-05", "second commit") val userActivity = aggregateBy(EMAIL_YEAR) @@ -50,8 +39,8 @@ } it should "keep as separate rows activity from the same author on two different year" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2018-09-01", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2018-09-01", "second commit") val userActivity = aggregateBy(EMAIL_YEAR) @@ -66,8 +55,8 @@ } it should "keep as separate rows activity from two different authors on the same year" in { - commit("john", "2017-08-01", "first commit") - commit("bob", "2017-12-05", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("bob", "2017-12-05", "second commit") val userActivity = aggregateBy(EMAIL_YEAR) @@ -79,8 +68,8 @@ } "AggregatedHistogramFilter by email and month" should "aggregate two commits from the same author the same month" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2017-08-05", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2017-08-05", "second commit") val userActivity = aggregateBy(EMAIL_MONTH) @@ -90,8 +79,8 @@ } it should "keep as separate rows activity from the same author on two different months" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2017-09-01", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2017-09-01", "second commit") val userActivity = aggregateBy(EMAIL_MONTH) @@ -106,8 +95,8 @@ } it should "keep as separate rows activity from two different authors on the same month" in { - commit("john", "2017-08-01", "first commit") - commit("bob", "2017-08-05", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("bob", "2017-08-05", "second commit") val userActivity = aggregateBy(EMAIL_MONTH) @@ -119,8 +108,8 @@ } "AggregatedHistogramFilter by email and day" should "aggregate two commits of the same author the same day" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2017-08-01", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2017-08-01", "second commit") val userActivity = aggregateBy(EMAIL_DAY) @@ -130,8 +119,8 @@ } it should "keep as separate rows activity from the same author on two different days" in { - commit("john", "2017-08-01", "first commit") - commit("john", "2017-08-02", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("john", "2017-08-02", "second commit") val userActivity = aggregateBy(EMAIL_DAY) @@ -146,8 +135,8 @@ } it should "keep as separate rows activity from two different authors on the same day" in { - commit("john", "2017-08-01", "first commit") - commit("bob", "2017-08-01", "second commit") + commitAtDate("john", "2017-08-01", "first commit") + commitAtDate("bob", "2017-08-01", "second commit") val userActivity = aggregateBy(EMAIL_DAY) @@ -159,8 +148,8 @@ } "AggregatedHistogramFilter by email and hour" should "aggregate two commits of the same author on the same hour" in { - commit("john", "2017-08-01 10:15:03", "first commit") - commit("john", "2017-08-01 10:45:01", "second commit") + commitAtDate("john", "2017-08-01 10:15:03", "first commit") + commitAtDate("john", "2017-08-01 10:45:01", "second commit") val userActivity = aggregateBy(EMAIL_HOUR) @@ -170,8 +159,8 @@ } it should "keep separate commits from the same author on different hours" in { - commit("john", "2017-08-01 10:15:03", "first commit") - commit("john", "2017-08-01 11:30:01", "second commit") + commitAtDate("john", "2017-08-01 10:15:03", "first commit") + commitAtDate("john", "2017-08-01 11:30:01", "second commit") val userActivity = aggregateBy(EMAIL_HOUR) @@ -185,8 +174,8 @@ } it should "keep separate commits from different authors on the same hour" in { - commit("john", "2017-08-01 10:15:03", "first commit") - commit("bob", "2017-08-01 10:20:00", "second commit") + commitAtDate("john", "2017-08-01 10:15:03", "first commit") + commitAtDate("bob", "2017-08-01 10:20:00", "second commit") val userActivity = aggregateBy(EMAIL_HOUR)
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala index b5d7482..95784fd 100644 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsCommentLinkSpec.scala
@@ -14,14 +14,12 @@ package com.googlesource.gerrit.plugins.analytics.test -import java.util.{Arrays, Date} - import com.google.gerrit.extensions.api.projects.CommentLinkInfo import com.googlesource.gerrit.plugins.analytics.IssueInfo import com.googlesource.gerrit.plugins.analytics.common.{CommitsStatistics, Statistics} import org.eclipse.jgit.internal.storage.file.FileRepository -import org.eclipse.jgit.revwalk.RevCommit import org.scalatest.{FlatSpec, Inside, Matchers} + import scala.collection.JavaConverters._ class CommitStatisticsCommentLinkSpec extends FlatSpec with GitTestCase with Matchers with Inside { @@ -34,12 +32,6 @@ info } - def commit(committer: String, fileName: String, content: String, message: Option[String] = None): RevCommit = { - val date = new Date() - val person = newPersonIdent(committer, committer, date) - add(testRepo, fileName, content, author = person, committer = author, message = message.getOrElse("** no message **")) - } - class TestEnvironment(val repo: FileRepository = new FileRepository(testRepo), val commentLinks: java.util.List[CommentLinkInfo] = Seq( createCommentLinkInfo(pattern = "(bug\\s+#?)(\\d+)",
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala index df4260a..6db1d00 100644 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/CommitStatisticsSpec.scala
@@ -14,18 +14,12 @@ package com.googlesource.gerrit.plugins.analytics.test -import java.util.Date - -import com.google.common.collect.Sets import com.google.common.collect.Sets.newHashSet import com.googlesource.gerrit.plugins.analytics.CommitInfo import com.googlesource.gerrit.plugins.analytics.common.{CommitsStatistics, Statistics} -import org.eclipse.jgit.api.{Git, MergeResult} import org.eclipse.jgit.internal.storage.file.FileRepository -import org.eclipse.jgit.revwalk.RevCommit import org.scalatest.{FlatSpec, Inside, Matchers} - class CommitStatisticsSpec extends FlatSpec with GitTestCase with Matchers with Inside { @@ -34,24 +28,6 @@ val stats = new Statistics(repo) } - def commit(committer: String, fileName: String, content: String): RevCommit = { - val date = new Date() - val person = newPersonIdent(committer, committer, date) - add(testRepo, fileName, content, author = person, committer = author) - } - - def mergeCommit(committer: String, fname: String, content: String): MergeResult = { - val currentBranch = Git.open(testRepo).getRepository.getBranch - val tmpBranch = branch(testRepo, "tmp") - try { - val commitToMerge = commit(committer, fname, content) - checkout(currentBranch) - mergeBranch("tmp", true) - } finally { - deleteBranch(testRepo, tmpBranch.getName) - } - } - "CommitStatistics" should "stats a single file added" in new TestEnvironment { val change = commit("user", "file1.txt", "line1\nline2")
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/GitTestCase.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/GitTestCase.scala index 8a1dcf7..1449a35 100644 --- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/GitTestCase.scala +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/GitTestCase.scala
@@ -40,6 +40,7 @@ import java.text.MessageFormat import java.util.Date +import com.googlesource.gerrit.plugins.analytics.common.DateConversions.isoStringToLongDate import com.googlesource.gerrit.plugins.analytics.common.ManagedResource.use import org.eclipse.jgit.api.MergeCommand.FastForwardMode import org.eclipse.jgit.api.{Git, MergeResult} @@ -50,6 +51,7 @@ import org.eclipse.jgit.revwalk.RevCommit import org.gitective.core.CommitUtils import org.scalatest.{BeforeAndAfterEach, Suite} +import com.googlesource.gerrit.plugins.analytics.common.DateConversions._ /** @@ -391,4 +393,40 @@ git.commit.setOnly(path).setMessage(message).setAuthor(author).setCommitter(committer).call } } ensuring (_ != null, "Unable to commit delete operation") + + /** + * commit specified content into a file, as committer + * + * @param committer - the author of this commit + * @param fileName - the name of the file + * @param content - the content of the file + * @param when - the date of the commit + * @return RevCommit + * + */ + protected def commit(committer: String, fileName: String, content: String, when: Date = new Date(), message: Option[String] = None): RevCommit = { + val person = newPersonIdent(committer, committer, when) + add(testRepo, fileName, content, author = person, committer = author, message = message.getOrElse("** no message **")) + } + + /** + * commit specified content into a file, as committer and merge into current branch + * + * @param committer - the author of this commit + * @param fileName - the name of the file + * @param content - the content of the file + * @return MergeResult + * + */ + protected def mergeCommit(committer: String, fileName: String, content: String): MergeResult = { + val currentBranch = Git.open(testRepo).getRepository.getBranch + val tmpBranch = branch(testRepo, "tmp") + try { + commit(committer, fileName, content) + checkout(currentBranch) + mergeBranch(tmpBranch.getName, withCommit = true) + } finally { + deleteBranch(testRepo, tmpBranch.getName) + } + } } \ No newline at end of file
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/UserActivitySummarySpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/UserActivitySummarySpec.scala new file mode 100644 index 0000000..57417cd --- /dev/null +++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/UserActivitySummarySpec.scala
@@ -0,0 +1,39 @@ +package com.googlesource.gerrit.plugins.analytics.test + +import com.googlesource.gerrit.plugins.analytics.UserActivitySummary +import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.EMAIL +import com.googlesource.gerrit.plugins.analytics.common.{Statistics, TestUtils} +import org.eclipse.jgit.internal.storage.file.FileRepository +import org.scalatest.{FlatSpec, Matchers} + +class UserActivitySummarySpec extends FlatSpec with GitTestCase with TestUtils with Matchers { + + "numCommits" should "count only comments filtered by their merge status" in { + val personEmail = "aCommitter@aCompany.com" + + // we want merge and non-merge commits to be authored by same person, so that they can be aggregated together + val repo = getRepoOwnedByPerson(personEmail) + + commit(personEmail, fileName="aFile.txt", content="some content") + mergeCommit(personEmail, fileName = "anotherFile.txt", content="some other content") + val aggregatedCommits = aggregateBy(EMAIL) + + val List(nonMergeSummary, mergeSummary) = UserActivitySummary.apply(new Statistics(repo))(aggregatedCommits.head) + + nonMergeSummary.numCommits should be(2) + mergeSummary.numCommits should be(1) + } + + def getRepoOwnedByPerson(email: String = author.getEmailAddress): FileRepository = { + val repo = new FileRepository(testRepo) + + val conf = repo.getConfig + conf.load() + conf.setString("user", null, "name", email) + conf.setString("user", null, "email", email) + conf.save() + + repo + } + +}