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/build.sbt b/build.sbt
index 6564192..00adcc9 100644
--- a/build.sbt
+++ b/build.sbt
@@ -1,6 +1,6 @@
 enablePlugins(GitVersioning)
 
-val gerritApiVersion = "2.16-SNAPSHOT"
+val gerritApiVersion = "2.16-rc1"
 
 val pluginName = "analytics"
 
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 467dd65..34cae77 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
@@ -138,6 +138,7 @@
                                     histogram: UserActivityHistogram,
                                     gsonFmt: GsonFormatter) {
   import RichBoolean._
+
   import scala.collection.JavaConverters._
 
   def get(projectRes: ProjectResource, startDate: Option[Long], stopDate: Option[Long],
@@ -148,15 +149,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
     }
   }
@@ -186,24 +187,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 b14bff8..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(tmpBranch.getName, 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 a511420..c7a3850 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
+  }
+
+}